[clang] [clang-tools-extra] [ASTMatchers] fix `isStaticStorageClass` not matching definitions of forward declared functions (PR #120027)
via cfe-commits
cfe-commits at lists.llvm.org
Sun Dec 15 16:21:17 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tidy
Author: Julian Schmidt (5chmidti)
<details>
<summary>Changes</summary>
```c++
static void foo();
void foo() {}
struct A {
static void bar();
};
void A::bar() {}
```
Both definitions refer to their previous declaration, but were not
considered `static`, because the matcher did not check the canonical
declaration.
---
Full diff: https://github.com/llvm/llvm-project/pull/120027.diff
7 Files Affected:
- (modified) clang-tools-extra/clang-tidy/bugprone/VirtualNearMissCheck.cpp (+3-4)
- (modified) clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp (+2-4)
- (modified) clang-tools-extra/clang-tidy/readability/MakeMemberFunctionConstCheck.cpp (+4-5)
- (modified) clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp (+2-6)
- (modified) clang/docs/LibASTMatchersReference.html (+3-1)
- (modified) clang/include/clang/ASTMatchers/ASTMatchers.h (+4-2)
- (modified) clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp (+8)
``````````diff
diff --git a/clang-tools-extra/clang-tidy/bugprone/VirtualNearMissCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/VirtualNearMissCheck.cpp
index 76fa2d916f0e86..c83c175ce4bdd5 100644
--- a/clang-tools-extra/clang-tidy/bugprone/VirtualNearMissCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/VirtualNearMissCheck.cpp
@@ -10,6 +10,7 @@
#include "clang/AST/ASTContext.h"
#include "clang/AST/CXXInheritance.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/Lex/Lexer.h"
using namespace clang::ast_matchers;
@@ -17,8 +18,6 @@ using namespace clang::ast_matchers;
namespace clang::tidy::bugprone {
namespace {
-AST_MATCHER(CXXMethodDecl, isStatic) { return Node.isStatic(); }
-
AST_MATCHER(CXXMethodDecl, isOverloadedOperator) {
return Node.isOverloadedOperator();
}
@@ -216,8 +215,8 @@ void VirtualNearMissCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(
cxxMethodDecl(
unless(anyOf(isOverride(), isImplicit(), cxxConstructorDecl(),
- cxxDestructorDecl(), cxxConversionDecl(), isStatic(),
- isOverloadedOperator())))
+ cxxDestructorDecl(), cxxConversionDecl(),
+ isStaticStorageClass(), isOverloadedOperator())))
.bind("method"),
this);
}
diff --git a/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp b/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp
index 1284df6bd99cfd..42fc9d36ac41d1 100644
--- a/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp
@@ -11,15 +11,13 @@
#include "clang/AST/DeclCXX.h"
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Lex/Lexer.h"
using namespace clang::ast_matchers;
namespace clang::tidy::readability {
-
-AST_MATCHER(CXXMethodDecl, isStatic) { return Node.isStatic(); }
-
AST_MATCHER(CXXMethodDecl, hasTrivialBody) { return Node.hasTrivialBody(); }
AST_MATCHER(CXXMethodDecl, isOverloadedOperator) {
@@ -79,7 +77,7 @@ void ConvertMemberFunctionsToStatic::registerMatchers(MatchFinder *Finder) {
cxxMethodDecl(
isDefinition(), isUserProvided(),
unless(anyOf(
- isExpansionInSystemHeader(), isVirtual(), isStatic(),
+ isExpansionInSystemHeader(), isVirtual(), isStaticStorageClass(),
hasTrivialBody(), isOverloadedOperator(), cxxConstructorDecl(),
cxxDestructorDecl(), cxxConversionDecl(), isTemplate(),
isDependentContext(),
diff --git a/clang-tools-extra/clang-tidy/readability/MakeMemberFunctionConstCheck.cpp b/clang-tools-extra/clang-tidy/readability/MakeMemberFunctionConstCheck.cpp
index d42fcba70e81b4..adf4584e75f9e5 100644
--- a/clang-tools-extra/clang-tidy/readability/MakeMemberFunctionConstCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/MakeMemberFunctionConstCheck.cpp
@@ -11,14 +11,12 @@
#include "clang/AST/ParentMapContext.h"
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/Lex/Lexer.h"
using namespace clang::ast_matchers;
namespace clang::tidy::readability {
-
-AST_MATCHER(CXXMethodDecl, isStatic) { return Node.isStatic(); }
-
AST_MATCHER(CXXMethodDecl, hasTrivialBody) { return Node.hasTrivialBody(); }
AST_MATCHER(CXXRecordDecl, hasAnyDependentBases) {
@@ -222,8 +220,9 @@ void MakeMemberFunctionConstCheck::registerMatchers(MatchFinder *Finder) {
isDefinition(), isUserProvided(),
unless(anyOf(
isExpansionInSystemHeader(), isVirtual(), isConst(),
- isStatic(), hasTrivialBody(), cxxConstructorDecl(),
- cxxDestructorDecl(), isTemplate(), isDependentContext(),
+ isStaticStorageClass(), hasTrivialBody(),
+ cxxConstructorDecl(), cxxDestructorDecl(), isTemplate(),
+ isDependentContext(),
ofClass(anyOf(isLambda(),
hasAnyDependentBases()) // Method might become
// virtual depending on
diff --git a/clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp b/clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
index 08adc7134cfea2..383695fab0ba97 100644
--- a/clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
@@ -9,16 +9,12 @@
#include "StaticAccessedThroughInstanceCheck.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
#include "llvm/ADT/StringRef.h"
using namespace clang::ast_matchers;
namespace clang::tidy::readability {
-
-namespace {
-AST_MATCHER(CXXMethodDecl, isStatic) { return Node.isStatic(); }
-} // namespace
-
static unsigned getNameSpecifierNestingLevel(const QualType &QType) {
if (const auto *ElType = QType->getAs<ElaboratedType>()) {
if (const NestedNameSpecifier *NestedSpecifiers = ElType->getQualifier()) {
@@ -42,7 +38,7 @@ void StaticAccessedThroughInstanceCheck::storeOptions(
void StaticAccessedThroughInstanceCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(
- memberExpr(hasDeclaration(anyOf(cxxMethodDecl(isStatic()),
+ memberExpr(hasDeclaration(anyOf(cxxMethodDecl(isStaticStorageClass()),
varDecl(hasStaticStorageDuration()),
enumConstantDecl())))
.bind("memberExpression"),
diff --git a/clang/docs/LibASTMatchersReference.html b/clang/docs/LibASTMatchersReference.html
index f18e9cf1341696..fe24bd3ab9a0cd 100644
--- a/clang/docs/LibASTMatchersReference.html
+++ b/clang/docs/LibASTMatchersReference.html
@@ -4683,8 +4683,10 @@ <h2 id="narrowing-matchers">Narrowing Matchers</h2>
static int i = 0;
extern int j;
int k;
+ static void l();
+ void l() {}
functionDecl(isStaticStorageClass())
- matches the function declaration f.
+ matches the function declaration of f and l, and the definition of l.
varDecl(isStaticStorageClass())
matches the variable declaration i.
</pre></td></tr>
diff --git a/clang/include/clang/ASTMatchers/ASTMatchers.h b/clang/include/clang/ASTMatchers/ASTMatchers.h
index 897aa25dc95cc1..e7ab6c184349e4 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchers.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -5418,15 +5418,17 @@ AST_POLYMORPHIC_MATCHER(isExternC, AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl,
/// static int i = 0;
/// extern int j;
/// int k;
+/// static void l();
+/// void l() {}
/// \endcode
/// functionDecl(isStaticStorageClass())
-/// matches the function declaration f.
+/// matches the function declaration of f and l, and the definition of l.
/// varDecl(isStaticStorageClass())
/// matches the variable declaration i.
AST_POLYMORPHIC_MATCHER(isStaticStorageClass,
AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl,
VarDecl)) {
- return Node.getStorageClass() == SC_Static;
+ return Node.getCanonicalDecl()->getStorageClass() == SC_Static;
}
/// Matches deleted function declarations.
diff --git a/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp b/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
index 056b7c7b571ef4..f1efba5b0650fa 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ b/clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1826,6 +1826,14 @@ TEST_P(ASTMatchersTest, IsStaticStorageClass) {
EXPECT_TRUE(notMatches("int i = 1;", varDecl(isStaticStorageClass())));
EXPECT_TRUE(notMatches("extern int i;", varDecl(isStaticStorageClass())));
EXPECT_TRUE(notMatches("void f() {}", functionDecl(isStaticStorageClass())));
+
+ if (!GetParam().isCXX())
+ return;
+
+ EXPECT_TRUE(matches("static void foo(); void foo() {}",
+ functionDecl(isDefinition(), isStaticStorageClass())));
+ EXPECT_TRUE(matches("struct A { static void bar(); }; void A::bar() {}",
+ cxxMethodDecl(isDefinition(), isStaticStorageClass())));
}
TEST_P(ASTMatchersTest, IsDefaulted) {
``````````
</details>
https://github.com/llvm/llvm-project/pull/120027
More information about the cfe-commits
mailing list