cpp plugin: visitor & context madness
mail at milianw.de
Tue Oct 25 20:32:15 UTC 2011
I want to cleanup the cpp plugin a bit after I was hit by the bug fixed by:
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
- 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
mail at milianw.de
More information about the KDevelop-devel