aaron.ballman added a comment.
In D112491#3088363 <https://reviews.llvm.org/D112491#3088363>, @jcking1034
wrote:
> @aaron.ballman for the purpose of these matchers, what @ymandel said is
> correct - the goal is to allow `LambdaCapture`s themselves to be bindable.
Should we be discussing deprecating the non-bindable matchers for lambda
captures? I guess the part that worries me is there are now two ways to match
the same sorts of constructs, and I'm a bit worried it won't be clear which one
to reach for and when. Do you think this will be a usability concern for users?
================
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();
----------------
jcking1034 wrote:
> ymandel wrote:
> > 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?
> I would expect @aaron.ballman's initializer example to match, and I added a
> similar test case to the one described. I think that if a capture does not
> have an initializer, then `refersToVarDecl` will match on the variable
> declaration before the lambda. However, if a capture does have an
> initializer, that initializer itself seems to be represented as a `VarDecl`
> in the AST, which is the `VarDecl` that gets matched.
>
> For that reason, I think we may not need to have a separate matcher on the
> name of the capture itself. Additionally, since captures with/without
> initializers are both represented the same way, there may not be a good way
> to distinguish between them, so matchers for initializers may not be possible.
> I think that if a capture does not have an initializer, then refersToVarDecl
> will match on the variable declaration before the lambda. However, if a
> capture does have an initializer, that initializer itself seems to be
> represented as a VarDecl in the AST, which is the VarDecl that gets matched.
Oof, that'd be confusing! :-(
> For that reason, I think we may not need to have a separate matcher on the
> name of the capture itself.
Er, but there are init captures where you can introduce a whole new
declaration. I think we do want to be able to match on that, right? e.g.,
```
[x = 12](){ return x; }();
```
> Additionally, since captures with/without initializers are both represented
> the same way, there may not be a good way to distinguish between them, so
> matchers for initializers may not be possible.
That's a bummer! :-( If this turns out to be a limitation, we should probably
document it as such.
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
