[PATCH] D72414: Add new AST matchers `hasAnyCapture` and `capturesThis`

2020-01-09 Thread Reid via Phabricator via cfe-commits
rhiro marked an inline comment as done.
rhiro added a comment.

Thanks for the review!  I regenerated the docs and switched the `this` capture 
case to be an override of `hasAnyCapture`.




Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:4056
+///   matches [this](){};
+AST_MATCHER(LambdaExpr, capturesThis) {
+  for (const LambdaCapture &Capture : Node.captures()) {

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, 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.


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


[PATCH] D72414: Add new AST matchers `hasAnyCapture` and `capturesThis`

2020-01-09 Thread Reid via Phabricator via cfe-commits
rhiro updated this revision to Diff 237195.
rhiro added a comment.

- Change the hasCaptureThis functionality to be an override of hasAnyCapture 
with cxxThisExpr argument. Also generate the documentation and update tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72414

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -454,6 +454,26 @@
   objcMessageExpr(hasReceiver(declRefExpr(to(varDecl(hasName("x";
 }
 
+TEST(Matcher, HasAnyCapture) {
+  auto HasCaptureX = lambdaExpr(hasAnyCapture(varDecl(hasName("x";
+  EXPECT_TRUE(matches("void f() { int x = 3; [x](){}; }", HasCaptureX));
+  EXPECT_TRUE(matches("void f() { int x = 3; [&x](){}; }", HasCaptureX));
+  EXPECT_TRUE(notMatches("void f() { [](){}; }", HasCaptureX));
+  EXPECT_TRUE(notMatches("void f() { int z = 3; [&z](){}; }", HasCaptureX));
+  EXPECT_TRUE(
+  notMatches("struct a { void f() { [this](){}; }; };", HasCaptureX));
+}
+
+TEST(Matcher, CapturesThis) {
+  auto HasCaptureThis = lambdaExpr(hasAnyCapture(cxxThisExpr()));
+  EXPECT_TRUE(
+  matches("struct a { void f() { [this](){}; }; };", HasCaptureThis));
+  EXPECT_TRUE(notMatches("void f() { [](){}; }", HasCaptureThis));
+  EXPECT_TRUE(notMatches("void f() { int x = 3; [x](){}; }", HasCaptureThis));
+  EXPECT_TRUE(notMatches("void f() { int x = 3; [&x](){}; }", HasCaptureThis));
+  EXPECT_TRUE(notMatches("void f() { int z = 3; [&z](){}; }", HasCaptureThis));
+}
+
 TEST(Matcher, isClassMessage) {
   EXPECT_TRUE(matchesObjC(
   "@interface NSString +(NSString *) stringWithFormat; @end "
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -105,6 +105,7 @@
   // equalsNode
 
   REGISTER_OVERLOADED_2(callee);
+  REGISTER_OVERLOADED_2(hasAnyCapture);
   REGISTER_OVERLOADED_2(hasPrefix);
   REGISTER_OVERLOADED_2(hasType);
   REGISTER_OVERLOADED_2(ignoringParens);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -55,6 +55,7 @@
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
+#include "clang/AST/LambdaCapture.h"
 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/OpenMPClause.h"
 #include "clang/AST/OperationKinds.h"
@@ -4014,6 +4015,54 @@
   return false;
 }
 
+/// Matches any capture of a lambda expression.
+///
+/// Given
+/// \code
+///   void foo() {
+/// int x;
+/// auto f = [x](){};
+///   }
+/// \endcode
+/// lambdaExpr(hasAnyCapture(anything()))
+///   matches [x](){};
+AST_MATCHER_P_OVERLOAD(LambdaExpr, hasAnyCapture, internal::Matcher,
+   InnerMatcher, 0) {
+  for (const LambdaCapture &Capture : Node.captures()) {
+if (Capture.capturesThis()) {
+  continue;
+}
+BoundNodesTreeBuilder Result(*Builder);
+if (InnerMatcher.matches(*Capture.getCapturedVar(), Finder, &Result)) {
+  *Builder = std::move(Result);
+  return true;
+}
+  }
+  return false;
+}
+
+/// Matches any capture of 'this' in a lambda expression.
+///
+/// Given
+/// \code
+///   struct foo {
+/// void bar() {
+///   auto f = [this](){};
+/// }
+///   }
+/// \endcode
+/// lambdaExpr(hasAnyCapture(cxxThisExpr()))
+///   matches [this](){};
+AST_MATCHER_P_OVERLOAD(LambdaExpr, hasAnyCapture,
+   internal::Matcher, InnerMatcher, 1) {
+  for (const LambdaCapture &Capture : Node.captures()) {
+if (Capture.capturesThis()) {
+  return true;
+}
+  }
+  return false;
+}
+
 /// Matches a constructor call expression which uses list initialization.
 AST_MATCHER(CXXConstructExpr, isListInitialization) {
   return Node.isListInitialization();
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -5098,15 +5098,15 @@
 Matches selection statements with initializer.
 
 Given:
- void foo() { 
+ void foo() {
if (int i = foobar(); i > 0) {}
switch (int i = foobar(); i) {}
-   for (auto& a = get_range(); auto& x : a) {} 
+   for (auto& a = get_range(); auto& x : a) {}
  }
- void bar() { 
+ void bar() {
if (foobar() > 0) {}
switch (foobar()) {}
-   for (auto& x : get_range()) {} 

[PATCH] D72414: Add new AST matchers `hasAnyCapture` and `capturesThis`

2020-01-09 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

Thank you for this patch! You should also register the matcher(s) in 
Registry.cpp so that they'll work from clang-query. Also, you need to 
regenerate the documentation by running `clang\docs\tools\dump_ast_matchers.py`




Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:4056
+///   matches [this](){};
+AST_MATCHER(LambdaExpr, capturesThis) {
+  for (const LambdaCapture &Capture : Node.captures()) {

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.


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