njames93 updated this revision to Diff 442788. njames93 marked an inline comment as done. njames93 added a comment.
Address comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129202/new/ https://reviews.llvm.org/D129202 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Sema/SemaExpr.cpp clang/test/SemaCXX/warn-self-assign-builtin.cpp clang/test/SemaCXX/warn-self-assign-field-builtin.cpp
Index: clang/test/SemaCXX/warn-self-assign-field-builtin.cpp =================================================================== --- clang/test/SemaCXX/warn-self-assign-field-builtin.cpp +++ clang/test/SemaCXX/warn-self-assign-field-builtin.cpp @@ -4,6 +4,8 @@ int a; int b; + C(int a, int b) : a(a), b(b) {} + void f() { a = a; // expected-warning {{assigning field to itself}} b = b; // expected-warning {{assigning field to itself}} 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,26 @@ g<int>(); g<S>(); } + +struct Foo { + int A; + + Foo(int A) : A(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,36 @@ : 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 (!isa<ParmVarDecl>(RHSDecl)) + return; + if (const auto *Method = + dyn_cast_or_null<CXXMethodDecl>(S.getCurFunctionDecl(true))) { + + const 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">; Index: clang/docs/ReleaseNotes.rst =================================================================== --- clang/docs/ReleaseNotes.rst +++ clang/docs/ReleaseNotes.rst @@ -279,6 +279,9 @@ unevaluated operands of a ``typeid`` expression, as they are now modeled correctly in the CFG. This fixes `Issue 21668 <https://github.com/llvm/llvm-project/issues/21668>`_. +- ``-Wself-assign`` and ``-Wself-assign-overloaded`` will suggest a fix if the + decl being assigned is a parameter that shadows a data member of the contained + class. Non-comprehensive list of changes in this release -------------------------------------------------
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits