aaron.ballman added inline comments.

================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:4056
+///   matches [this](){};
+AST_MATCHER(LambdaExpr, capturesThis) {
+  for (const LambdaCapture &Capture : Node.captures()) {
----------------
rhiro wrote:
> aaron.ballman wrote:
> > I'm a bit less certain about the utility of this matcher. I would have 
> > assumed you could do `hasAnyCapture(cxxThisExpr())` to get the same 
> > behavior, but I am guessing that won't work -- though you may be able to 
> > add an overload to `hasAnyCapture()` to make that work. I think that might 
> > be a better approach, if it works.
> Thanks for that idea, it does seem a bit cleaner.
> 
> It would be nice if we could write a Matcher<LambdaCapture>, so that the 
> macro composition could be more generically nested. e.g. hasAnyCapture that 
> just loops over the LambdaCaptures and passes them to the child matcher.  
> However, when I tried this, it became apparent that it is not a Node type 
> that can be matched on, and would require nontrivial changes.
Yeah, I think this would require some work within the guts of the AST matchers 
and I'm not certain it would be worth the effort yet.


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:4032-4034
+    if (Capture.capturesThis()) {
+      continue;
+    }
----------------
I would prefer to see this written as: `if (Capture.capturesVariable()) { ... 
}` so it's a bit more resilient to other non-var captures like VLA captures. 


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:4058-4063
+  for (const LambdaCapture &Capture : Node.captures()) {
+    if (Capture.capturesThis()) {
+      return true;
+    }
+  }
+  return false;
----------------
`return llvm::any_of(Node.captures(), [](const LambdaCapture &LC) { return 
LC.capturesThis(); });`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72414



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to