[PATCH] D130665: [clang-tidy] Fix false negative in readability-convert-member-functions-to-static

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added reviewers: aaron.ballman, alexfh, LegalizeAdulthood.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a project: All.
njames93 requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

A nested class in a member function can erroneously confuse the 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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130665

Files:
  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


Index: clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/convert-member-functions-to-static.cpp
@@ -45,6 +45,24 @@
     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; }
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -105,6 +105,10 @@
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+- 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.
+
 Removed checks
 ^^^^^^^^^^^^^^
 
Index: clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp
+++ clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp
@@ -63,6 +63,11 @@
       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.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D130665.448166.patch
Type: text/x-patch
Size: 2214 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220727/fd366e48/attachment.bin>