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

2021-11-08 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc0b298fc213c: Add `LambdaCapture`-related matchers. 
(authored by jcking1034, committed by ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112491

Files:
  clang/docs/LibASTMatchersReference.html
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/ASTTypeTraits.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/AST/ASTTypeTraits.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -563,26 +563,6 @@
   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; [](){}; }", HasCaptureX));
-  EXPECT_TRUE(notMatches("void f() { [](){}; }", HasCaptureX));
-  EXPECT_TRUE(notMatches("void f() { int z = 3; [](){}; }", 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; [](){}; }", HasCaptureThis));
-  EXPECT_TRUE(notMatches("void f() { int z = 3; [](){}; }", HasCaptureThis));
-}
-
 TEST(Matcher, MatchesMethodsOnLambda) {
   StringRef Code = R"cpp(
 struct A {
Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -2237,6 +2237,65 @@
  varDecl(hasName("ss"), hasTypeLoc(elaboratedTypeLoc();
 }
 
+TEST_P(ASTMatchersTest, LambdaCaptureTest) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [cc](){ return cc; }; }",
+  lambdaExpr(hasAnyCapture(lambdaCapture();
+}
+
+TEST_P(ASTMatchersTest, LambdaCaptureTest_BindsToCaptureOfVarDecl) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(
+  hasAnyCapture(lambdaCapture(capturesVar(varDecl(hasName("cc"));
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [cc](){ return cc; }; }",
+  matcher));
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [](){ return cc; }; }",
+  matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f = [=](){ return cc; }; }", matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f = [&](){ return cc; }; }", matcher));
+}
+
+TEST_P(ASTMatchersTest, LambdaCaptureTest_BindsToCaptureWithInitializer) {
+  if (!GetParam().isCXX14OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(hasAnyCapture(lambdaCapture(capturesVar(
+  varDecl(hasName("cc"), hasInitializer(integerLiteral(equals(1;
+  EXPECT_TRUE(
+  matches("int main() { auto lambda = [cc = 1] {return cc;}; }", matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc = 2; auto lambda = [cc = 1] {return cc;}; }",
+  matcher));
+}
+
+TEST_P(ASTMatchersTest, LambdaCaptureTest_DoesNotBindToCaptureOfVarDecl) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(
+  hasAnyCapture(lambdaCapture(capturesVar(varDecl(hasName("cc"));
+  EXPECT_FALSE(matches("int main() { auto f = [](){ return 5; }; }", matcher));
+  EXPECT_FALSE(matches("int main() { int xx; auto f = [xx](){ return xx; }; }",
+   matcher));
+}
+
+TEST_P(ASTMatchersTest,
+   LambdaCaptureTest_DoesNotBindToCaptureWithInitializerAndDifferentName) {
+  if (!GetParam().isCXX14OrLater()) {
+return;
+  }
+  EXPECT_FALSE(matches(
+  "int main() { auto lambda = [xx = 1] {return xx;}; }",
+  lambdaExpr(hasAnyCapture(lambdaCapture(capturesVar(varDecl(
+  hasName("cc"), hasInitializer(integerLiteral(equals(1));
+}
+
 TEST(ASTMatchersTestObjC, ObjCMessageExpr) {
   // Don't find ObjCMessageExpr where none are present.
   EXPECT_TRUE(notMatchesObjC("", objcMessageExpr(anything(;
Index: 

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

2021-11-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.

This continues to LGTM, thank you for it!


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


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

2021-11-08 Thread James King via Phabricator via cfe-commits
jcking1034 added a comment.

@aaron.ballman Just wanted to confirm with you that the work here and release 
notes look good and can be wrapped up so that I can have @ymandel submit!


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


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

2021-11-04 Thread James King via Phabricator via cfe-commits
jcking1034 marked an inline comment as done.
jcking1034 added a comment.

@fowles @aaron.ballman I'll take a look at `forEachCapture` in the next patch. 
Also, I've discovered that the `VarDecl` node has a member function 
`isInitCapture` that seems like it could allow us to distinguish between 
captures with and without initializers (link 
)
 - I want to play around with that, as well.


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


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

2021-11-04 Thread James King via Phabricator via cfe-commits
jcking1034 marked an inline comment as done.
jcking1034 added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:223-229
+- ``LambdaCapture`` AST Matchers are now available. These matchers allow for
+  the binding of ``LambdaCapture`` nodes, and include the ``lambdaCapture``,
+  ``capturesVar``, and ``capturesThis`` matchers. In addition, the
+  ``hasAnyCapture`` matcher has been updated to accept an inner matcher of
+  type ``Matcher`` - its original interface accepted an inner
+  matcher of type ``Matcher`` or ``Matcher``, but did
+  not allow for the binding of ``LambdaCapture`` nodes.

aaron.ballman wrote:
> We should have an additional note about the removal of the old matchers.
I believe that the only change made to the old matchers is that `hasAnyCapture` 
now accepts an inner matcher of type `Matcher`, and no longer 
accepts inner matchers of type `Matcher` or `Matcher`. 
I've revised this note and broken it up into two points for clarity.


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


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

2021-11-04 Thread James King via Phabricator via cfe-commits
jcking1034 updated this revision to Diff 384835.
jcking1034 added a comment.

Update release notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112491

Files:
  clang/docs/LibASTMatchersReference.html
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/ASTTypeTraits.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/AST/ASTTypeTraits.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -563,26 +563,6 @@
   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; [](){}; }", HasCaptureX));
-  EXPECT_TRUE(notMatches("void f() { [](){}; }", HasCaptureX));
-  EXPECT_TRUE(notMatches("void f() { int z = 3; [](){}; }", 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; [](){}; }", HasCaptureThis));
-  EXPECT_TRUE(notMatches("void f() { int z = 3; [](){}; }", HasCaptureThis));
-}
-
 TEST(Matcher, MatchesMethodsOnLambda) {
   StringRef Code = R"cpp(
 struct A {
Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -2237,6 +2237,65 @@
  varDecl(hasName("ss"), hasTypeLoc(elaboratedTypeLoc();
 }
 
+TEST_P(ASTMatchersTest, LambdaCaptureTest) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [cc](){ return cc; }; }",
+  lambdaExpr(hasAnyCapture(lambdaCapture();
+}
+
+TEST_P(ASTMatchersTest, LambdaCaptureTest_BindsToCaptureOfVarDecl) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(
+  hasAnyCapture(lambdaCapture(capturesVar(varDecl(hasName("cc"));
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [cc](){ return cc; }; }",
+  matcher));
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [](){ return cc; }; }",
+  matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f = [=](){ return cc; }; }", matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f = [&](){ return cc; }; }", matcher));
+}
+
+TEST_P(ASTMatchersTest, LambdaCaptureTest_BindsToCaptureWithInitializer) {
+  if (!GetParam().isCXX14OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(hasAnyCapture(lambdaCapture(capturesVar(
+  varDecl(hasName("cc"), hasInitializer(integerLiteral(equals(1;
+  EXPECT_TRUE(
+  matches("int main() { auto lambda = [cc = 1] {return cc;}; }", matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc = 2; auto lambda = [cc = 1] {return cc;}; }",
+  matcher));
+}
+
+TEST_P(ASTMatchersTest, LambdaCaptureTest_DoesNotBindToCaptureOfVarDecl) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(
+  hasAnyCapture(lambdaCapture(capturesVar(varDecl(hasName("cc"));
+  EXPECT_FALSE(matches("int main() { auto f = [](){ return 5; }; }", matcher));
+  EXPECT_FALSE(matches("int main() { int xx; auto f = [xx](){ return xx; }; }",
+   matcher));
+}
+
+TEST_P(ASTMatchersTest,
+   LambdaCaptureTest_DoesNotBindToCaptureWithInitializerAndDifferentName) {
+  if (!GetParam().isCXX14OrLater()) {
+return;
+  }
+  EXPECT_FALSE(matches(
+  "int main() { auto lambda = [xx = 1] {return xx;}; }",
+  lambdaExpr(hasAnyCapture(lambdaCapture(capturesVar(varDecl(
+  hasName("cc"), hasInitializer(integerLiteral(equals(1));
+}
+
 TEST(ASTMatchersTestObjC, ObjCMessageExpr) {
   // Don't find ObjCMessageExpr where none are present.
   EXPECT_TRUE(notMatchesObjC("", objcMessageExpr(anything(;
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===

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

2021-11-04 Thread Matt Kulukundis via Phabricator via cfe-commits
fowles added a comment.

In D112491#3108631 , @aaron.ballman 
wrote:

> That might be good follow-on work (I wouldn't insist on it for this patch 
> though).

Completely agreed, just something that occurred to me as the next thing I will 
need when building on this.


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


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

2021-11-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D112491#3107361 , @fowles wrote:

> Do we also want a `forEachCapture` matcher?

That might be good follow-on work (I wouldn't insist on it for this patch 
though).




Comment at: clang/docs/ReleaseNotes.rst:223-229
+- ``LambdaCapture`` AST Matchers are now available. These matchers allow for
+  the binding of ``LambdaCapture`` nodes, and include the ``lambdaCapture``,
+  ``capturesVar``, and ``capturesThis`` matchers. In addition, the
+  ``hasAnyCapture`` matcher has been updated to accept an inner matcher of
+  type ``Matcher`` - its original interface accepted an inner
+  matcher of type ``Matcher`` or ``Matcher``, but did
+  not allow for the binding of ``LambdaCapture`` nodes.

We should have an additional note about the removal of the old matchers.


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


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

2021-11-03 Thread Matt Kulukundis via Phabricator via cfe-commits
fowles added a comment.

Do we also want a `forEachCapture` matcher?


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


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

2021-11-03 Thread James King via Phabricator via cfe-commits
jcking1034 updated this revision to Diff 384578.
jcking1034 added a comment.

Rebase onto main.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112491

Files:
  clang/docs/LibASTMatchersReference.html
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/ASTTypeTraits.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/AST/ASTTypeTraits.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -563,26 +563,6 @@
   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; [](){}; }", HasCaptureX));
-  EXPECT_TRUE(notMatches("void f() { [](){}; }", HasCaptureX));
-  EXPECT_TRUE(notMatches("void f() { int z = 3; [](){}; }", 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; [](){}; }", HasCaptureThis));
-  EXPECT_TRUE(notMatches("void f() { int z = 3; [](){}; }", HasCaptureThis));
-}
-
 TEST(Matcher, MatchesMethodsOnLambda) {
   StringRef Code = R"cpp(
 struct A {
Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -2237,6 +2237,65 @@
  varDecl(hasName("ss"), hasTypeLoc(elaboratedTypeLoc();
 }
 
+TEST_P(ASTMatchersTest, LambdaCaptureTest) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [cc](){ return cc; }; }",
+  lambdaExpr(hasAnyCapture(lambdaCapture();
+}
+
+TEST_P(ASTMatchersTest, LambdaCaptureTest_BindsToCaptureOfVarDecl) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(
+  hasAnyCapture(lambdaCapture(capturesVar(varDecl(hasName("cc"));
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [cc](){ return cc; }; }",
+  matcher));
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [](){ return cc; }; }",
+  matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f = [=](){ return cc; }; }", matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f = [&](){ return cc; }; }", matcher));
+}
+
+TEST_P(ASTMatchersTest, LambdaCaptureTest_BindsToCaptureWithInitializer) {
+  if (!GetParam().isCXX14OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(hasAnyCapture(lambdaCapture(capturesVar(
+  varDecl(hasName("cc"), hasInitializer(integerLiteral(equals(1;
+  EXPECT_TRUE(
+  matches("int main() { auto lambda = [cc = 1] {return cc;}; }", matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc = 2; auto lambda = [cc = 1] {return cc;}; }",
+  matcher));
+}
+
+TEST_P(ASTMatchersTest, LambdaCaptureTest_DoesNotBindToCaptureOfVarDecl) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(
+  hasAnyCapture(lambdaCapture(capturesVar(varDecl(hasName("cc"));
+  EXPECT_FALSE(matches("int main() { auto f = [](){ return 5; }; }", matcher));
+  EXPECT_FALSE(matches("int main() { int xx; auto f = [xx](){ return xx; }; }",
+   matcher));
+}
+
+TEST_P(ASTMatchersTest,
+   LambdaCaptureTest_DoesNotBindToCaptureWithInitializerAndDifferentName) {
+  if (!GetParam().isCXX14OrLater()) {
+return;
+  }
+  EXPECT_FALSE(matches(
+  "int main() { auto lambda = [xx = 1] {return xx;}; }",
+  lambdaExpr(hasAnyCapture(lambdaCapture(capturesVar(varDecl(
+  hasName("cc"), hasInitializer(integerLiteral(equals(1));
+}
+
 TEST(ASTMatchersTestObjC, ObjCMessageExpr) {
   // Don't find ObjCMessageExpr where none are present.
   EXPECT_TRUE(notMatchesObjC("", objcMessageExpr(anything(;
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- 

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

2021-11-02 Thread James King via Phabricator via cfe-commits
jcking1034 updated this revision to Diff 384114.
jcking1034 added a comment.

Update documentation for `capturesVar` matcher.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112491

Files:
  clang/docs/LibASTMatchersReference.html
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/ASTTypeTraits.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/AST/ASTTypeTraits.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -563,26 +563,6 @@
   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; [](){}; }", HasCaptureX));
-  EXPECT_TRUE(notMatches("void f() { [](){}; }", HasCaptureX));
-  EXPECT_TRUE(notMatches("void f() { int z = 3; [](){}; }", 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; [](){}; }", HasCaptureThis));
-  EXPECT_TRUE(notMatches("void f() { int z = 3; [](){}; }", HasCaptureThis));
-}
-
 TEST(Matcher, MatchesMethodsOnLambda) {
   StringRef Code = R"cpp(
 struct A {
Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -2237,6 +2237,65 @@
  varDecl(hasName("ss"), hasTypeLoc(elaboratedTypeLoc();
 }
 
+TEST_P(ASTMatchersTest, LambdaCaptureTest) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [cc](){ return cc; }; }",
+  lambdaExpr(hasAnyCapture(lambdaCapture();
+}
+
+TEST_P(ASTMatchersTest, LambdaCaptureTest_BindsToCaptureOfVarDecl) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(
+  hasAnyCapture(lambdaCapture(capturesVar(varDecl(hasName("cc"));
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [cc](){ return cc; }; }",
+  matcher));
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [](){ return cc; }; }",
+  matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f = [=](){ return cc; }; }", matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f = [&](){ return cc; }; }", matcher));
+}
+
+TEST_P(ASTMatchersTest, LambdaCaptureTest_BindsToCaptureWithInitializer) {
+  if (!GetParam().isCXX14OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(hasAnyCapture(lambdaCapture(capturesVar(
+  varDecl(hasName("cc"), hasInitializer(integerLiteral(equals(1;
+  EXPECT_TRUE(
+  matches("int main() { auto lambda = [cc = 1] {return cc;}; }", matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc = 2; auto lambda = [cc = 1] {return cc;}; }",
+  matcher));
+}
+
+TEST_P(ASTMatchersTest, LambdaCaptureTest_DoesNotBindToCaptureOfVarDecl) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(
+  hasAnyCapture(lambdaCapture(capturesVar(varDecl(hasName("cc"));
+  EXPECT_FALSE(matches("int main() { auto f = [](){ return 5; }; }", matcher));
+  EXPECT_FALSE(matches("int main() { int xx; auto f = [xx](){ return xx; }; }",
+   matcher));
+}
+
+TEST_P(ASTMatchersTest,
+   LambdaCaptureTest_DoesNotBindToCaptureWithInitializerAndDifferentName) {
+  if (!GetParam().isCXX14OrLater()) {
+return;
+  }
+  EXPECT_FALSE(matches(
+  "int main() { auto lambda = [xx = 1] {return xx;}; }",
+  lambdaExpr(hasAnyCapture(lambdaCapture(capturesVar(varDecl(
+  hasName("cc"), hasInitializer(integerLiteral(equals(1));
+}
+
 TEST(ASTMatchersTestObjC, ObjCMessageExpr) {
   // Don't find ObjCMessageExpr where none are present.
   EXPECT_TRUE(notMatchesObjC("", objcMessageExpr(anything(;
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

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

2021-11-02 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Thanks for the changes, this looks great to me now.




Comment at: clang/docs/LibASTMatchersReference.html:8368
+lambdaExpr(hasAnyCapture(lambdaCapture(capturesVar(hasName("x",
+capturesVar(hasName("x")) matches `int x` and `x = 1`.
 

fowles wrote:
> I think this should be "matches `x` and `x = 1`
either that or `... hasName("x") matches ...`

Maybe it's worth explicitly saying in the doc text something like "this can be 
a separate variable captured by value or reference, or a synthesized variable 
if the capture has an initializer".

(There may end up being some confusion about the fact that these variables 
don't appear in AST dumps, but I don't think that's your problem to deal with 
here)


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


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

2021-11-01 Thread Matt Kulukundis via Phabricator via cfe-commits
fowles added a comment.

This is great!




Comment at: clang/docs/LibASTMatchersReference.html:8368
+lambdaExpr(hasAnyCapture(lambdaCapture(capturesVar(hasName("x",
+capturesVar(hasName("x")) matches `int x` and `x = 1`.
 

I think this should be "matches `x` and `x = 1`


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


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

2021-11-01 Thread James King via Phabricator via cfe-commits
jcking1034 updated this revision to Diff 383823.
jcking1034 marked 4 inline comments as done.
jcking1034 added a comment.

Update missed names; remove original implementations of `hasAnyCapture`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112491

Files:
  clang/docs/LibASTMatchersReference.html
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/ASTTypeTraits.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/AST/ASTTypeTraits.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -563,26 +563,6 @@
   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; [](){}; }", HasCaptureX));
-  EXPECT_TRUE(notMatches("void f() { [](){}; }", HasCaptureX));
-  EXPECT_TRUE(notMatches("void f() { int z = 3; [](){}; }", 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; [](){}; }", HasCaptureThis));
-  EXPECT_TRUE(notMatches("void f() { int z = 3; [](){}; }", HasCaptureThis));
-}
-
 TEST(Matcher, MatchesMethodsOnLambda) {
   StringRef Code = R"cpp(
 struct A {
Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -2237,6 +2237,65 @@
  varDecl(hasName("ss"), hasTypeLoc(elaboratedTypeLoc();
 }
 
+TEST_P(ASTMatchersTest, LambdaCaptureTest) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [cc](){ return cc; }; }",
+  lambdaExpr(hasAnyCapture(lambdaCapture();
+}
+
+TEST_P(ASTMatchersTest, LambdaCaptureTest_BindsToCaptureOfVarDecl) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(
+  hasAnyCapture(lambdaCapture(capturesVar(varDecl(hasName("cc"));
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [cc](){ return cc; }; }",
+  matcher));
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [](){ return cc; }; }",
+  matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f = [=](){ return cc; }; }", matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f = [&](){ return cc; }; }", matcher));
+}
+
+TEST_P(ASTMatchersTest, LambdaCaptureTest_BindsToCaptureWithInitializer) {
+  if (!GetParam().isCXX14OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(hasAnyCapture(lambdaCapture(capturesVar(
+  varDecl(hasName("cc"), hasInitializer(integerLiteral(equals(1;
+  EXPECT_TRUE(
+  matches("int main() { auto lambda = [cc = 1] {return cc;}; }", matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc = 2; auto lambda = [cc = 1] {return cc;}; }",
+  matcher));
+}
+
+TEST_P(ASTMatchersTest, LambdaCaptureTest_DoesNotBindToCaptureOfVarDecl) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(
+  hasAnyCapture(lambdaCapture(capturesVar(varDecl(hasName("cc"));
+  EXPECT_FALSE(matches("int main() { auto f = [](){ return 5; }; }", matcher));
+  EXPECT_FALSE(matches("int main() { int xx; auto f = [xx](){ return xx; }; }",
+   matcher));
+}
+
+TEST_P(ASTMatchersTest,
+   LambdaCaptureTest_DoesNotBindToCaptureWithInitializerAndDifferentName) {
+  if (!GetParam().isCXX14OrLater()) {
+return;
+  }
+  EXPECT_FALSE(matches(
+  "int main() { auto lambda = [xx = 1] {return xx;}; }",
+  lambdaExpr(hasAnyCapture(lambdaCapture(capturesVar(varDecl(
+  hasName("cc"), hasInitializer(integerLiteral(equals(1));
+}
+
 TEST(ASTMatchersTestObjC, ObjCMessageExpr) {
   // Don't find ObjCMessageExpr where none are present.
   EXPECT_TRUE(notMatchesObjC("", objcMessageExpr(anything(;
Index: 

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

2021-11-01 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added inline comments.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:4630-4632
+/// lambdaExpr(hasAnyCapture(lambdaCapture(refersToVarDecl(hasName("x",
+/// refersToVarDecl(hasName("x")) matches `int x` and `x = 1`.
+AST_MATCHER_P(LambdaCapture, capturesVar, internal::Matcher,

update to new matcher name.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:4650
+/// \endcode
+/// lambdaExpr(hasAnyCapture(lambdaCapture(refersToThis(
+///   matches `[this]() { return cc; }`.





Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:4629-4630
+///   matches `[x](){}`.
+AST_MATCHER_P(LambdaCapture, refersToVarDecl, internal::Matcher,
+  InnerMatcher) {
+  auto *capturedVar = Node.getCapturedVar();

aaron.ballman wrote:
> jcking1034 wrote:
> > aaron.ballman wrote:
> > > 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.
> > For the example you've provided, these can be matched with the 
> > `refersToVarDecl` matcher, as seen in the test 
> > `LambdaCaptureTest_BindsToCaptureWithInitializer`. I've gone ahead and 
> > updated the documentation to include an example with an initializer.
> > 
> > Having that limitation with initializer representation is definitely a 
> > concern, though. Looking through the [[ 
> > https://clang.llvm.org/doxygen/LambdaCapture_8h_source.html | source ]] for 
> > the `LambdaCapture` class, the documentation for the `DeclAndBits` (line 
> > 42-48) suggests that there isn't a distinguishment between the two cases. 
> > However, do you think it's feasible to update the classes related to 
> > `LambdaCapture` obtain and store this information (possibly through 
> > updating the `LambdaCaptureKind` enum, updating the constructor/fields of 
> > the class, etc)?
> > However, do you think it's feasible to update the classes related 

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

2021-11-01 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment.

+1 to removing the old versions of these matchers.


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


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

2021-11-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D112491#3096935 , @sammccall wrote:

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

I did some looking around to see if I could find any uses of the matcher in the 
wild, and I can't spot any that aren't in a Clang fork. I think there's plenty 
of time for us to get feedback from early adopters if removing the old 
interface causes severe pain for anyone. So I'm on board with replacing the old 
APIs, but we should definitely have a release note explaining what's happening.


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


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

2021-10-29 Thread James King via Phabricator via cfe-commits
jcking1034 added inline comments.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:4652
+///   matches `[this]() { return cc; }`.
+AST_MATCHER(LambdaCapture, refersToThis) { return Node.capturesThis(); }
+

sammccall wrote:
> Again, why `refersToThis` rather than `capturesThis`, which is more specific 
> and matches the AST?
Initially, I think I saw that there were a few `TemplateArgument` matchers of 
the form `refersToX`, so I wanted to maintain some consistency with that. But I 
think I your suggestion makes things clearer, so will opt for that.


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


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

2021-10-29 Thread James King via Phabricator via cfe-commits
jcking1034 updated this revision to Diff 383458.
jcking1034 marked 2 inline comments as done.
jcking1034 added a comment.

Update matcher names for clarity.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112491

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/AST/ASTTypeTraits.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/AST/ASTTypeTraits.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -2237,6 +2237,66 @@
  varDecl(hasName("ss"), hasTypeLoc(elaboratedTypeLoc();
 }
 
+TEST_P(ASTMatchersTest, LambdaCaptureTest) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [cc](){ return cc; }; }",
+  lambdaExpr(hasAnyCapture(lambdaCapture();
+}
+
+TEST_P(ASTMatchersTest, LambdaCaptureTest_BindsToCaptureReferringToVarDecl) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(
+  hasAnyCapture(lambdaCapture(capturesVar(varDecl(hasName("cc"));
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [cc](){ return cc; }; }",
+  matcher));
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [](){ return cc; }; }",
+  matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f = [=](){ return cc; }; }", matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f = [&](){ return cc; }; }", matcher));
+}
+
+TEST_P(ASTMatchersTest, LambdaCaptureTest_BindsToCaptureWithInitializer) {
+  if (!GetParam().isCXX14OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(hasAnyCapture(lambdaCapture(capturesVar(
+  varDecl(hasName("cc"), hasInitializer(integerLiteral(equals(1;
+  EXPECT_TRUE(
+  matches("int main() { auto lambda = [cc = 1] {return cc;}; }", matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc = 2; auto lambda = [cc = 1] {return cc;}; }",
+  matcher));
+}
+
+TEST_P(ASTMatchersTest,
+   LambdaCaptureTest_DoesNotBindToCaptureReferringToVarDecl) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(
+  hasAnyCapture(lambdaCapture(capturesVar(varDecl(hasName("cc"));
+  EXPECT_FALSE(matches("int main() { auto f = [](){ return 5; }; }", matcher));
+  EXPECT_FALSE(matches("int main() { int xx; auto f = [xx](){ return xx; }; }",
+   matcher));
+}
+
+TEST_P(ASTMatchersTest,
+   LambdaCaptureTest_DoesNotBindToCaptureWithInitializerAndDifferentName) {
+  if (!GetParam().isCXX14OrLater()) {
+return;
+  }
+  EXPECT_FALSE(matches(
+  "int main() { auto lambda = [xx = 1] {return xx;}; }",
+  lambdaExpr(hasAnyCapture(lambdaCapture(capturesVar(varDecl(
+  hasName("cc"), hasInitializer(integerLiteral(equals(1));
+}
+
 TEST(ASTMatchersTestObjC, ObjCMessageExpr) {
   // Don't find ObjCMessageExpr where none are present.
   EXPECT_TRUE(notMatchesObjC("", objcMessageExpr(anything(;
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -4445,5 +4445,42 @@
   cxxRecordDecl(hasName("Derived"),
 hasDirectBase(hasType(cxxRecordDecl(hasName("Base")));
 }
+
+TEST_P(ASTMatchersTest, RefersToThis) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(hasAnyCapture(lambdaCapture(capturesThis(;
+  EXPECT_TRUE(matches("class C { int cc; int f() { auto l = [this](){ return "
+  "cc; }; return l(); } };",
+  matcher));
+  EXPECT_TRUE(matches("class C { int cc; int f() { auto l = [=](){ return cc; "
+  "}; return l(); } };",
+  matcher));
+  EXPECT_TRUE(matches("class C { int cc; int f() { auto l = [&](){ return cc; "
+  "}; return l(); } };",
+  matcher));
+  EXPECT_FALSE(matches("class C { int cc; int f() { auto l = [cc](){ return "
+   "cc; }; return l(); } };",
+   matcher));
+  EXPECT_FALSE(matches("class C { int this; int f() { auto l = [this](){ "
+   "return this; }; return l(); } };",
+   matcher));
+}
+
+TEST_P(ASTMatchersTest, IsImplicit_LambdaCapture) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(hasAnyCapture(
+  

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

2021-10-29 Thread 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,
+  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

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


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

2021-10-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a reviewer: sammccall.
aaron.ballman added a subscriber: sammccall.
aaron.ballman added a comment.

In general, I'm happy with this. Adding @sammccall in case I've missed anything 
regarding the AST matcher internals or design concerns.




Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:4629-4630
+///   matches `[x](){}`.
+AST_MATCHER_P(LambdaCapture, refersToVarDecl, internal::Matcher,
+  InnerMatcher) {
+  auto *capturedVar = Node.getCapturedVar();

jcking1034 wrote:
> aaron.ballman wrote:
> > 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.
> For the example you've provided, these can be matched with the 
> `refersToVarDecl` matcher, as seen in the test 
> `LambdaCaptureTest_BindsToCaptureWithInitializer`. I've gone ahead and 
> updated the documentation to include an example with an initializer.
> 
> Having that limitation with initializer representation is definitely a 
> concern, though. Looking through the [[ 
> https://clang.llvm.org/doxygen/LambdaCapture_8h_source.html | source ]] for 
> the `LambdaCapture` class, the documentation for the `DeclAndBits` (line 
> 42-48) suggests that there isn't a distinguishment between the two cases. 
> However, do you think it's feasible to update the classes related to 
> `LambdaCapture` obtain and store this information (possibly through updating 
> the `LambdaCaptureKind` enum, updating the constructor/fields of the class, 
> etc)?
> However, do you think it's feasible to update the classes related to 
> LambdaCapture obtain and store this information (possibly through updating 
> the LambdaCaptureKind enum, updating the constructor/fields of the class, 
> etc)?

I think that would make sense (thought perhaps as an orthogonal patch). That we 
don't track init captures seems like a deficiency of the AST to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112491

___

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

2021-10-28 Thread James King via Phabricator via cfe-commits
jcking1034 updated this revision to Diff 383128.
jcking1034 added a comment.

Regenerate documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112491

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/AST/ASTTypeTraits.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/AST/ASTTypeTraits.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -2237,6 +2237,66 @@
  varDecl(hasName("ss"), hasTypeLoc(elaboratedTypeLoc();
 }
 
+TEST_P(ASTMatchersTest, LambdaCaptureTest) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [cc](){ return cc; }; }",
+  lambdaExpr(hasAnyCapture(lambdaCapture();
+}
+
+TEST_P(ASTMatchersTest, LambdaCaptureTest_BindsToCaptureReferringToVarDecl) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(
+  hasAnyCapture(lambdaCapture(refersToVarDecl(varDecl(hasName("cc"));
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [cc](){ return cc; }; }",
+  matcher));
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [](){ return cc; }; }",
+  matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f = [=](){ return cc; }; }", matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f = [&](){ return cc; }; }", matcher));
+}
+
+TEST_P(ASTMatchersTest, LambdaCaptureTest_BindsToCaptureWithInitializer) {
+  if (!GetParam().isCXX14OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(hasAnyCapture(lambdaCapture(refersToVarDecl(
+  varDecl(hasName("cc"), hasInitializer(integerLiteral(equals(1;
+  EXPECT_TRUE(
+  matches("int main() { auto lambda = [cc = 1] {return cc;}; }", matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc = 2; auto lambda = [cc = 1] {return cc;}; }",
+  matcher));
+}
+
+TEST_P(ASTMatchersTest,
+   LambdaCaptureTest_DoesNotBindToCaptureReferringToVarDecl) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(
+  hasAnyCapture(lambdaCapture(refersToVarDecl(varDecl(hasName("cc"));
+  EXPECT_FALSE(matches("int main() { auto f = [](){ return 5; }; }", matcher));
+  EXPECT_FALSE(matches("int main() { int xx; auto f = [xx](){ return xx; }; }",
+   matcher));
+}
+
+TEST_P(ASTMatchersTest,
+   LambdaCaptureTest_DoesNotBindToCaptureWithInitializerAndDifferentName) {
+  if (!GetParam().isCXX14OrLater()) {
+return;
+  }
+  EXPECT_FALSE(matches(
+  "int main() { auto lambda = [xx = 1] {return xx;}; }",
+  lambdaExpr(hasAnyCapture(lambdaCapture(refersToVarDecl(varDecl(
+  hasName("cc"), hasInitializer(integerLiteral(equals(1));
+}
+
 TEST(ASTMatchersTestObjC, ObjCMessageExpr) {
   // Don't find ObjCMessageExpr where none are present.
   EXPECT_TRUE(notMatchesObjC("", objcMessageExpr(anything(;
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -4445,5 +4445,42 @@
   cxxRecordDecl(hasName("Derived"),
 hasDirectBase(hasType(cxxRecordDecl(hasName("Base")));
 }
+
+TEST_P(ASTMatchersTest, RefersToThis) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(hasAnyCapture(lambdaCapture(refersToThis(;
+  EXPECT_TRUE(matches("class C { int cc; int f() { auto l = [this](){ return "
+  "cc; }; return l(); } };",
+  matcher));
+  EXPECT_TRUE(matches("class C { int cc; int f() { auto l = [=](){ return cc; "
+  "}; return l(); } };",
+  matcher));
+  EXPECT_TRUE(matches("class C { int cc; int f() { auto l = [&](){ return cc; "
+  "}; return l(); } };",
+  matcher));
+  EXPECT_FALSE(matches("class C { int cc; int f() { auto l = [cc](){ return "
+   "cc; }; return l(); } };",
+   matcher));
+  EXPECT_FALSE(matches("class C { int this; int f() { auto l = [this](){ "
+   "return this; }; return l(); } };",
+   matcher));
+}
+
+TEST_P(ASTMatchersTest, IsImplicit_LambdaCapture) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(hasAnyCapture(
+  lambdaCapture(isImplicit(), 

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

2021-10-28 Thread James King via Phabricator via cfe-commits
jcking1034 updated this revision to Diff 383126.
jcking1034 added a comment.

Deprecate original overloads of `hasAnyCapture`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112491

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/AST/ASTTypeTraits.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/AST/ASTTypeTraits.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -2237,6 +2237,66 @@
  varDecl(hasName("ss"), hasTypeLoc(elaboratedTypeLoc();
 }
 
+TEST_P(ASTMatchersTest, LambdaCaptureTest) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [cc](){ return cc; }; }",
+  lambdaExpr(hasAnyCapture(lambdaCapture();
+}
+
+TEST_P(ASTMatchersTest, LambdaCaptureTest_BindsToCaptureReferringToVarDecl) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(
+  hasAnyCapture(lambdaCapture(refersToVarDecl(varDecl(hasName("cc"));
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [cc](){ return cc; }; }",
+  matcher));
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [](){ return cc; }; }",
+  matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f = [=](){ return cc; }; }", matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f = [&](){ return cc; }; }", matcher));
+}
+
+TEST_P(ASTMatchersTest, LambdaCaptureTest_BindsToCaptureWithInitializer) {
+  if (!GetParam().isCXX14OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(hasAnyCapture(lambdaCapture(refersToVarDecl(
+  varDecl(hasName("cc"), hasInitializer(integerLiteral(equals(1;
+  EXPECT_TRUE(
+  matches("int main() { auto lambda = [cc = 1] {return cc;}; }", matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc = 2; auto lambda = [cc = 1] {return cc;}; }",
+  matcher));
+}
+
+TEST_P(ASTMatchersTest,
+   LambdaCaptureTest_DoesNotBindToCaptureReferringToVarDecl) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(
+  hasAnyCapture(lambdaCapture(refersToVarDecl(varDecl(hasName("cc"));
+  EXPECT_FALSE(matches("int main() { auto f = [](){ return 5; }; }", matcher));
+  EXPECT_FALSE(matches("int main() { int xx; auto f = [xx](){ return xx; }; }",
+   matcher));
+}
+
+TEST_P(ASTMatchersTest,
+   LambdaCaptureTest_DoesNotBindToCaptureWithInitializerAndDifferentName) {
+  if (!GetParam().isCXX14OrLater()) {
+return;
+  }
+  EXPECT_FALSE(matches(
+  "int main() { auto lambda = [xx = 1] {return xx;}; }",
+  lambdaExpr(hasAnyCapture(lambdaCapture(refersToVarDecl(varDecl(
+  hasName("cc"), hasInitializer(integerLiteral(equals(1));
+}
+
 TEST(ASTMatchersTestObjC, ObjCMessageExpr) {
   // Don't find ObjCMessageExpr where none are present.
   EXPECT_TRUE(notMatchesObjC("", objcMessageExpr(anything(;
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -4445,5 +4445,42 @@
   cxxRecordDecl(hasName("Derived"),
 hasDirectBase(hasType(cxxRecordDecl(hasName("Base")));
 }
+
+TEST_P(ASTMatchersTest, RefersToThis) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(hasAnyCapture(lambdaCapture(refersToThis(;
+  EXPECT_TRUE(matches("class C { int cc; int f() { auto l = [this](){ return "
+  "cc; }; return l(); } };",
+  matcher));
+  EXPECT_TRUE(matches("class C { int cc; int f() { auto l = [=](){ return cc; "
+  "}; return l(); } };",
+  matcher));
+  EXPECT_TRUE(matches("class C { int cc; int f() { auto l = [&](){ return cc; "
+  "}; return l(); } };",
+  matcher));
+  EXPECT_FALSE(matches("class C { int cc; int f() { auto l = [cc](){ return "
+   "cc; }; return l(); } };",
+   matcher));
+  EXPECT_FALSE(matches("class C { int this; int f() { auto l = [this](){ "
+   "return this; }; return l(); } };",
+   matcher));
+}
+
+TEST_P(ASTMatchersTest, IsImplicit_LambdaCapture) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(hasAnyCapture(
+  

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

2021-10-28 Thread James King via Phabricator via cfe-commits
jcking1034 marked an inline comment as not done.
jcking1034 added a comment.

I agree that having two ways to match the same thing is a usability concern and 
could definitely be confusing. Deprecating non-bindable matchers could be a 
possibility and is probably the right way to go if we choose to include these 
matchers, but I'm not sure of if doing so will have any side effects.




Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:4629-4630
+///   matches `[x](){}`.
+AST_MATCHER_P(LambdaCapture, refersToVarDecl, internal::Matcher,
+  InnerMatcher) {
+  auto *capturedVar = Node.getCapturedVar();

aaron.ballman wrote:
> 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.
For the example you've provided, these can be matched with the 
`refersToVarDecl` matcher, as seen in the test 
`LambdaCaptureTest_BindsToCaptureWithInitializer`. I've gone ahead and updated 
the documentation to include an example with an initializer.

Having that limitation with initializer representation is definitely a concern, 
though. Looking through the [[ 
https://clang.llvm.org/doxygen/LambdaCapture_8h_source.html | source ]] for the 
`LambdaCapture` class, the documentation for the `DeclAndBits` (line 42-48) 
suggests that there isn't a distinguishment between the two cases. However, do 
you think it's feasible to update the classes related to `LambdaCapture` obtain 
and store this information (possibly through updating the `LambdaCaptureKind` 
enum, updating the constructor/fields of the class, etc)?


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


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

2021-10-28 Thread James King via Phabricator via cfe-commits
jcking1034 updated this revision to Diff 383115.
jcking1034 added a comment.

Update comment with additional example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112491

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/AST/ASTTypeTraits.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/AST/ASTTypeTraits.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -2237,6 +2237,66 @@
  varDecl(hasName("ss"), hasTypeLoc(elaboratedTypeLoc();
 }
 
+TEST_P(ASTMatchersTest, LambdaCaptureTest) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [cc](){ return cc; }; }",
+  lambdaExpr(hasAnyCapture(lambdaCapture();
+}
+
+TEST_P(ASTMatchersTest, LambdaCaptureTest_BindsToCaptureReferringToVarDecl) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(
+  hasAnyCapture(lambdaCapture(refersToVarDecl(varDecl(hasName("cc"));
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [cc](){ return cc; }; }",
+  matcher));
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [](){ return cc; }; }",
+  matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f = [=](){ return cc; }; }", matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f = [&](){ return cc; }; }", matcher));
+}
+
+TEST_P(ASTMatchersTest, LambdaCaptureTest_BindsToCaptureWithInitializer) {
+  if (!GetParam().isCXX14OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(hasAnyCapture(lambdaCapture(refersToVarDecl(
+  varDecl(hasName("cc"), hasInitializer(integerLiteral(equals(1;
+  EXPECT_TRUE(
+  matches("int main() { auto lambda = [cc = 1] {return cc;}; }", matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc = 2; auto lambda = [cc = 1] {return cc;}; }",
+  matcher));
+}
+
+TEST_P(ASTMatchersTest,
+   LambdaCaptureTest_DoesNotBindToCaptureReferringToVarDecl) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(
+  hasAnyCapture(lambdaCapture(refersToVarDecl(varDecl(hasName("cc"));
+  EXPECT_FALSE(matches("int main() { auto f = [](){ return 5; }; }", matcher));
+  EXPECT_FALSE(matches("int main() { int xx; auto f = [xx](){ return xx; }; }",
+   matcher));
+}
+
+TEST_P(ASTMatchersTest,
+   LambdaCaptureTest_DoesNotBindToCaptureWithInitializerAndDifferentName) {
+  if (!GetParam().isCXX14OrLater()) {
+return;
+  }
+  EXPECT_FALSE(matches(
+  "int main() { auto lambda = [xx = 1] {return xx;}; }",
+  lambdaExpr(hasAnyCapture(lambdaCapture(refersToVarDecl(varDecl(
+  hasName("cc"), hasInitializer(integerLiteral(equals(1));
+}
+
 TEST(ASTMatchersTestObjC, ObjCMessageExpr) {
   // Don't find ObjCMessageExpr where none are present.
   EXPECT_TRUE(notMatchesObjC("", objcMessageExpr(anything(;
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -4445,5 +4445,42 @@
   cxxRecordDecl(hasName("Derived"),
 hasDirectBase(hasType(cxxRecordDecl(hasName("Base")));
 }
+
+TEST_P(ASTMatchersTest, RefersToThis) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(hasAnyCapture(lambdaCapture(refersToThis(;
+  EXPECT_TRUE(matches("class C { int cc; int f() { auto l = [this](){ return "
+  "cc; }; return l(); } };",
+  matcher));
+  EXPECT_TRUE(matches("class C { int cc; int f() { auto l = [=](){ return cc; "
+  "}; return l(); } };",
+  matcher));
+  EXPECT_TRUE(matches("class C { int cc; int f() { auto l = [&](){ return cc; "
+  "}; return l(); } };",
+  matcher));
+  EXPECT_FALSE(matches("class C { int cc; int f() { auto l = [cc](){ return "
+   "cc; }; return l(); } };",
+   matcher));
+  EXPECT_FALSE(matches("class C { int this; int f() { auto l = [this](){ "
+   "return this; }; return l(); } };",
+   matcher));
+}
+
+TEST_P(ASTMatchersTest, IsImplicit_LambdaCapture) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(hasAnyCapture(
+  

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

2021-10-27 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In 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,
+  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


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

2021-10-26 Thread James King via Phabricator via cfe-commits
jcking1034 added a comment.

@aaron.ballman for the purpose of these matchers, what @ymandel said is correct 
- the goal is to allow `LambdaCapture`s themselves to be bindable.




Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:4629-4630
+///   matches `[x](){}`.
+AST_MATCHER_P(LambdaCapture, refersToVarDecl, internal::Matcher,
+  InnerMatcher) {
+  auto *capturedVar = Node.getCapturedVar();

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.


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


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

2021-10-26 Thread James King via Phabricator via cfe-commits
jcking1034 updated this revision to Diff 382410.
jcking1034 added a comment.

Update comments and tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112491

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/AST/ASTTypeTraits.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/AST/ASTTypeTraits.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -2237,6 +2237,66 @@
  varDecl(hasName("ss"), hasTypeLoc(elaboratedTypeLoc();
 }
 
+TEST_P(ASTMatchersTest, LambdaCaptureTest) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [cc](){ return cc; }; }",
+  lambdaExpr(hasAnyCapture(lambdaCapture();
+}
+
+TEST_P(ASTMatchersTest, LambdaCaptureTest_BindsToCaptureReferringToVarDecl) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(
+  hasAnyCapture(lambdaCapture(refersToVarDecl(varDecl(hasName("cc"));
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [cc](){ return cc; }; }",
+  matcher));
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [](){ return cc; }; }",
+  matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f = [=](){ return cc; }; }", matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f = [&](){ return cc; }; }", matcher));
+}
+
+TEST_P(ASTMatchersTest, LambdaCaptureTest_BindsToCaptureWithInitializer) {
+  if (!GetParam().isCXX14OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(hasAnyCapture(lambdaCapture(refersToVarDecl(
+  varDecl(hasName("cc"), hasInitializer(integerLiteral(equals(1;
+  EXPECT_TRUE(
+  matches("int main() { auto lambda = [cc = 1] {return cc;}; }", matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc = 2; auto lambda = [cc = 1] {return cc;}; }",
+  matcher));
+}
+
+TEST_P(ASTMatchersTest,
+   LambdaCaptureTest_DoesNotBindToCaptureReferringToVarDecl) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(
+  hasAnyCapture(lambdaCapture(refersToVarDecl(varDecl(hasName("cc"));
+  EXPECT_FALSE(matches("int main() { auto f = [](){ return 5; }; }", matcher));
+  EXPECT_FALSE(matches("int main() { int xx; auto f = [xx](){ return xx; }; }",
+   matcher));
+}
+
+TEST_P(ASTMatchersTest,
+   LambdaCaptureTest_DoesNotBindToCaptureWithInitializerAndDifferentName) {
+  if (!GetParam().isCXX14OrLater()) {
+return;
+  }
+  EXPECT_FALSE(matches(
+  "int main() { auto lambda = [xx = 1] {return xx;}; }",
+  lambdaExpr(hasAnyCapture(lambdaCapture(refersToVarDecl(varDecl(
+  hasName("cc"), hasInitializer(integerLiteral(equals(1));
+}
+
 TEST(ASTMatchersTestObjC, ObjCMessageExpr) {
   // Don't find ObjCMessageExpr where none are present.
   EXPECT_TRUE(notMatchesObjC("", objcMessageExpr(anything(;
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -4445,5 +4445,42 @@
   cxxRecordDecl(hasName("Derived"),
 hasDirectBase(hasType(cxxRecordDecl(hasName("Base")));
 }
+
+TEST_P(ASTMatchersTest, RefersToThis) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(hasAnyCapture(lambdaCapture(refersToThis(;
+  EXPECT_TRUE(matches("class C { int cc; int f() { auto l = [this](){ return "
+  "cc; }; return l(); } };",
+  matcher));
+  EXPECT_TRUE(matches("class C { int cc; int f() { auto l = [=](){ return cc; "
+  "}; return l(); } };",
+  matcher));
+  EXPECT_TRUE(matches("class C { int cc; int f() { auto l = [&](){ return cc; "
+  "}; return l(); } };",
+  matcher));
+  EXPECT_FALSE(matches("class C { int cc; int f() { auto l = [cc](){ return "
+   "cc; }; return l(); } };",
+   matcher));
+  EXPECT_FALSE(matches("class C { int this; int f() { auto l = [this](){ "
+   "return this; }; return l(); } };",
+   matcher));
+}
+
+TEST_P(ASTMatchersTest, IsImplicit_LambdaCapture) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(hasAnyCapture(
+  lambdaCapture(isImplicit(), 

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

2021-10-26 Thread James King via Phabricator via cfe-commits
jcking1034 updated this revision to Diff 382397.
jcking1034 marked an inline comment as done.
jcking1034 added a comment.

Update comment with example


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112491

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/AST/ASTTypeTraits.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/AST/ASTTypeTraits.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -2237,6 +2237,52 @@
  varDecl(hasName("ss"), hasTypeLoc(elaboratedTypeLoc();
 }
 
+TEST_P(ASTMatchersTest, LambdaCaptureTest) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [cc](){ return cc; }; }",
+  lambdaExpr(hasAnyCapture(lambdaCapture();
+}
+
+TEST_P(ASTMatchersTest, LambdaCaptureTest_BindsToCaptureReferringToVarDecl) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(
+  hasAnyCapture(lambdaCapture(refersToVarDecl(varDecl(hasName("cc"));
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [cc](){ return cc; }; }",
+  matcher));
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [](){ return cc; }; }",
+  matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f = [=](){ return cc; }; }", matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f = [&](){ return cc; }; }", matcher));
+  if (!GetParam().isCXX14OrLater()) {
+return;
+  }
+  EXPECT_TRUE(
+  matches("int main() { auto lambda = [cc = 1] {return cc;}; }", matcher));
+}
+
+TEST_P(ASTMatchersTest,
+   LambdaCaptureTest_DoesNotBindToCaptureReferringToVarDecl) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(
+  hasAnyCapture(lambdaCapture(refersToVarDecl(varDecl(hasName("cc"));
+  EXPECT_FALSE(matches("int main() { auto f = [](){ return 5; }; }", matcher));
+  EXPECT_FALSE(matches("int main() { int xx; auto f = [xx](){ return xx; }; }",
+   matcher));
+  if (!GetParam().isCXX14OrLater()) {
+return;
+  }
+  EXPECT_FALSE(
+  matches("int main() { auto lambda = [xx = 1] {return xx;}; }", matcher));
+}
+
 TEST(ASTMatchersTestObjC, ObjCMessageExpr) {
   // Don't find ObjCMessageExpr where none are present.
   EXPECT_TRUE(notMatchesObjC("", objcMessageExpr(anything(;
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -4445,5 +4445,42 @@
   cxxRecordDecl(hasName("Derived"),
 hasDirectBase(hasType(cxxRecordDecl(hasName("Base")));
 }
+
+TEST_P(ASTMatchersTest, RefersToThis) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(hasAnyCapture(lambdaCapture(refersToThis(;
+  EXPECT_TRUE(matches("class C { int cc; int f() { auto l = [this](){ return "
+  "cc; }; return l(); } };",
+  matcher));
+  EXPECT_TRUE(matches("class C { int cc; int f() { auto l = [=](){ return cc; "
+  "}; return l(); } };",
+  matcher));
+  EXPECT_TRUE(matches("class C { int cc; int f() { auto l = [&](){ return cc; "
+  "}; return l(); } };",
+  matcher));
+  EXPECT_FALSE(matches("class C { int cc; int f() { auto l = [cc](){ return "
+   "cc; }; return l(); } };",
+   matcher));
+  EXPECT_FALSE(matches("class C { int this; int f() { auto l = [this](){ "
+   "return this; }; return l(); } };",
+   matcher));
+}
+
+TEST_P(ASTMatchersTest, IsImplicit_LambdaCapture) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(hasAnyCapture(
+  lambdaCapture(isImplicit(), refersToVarDecl(varDecl(hasName("cc"));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f = [&](){ return cc; }; }", matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f = [=](){ return cc; }; }", matcher));
+  EXPECT_FALSE(matches("int main() { int cc; auto f = [cc](){ return cc; }; }",
+   matcher));
+}
+
 } // namespace ast_matchers
 } // namespace clang
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- 

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

2021-10-26 Thread James King via Phabricator via cfe-commits
jcking1034 updated this revision to Diff 382391.
jcking1034 marked 3 inline comments as done.
jcking1034 added a comment.

Revert changes to ASTMatchFinder


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112491

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/AST/ASTTypeTraits.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/AST/ASTTypeTraits.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -2237,6 +2237,52 @@
  varDecl(hasName("ss"), hasTypeLoc(elaboratedTypeLoc();
 }
 
+TEST_P(ASTMatchersTest, LambdaCaptureTest) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [cc](){ return cc; }; }",
+  lambdaExpr(hasAnyCapture(lambdaCapture();
+}
+
+TEST_P(ASTMatchersTest, LambdaCaptureTest_BindsToCaptureReferringToVarDecl) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(
+  hasAnyCapture(lambdaCapture(refersToVarDecl(varDecl(hasName("cc"));
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [cc](){ return cc; }; }",
+  matcher));
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [](){ return cc; }; }",
+  matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f = [=](){ return cc; }; }", matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f = [&](){ return cc; }; }", matcher));
+  if (!GetParam().isCXX14OrLater()) {
+return;
+  }
+  EXPECT_TRUE(
+  matches("int main() { auto lambda = [cc = 1] {return cc;}; }", matcher));
+}
+
+TEST_P(ASTMatchersTest,
+   LambdaCaptureTest_DoesNotBindToCaptureReferringToVarDecl) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(
+  hasAnyCapture(lambdaCapture(refersToVarDecl(varDecl(hasName("cc"));
+  EXPECT_FALSE(matches("int main() { auto f = [](){ return 5; }; }", matcher));
+  EXPECT_FALSE(matches("int main() { int xx; auto f = [xx](){ return xx; }; }",
+   matcher));
+  if (!GetParam().isCXX14OrLater()) {
+return;
+  }
+  EXPECT_FALSE(
+  matches("int main() { auto lambda = [xx = 1] {return xx;}; }", matcher));
+}
+
 TEST(ASTMatchersTestObjC, ObjCMessageExpr) {
   // Don't find ObjCMessageExpr where none are present.
   EXPECT_TRUE(notMatchesObjC("", objcMessageExpr(anything(;
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -4445,5 +4445,42 @@
   cxxRecordDecl(hasName("Derived"),
 hasDirectBase(hasType(cxxRecordDecl(hasName("Base")));
 }
+
+TEST_P(ASTMatchersTest, RefersToThis) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(hasAnyCapture(lambdaCapture(refersToThis(;
+  EXPECT_TRUE(matches("class C { int cc; int f() { auto l = [this](){ return "
+  "cc; }; return l(); } };",
+  matcher));
+  EXPECT_TRUE(matches("class C { int cc; int f() { auto l = [=](){ return cc; "
+  "}; return l(); } };",
+  matcher));
+  EXPECT_TRUE(matches("class C { int cc; int f() { auto l = [&](){ return cc; "
+  "}; return l(); } };",
+  matcher));
+  EXPECT_FALSE(matches("class C { int cc; int f() { auto l = [cc](){ return "
+   "cc; }; return l(); } };",
+   matcher));
+  EXPECT_FALSE(matches("class C { int this; int f() { auto l = [this](){ "
+   "return this; }; return l(); } };",
+   matcher));
+}
+
+TEST_P(ASTMatchersTest, IsImplicit_LambdaCapture) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(hasAnyCapture(
+  lambdaCapture(isImplicit(), refersToVarDecl(varDecl(hasName("cc"));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f = [&](){ return cc; }; }", matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f = [=](){ return cc; }; }", matcher));
+  EXPECT_FALSE(matches("int main() { int cc; auto f = [cc](){ return cc; }; }",
+   matcher));
+}
+
 } // namespace ast_matchers
 } // namespace clang
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- 

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

2021-10-26 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
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 , MatchCallback 
*Action);
   void addMatcher(const AttrMatcher , 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> Attr;
+std::vector> 
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,
+  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


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

2021-10-26 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

I'm a bit confused as to why this is necessary. We already have 
`lambdaExpr(hasAnyCapture(varDecl(hasName("whatever"` and 
`lambdaExpr(hasAnyCapture(cxxThisExpr()))`, so I'm not certain why we need this 
as a parallel construct?




Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:4629-4630
+///   matches `[x](){}`.
+AST_MATCHER_P(LambdaCapture, refersToVarDecl, internal::Matcher,
+  InnerMatcher) {
+  auto *capturedVar = Node.getCapturedVar();

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


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


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

2021-10-25 Thread James King via Phabricator via cfe-commits
jcking1034 created this revision.
jcking1034 added reviewers: ymandel, aaron.ballman.
jcking1034 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This provides better support for `LambdaCapture`s, and implements several
`LambdaCapture`-related matchers. This does not update how lambdas are
traversed. As a result, something like trying to match `lambdaCapture()` by
itself will not work - it must be used as an inner matcher.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112491

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/AST/ASTTypeTraits.h
  clang/include/clang/ASTMatchers/ASTMatchFinder.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/AST/ASTTypeTraits.cpp
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -2237,6 +2237,52 @@
  varDecl(hasName("ss"), hasTypeLoc(elaboratedTypeLoc();
 }
 
+TEST_P(ASTMatchersTest, LambdaCaptureTest) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [cc](){ return cc; }; }",
+  lambdaExpr(hasAnyCapture(lambdaCapture();
+}
+
+TEST_P(ASTMatchersTest, LambdaCaptureTest_BindsToCaptureReferringToVarDecl) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(
+  hasAnyCapture(lambdaCapture(refersToVarDecl(varDecl(hasName("cc"));
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [cc](){ return cc; }; }",
+  matcher));
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [](){ return cc; }; }",
+  matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f = [=](){ return cc; }; }", matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f = [&](){ return cc; }; }", matcher));
+  if (!GetParam().isCXX14OrLater()) {
+return;
+  }
+  EXPECT_TRUE(
+  matches("int main() { auto lambda = [cc = 1] {return cc;}; }", matcher));
+}
+
+TEST_P(ASTMatchersTest,
+   LambdaCaptureTest_DoesNotBindToCaptureReferringToVarDecl) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(
+  hasAnyCapture(lambdaCapture(refersToVarDecl(varDecl(hasName("cc"));
+  EXPECT_FALSE(matches("int main() { auto f = [](){ return 5; }; }", matcher));
+  EXPECT_FALSE(matches("int main() { int xx; auto f = [xx](){ return xx; }; }",
+   matcher));
+  if (!GetParam().isCXX14OrLater()) {
+return;
+  }
+  EXPECT_FALSE(
+  matches("int main() { auto lambda = [xx = 1] {return xx;}; }", matcher));
+}
+
 TEST(ASTMatchersTestObjC, ObjCMessageExpr) {
   // Don't find ObjCMessageExpr where none are present.
   EXPECT_TRUE(notMatchesObjC("", objcMessageExpr(anything(;
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -4445,5 +4445,42 @@
   cxxRecordDecl(hasName("Derived"),
 hasDirectBase(hasType(cxxRecordDecl(hasName("Base")));
 }
+
+TEST_P(ASTMatchersTest, RefersToThis) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(hasAnyCapture(lambdaCapture(refersToThis(;
+  EXPECT_TRUE(matches("class C { int cc; int f() { auto l = [this](){ return "
+  "cc; }; return l(); } };",
+  matcher));
+  EXPECT_TRUE(matches("class C { int cc; int f() { auto l = [=](){ return cc; "
+  "}; return l(); } };",
+  matcher));
+  EXPECT_TRUE(matches("class C { int cc; int f() { auto l = [&](){ return cc; "
+  "}; return l(); } };",
+  matcher));
+  EXPECT_FALSE(matches("class C { int cc; int f() { auto l = [cc](){ return "
+   "cc; }; return l(); } };",
+   matcher));
+  EXPECT_FALSE(matches("class C { int this; int f() { auto l = [this](){ "
+   "return this; }; return l(); } };",
+   matcher));
+}
+
+TEST_P(ASTMatchersTest, IsImplicit_LambdaCapture) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(hasAnyCapture(
+  lambdaCapture(isImplicit(), refersToVarDecl(varDecl(hasName("cc"));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f = [&](){ return cc; }; }", matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f =