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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D56444#1350849 <https://reviews.llvm.org/D56444#1350849>, @JonasToth wrote:

> In D56444#1350746 <https://reviews.llvm.org/D56444#1350746>, @sammccall wrote:
>
> > @klimek: would it be better to preserve the odd behavior of the `functionDecl()` matcher, and add a new `functionOrLambdaDecl()`? It seems too surprising to me.
>
>
> We can change the clang-tidy check as well. I think lambdas should be considered functionDecls, shouldnt they? WDYT @aaron.ballman ?


I think it depends on whether we want the AST matchers to match what the user wrote (syntax) or how the program behaves (semantics). If the user writes a lambda, they did not syntactically write a function declaration and so it makes sense for `functionDecl()` to not match. However, the semantics of a lambda are that they act as a functor (in effect) which includes a class declaration, a function declaration for the function call operator, conversion operators, etc and so it does make sense for users to want to match those semantic components. 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.

It's always bothered me that tools like clang-query have behavior like this around lambdas, given:

  int main() {
    auto l = [](){};
  }

you get behavior like:

  clang-query> m cxxRecordDecl()
  
  Match #1:
  
  1 match.
  clang-query>

> Should still be fixed for 8.0 (probably with this patch). The refactoring is more realistic to land in 9.0 i guess?

9.0, IMO.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56444