fwolff created this revision.
fwolff added reviewers: aaron.ballman, simon.giesecke, jcking1034.
fwolff added a project: clang-tools-extra.
Herald added subscribers: carlosgalvezp, xazax.hun.
fwolff requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Fixes PR#52519 <https://bugs.llvm.org/show_bug.cgi?id=52519>. The problem
actually has nothing to do with the out-of-line definition being `inline`; the
problem is that `hasDeclaration()` of the `memberExpr()` will match the
out-of-line definition, which obviously isn't marked `static`, so
`isStaticStorageClass()` won't match. To find out whether a member function is
`static`, one needs to look at the //canonical// declaration, but there doesn't
seem to be a matcher for that yet, so I've added an `isStatic()` narrowing
matcher for `CXXMethodDecl`s, similar to the `isConst()` matcher that already
exists.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D115106
Files:
clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/readability-static-accessed-through-instance.cpp
clang/docs/LibASTMatchersReference.html
clang/include/clang/ASTMatchers/ASTMatchers.h
clang/lib/ASTMatchers/Dynamic/Registry.cpp
clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===================================================================
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2017,6 +2017,17 @@
notMatches("struct A { void foo(); };", cxxMethodDecl(isConst())));
}
+TEST_P(ASTMatchersTest, IsStatic) {
+ if (!GetParam().isCXX()) {
+ return;
+ }
+
+ EXPECT_TRUE(
+ matches("struct A { static void foo(); };", cxxMethodDecl(isStatic())));
+ EXPECT_TRUE(
+ notMatches("struct A { void foo(); };", cxxMethodDecl(isStatic())));
+}
+
TEST_P(ASTMatchersTest, IsOverride) {
if (!GetParam().isCXX()) {
return;
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===================================================================
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -451,6 +451,7 @@
REGISTER_MATCHER(isSharedKind);
REGISTER_MATCHER(isSignedInteger);
REGISTER_MATCHER(isStandaloneDirective);
+ REGISTER_MATCHER(isStatic);
REGISTER_MATCHER(isStaticLocal);
REGISTER_MATCHER(isStaticStorageClass);
REGISTER_MATCHER(isStruct);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===================================================================
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -6003,6 +6003,19 @@
return Node.isConst();
}
+/// Matches if the given method declaration is static.
+///
+/// Given
+/// \code
+/// struct A {
+/// static void foo();
+/// void bar();
+/// };
+/// \endcode
+///
+/// cxxMethodDecl(isStatic()) matches A::foo() but not A::bar()
+AST_MATCHER(CXXMethodDecl, isStatic) { return Node.isStatic(); }
+
/// Matches if the given method declaration declares a copy assignment
/// operator.
///
Index: clang/docs/LibASTMatchersReference.html
===================================================================
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -3322,6 +3322,19 @@
</pre></td></tr>
+<tr><td>Matcher<<a href="https://clang.llvm.org/doxygen/classclang_1_1CXXMethodDecl.html";>CXXMethodDecl</a>></td><td class="name" onclick="toggle('isStatic0')"><a name="isStatic0Anchor">isStatic</a></td><td></td></tr>
+<tr><td colspan="4" class="doc" id="isStatic0"><pre>Matches if the given method declaration is static.
+
+Given
+struct A {
+ static void foo();
+ void bar();
+};
+
+cxxMethodDecl(isStatic()) matches A::foo() but not A::bar()
+</pre></td></tr>
+
+
<tr><td>Matcher<<a href="https://clang.llvm.org/doxygen/classclang_1_1CXXMethodDecl.html";>CXXMethodDecl</a>></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.
Index: clang-tools-extra/test/clang-tidy/checkers/readability-static-accessed-through-instance.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/readability-static-accessed-through-instance.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-static-accessed-through-instance.cpp
@@ -263,3 +263,20 @@
// CHECK-MESSAGES-NOT: :[[@LINE-1]]:10: warning: static member
} // namespace Bugzilla_48758
+
+// https://bugs.llvm.org/show_bug.cgi?id=52519
+struct PR52519 {
+ static PR52519 &getInstance();
+ static int getInt();
+};
+
+int PR52519::getInt() {
+ return 42;
+}
+
+int PR52519_bar() {
+ auto &foo = PR52519::getInstance();
+ return foo.getInt();
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: static member accessed through instance [readability-static-accessed-through-instance]
+ // CHECK-FIXES: {{^}} return PR52519::getInt();{{$}}
+}
Index: clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
@@ -39,7 +39,7 @@
void StaticAccessedThroughInstanceCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(
- memberExpr(hasDeclaration(anyOf(cxxMethodDecl(isStaticStorageClass()),
+ memberExpr(hasDeclaration(anyOf(cxxMethodDecl(isStatic()),
varDecl(hasStaticStorageDuration()))))
.bind("memberExpression"),
this);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
