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 <[email protected]> Date: Sun, 12 Apr 2026 19:05:58 +0530 Subject: [PATCH 01/10] [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 <[email protected]> Date: Sun, 12 Apr 2026 19:20:18 +0530 Subject: [PATCH 02/10] 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 <[email protected]> Date: Sun, 12 Apr 2026 19:22:04 +0530 Subject: [PATCH 03/10] 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 <[email protected]> Date: Mon, 13 Apr 2026 15:08:58 +0530 Subject: [PATCH 04/10] 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 <[email protected]> Date: Mon, 13 Apr 2026 15:27:30 +0530 Subject: [PATCH 05/10] 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 <[email protected]> Date: Mon, 13 Apr 2026 16:09:46 +0530 Subject: [PATCH 06/10] 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 <[email protected]> Date: Tue, 14 Apr 2026 11:10:38 +0530 Subject: [PATCH 07/10] 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 <[email protected]> Date: Tue, 14 Apr 2026 11:23:44 +0530 Subject: [PATCH 08/10] 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 <[email protected]> Date: Tue, 14 Apr 2026 11:29:33 +0530 Subject: [PATCH 09/10] 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 <[email protected]> Date: Tue, 14 Apr 2026 12:19:03 +0530 Subject: [PATCH 10/10] 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; }; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
