[clang] [clang-tools-extra] [ASTMatchers] fix `isStaticStorageClass` not matching definitions of forward declared functions (PR #120027)
Julian Schmidt via cfe-commits
cfe-commits at lists.llvm.org
Sun Dec 15 16:20:53 PST 2024
https://github.com/5chmidti created https://github.com/llvm/llvm-project/pull/120027
```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.
>From 000e5322b0d3371df9a1861ad6c1aa010c52be33 Mon Sep 17 00:00:00 2001
From: Julian Schmidt <git.julian.schmidt at gmail.com>
Date: Mon, 16 Dec 2024 01:18:06 +0100
Subject: [PATCH] [ASTMatchers] fix `isStaticStorageClass` not matching
definitions of forward declared functions
```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.
---
.../clang-tidy/bugprone/VirtualNearMissCheck.cpp | 7 +++----
.../readability/ConvertMemberFunctionsToStatic.cpp | 6 ++----
.../readability/MakeMemberFunctionConstCheck.cpp | 9 ++++-----
.../readability/StaticAccessedThroughInstanceCheck.cpp | 8 ++------
clang/docs/LibASTMatchersReference.html | 4 +++-
clang/include/clang/ASTMatchers/ASTMatchers.h | 6 ++++--
clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp | 8 ++++++++
7 files changed, 26 insertions(+), 22 deletions(-)
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) {
More information about the cfe-commits
mailing list