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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits