cpp plugin: visitor & context madness

[Milian Wolff]

Hey all,

I want to cleanup the cpp plugin a bit after I was hit by the bug fixed by:
http://commits.kde.org/kdevelop/5a68522b2197da7442ca34bd31a89dd7db013d66

while at it I found a few strange things I'd like to have some feedback on:

a) NameASTVisitor & TypeASTVisitor have m_context + m_localContext which are 
both set via the ctor and never changed thereafter. the only use I could find 
is ExpressionVisitor::visitSignalSlotExpression - could someone 
explain/comment on this?

b) neither NameASTVisitor nor TypeASTVisitor track the "current" context, i.e. 
node->ducontext and just pass along what they found in their ctor - is this 
OK? should we not maybe also overload visit() there and introduce a 
currentContext (or could we reuse one of m_context/m_localContex)?

c) the expression visitor has two TopDUContext* members: m_source and and 
m_topContext. the former gets set in the ctor, the latter in parse(). why are 
two required, what is the difference? could someone comment/explain?

d) the ExpressionParser takes a raw TopDUContext* (default = 0) argument but a 
DUContextPointer for the context argument. the latter and the code in 
evaluateType make it clear that this can be used later on, e.g. in code 
completion. so 

- shouldn't the topcontext argument also be a smart pointer?
- we set the ast->ducontext to context.data() and do one more check for 
validity, but down below the ExpressionVisitor will be used which never again 
does any checks. isn't this *very* unsafe?

the API dox of the expression parser says:

> The duchain does not strictly need to be locked when this is called, but it
> should be locked if the entity evaluating the expression has no control over
> the lifetime of @p context 

can we somehow make this clearer on an API-level - ideas anyone?

e) the visitor and some other api uses "const DUContext*", others expect just 
a DUContext*. Theroetically I prefer the former, but as soon as you try to use 
DUContextPointer / IndexedDUContext or similar contains, you must use a 
const_cast - or am I missing something? Could this be improved? Should we use 
plain non-const pointers everywhere?

I'll work on these things a bit and try to
-- 
Milian Wolff
mail at milianw.de
http://milianw.de