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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
