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

Reply via email to