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

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

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

> In D56444#1351056 <https://reviews.llvm.org/D56444#1351056>, @aaron.ballman wrote:
>
> > Given that, I kind of think we should have functionDecl() match only functions, and give users some other way to match the semantic declarations in a consistent manner. Alternatively, we could decide semantics are what we want to match (because it's what the AST encodes) and instead we give users a way to request to only match syntax.
>
>
> I believe matching the implied semantic nodes is how closer to how matchers behave in general (corresponding to the fact that the ASTMatcher RecursiveASTVisitor sets `shouldVisitImplicitCode` to true). e.g.
>
>   $ cat ~/test.cc
>   void foo() { for (char c : "abc") {} }
>   $ bin/clang-query ~/test.cc --
>   clang-query> set output detailed-ast
>   clang-query> match binaryOperator()
>  
>   Match #1:
>  
>   Binding for "root":
>   BinaryOperator 0x471f038 </usr/local/google/home/sammccall/test.cc:1:26> '_Bool' '!='
>   |-ImplicitCastExpr 0x471f008 <col:26> 'const char *':'const char *' <LValueToRValue>
>   | `-DeclRefExpr 0x471efc8 <col:26> 'const char *':'const char *' lvalue Var 0x471ed48 '__begin1' 'const char *':'const char *'
>   `-ImplicitCastExpr 0x471f020 <col:26> 'const char *':'const char *' <LValueToRValue>
>     `-DeclRefExpr 0x471efe8 <col:26> 'const char *':'const char *' lvalue Var 0x471edb8 '__end1' 'const char *':'const char *'
>   etc
>
>
> Obviously this is only true when such nodes are present in the AST at the time of matching (if I'm understanding Manuel's comment correctly).


Note that I've fixed that too,

http://ec2-18-191-7-3.us-east-2.compute.amazonaws.com:10240/z/7sLs34

but both the matcher and the dumper need to get the new behavior at the same time. I agree that we shouldn't match on implicit nodes. See also

http://ec2-18-191-7-3.us-east-2.compute.amazonaws.com:10240/z/sNny36

See how Manuel had to explain this here years ago: https://youtu.be/VqCkCDFLSsc?t=1748

There's more that we should do to simplify AST dumping and matching: http://ec2-18-191-7-3.us-east-2.compute.amazonaws.com:10240/z/uBshw1

My point being that we shouldn't try to rush some way of treating lambdas like functionDecl()s or anything like that.

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.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56444