[ASTMatchers] fix isStaticStorageClass not matching definitions of forward declared functions
#120027
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
5chmidti
added
clang-tidy
clang:frontend
Member
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-tidy Author: Julian Schmidt (5chmidti) Changesstatic void foo();
void foo() {}
struct A {
static void bar();
};
void A::bar() {}Both definitions refer to their previous declaration, but were not Full diff: https://github.com/llvm/llvm-project/pull/120027.diff 7 Files Affected:
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) {
|
5chmidti
force-pushed
the
clang_is_Stat_storage_class_forward_decls
branch
from
000e532 to
35a1cec
Compare
…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.
5chmidti
force-pushed
the
clang_is_Stat_storage_class_forward_decls
branch
from
35a1cec to
07dd31d
Compare
Contributor
Author
|
Another solution would be to explicitly document the definition is not matching with |
HerrCai0907
reviewed
Dec 16, 2024
Comment on lines
+1833
to
+1834
| EXPECT_TRUE(matches("static void foo(); void foo() {}", | ||
| functionDecl(isDefinition(), isStaticStorageClass()))); |
Contributor
There was a problem hiding this comment.
static function also available in C.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Both definitions refer to their previous declaration, but were not
considered
static, because the matcher did not check the canonicaldeclaration.