llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tools-extra
Author: Gaurav Dhingra (gxyd)
<details>
<summary>Changes</summary>
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
---
Full diff: https://github.com/llvm/llvm-project/pull/191712.diff
3 Files Affected:
- (modified)
clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStaticCheck.cpp
(+30-1)
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5)
- (modified)
clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static.cpp
(+35)
``````````diff
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/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:
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..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
@@ -235,3 +235,38 @@ 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) {
+ ;
+ };
+
+ void j() {
+ this->i++;
+ }
+ int j() const {
+ ;
+ }
+
+ int i = 0;
+};
``````````
</details>
https://github.com/llvm/llvm-project/pull/191712
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
