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

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

This looks really sensible to me too. Some naming bikeshedding, but my main question is migration.

Supporting two things is indeed annoying and confusing. I agree it's not worth keeping the old way forever just to avoid writing `refersToVarDecl`.
The question is: how soon can we get rid of it? We should consider whether we can do it in this patch: just replace the old matcher with this one.

AFAICT the old matcher has no uses in-tree beyond tests/registration. FWIW it has no uses in our out-of-tree repo either (which generally is a heavy matchers user).
I'm sure it has users somewhere, but probably few, and the mechanical difficulty of updating them is low. Given that I think we should just break them, rather than deal with overloading and deprecation warnings and future cleanup just to put off breaking them for one more release cycle.

This is a tradeoff, to me it seems but reasonable people can disagree on the importance of stability. Aaron, happy to go with whatever you decide.



================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:4632
+/// refersToVarDecl(hasName("x")) matches `int x` and `x = 1`.
+AST_MATCHER_P(LambdaCapture, refersToVarDecl, internal::Matcher<VarDecl>,
+              InnerMatcher) {
----------------
I think `capturesVar` may be a clearer/more direct name.

For example given `int y; [x(y)] { return x; }` I would naively expect `refersToVarDecl(hasName("y"))` to match the capture.


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:4652
+///   matches `[this]() { return cc; }`.
+AST_MATCHER(LambdaCapture, refersToThis) { return Node.capturesThis(); }
+
----------------
Again, why `refersToThis` rather than `capturesThis`, which is more specific and matches the AST?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112491