[PATCH] D112491: Add `LambdaCapture`-related matchers.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

Nice! Per Aaron's comment, it's probably worth expanding the patch description to explain the motivation. I assume that the key is making these first-class and therefore bindable?



================
Comment at: clang/include/clang/ASTMatchers/ASTMatchFinder.h:170
                   MatchCallback *Action);
+  void addMatcher(const LambdaCaptureMatcher &NodeMatch, MatchCallback *Action);
   void addMatcher(const AttrMatcher &NodeMatch, MatchCallback *Action);
----------------
Since top-level metching doesn't work, I think we *don't* want to add this overload.


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchFinder.h:225
     std::vector<std::pair<AttrMatcher, MatchCallback *>> Attr;
+    std::vector<std::pair<LambdaCaptureMatcher, MatchCallback *>> LambdaCapture;
     /// All the callbacks in one container to simplify iteration.
----------------
i guess remove this too, if you remove the overload?


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:4595
+
+/// Matches any capture of a lambda expression.
+///
----------------



================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:4597-4605
+/// Given
+/// \code
+///   void foo() {
+///     int t = 5;
+///     auto f = [=](){ return t; };
+///   }
+/// \endcode
----------------
Maybe add some more examples? e.g. demonstrating that you can match implicit captures seems valuable. For that same code snippet, maybe something like:

... lambdaCapture(refersToVarDecl(hasName("t")))


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:4629-4630
+///   matches `[x](){}`.
+AST_MATCHER_P(LambdaCapture, refersToVarDecl, internal::Matcher<VarDecl>,
+              InnerMatcher) {
+  auto *capturedVar = Node.getCapturedVar();
----------------
aaron.ballman wrote:
> The name here is a bit unclear -- whether it is the matcher matching `int x;` or the `x` from the capture is not clear from the name. The comment suggests it's matching `x` from the capture, but I think it's actually matching the `int x;` variable declaration.
> 
> Being clear on what's matched here is important when we think about initializers:
> ```
> void foo() {
>   int x = 12;
>   auto f = [x = 100](){};
> }
> ```
> and
> ```
> lambdaExpr(hasAnyCapture(lambdaCapture(refersToVarDecl(hasName("x"), hasInitializer(integerLiteral(equals(100))))))
> ```
> Would you expect this to match? (This might be a good test case to add.)
In a similar vein, do we want a separate matcher on the name of the capture itself? e.g. an overload of `hasName`? And what about matchers for the initializers?  Those don't have to land in this patch, but do you think those would be doable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112491