Review Request 119526: Ensure we're not visiting cursors twice

[Kevin Funk]

> On July 29, 2014, 5:11 a.m., Olivier Jean de Gaalon wrote:
> > I would prefer to look into this issue deeper, adding a hash of visited cursors isn't really an acceptable solution IMO.
> > 
> > Also not sure what is meant by "Apparently just 'recursing' vs. 'visiting children explicitly' results in a different AST traversal"
> > 
> > There should be no difference between clang_visitChildren on a cursor vs returning CXChildVisitRecurse. If we are doing both that is a problem, but that doesn't seem to be the case (IIRC we don't ever use CXChildVisitRecurse). On the other hand, returning CXChildVisitContinue means no further recursion is desired (ie, skip the children of this cursor).
> > 
> > This looks like a defect in the clang AST. Looking at it with clang-parser I see no reason for the StructDecl to be a child of the variable declaration. A workaround is still needed, but it should target this pattern without hashing every cursor in the TU, which would really be a last resort.

"There should be no difference between clang_visitChildren on a cursor vs returning CXChildVisitRecurse. If we are doing both that is a problem, but that doesn't seem to be the case (IIRC we don't ever use CXChildVisitRecurse). On the other hand, returning CXChildVisitContinue means no further recursion is desired (ie, skip the children of this cursor)."

=> Damn, I think that's indeed the case. I was playing around with DebugVisitor, removed the call to `clang_visitChildren` but forgot to make it return `CXChildVisit_Recurse` instead. So, indeed, even with `CXChildVisit_Recurse` the traversal is the same.

The issue still remains. We're visiting some cursors multiple times. I also don't really see why the StructDecl needs to be a child of the VarDecl. Maybe worth asking on the mailing list -- I'll do that.

I think it's still sane to have this committed for now (with a big-fat TODO), otherwise we and up duplicating decls and types in the DUChain.


- Kevin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119526/#review63378
-----------------------------------------------------------


On July 28, 2014, 8:25 p.m., Kevin Funk wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119526/
> -----------------------------------------------------------
> 
> (Updated July 28, 2014, 8:25 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdev-clang
> 
> 
> Description
> -------
> 
> Ensure we're not visiting cursors twice
> 
> 
> Diffs
> -----
> 
>   duchain/tuduchain.h 00ee23ba0da851456fd2a2641689514d25240389 
>   duchain/tuduchain.cpp a9e249e16a1d82c95c360b40575568abc1311444 
>   tests/test_duchain.h 37ce20f398ea73d2a36d753a6421bb3015898f4d 
>   tests/test_duchain.cpp 89f5005149421b8b3ef1a9def3d9e6222e3d6b0a 
> 
> Diff: https://git.reviewboard.kde.org/r/119526/diff/
> 
> 
> Testing
> -------
> 
> Please check, maybe I'm missing something.
> 
> 
> Thanks,
> 
> Kevin Funk
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20140729/ec38e2c3/attachment.html>