Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang][ASTMatcher] Add matchers for isExplicitObjectMemberFunction() #84446

Merged

Conversation

steakhal
Copy link
Contributor

@steakhal steakhal commented Mar 8, 2024

Note that this patch will be necessary to fix forEachArgumentWithParam() and forEachArgumentWithParamType() matchers for deducing "this"; which is my true motivation. There the bug is that with explicit obj params, one should not adjust the number of arguments in presence of CXXMethodDecls, and this causes a mismatch there mapping the argument to the wrong param. But, I'll come back there once we have this matcher.

@steakhal steakhal added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Mar 8, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 8, 2024

@llvm/pr-subscribers-clang

Author: Balazs Benics (steakhal)

Changes

Note that this patch will be necessary to fix forEachArgumentWithParam() and forEachArgumentWithParamType() matchers for deducing "this"; which is my true motivation.


Full diff: https://github.com/llvm/llvm-project/pull/84446.diff

10 Files Affected:

  • (modified) clang/docs/LibASTMatchersReference.html (+15)
  • (modified) clang/docs/ReleaseNotes.rst (+1)
  • (modified) clang/include/clang/ASTMatchers/ASTMatchers.h (+19)
  • (modified) clang/include/clang/Testing/CommandLineArgs.h (+1)
  • (modified) clang/include/clang/Testing/TestClangConfig.h (+11-5)
  • (modified) clang/lib/ASTMatchers/Dynamic/Registry.cpp (+1)
  • (modified) clang/lib/Testing/CommandLineArgs.cpp (+7)
  • (modified) clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp (+14)
  • (modified) clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp (+1-1)
  • (modified) clang/unittests/ASTMatchers/ASTMatchersTest.h (+10-4)
diff --git a/clang/docs/LibASTMatchersReference.html b/clang/docs/LibASTMatchersReference.html
index 8a06084955aa6b..bb1b68f6671b1a 100644
--- a/clang/docs/LibASTMatchersReference.html
+++ b/clang/docs/LibASTMatchersReference.html
@@ -3546,6 +3546,21 @@ <h2 id="narrowing-matchers">Narrowing Matchers</h2>
 </pre></td></tr>
 
 
+<tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1CXXMethodDecl.html">CXXMethodDecl</a>&gt;</td><td class="name" onclick="toggle('isExplicitObjectMemberFunction0')"><a name="isExplicitObjectMemberFunction0Anchor">isExplicitObjectMemberFunction</a></td><td></td></tr>
+<tr><td colspan="4" class="doc" id="isExplicitObjectMemberFunction0"><pre>Matches if the given method declaration declares a member function with an explicit object parameter.
+
+Given
+struct A {
+  int operator-(this A, int);
+  void fun(this A &&self);
+  static int operator()(int);
+  int operator+(int);
+};
+
+cxxMethodDecl(isExplicitObjectMemberFunction()) matches the first two methods but not the last two.
+</pre></td></tr>
+
+
 <tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1CXXMethodDecl.html">CXXMethodDecl</a>&gt;</td><td class="name" onclick="toggle('isCopyAssignmentOperator0')"><a name="isCopyAssignmentOperator0Anchor">isCopyAssignmentOperator</a></td><td></td></tr>
 <tr><td colspan="4" class="doc" id="isCopyAssignmentOperator0"><pre>Matches if the given method declaration declares a copy assignment
 operator.
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 942820a5268576..898380f94b7956 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -415,6 +415,7 @@ AST Matchers
 ------------
 
 - ``isInStdNamespace`` now supports Decl declared with ``extern "C++"``.
+- Add ``isExplicitObjectMemberFunction``.
 
 clang-format
 ------------
diff --git a/clang/include/clang/ASTMatchers/ASTMatchers.h b/clang/include/clang/ASTMatchers/ASTMatchers.h
index ced89ff127ab3e..96dbcdc344e131 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchers.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -6366,6 +6366,25 @@ AST_MATCHER(CXXMethodDecl, isConst) {
   return Node.isConst();
 }
 
+/// Matches if the given method declaration declares a member function with an
+/// explicit object parameter.
+///
+/// Given
+/// \code
+/// struct A {
+///  int operator-(this A, int);
+///  void fun(this A &&self);
+///  static int operator()(int);
+///  int operator+(int);
+/// };
+/// \endcode
+///
+/// cxxMethodDecl(isExplicitObjectMemberFunction()) matches the first two
+/// methods but not the last two.
+AST_MATCHER(CXXMethodDecl, isExplicitObjectMemberFunction) {
+  return Node.isExplicitObjectMemberFunction();
+}
+
 /// Matches if the given method declaration declares a copy assignment
 /// operator.
 ///
diff --git a/clang/include/clang/Testing/CommandLineArgs.h b/clang/include/clang/Testing/CommandLineArgs.h
index 4dd28718dfa67c..e71907e8bbd0c6 100644
--- a/clang/include/clang/Testing/CommandLineArgs.h
+++ b/clang/include/clang/Testing/CommandLineArgs.h
@@ -28,6 +28,7 @@ enum TestLanguage {
   Lang_CXX14,
   Lang_CXX17,
   Lang_CXX20,
+  Lang_CXX23,
   Lang_OpenCL,
   Lang_OBJC,
   Lang_OBJCXX
diff --git a/clang/include/clang/Testing/TestClangConfig.h b/clang/include/clang/Testing/TestClangConfig.h
index 92d5cc3cff994f..1b4efca80e9d47 100644
--- a/clang/include/clang/Testing/TestClangConfig.h
+++ b/clang/include/clang/Testing/TestClangConfig.h
@@ -34,24 +34,30 @@ struct TestClangConfig {
   bool isCXX() const {
     return Language == Lang_CXX03 || Language == Lang_CXX11 ||
            Language == Lang_CXX14 || Language == Lang_CXX17 ||
-           Language == Lang_CXX20;
+           Language == Lang_CXX20 || Language == Lang_CXX23;
   }
 
   bool isCXX11OrLater() const {
     return Language == Lang_CXX11 || Language == Lang_CXX14 ||
-           Language == Lang_CXX17 || Language == Lang_CXX20;
+           Language == Lang_CXX17 || Language == Lang_CXX20 ||
+           Language == Lang_CXX23;
   }
 
   bool isCXX14OrLater() const {
     return Language == Lang_CXX14 || Language == Lang_CXX17 ||
-           Language == Lang_CXX20;
+           Language == Lang_CXX20 || Language == Lang_CXX23;
   }
 
   bool isCXX17OrLater() const {
-    return Language == Lang_CXX17 || Language == Lang_CXX20;
+    return Language == Lang_CXX17 || Language == Lang_CXX20 ||
+           Language == Lang_CXX23;
   }
 
-  bool isCXX20OrLater() const { return Language == Lang_CXX20; }
+  bool isCXX20OrLater() const {
+    return Language == Lang_CXX20 || Language == Lang_CXX23;
+  }
+
+  bool isCXX23OrLater() const { return Language == Lang_CXX23; }
 
   bool supportsCXXDynamicExceptionSpecification() const {
     return Language == Lang_CXX03 || Language == Lang_CXX11 ||
diff --git a/clang/lib/ASTMatchers/Dynamic/Registry.cpp b/clang/lib/ASTMatchers/Dynamic/Registry.cpp
index 15dad022df5fe0..2c75e6beb74301 100644
--- a/clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ b/clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -432,6 +432,7 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(isExpansionInMainFile);
   REGISTER_MATCHER(isExpansionInSystemHeader);
   REGISTER_MATCHER(isExplicit);
+  REGISTER_MATCHER(isExplicitObjectMemberFunction);
   REGISTER_MATCHER(isExplicitTemplateSpecialization);
   REGISTER_MATCHER(isExpr);
   REGISTER_MATCHER(isExternC);
diff --git a/clang/lib/Testing/CommandLineArgs.cpp b/clang/lib/Testing/CommandLineArgs.cpp
index 0da087c33e3f4e..3abc689b93e8d0 100644
--- a/clang/lib/Testing/CommandLineArgs.cpp
+++ b/clang/lib/Testing/CommandLineArgs.cpp
@@ -37,6 +37,9 @@ std::vector<std::string> getCommandLineArgsForTesting(TestLanguage Lang) {
   case Lang_CXX20:
     Args = {"-std=c++20", "-frtti"};
     break;
+  case Lang_CXX23:
+    Args = {"-std=c++23", "-frtti"};
+    break;
   case Lang_OBJC:
     Args = {"-x", "objective-c", "-frtti", "-fobjc-nonfragile-abi"};
     break;
@@ -73,6 +76,9 @@ std::vector<std::string> getCC1ArgsForTesting(TestLanguage Lang) {
   case Lang_CXX20:
     Args = {"-std=c++20"};
     break;
+  case Lang_CXX23:
+    Args = {"-std=c++23"};
+    break;
   case Lang_OBJC:
     Args = {"-xobjective-c"};
     break;
@@ -96,6 +102,7 @@ StringRef getFilenameForTesting(TestLanguage Lang) {
   case Lang_CXX14:
   case Lang_CXX17:
   case Lang_CXX20:
+  case Lang_CXX23:
     return "input.cc";
 
   case Lang_OpenCL:
diff --git a/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp b/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
index b75da7bc1ed069..87774b00956a5a 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ b/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2107,6 +2107,20 @@ TEST_P(ASTMatchersTest, IsPure) {
   EXPECT_TRUE(notMatches("class X { int f(); };", cxxMethodDecl(isPure())));
 }
 
+TEST_P(ASTMatchersTest, IsExplicitObjectMemberFunction) {
+  if (!GetParam().isCXX23OrLater()) {
+    return;
+  }
+
+  auto ExpObjParamFn = cxxMethodDecl(isExplicitObjectMemberFunction());
+  EXPECT_TRUE(
+      notMatches("struct A { static int operator()(int); };", ExpObjParamFn));
+  EXPECT_TRUE(notMatches("struct A { int operator+(int); };", ExpObjParamFn));
+  EXPECT_TRUE(
+      matches("struct A { int operator-(this A, int); };", ExpObjParamFn));
+  EXPECT_TRUE(matches("struct A { void fun(this A &&self); };", ExpObjParamFn));
+}
+
 TEST_P(ASTMatchersTest, IsCopyAssignmentOperator) {
   if (!GetParam().isCXX()) {
     return;
diff --git a/clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp b/clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
index ae30c03126d768..0edc65162fbe3f 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ b/clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -2754,7 +2754,7 @@ TEST(MatchFinderAPI, MatchesDynamic) {
 static std::vector<TestClangConfig> allTestClangConfigs() {
   std::vector<TestClangConfig> all_configs;
   for (TestLanguage lang : {Lang_C89, Lang_C99, Lang_CXX03, Lang_CXX11,
-                            Lang_CXX14, Lang_CXX17, Lang_CXX20}) {
+                            Lang_CXX14, Lang_CXX17, Lang_CXX20, Lang_CXX23}) {
     TestClangConfig config;
     config.Language = lang;
 
diff --git a/clang/unittests/ASTMatchers/ASTMatchersTest.h b/clang/unittests/ASTMatchers/ASTMatchersTest.h
index 79c6186054839c..1ed1b5958a8b3a 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersTest.h
+++ b/clang/unittests/ASTMatchers/ASTMatchersTest.h
@@ -62,22 +62,28 @@ class VerifyMatch : public MatchFinder::MatchCallback {
 
 inline ArrayRef<TestLanguage> langCxx11OrLater() {
   static const TestLanguage Result[] = {Lang_CXX11, Lang_CXX14, Lang_CXX17,
-                                        Lang_CXX20};
+                                        Lang_CXX20, Lang_CXX23};
   return ArrayRef<TestLanguage>(Result);
 }
 
 inline ArrayRef<TestLanguage> langCxx14OrLater() {
-  static const TestLanguage Result[] = {Lang_CXX14, Lang_CXX17, Lang_CXX20};
+  static const TestLanguage Result[] = {Lang_CXX14, Lang_CXX17, Lang_CXX20,
+                                        Lang_CXX23};
   return ArrayRef<TestLanguage>(Result);
 }
 
 inline ArrayRef<TestLanguage> langCxx17OrLater() {
-  static const TestLanguage Result[] = {Lang_CXX17, Lang_CXX20};
+  static const TestLanguage Result[] = {Lang_CXX17, Lang_CXX20, Lang_CXX23};
   return ArrayRef<TestLanguage>(Result);
 }
 
 inline ArrayRef<TestLanguage> langCxx20OrLater() {
-  static const TestLanguage Result[] = {Lang_CXX20};
+  static const TestLanguage Result[] = {Lang_CXX20, Lang_CXX23};
+  return ArrayRef<TestLanguage>(Result);
+}
+
+inline ArrayRef<TestLanguage> langCxx23OrLater() {
+  static const TestLanguage Result[] = {Lang_CXX23};
   return ArrayRef<TestLanguage>(Result);
 }
 

Copy link
Member

@PiotrZSL PiotrZSL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@steakhal
Copy link
Contributor Author

steakhal commented Mar 8, 2024

Thanks for the prompt review @PiotrZSL!
I plan to merge it by the end of the day, unless someone objects.

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@steakhal steakhal merged commit 7457e2c into llvm:main Mar 8, 2024
8 checks passed
@steakhal steakhal deleted the add-matcher-for-isExplicitObjectMemberFunction branch March 8, 2024 12:57
@steakhal
Copy link
Contributor Author

steakhal commented Mar 8, 2024

I had to push a fixup commit, because I forgot about one switch-case. f07157e
I pushed it already to fix the build bot, but let me know if you disagree with the fix.

steakhal added a commit to steakhal/llvm-project that referenced this pull request Mar 12, 2024
…" operator calls

This is a follow-up commit of llvm#84446.
In this patch, I demonstrate that `forEachArgumentWithParam` and
`forEachArgumentWithParamType` did not correctly handle the presence of
the explicit object parameter for operator calls.

Prior to this patch, the matcher would skip the first (and only) argument
of the operator call if the explicit object param was used.

Note that I had to move the definition of `isExplicitObjectMemberFunction`,
to be declared before the matcher I fix to be visible.
steakhal added a commit that referenced this pull request Mar 12, 2024
…" operator calls (#84887)

This is a follow-up commit of #84446.
In this patch, I demonstrate that `forEachArgumentWithParam` and
`forEachArgumentWithParamType` did not correctly handle the presence of
the explicit object parameter for operator calls.

Prior to this patch, the matcher would skip the first (and only)
argument of the operator call if the explicit object param was used.

Note that I had to move the definition of
`isExplicitObjectMemberFunction`, to be declared before the matcher I
fix to be visible.

I also had to do some gymnastics for passing the language standard
version command-line flags to the invocation as
`matchAndVerifyResultTrue` wasn't really considered for non-c++11 code.
See the that it always prepends `-std=gnu++11` to the command-line
arguments. I workarounded it by accepting extra args, which get
appended, thus possibly overriding the hardcoded arguments.

I'm not sure if this qualifies for backporting to clang-18 (probably not
because its not a crash, but a semantic problem), but I figure it might
be useful for some vendors (like us).
But we are also happy to cherry-pick this fix to downstream. Let me know
if you want this to be backported or not.

CPP-5074
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants