Author: Nathan James Date: 2023-06-24T14:35:11Z New Revision: 67e05d380c2253319c22451d340e2e3c2043b6d8
URL: https://github.com/llvm/llvm-project/commit/67e05d380c2253319c22451d340e2e3c2043b6d8 DIFF: https://github.com/llvm/llvm-project/commit/67e05d380c2253319c22451d340e2e3c2043b6d8.diff LOG: [clang-tidy] Fix false negative in readability-convert-member-functions-to-static A nested class in a member function can erroneously confuse the check into thinking that any CXXThisExpr found relate to the outer class, preventing any warnings. Fix this by not traversing any nested classes. Fixes https://github.com/llvm/llvm-project/issues/56765 Reviewed By: PiotrZSL Differential Revision: https://reviews.llvm.org/D130665 Added: Modified: clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp b/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp index 0ef7d2955ff76..1284df6bd99cf 100644 --- a/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp +++ b/clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp @@ -61,6 +61,11 @@ AST_MATCHER(CXXMethodDecl, usesThis) { Used = true; return false; // Stop traversal. } + + // 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; } + } UsageOfThis; // TraverseStmt does not modify its argument. diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index e049ff87939a6..03426772f6037 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -412,6 +412,10 @@ Changes in existing checks ``std::array`` objects to default constructed ones. The behavior for this and other relevant classes can now be configured with a new option. +- Fixed a false negative in :doc:`readability-convert-member-functions-to-static + <clang-tidy/checks/readability/convert-member-functions-to-static>` when a + nested class in a member function uses a ``this`` pointer. + - Fixed reading `HungarianNotation.CString.*` options in :doc:`readability-identifier-naming <clang-tidy/checks/readability/identifier-naming>` 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 9612fa9de8c20..5ec1f221b2207 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 @@ -45,6 +45,24 @@ class A { static_field = 1; } + void static_nested() { + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: method 'static_nested' can be made static + // CHECK-FIXES: {{^}} static void static_nested() { + struct Nested { + int Foo; + int getFoo() { return Foo; } + }; + } + + void write_nested() { + struct Nested { + int Foo; + int getFoo() { return Foo; } + }; + // Ensure we still detect usages of `this` once we leave the nested class definition. + field = 1; + } + static int already_static() { return static_field; } int already_const() const { return field; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
