[clang-tools-extra] [clang-tidy] Fix false positive in readability-convert-member-functions-to-static for const overloads (PR #191712)
Gaurav Dhingra via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 16 02:17:34 PDT 2026
https://github.com/gxyd updated https://github.com/llvm/llvm-project/pull/191712
>From 37c1904c6030c79d3fa52c6e410a090cbbcd7cae Mon Sep 17 00:00:00 2001
From: Gaurav Dhingra <gauravdhingra.gxyd at gmail.com>
Date: Sun, 12 Apr 2026 19:05:58 +0530
Subject: [PATCH 01/11] [clang-tidy] Fix false positive in
readability-convert-member-functions-to-static for const overloads
Don't suggest conversion of overloaded + same signature methods
differing in `const`ness to replace the `const` method with `static`.
E.g.;
```
void S::f(); // method1
void S::f() const; // method2
```
method2 can't have it's `const` replaced with `static`.
Fixes #149152
---
.../ConvertMemberFunctionsToStaticCheck.cpp | 31 ++++++++++++++++++-
.../convert-member-functions-to-static.cpp | 28 +++++++++++++++++
2 files changed, 58 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStaticCheck.cpp b/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStaticCheck.cpp
index 4bca9686e94e1..a0066a58f6f96 100644
--- a/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStaticCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStaticCheck.cpp
@@ -76,6 +76,35 @@ AST_MATCHER(CXXMethodDecl, usesThis) {
return UsageOfThis.Used;
}
+static bool hasSameParameterTypes(const CXXMethodDecl &MD1,
+ const CXXMethodDecl &MD2) {
+ if (MD1.getNumParams() != MD2.getNumParams())
+ return false;
+ for (unsigned I = 0, E = MD1.getNumParams(); I < E; ++I)
+ if (MD1.getParamDecl(I)->getType().getCanonicalType() !=
+ MD2.getParamDecl(I)->getType().getCanonicalType())
+ return false;
+ return true;
+}
+
+AST_MATCHER(CXXMethodDecl, hasNonConstOverload) {
+ const auto *Method = &Node;
+ const DeclContext::lookup_result LookupResult =
+ Method->getParent()->lookup(Method->getNameInfo().getName());
+ if (LookupResult.isSingleResult())
+ return false;
+
+ for (const Decl *D : LookupResult) {
+ if (const auto *Overload = dyn_cast<CXXMethodDecl>(D)) {
+ if (Overload != Method && !Overload->isConst() &&
+ hasSameParameterTypes(*Method, *Overload)) {
+ return true;
+ }
+ }
+ }
+ return false;
+}
+
} // namespace
void ConvertMemberFunctionsToStaticCheck::registerMatchers(
@@ -87,7 +116,7 @@ void ConvertMemberFunctionsToStaticCheck::registerMatchers(
isVirtual(), isStatic(), hasTrivialBody(), isOverloadedOperator(),
cxxConstructorDecl(), cxxDestructorDecl(), cxxConversionDecl(),
isExplicitObjectMemberFunction(), isTemplate(),
- isDependentContext(),
+ isDependentContext(), allOf(isConst(), hasNonConstOverload()),
ofClass(anyOf(
isLambda(),
hasAnyDependentBases()) // Method might become virtual
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static.cpp
index f9330388c1174..7d0eb858b9daa 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static.cpp
@@ -235,3 +235,31 @@ struct NoFixitInMacro {
return;
}
};
+
+
+struct OverloadedMethods {
+ void f() {
+ this->i++;
+ }
+ void f() const {
+ ;
+ };
+
+ void g(int) {
+ this->i++;
+ }
+ void g(int) const {
+ ;
+ };
+
+ void h(int) {
+ this->i++;
+ };
+ void h(float) const {
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: method 'h' can be made static
+ // CHECK-FIXES: static void h(float) {
+ ;
+ };
+
+ int i = 0;
+};
>From bfcfe0e9014ad42fe71e4167e4ecb4067c381688 Mon Sep 17 00:00:00 2001
From: Gaurav Dhingra <gauravdhingra.gxyd at gmail.com>
Date: Sun, 12 Apr 2026 19:20:18 +0530
Subject: [PATCH 02/11] add ReleaseNotes entry
---
clang-tools-extra/docs/ReleaseNotes.rst | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 02315415b975f..3c26e47926f05 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -412,6 +412,11 @@ Changes in existing checks
- Reduce verbosity by removing the note indicating source location of the
``empty`` function.
+- Improved :doc:`readability-convert-member-functions-to-static
+ <clang-tidy/checks/readability/convert-member-functions-to-static>` check by
+ avoiding false positive on ``const`` member functions to static when they are
+ a part of const/non-const overload pair with same signature.
+
- Improved :doc:`readability-else-after-return
<clang-tidy/checks/readability/else-after-return>` check:
>From 697832378ff9cd701f3858c4fdd49e9b8890c116 Mon Sep 17 00:00:00 2001
From: Gaurav Dhingra <gauravdhingra.gxyd at gmail.com>
Date: Sun, 12 Apr 2026 19:22:04 +0530
Subject: [PATCH 03/11] add one more test case
---
.../readability/convert-member-functions-to-static.cpp | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static.cpp
index 7d0eb858b9daa..7681e780f4624 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static.cpp
@@ -261,5 +261,12 @@ struct OverloadedMethods {
;
};
+ void j() {
+ this->i++;
+ }
+ int j() const {
+ ;
+ }
+
int i = 0;
};
>From 43da6dc60d518d4102ce8f8d3acb915c0312107a Mon Sep 17 00:00:00 2001
From: Gaurav Dhingra <gauravdhingra.gxyd at gmail.com>
Date: Mon, 13 Apr 2026 15:08:58 +0530
Subject: [PATCH 04/11] refactor hasNonConstOverload to use less nesting
---
.../ConvertMemberFunctionsToStaticCheck.cpp | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStaticCheck.cpp b/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStaticCheck.cpp
index a0066a58f6f96..1b22923c33b21 100644
--- a/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStaticCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStaticCheck.cpp
@@ -95,12 +95,10 @@ AST_MATCHER(CXXMethodDecl, hasNonConstOverload) {
return false;
for (const Decl *D : LookupResult) {
- if (const auto *Overload = dyn_cast<CXXMethodDecl>(D)) {
- if (Overload != Method && !Overload->isConst() &&
- hasSameParameterTypes(*Method, *Overload)) {
- return true;
- }
- }
+ const auto *Overload = dyn_cast<CXXMethodDecl>(D);
+ if (Overload && Overload != Method && !Overload->isConst() &&
+ hasSameParameterTypes(*Method, *Overload))
+ return true;
}
return false;
}
>From 1886a236279190dc3fd816dd498d91d0cfbd9552 Mon Sep 17 00:00:00 2001
From: Gaurav Dhingra <gauravdhingra.gxyd at gmail.com>
Date: Mon, 13 Apr 2026 15:27:30 +0530
Subject: [PATCH 05/11] remove 'static' keyword from `hasSameParameterTypes`
function
---
.../readability/ConvertMemberFunctionsToStaticCheck.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStaticCheck.cpp b/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStaticCheck.cpp
index 1b22923c33b21..2679b565a0acf 100644
--- a/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStaticCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStaticCheck.cpp
@@ -76,7 +76,7 @@ AST_MATCHER(CXXMethodDecl, usesThis) {
return UsageOfThis.Used;
}
-static bool hasSameParameterTypes(const CXXMethodDecl &MD1,
+bool hasSameParameterTypes(const CXXMethodDecl &MD1,
const CXXMethodDecl &MD2) {
if (MD1.getNumParams() != MD2.getNumParams())
return false;
>From 4c95d250bb21c912c1200ec75376e12283d8fa99 Mon Sep 17 00:00:00 2001
From: Gaurav Dhingra <gauravdhingra.gxyd at gmail.com>
Date: Mon, 13 Apr 2026 16:09:46 +0530
Subject: [PATCH 06/11] use std::any_of to loop on LookupResult
---
.../ConvertMemberFunctionsToStaticCheck.cpp | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStaticCheck.cpp b/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStaticCheck.cpp
index 2679b565a0acf..6878b0f92a9ed 100644
--- a/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStaticCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStaticCheck.cpp
@@ -76,8 +76,7 @@ AST_MATCHER(CXXMethodDecl, usesThis) {
return UsageOfThis.Used;
}
-bool hasSameParameterTypes(const CXXMethodDecl &MD1,
- const CXXMethodDecl &MD2) {
+bool hasSameParameterTypes(const CXXMethodDecl &MD1, const CXXMethodDecl &MD2) {
if (MD1.getNumParams() != MD2.getNumParams())
return false;
for (unsigned I = 0, E = MD1.getNumParams(); I < E; ++I)
@@ -94,13 +93,12 @@ AST_MATCHER(CXXMethodDecl, hasNonConstOverload) {
if (LookupResult.isSingleResult())
return false;
- for (const Decl *D : LookupResult) {
- const auto *Overload = dyn_cast<CXXMethodDecl>(D);
- if (Overload && Overload != Method && !Overload->isConst() &&
- hasSameParameterTypes(*Method, *Overload))
- return true;
- }
- return false;
+ return std::any_of(
+ LookupResult.begin(), LookupResult.end(), [Method](const Decl *D) {
+ const auto *Overload = dyn_cast<CXXMethodDecl>(D);
+ return Overload && Overload != Method && !Overload->isConst() &&
+ hasSameParameterTypes(*Method, *Overload);
+ });
}
} // namespace
>From 4bd5a3cb96abddfed7a15694bb136294cac68086 Mon Sep 17 00:00:00 2001
From: Gaurav Dhingra <gauravdhingra.gxyd at gmail.com>
Date: Tue, 14 Apr 2026 11:10:38 +0530
Subject: [PATCH 07/11] refactor hasNonConstOverload to be a static function
this avoids LLVM's error llvm-prefer-static-over-anonymous-namespace
as now it's outside the anonymous namespace
---
.../ConvertMemberFunctionsToStaticCheck.cpp | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStaticCheck.cpp b/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStaticCheck.cpp
index 6878b0f92a9ed..b8ee33daeee4d 100644
--- a/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStaticCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStaticCheck.cpp
@@ -75,8 +75,9 @@ AST_MATCHER(CXXMethodDecl, usesThis) {
return UsageOfThis.Used;
}
+} // namespace
-bool hasSameParameterTypes(const CXXMethodDecl &MD1, const CXXMethodDecl &MD2) {
+static bool hasSameParameterTypes(const CXXMethodDecl &MD1, const CXXMethodDecl &MD2) {
if (MD1.getNumParams() != MD2.getNumParams())
return false;
for (unsigned I = 0, E = MD1.getNumParams(); I < E; ++I)
@@ -86,7 +87,7 @@ bool hasSameParameterTypes(const CXXMethodDecl &MD1, const CXXMethodDecl &MD2) {
return true;
}
-AST_MATCHER(CXXMethodDecl, hasNonConstOverload) {
+static bool hasNonConstOverload(const CXXMethodDecl &Node) {
const auto *Method = &Node;
const DeclContext::lookup_result LookupResult =
Method->getParent()->lookup(Method->getNameInfo().getName());
@@ -101,8 +102,6 @@ AST_MATCHER(CXXMethodDecl, hasNonConstOverload) {
});
}
-} // namespace
-
void ConvertMemberFunctionsToStaticCheck::registerMatchers(
MatchFinder *Finder) {
Finder->addMatcher(
@@ -112,7 +111,7 @@ void ConvertMemberFunctionsToStaticCheck::registerMatchers(
isVirtual(), isStatic(), hasTrivialBody(), isOverloadedOperator(),
cxxConstructorDecl(), cxxDestructorDecl(), cxxConversionDecl(),
isExplicitObjectMemberFunction(), isTemplate(),
- isDependentContext(), allOf(isConst(), hasNonConstOverload()),
+ isDependentContext(),
ofClass(anyOf(
isLambda(),
hasAnyDependentBases()) // Method might become virtual
@@ -160,6 +159,9 @@ void ConvertMemberFunctionsToStaticCheck::check(
const MatchFinder::MatchResult &Result) {
const auto *Definition = Result.Nodes.getNodeAs<CXXMethodDecl>("x");
+ if (Definition->isConst() && hasNonConstOverload(*Definition))
+ return;
+
// TODO: For out-of-line declarations, don't modify the source if the header
// is excluded by the -header-filter option.
const DiagnosticBuilder Diag =
>From 5b4eabd2c019abb39b2ea16059d6cade1ebb265a Mon Sep 17 00:00:00 2001
From: Gaurav Dhingra <gauravdhingra.gxyd at gmail.com>
Date: Tue, 14 Apr 2026 11:23:44 +0530
Subject: [PATCH 08/11] update Release Notes entry
---
clang-tools-extra/docs/ReleaseNotes.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 3c26e47926f05..16e5609083a78 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -415,7 +415,7 @@ Changes in existing checks
- Improved :doc:`readability-convert-member-functions-to-static
<clang-tidy/checks/readability/convert-member-functions-to-static>` check by
avoiding false positive on ``const`` member functions to static when they are
- a part of const/non-const overload pair with same signature.
+ a part of const/non-const overload pair.
- Improved :doc:`readability-else-after-return
<clang-tidy/checks/readability/else-after-return>` check:
>From 94a830ca94d839a75fb119771e458577e64f0903 Mon Sep 17 00:00:00 2001
From: Gaurav Dhingra <gauravdhingra.gxyd at gmail.com>
Date: Tue, 14 Apr 2026 11:29:33 +0530
Subject: [PATCH 09/11] use llvm variant instead of std variant of ``any_of``
---
.../readability/ConvertMemberFunctionsToStaticCheck.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStaticCheck.cpp b/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStaticCheck.cpp
index b8ee33daeee4d..1cd8cd1e8f9c7 100644
--- a/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStaticCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStaticCheck.cpp
@@ -94,8 +94,8 @@ static bool hasNonConstOverload(const CXXMethodDecl &Node) {
if (LookupResult.isSingleResult())
return false;
- return std::any_of(
- LookupResult.begin(), LookupResult.end(), [Method](const Decl *D) {
+ return llvm::any_of(
+ LookupResult, [Method](const Decl *D) {
const auto *Overload = dyn_cast<CXXMethodDecl>(D);
return Overload && Overload != Method && !Overload->isConst() &&
hasSameParameterTypes(*Method, *Overload);
>From 22b582e640146e39aee569960b6f1a0492aa2e2f Mon Sep 17 00:00:00 2001
From: Gaurav Dhingra <gauravdhingra.gxyd at gmail.com>
Date: Tue, 14 Apr 2026 12:19:03 +0530
Subject: [PATCH 10/11] add comment and improve readability
---
.../ConvertMemberFunctionsToStaticCheck.cpp | 11 ++++----
.../convert-member-functions-to-static.cpp | 28 +++++--------------
2 files changed, 12 insertions(+), 27 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStaticCheck.cpp b/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStaticCheck.cpp
index 1cd8cd1e8f9c7..c331e0c04a059 100644
--- a/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStaticCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStaticCheck.cpp
@@ -94,12 +94,11 @@ static bool hasNonConstOverload(const CXXMethodDecl &Node) {
if (LookupResult.isSingleResult())
return false;
- return llvm::any_of(
- LookupResult, [Method](const Decl *D) {
- const auto *Overload = dyn_cast<CXXMethodDecl>(D);
- return Overload && Overload != Method && !Overload->isConst() &&
- hasSameParameterTypes(*Method, *Overload);
- });
+ return llvm::any_of(LookupResult, [Method](const Decl *D) {
+ const auto *Overload = dyn_cast<CXXMethodDecl>(D);
+ return Overload && Overload != Method && !Overload->isConst() &&
+ hasSameParameterTypes(*Method, *Overload);
+ });
}
void ConvertMemberFunctionsToStaticCheck::registerMatchers(
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static.cpp
index 7681e780f4624..ba8479b486535 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static.cpp
@@ -238,35 +238,21 @@ struct NoFixitInMacro {
struct OverloadedMethods {
- void f() {
- this->i++;
- }
- void f() const {
- ;
- };
+ void f() { this->i++; }
+ void f() const { ; }; // `;` is necessary to ensure non trivial body
- void g(int) {
- this->i++;
- }
- void g(int) const {
- ;
- };
+ void g(int) { this->i++; }
+ void g(int) const { ; };
- void h(int) {
- this->i++;
- };
+ void h(int) { this->i++; };
void h(float) const {
// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: method 'h' can be made static
// CHECK-FIXES: static void h(float) {
;
};
- void j() {
- this->i++;
- }
- int j() const {
- ;
- }
+ void j() { this->i++; }
+ int j() const { return 1; }
int i = 0;
};
>From 7c707a3a87e902110f548f656519805a41b69cde Mon Sep 17 00:00:00 2001
From: Gaurav Dhingra <gauravdhingra.gxyd at gmail.com>
Date: Thu, 16 Apr 2026 14:46:45 +0530
Subject: [PATCH 11/11] fix raised clang-format issue
---
.../readability/ConvertMemberFunctionsToStaticCheck.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStaticCheck.cpp b/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStaticCheck.cpp
index c331e0c04a059..87591be743d4e 100644
--- a/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStaticCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStaticCheck.cpp
@@ -77,7 +77,8 @@ AST_MATCHER(CXXMethodDecl, usesThis) {
}
} // namespace
-static bool hasSameParameterTypes(const CXXMethodDecl &MD1, const CXXMethodDecl &MD2) {
+static bool hasSameParameterTypes(const CXXMethodDecl &MD1,
+ const CXXMethodDecl &MD2) {
if (MD1.getNumParams() != MD2.getNumParams())
return false;
for (unsigned I = 0, E = MD1.getNumParams(); I < E; ++I)
More information about the cfe-commits
mailing list