-
Notifications
You must be signed in to change notification settings - Fork 12.3k
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 dependentScopeDeclRefExpr
Matcher
#120996
[Clang][ASTMatcher] Add dependentScopeDeclRefExpr
Matcher
#120996
Conversation
@llvm/pr-subscribers-clang Author: Amr Hesham (AmrDeveloper) ChangesAdd AST Matcher for Fixes: #120937 Full diff: https://github.com/llvm/llvm-project/pull/120996.diff 7 Files Affected:
diff --git a/clang/docs/LibASTMatchersReference.html b/clang/docs/LibASTMatchersReference.html
index f18e9cf1341696..ddc99020604c94 100644
--- a/clang/docs/LibASTMatchersReference.html
+++ b/clang/docs/LibASTMatchersReference.html
@@ -1842,6 +1842,12 @@ <h2 id="decl-matchers">Node Matchers</h2>
if (x) {}
</pre></td></tr>
+<tr><td>Matcher<<a href="https://clang.llvm.org/doxygen/classclang_1_1Stmt.html">Stmt</a>></td><td class="name" onclick="toggle('dependentScopeDeclRefExpr0')"><a name="dependentScopeDeclRefExpr0Anchor">dependentScopeDeclRefExpr</a></td><td>Matcher<<a href="https://clang.llvm.org/doxygen/classclang_1_1DependentScopeDeclRefExpr.html">DependentScopeDeclRefExpr</a>>...</td></tr>
+<tr><td colspan="4" class="doc" id="dependentScopeDeclRefExpr0"><pre>Matches expressions that refer to dependent scope declarations.
+
+Example matches T::v
+ template <class T> class X : T { void f() { T::v; } };
+</pre></td></tr>
<tr><td>Matcher<<a href="https://clang.llvm.org/doxygen/classclang_1_1Stmt.html">Stmt</a>></td><td class="name" onclick="toggle('declStmt0')"><a name="declStmt0Anchor">declStmt</a></td><td>Matcher<<a href="https://clang.llvm.org/doxygen/classclang_1_1DeclStmt.html">DeclStmt</a>>...</td></tr>
<tr><td colspan="4" class="doc" id="declStmt0"><pre>Matches declaration statements.
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8b984ecaefecaf..2a84a4704f0c21 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1101,6 +1101,8 @@ AST Matchers
- Ensure ``pointee`` matches Objective-C pointer types.
+- Add ``dependentScopeDeclRefExpr`` matcher to match expressions that refer to dependent scope declarations.
+
clang-format
------------
diff --git a/clang/include/clang/ASTMatchers/ASTMatchers.h b/clang/include/clang/ASTMatchers/ASTMatchers.h
index 897aa25dc95cc1..fd8d9c99eca163 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchers.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -2125,6 +2125,15 @@ extern const internal::VariadicDynCastAllOfMatcher<Stmt, Expr> expr;
extern const internal::VariadicDynCastAllOfMatcher<Stmt, DeclRefExpr>
declRefExpr;
+/// Matches expressions that refer to dependent scope declarations.
+///
+/// example matches T::v;
+/// \code
+/// template <class T> class X : T { void f() { T::v; } };
+/// \endcode
+extern const internal::VariadicDynCastAllOfMatcher<Stmt, DependentScopeDeclRefExpr>
+ dependentScopeDeclRefExpr;
+
/// Matches a reference to an ObjCIvar.
///
/// Example: matches "a" in "init" method:
diff --git a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
index bf9dc5f2373f9e..ebe30f97b08268 100644
--- a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -924,6 +924,7 @@ const internal::VariadicDynCastAllOfMatcher<Stmt, CXXRewrittenBinaryOperator>
const internal::VariadicDynCastAllOfMatcher<Stmt, CXXFoldExpr> cxxFoldExpr;
const internal::VariadicDynCastAllOfMatcher<Stmt, Expr> expr;
const internal::VariadicDynCastAllOfMatcher<Stmt, DeclRefExpr> declRefExpr;
+const internal::VariadicDynCastAllOfMatcher<Stmt, DependentScopeDeclRefExpr> dependentScopeDeclRefExpr;
const internal::VariadicDynCastAllOfMatcher<Stmt, ObjCIvarRefExpr> objcIvarRefExpr;
const internal::VariadicDynCastAllOfMatcher<Stmt, BlockExpr> blockExpr;
const internal::VariadicDynCastAllOfMatcher<Stmt, IfStmt> ifStmt;
diff --git a/clang/lib/ASTMatchers/Dynamic/Registry.cpp b/clang/lib/ASTMatchers/Dynamic/Registry.cpp
index 837633fb2f0601..685d626d2978bf 100644
--- a/clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ b/clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -222,6 +222,7 @@ RegistryMaps::RegistryMaps() {
REGISTER_MATCHER(decompositionDecl);
REGISTER_MATCHER(declCountIs);
REGISTER_MATCHER(declRefExpr);
+ REGISTER_MATCHER(dependentScopeDeclRefExpr);
REGISTER_MATCHER(declStmt);
REGISTER_MATCHER(declaratorDecl);
REGISTER_MATCHER(decltypeType);
diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp
index f3f314b723dfc6..ec062a5cc953b8 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -3172,9 +3172,6 @@ TEST_P(ImportDecl, ImportFieldOrder) {
recordDecl(hasFieldOrder({"b", "a"})));
}
-const internal::VariadicDynCastAllOfMatcher<Expr, DependentScopeDeclRefExpr>
- dependentScopeDeclRefExpr;
-
TEST_P(ImportExpr, DependentScopeDeclRefExpr) {
MatchVerifier<Decl> Verifier;
testImport("template <typename T> struct S { static T foo; };"
diff --git a/clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp b/clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
index 9bc287e07224aa..0663cc35d7a52a 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ b/clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -556,6 +556,22 @@ TEST_P(ASTMatchersTest, DeclRefExpr) {
Reference));
}
+TEST_P(ASTMatchersTest, DependentScopeDeclRefExpr) {
+ if (!GetParam().isCXX()) {
+ // FIXME: Add a test for `dependentScopeDeclRefExpr()` that does not depend on C++.
+ return;
+ }
+
+ EXPECT_TRUE(
+ matches("template <class T> class X : T { void f() { T::v; } };",
+ dependentScopeDeclRefExpr()));
+
+ EXPECT_TRUE(
+ matches("template <typename T> struct S { static T Foo; };"
+ "template <typename T> void declToImport() { (void)S<T>::Foo; }",
+ dependentScopeDeclRefExpr()));
+}
+
TEST_P(ASTMatchersTest, CXXMemberCallExpr) {
if (!GetParam().isCXX()) {
return;
@@ -629,10 +645,9 @@ TEST_P(ASTMatchersTest, MemberExpr_MatchesVariable) {
EXPECT_TRUE(matches("template <class T>"
"class X : T { void f() { this->T::v; } };",
cxxDependentScopeMemberExpr()));
- // FIXME: Add a matcher for DependentScopeDeclRefExpr.
EXPECT_TRUE(
- notMatches("template <class T> class X : T { void f() { T::v; } };",
- cxxDependentScopeMemberExpr()));
+ matches("template <class T> class X : T { void f() { T::v; } };",
+ dependentScopeDeclRefExpr()));
EXPECT_TRUE(matches("template <class T> void x() { T t; t.v; }",
cxxDependentScopeMemberExpr()));
}
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
0e6e333
to
56b650f
Compare
56b650f
to
a0b7150
Compare
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, this looks pretty good!
@@ -556,6 +556,22 @@ TEST_P(ASTMatchersTest, DeclRefExpr) { | |||
Reference)); | |||
} | |||
|
|||
TEST_P(ASTMatchersTest, DependentScopeDeclRefExpr) { | |||
if (!GetParam().isCXX() || GetParam().hasDelayedTemplateParsing()) { | |||
// FIXME: Add a test for `dependentScopeDeclRefExpr()` that does not depend |
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.
The concept of template-dependence (and thus DependentScopeDeclRefExpr
nodes) only exist in C++, so it's expected that the test depends on C++.
Based on some other tests that check this condition, please revise the comment to instead say:
// FIXME: Fix this test to work with delayed template parsing.
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.
Thank you, I updated it :D
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, LGTM!
I will wait a few days before merging in case anyone else has feedback.
Went ahead and merged this. Thanks @AmrDeveloper! |
By the way, in case you're interested in doing another one, I filed #121240 about another missing matcher I came across. |
Sure, I am interested, Thank you :D |
Add AST Matcher for
DependentScopeDeclRefExpr
Fixes: #120937