https://github.com/flovent updated https://github.com/llvm/llvm-project/pull/141391
>From 3a7260050f8e8dc271c7fe2a5ec937026136fb6b Mon Sep 17 00:00:00 2001 From: fubowen <fubo...@protomail.com> Date: Sun, 25 May 2025 09:37:47 +0800 Subject: [PATCH 1/3] [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 <fubo...@protomail.com> Date: Sun, 25 May 2025 15:07:23 +0800 Subject: [PATCH 2/3] 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 <fubo...@protomail.com> Date: Sun, 25 May 2025 15:09:06 +0800 Subject: [PATCH 3/3] 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 &) {} }; -} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits