[PATCH] D129202: [clang] Add a fixit for warn-self-assign if LHS is a field with the same name as parameter on RHS

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 442559.
njames93 added a comment.

Remove unnecessary extra include.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129202/new/

https://reviews.llvm.org/D129202

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/warn-self-assign-builtin.cpp


Index: clang/test/SemaCXX/warn-self-assign-builtin.cpp
===================================================================
--- clang/test/SemaCXX/warn-self-assign-builtin.cpp
+++ clang/test/SemaCXX/warn-self-assign-builtin.cpp
@@ -65,3 +65,23 @@
   g<int>();
   g<S>();
 }
+
+struct Foo {
+  int A;
+  void setA(int A) {
+    A = A; // expected-warning{{explicitly assigning}} expected-note {{did you mean to assign to member 'A'}}
+  }
+
+  void setThroughLambda() {
+    [](int A) {
+      // To fix here we would need to insert an explicit capture 'this'
+      A = A; // expected-warning{{explicitly assigning}}
+    }(5);
+
+    [this](int A) {
+      this->A = 0;
+      // This fix would be possible by just adding this-> as above, but currently unsupported.
+      A = A; // expected-warning{{explicitly assigning}}
+    }(5);
+  }
+};
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -14634,6 +14634,35 @@
                           : diag::warn_self_assignment_overloaded)
       << LHSDeclRef->getType() << LHSExpr->getSourceRange()
       << RHSExpr->getSourceRange();
+
+  // Explore the case for adding 'this->' to the LHS, very common for setters
+  // struct A { int X;
+  // -void setX(int X) { X = X; }
+  // +void setX(int X) { this->X = X; }
+  // };
+
+  // Only consider parameter's for this fix.
+  if (!llvm::isa<ParmVarDecl>(RHSDecl))
+    return;
+  if (auto *Method =
+          llvm::dyn_cast_or_null<CXXMethodDecl>(S.getCurFunctionDecl(true))) {
+
+    CXXRecordDecl *Parent = Method->getParent();
+    // In theory this is fixable if the lambda explicitly captures this, but
+    // that's added complexity that's rarely going to be used.
+    if (Parent->isLambda())
+      return;
+
+    // Only check the fields declared in Parent, without traversing base classes
+    auto Field = llvm::find_if(
+        Parent->fields(), [Name(RHSDecl->getDeclName())](const FieldDecl *F) {
+          return F->getDeclName() == Name;
+        });
+    if (Field != Parent->fields().end())
+      S.Diag(LHSDeclRef->getBeginLoc(), diag::note_self_assign_insert_this)
+          << *Field
+          << FixItHint::CreateInsertion(LHSDeclRef->getBeginLoc(), "this->");
+  }
 }
 
 /// Check if a bitwise-& is performed on an Objective-C pointer.  This
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6613,6 +6613,8 @@
 def warn_self_move : Warning<
   "explicitly moving variable of type %0 to itself">,
   InGroup<SelfMove>, DefaultIgnore;
+def note_self_assign_insert_this : Note<
+  "did you mean to assign to member %0">;
 
 def err_builtin_move_forward_unsupported : Error<
   "unsupported signature for %q0">;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D129202.442559.patch
Type: text/x-patch
Size: 2939 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220706/a41b7f14/attachment.bin>