https://github.com/5chmidti updated https://github.com/llvm/llvm-project/pull/120027
>From 35a1cecb08d1827fb45c2e6bb06983ed363ca769 Mon Sep 17 00:00:00 2001 From: Julian Schmidt <git.julian.schm...@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/docs/ReleaseNotes.rst | 3 +++ clang/include/clang/ASTMatchers/ASTMatchers.h | 6 ++++-- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp | 8 ++++++++ 8 files changed, 29 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/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index befa411e882b4c..66d3cd6b53c7eb 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -1041,6 +1041,9 @@ AST Matchers - Ensure ``pointee`` matches Objective-C pointer types. +- Fixed `isStaticStorageClass` not matching the definition if the definition was + not marked `static` as well. + clang-format ------------ 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) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits