[PATCH] D56444: [AST] RecursiveASTVisitor visits lambda classes when implicit visitation is on.

Wed Jan 9 07:30:54 PST 2019

steveire added a comment.

In D56444#1351145 <https://reviews.llvm.org/D56444#1351145>, @sammccall wrote:

> In D56444#1351128 <https://reviews.llvm.org/D56444#1351128>, @aaron.ballman wrote:
>
> > In D56444#1351127 <https://reviews.llvm.org/D56444#1351127>, @steveire wrote:
> >
> > > I suggest not trying to make any such drastic changes for 8.0, try to fix the bug in a minimal way if possible, and have a more considered approach to the future of AST Matchers for after the release.
> >
> >
> > +1
>
>
> Can you elaborate?
>
> This patch is an attempt to solve the problem "some nodes associated with lambdas have no parents".

This is not a new kind of issue. I reported a variant of it too months ago: https://bugs.llvm.org/show_bug.cgi?id=37629.

As such I do think that this is not so urgent and can be fixed after the traverser class is extracted.

One of the problems is that visually tracking through parents/children of an AST dump does not represent the same relationships as can be expressed with hasParent() AST Matcher, because AST Matchers use the RAV and AST dumps do not.

However, we can make both use the same class (the generic traverser I am extracting from ASTDumper), and then that class of bug will be solved as I understand it.

> This manifests as assertion failures (or with assertions off, incorrect results) for some matchers, on some code - people have encountered several examples where this happens, it's hard to know where to target a more targeted fix.
>  The invariant violated is "RAV traverses every AST node (when implicit traversal is enabled)" - is there a way to fix this issue sufficiently without addressing that?

Yes - don't use RAV to traverse parents when AST-matching.

> Is the suggestion to make that change, but then modify the semantics of the `functionDecl()` etc matchers to hide it? Or something else?

My suggestion is to extract the traverser from ASTDumper first, then fix this issue.

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56444/new/

https://reviews.llvm.org/D56444