-
Notifications
You must be signed in to change notification settings - Fork 12.4k
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 a matcher for the name of a DependentScopeDeclRefExpr #121656
[Clang][ASTMatcher] Add a matcher for the name of a DependentScopeDeclRefExpr #121656
Conversation
@llvm/pr-subscribers-clang Author: Amr Hesham (AmrDeveloper) ChangesAdd the Example: Fixes: #121610 Full diff: https://github.com/llvm/llvm-project/pull/121656.diff 5 Files Affected:
diff --git a/clang/docs/LibASTMatchersReference.html b/clang/docs/LibASTMatchersReference.html
index fc557888013254..6a03aeb5eec2a1 100644
--- a/clang/docs/LibASTMatchersReference.html
+++ b/clang/docs/LibASTMatchersReference.html
@@ -3449,6 +3449,19 @@ <h2 id="narrowing-matchers">Narrowing Matchers</h2>
</pre></td></tr>
+<tr><td>Matcher<<a href="https://clang.llvm.org/doxygen/classclang_1_1DependentScopeDeclRefExpr.html">DependentScopeDeclRefExpr</a>></td><td class="name" onclick="toggle('hasDependentName0')"><a name="hasDependentName0Anchor">hasDependentName</a></td><td>std::string N</td></tr>
+<tr><td colspan="4" class="doc" id="hasDependentName0"><pre>Matches the dependent name of a dependent scope decl ref expr.
+
+Matches the dependent name of a dependent scope decl ref expr
+
+Given:
+
+ template <class T< class X : T { void f() { T::v; } };
+
+dependentScopeDeclRefExpr(hasDependentName("v")) matches `T::v`
+</pre></td></tr>
+
+
<tr><td>Matcher<<a href="https://clang.llvm.org/doxygen/classclang_1_1CXXDependentScopeMemberExpr.html">CXXDependentScopeMemberExpr</a>></td><td class="name" onclick="toggle('memberHasSameNameAsBoundNode0')"><a name="memberHasSameNameAsBoundNode0Anchor">memberHasSameNameAsBoundNode</a></td><td>std::string BindingID</td></tr>
<tr><td colspan="4" class="doc" id="memberHasSameNameAsBoundNode0"><pre>Matches template-dependent, but known, member names against an already-bound
node
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 5e75fc447636e0..4ef69ca4c743ed 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1116,6 +1116,8 @@ AST Matchers
- Add ``dependentTemplateSpecializationType`` matcher to match a dependent template specialization type.
+- Add ``hasDependentName`` matcher to match the dependent name of a dependent scope decl ref expr.
+
clang-format
------------
diff --git a/clang/include/clang/ASTMatchers/ASTMatchers.h b/clang/include/clang/ASTMatchers/ASTMatchers.h
index dd0fedb2cda2d4..6828fc6da1d5cf 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchers.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -3257,6 +3257,17 @@ AST_MATCHER_P(CXXDependentScopeMemberExpr, memberHasSameNameAsBoundNode,
});
}
+/// Matches the dependent name of a dependent scope decl ref expr
+///
+/// Given:
+/// \code
+/// template <class T> class X : T { void f() { T::v; } };
+/// \endcode
+/// \c dependentScopeDeclRefExpr(hasDependentName("v")) matches `T::v`
+AST_MATCHER_P(DependentScopeDeclRefExpr, hasDependentName, std::string, N) {
+ return Node.getDeclName().getAsString() == N;
+}
+
/// Matches C++ classes that are directly or indirectly derived from a class
/// matching \c Base, or Objective-C classes that directly or indirectly
/// subclass a class matching \c Base.
diff --git a/clang/lib/ASTMatchers/Dynamic/Registry.cpp b/clang/lib/ASTMatchers/Dynamic/Registry.cpp
index 97e6bbc093fe46..336d3a14f79559 100644
--- a/clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ b/clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -314,6 +314,7 @@ RegistryMaps::RegistryMaps() {
REGISTER_MATCHER(hasDeducedType);
REGISTER_MATCHER(hasDefaultArgument);
REGISTER_MATCHER(hasDefinition);
+ REGISTER_MATCHER(hasDependentName);
REGISTER_MATCHER(hasDescendant);
REGISTER_MATCHER(hasDestinationType);
REGISTER_MATCHER(hasDirectBase);
diff --git a/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp b/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
index 056b7c7b571ef4..4278e3d4fe5959 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ b/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2235,6 +2235,21 @@ TEST_P(ASTMatchersTest, ArgumentCountIs_CXXConstructExpr) {
Constructor1Arg));
}
+TEST_P(ASTMatchersTest, hasDependentName_DependentScopeDeclRefExpr) {
+ if (!GetParam().isCXX() || GetParam().hasDelayedTemplateParsing()) {
+ // FIXME: Fix this test to work with delayed template parsing.
+ return;
+ }
+
+ EXPECT_TRUE(matches("template <class T> class X : T { void f() { T::v; } };",
+ dependentScopeDeclRefExpr(hasDependentName("v"))));
+
+ EXPECT_TRUE(
+ matches("template <typename T> struct S { static T Foo; };"
+ "template <typename T> void declToImport() { (void)S<T>::Foo; }",
+ dependentScopeDeclRefExpr(hasDependentName("Foo"))));
+}
+
TEST(ASTMatchersTest, NamesMember_CXXDependentScopeMemberExpr) {
// Member functions:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for implementing this!
The patch looks pretty good, I just have some minor comments and a suggestion for a new test case.
@@ -2235,6 +2235,21 @@ TEST_P(ASTMatchersTest, ArgumentCountIs_CXXConstructExpr) { | |||
Constructor1Arg)); | |||
} | |||
|
|||
TEST_P(ASTMatchersTest, hasDependentName_DependentScopeDeclRefExpr) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: looking at other tests in this file, the test names start with a capital letter (e.g. HasSize_CXX
) even if the matcher they are testing starts with lowercase (e.g. hasSize
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
EXPECT_TRUE(matches("template <class T> class X : T { void f() { T::v; } };", | ||
dependentScopeDeclRefExpr(hasDependentName("v")))); | ||
|
||
EXPECT_TRUE( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to have a test case where the dependent name refers to a function (specifically a static member function, since I think that will get a DependentScopeDeclRefExpr
rather than a CXXDepdendentScopeMemberExpr
).
Come to think of it, this applies for the test for dependentScopeDeclRefExpr
itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, thank you for suggesting it :D
@@ -3257,6 +3257,17 @@ AST_MATCHER_P(CXXDependentScopeMemberExpr, memberHasSameNameAsBoundNode, | |||
}); | |||
} | |||
|
|||
/// Matches the dependent name of a dependent scope decl ref expr |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
likewise here, "of a DependentScopeDeclRefExpr"
clang/docs/ReleaseNotes.rst
Outdated
@@ -1116,6 +1116,8 @@ AST Matchers | |||
|
|||
- Add ``dependentTemplateSpecializationType`` matcher to match a dependent template specialization type. | |||
|
|||
- Add ``hasDependentName`` matcher to match the dependent name of a dependent scope decl ref expr. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
likewise here, "of a DependentScopeDeclRefExpr"
@@ -3449,6 +3449,19 @@ <h2 id="narrowing-matchers">Narrowing Matchers</h2> | |||
</pre></td></tr> | |||
|
|||
|
|||
<tr><td>Matcher<<a href="https://clang.llvm.org/doxygen/classclang_1_1DependentScopeDeclRefExpr.html">DependentScopeDeclRefExpr</a>></td><td class="name" onclick="toggle('hasDependentName0')"><a name="hasDependentName0Anchor">hasDependentName</a></td><td>std::string N</td></tr> | |||
<tr><td colspan="4" class="doc" id="hasDependentName0"><pre>Matches the dependent name of a dependent scope decl ref expr. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: "of a DependentScopeDeclRefExpr" would read better to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! The update looks good, I have one more minor suggestion for a change which I'll just go ahead and make before merging.
dependentScopeDeclRefExpr(hasDependentName("Foo")))); | ||
|
||
EXPECT_TRUE(matches("template <typename T> struct S { static T foo(); };" | ||
"template <typename T> void x() { S<T>::foo; }", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: it would make the example more realistic if the function is actually called, as in S<T>::foo();
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/76/builds/5845 Here is the relevant piece of the build log for the reference
|
Thank you |
We can also create a new matcher for If confirmed I can work on it too :D Thank you |
I don't think That said, it would be useful to have a matcher for the name of a |
Thanks for the explanation, I am interested to try to play around with it and will update you with result, |
Add the
hasDependentName
matcher to match the name ofDependentScopeDeclRefExpr
Example:
dependentScopeDeclRefExpr(hasDependentName("name"))
Fixes: #121610