[clang-tools-extra] [clang-tidy] Fix false positives with deducing this in `readability-convert-member-functions-to-static` check (PR #141391)
via cfe-commits
cfe-commits at lists.llvm.org
Sun May 25 02:01:40 PDT 2025
https://github.com/flovent updated https://github.com/llvm/llvm-project/pull/141391
>From 3a7260050f8e8dc271c7fe2a5ec937026136fb6b Mon Sep 17 00:00:00 2001
From: fubowen <fubowen at protomail.com>
Date: Sun, 25 May 2025 09:37:47 +0800
Subject: [PATCH 1/5] [clang-tidy] fix false positives with deducing this in
`readability-convert-member-functions-to-static` check
---
.../readability/ConvertMemberFunctionsToStatic.cpp | 10 ++++++++++
clang-tools-extra/docs/ReleaseNotes.rst | 4 ++++
...rt-member-functions-to-static-deducing-this.cpp | 14 ++++++++++++++
3 files changed, 28 insertions(+)
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static-deducing-this.cpp
diff --git a/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp b/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp
index 1284df6bd99cf..5cdd00414e01e 100644
--- a/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp
@@ -62,6 +62,16 @@ AST_MATCHER(CXXMethodDecl, usesThis) {
return false; // Stop traversal.
}
+ bool VisitDeclRefExpr(const DeclRefExpr *E) {
+ if (const auto *PVD = dyn_cast_if_present<ParmVarDecl>(E->getDecl());
+ PVD && PVD->isExplicitObjectParameter()) {
+ Used = true;
+ return false; // Stop traversal.
+ }
+
+ return true;
+ }
+
// If we enter a class declaration, don't traverse into it as any usages of
// `this` will correspond to the nested class.
bool TraverseCXXRecordDecl(CXXRecordDecl *RD) { return true; }
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 8611b5e0af272..a138272cc8e39 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -234,6 +234,10 @@ Changes in existing checks
tolerating fix-it breaking compilation when functions is used as pointers
to avoid matching usage of functions within the current compilation unit.
+- Improved :doc:`readability-convert-member-functions-to-static
+ <clang-tidy/checks/readability/convert-member-functions-to-static>` check by
+ fixing false positives on member functions with an explicit object parameter.
+
Removed checks
^^^^^^^^^^^^^^
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static-deducing-this.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static-deducing-this.cpp
new file mode 100644
index 0000000000000..bb9755abc9cfa
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static-deducing-this.cpp
@@ -0,0 +1,14 @@
+// RUN: %check_clang_tidy -std=c++23 %s readability-convert-member-functions-to-static %t
+
+namespace std{
+ class string {};
+ void println(const char *format, const std::string &str) {}
+}
+
+namespace PR141381 {
+struct Hello {
+ std::string str_;
+
+ void hello(this Hello &self) { std::println("Hello, {0}!", self.str_); }
+};
+}
>From 43f50053cb7d33f2bbbd21881feba80d8fb1738a Mon Sep 17 00:00:00 2001
From: fubowen <fubowen at protomail.com>
Date: Sun, 25 May 2025 15:07:23 +0800
Subject: [PATCH 2/5] don't traverse all DeclRefEx[r
---
.../ConvertMemberFunctionsToStatic.cpp | 18 ++++--------------
1 file changed, 4 insertions(+), 14 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp b/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp
index 5cdd00414e01e..40499fd0b99d9 100644
--- a/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp
@@ -62,16 +62,6 @@ AST_MATCHER(CXXMethodDecl, usesThis) {
return false; // Stop traversal.
}
- bool VisitDeclRefExpr(const DeclRefExpr *E) {
- if (const auto *PVD = dyn_cast_if_present<ParmVarDecl>(E->getDecl());
- PVD && PVD->isExplicitObjectParameter()) {
- Used = true;
- return false; // Stop traversal.
- }
-
- return true;
- }
-
// If we enter a class declaration, don't traverse into it as any usages of
// `this` will correspond to the nested class.
bool TraverseCXXRecordDecl(CXXRecordDecl *RD) { return true; }
@@ -89,10 +79,10 @@ void ConvertMemberFunctionsToStatic::registerMatchers(MatchFinder *Finder) {
cxxMethodDecl(
isDefinition(), isUserProvided(),
unless(anyOf(
- isExpansionInSystemHeader(), isVirtual(), isStatic(),
- hasTrivialBody(), isOverloadedOperator(), cxxConstructorDecl(),
- cxxDestructorDecl(), cxxConversionDecl(), isTemplate(),
- isDependentContext(),
+ isExplicitObjectMemberFunction(), isExpansionInSystemHeader(),
+ isVirtual(), isStatic(), hasTrivialBody(), isOverloadedOperator(),
+ cxxConstructorDecl(), cxxDestructorDecl(), cxxConversionDecl(),
+ isTemplate(), isDependentContext(),
ofClass(anyOf(
isLambda(),
hasAnyDependentBases()) // Method might become virtual
>From f80e17511489aafcf64c4830d1b35d18e5b50e5e Mon Sep 17 00:00:00 2001
From: fubowen <fubowen at protomail.com>
Date: Sun, 25 May 2025 15:09:06 +0800
Subject: [PATCH 3/5] improve test cases based on reviewd feedback
---
...rt-member-functions-to-static-deducing-this.cpp | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static-deducing-this.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static-deducing-this.cpp
index bb9755abc9cfa..ff704b2583eb9 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static-deducing-this.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static-deducing-this.cpp
@@ -5,10 +5,18 @@ namespace std{
void println(const char *format, const std::string &str) {}
}
-namespace PR141381 {
struct Hello {
std::string str_;
- void hello(this Hello &self) { std::println("Hello, {0}!", self.str_); }
+ void ByValueSelf(this Hello self) { std::println("Hello, {0}!", self.str_); }
+
+ void ByLRefSelf(this Hello &self) { std::println("Hello, {0}!", self.str_); }
+
+ void ByRRefSelf(this Hello&& self) {}
+
+ template<typename Self> void ByForwardRefSelf(this Self&& self) {}
+
+ void MultiParam(this Hello &self, int a, double b) {}
+
+ void UnnamedExplicitObjectParam(this Hello &) {}
};
-}
>From 09029bb90280ed891c362fccc8e98b3d096e0d7b Mon Sep 17 00:00:00 2001
From: fubowen <fubowen at protomail.com>
Date: Sun, 25 May 2025 16:59:02 +0800
Subject: [PATCH 4/5] adjust predicates' order
---
.../readability/ConvertMemberFunctionsToStatic.cpp | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp b/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp
index 40499fd0b99d9..beca824c8c8ce 100644
--- a/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp
@@ -79,10 +79,11 @@ void ConvertMemberFunctionsToStatic::registerMatchers(MatchFinder *Finder) {
cxxMethodDecl(
isDefinition(), isUserProvided(),
unless(anyOf(
- isExplicitObjectMemberFunction(), isExpansionInSystemHeader(),
- isVirtual(), isStatic(), hasTrivialBody(), isOverloadedOperator(),
- cxxConstructorDecl(), cxxDestructorDecl(), cxxConversionDecl(),
- isTemplate(), isDependentContext(),
+ isExpansionInSystemHeader(), isVirtual(), isStatic(),
+ hasTrivialBody(), isOverloadedOperator(), cxxConstructorDecl(),
+ cxxDestructorDecl(), cxxConversionDecl(),
+ isExplicitObjectMemberFunction(), isTemplate(),
+ isDependentContext(),
ofClass(anyOf(
isLambda(),
hasAnyDependentBases()) // Method might become virtual
>From f86e0c0f81031ed7a1231133daa6aad819c5c732 Mon Sep 17 00:00:00 2001
From: fubowen <fubowen at protomail.com>
Date: Sun, 25 May 2025 17:01:27 +0800
Subject: [PATCH 5/5] c++23 to c++23-or-later
---
.../convert-member-functions-to-static-deducing-this.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static-deducing-this.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static-deducing-this.cpp
index ff704b2583eb9..7974301aecce0 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static-deducing-this.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static-deducing-this.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy -std=c++23 %s readability-convert-member-functions-to-static %t
+// RUN: %check_clang_tidy -std=c++23-or-later %s readability-convert-member-functions-to-static %t
namespace std{
class string {};
More information about the cfe-commits
mailing list