njames93 updated this revision to Diff 443417.
njames93 added a comment.
Address comment.
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/include/clang/Sema/Sema.h
clang/lib/Sema/SemaChecking.cpp
clang/lib/Sema/SemaExpr.cpp
clang/test/SemaCXX/warn-self-assign-builtin.cpp
clang/test/SemaCXX/warn-self-assign-field-builtin.cpp
clang/test/SemaCXX/warn-self-move.cpp
Index: clang/test/SemaCXX/warn-self-move.cpp
===================================================================
--- clang/test/SemaCXX/warn-self-move.cpp
+++ clang/test/SemaCXX/warn-self-move.cpp
@@ -39,6 +39,9 @@
other.x = std::move(x);
other.x = std::move(other.x); // expected-warning{{explicitly moving}}
}
+ void withSuggest(int x) {
+ x = std::move(x); // expected-warning{{explicitly moving variable of type 'int' to itself; did you mean to move to member 'x'?}}
+ }
};
struct A {};
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 value of variable of type 'int' to itself; 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
@@ -14600,6 +14600,40 @@
return Opc;
}
+const FieldDecl *
+Sema::getSelfAssignmentClassMemberCandidate(const ValueDecl *SelfAssigned) {
+ // Explore the case for adding 'this->' to the LHS of a self assignment, very
+ // common for setters.
+ // struct A {
+ // int X;
+ // -void setX(int X) { X = X; }
+ // +void setX(int X) { this->X = X; }
+ // };
+
+ // Only consider parameters for self assignment fixes.
+ if (!isa<ParmVarDecl>(SelfAssigned))
+ return nullptr;
+ const auto *Method =
+ dyn_cast_or_null<CXXMethodDecl>(getCurFunctionDecl(true));
+ if (!Method)
+ return nullptr;
+
+ 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 nullptr;
+
+ // FIXME: Use an actual Lookup operation instead of just traversing fields
+ // in order to get base class fields.
+ auto Field =
+ llvm::find_if(Parent->fields(),
+ [Name(SelfAssigned->getDeclName())](const FieldDecl *F) {
+ return F->getDeclName() == Name;
+ });
+ return (Field != Parent->field_end()) ? *Field : nullptr;
+}
+
/// DiagnoseSelfAssignment - Emits a warning if a value is assigned to itself.
/// This warning suppressed in the event of macro expansions.
static void DiagnoseSelfAssignment(Sema &S, Expr *LHSExpr, Expr *RHSExpr,
@@ -14630,10 +14664,16 @@
if (RefTy->getPointeeType().isVolatileQualified())
return;
- S.Diag(OpLoc, IsBuiltin ? diag::warn_self_assignment_builtin
- : diag::warn_self_assignment_overloaded)
- << LHSDeclRef->getType() << LHSExpr->getSourceRange()
- << RHSExpr->getSourceRange();
+ auto Diag = S.Diag(OpLoc, IsBuiltin ? diag::warn_self_assignment_builtin
+ : diag::warn_self_assignment_overloaded)
+ << LHSDeclRef->getType() << LHSExpr->getSourceRange()
+ << RHSExpr->getSourceRange();
+ if (const FieldDecl *SelfAssignField =
+ S.getSelfAssignmentClassMemberCandidate(RHSDecl))
+ Diag << 1 << SelfAssignField
+ << FixItHint::CreateInsertion(LHSDeclRef->getBeginLoc(), "this->");
+ else
+ Diag << 0;
}
/// Check if a bitwise-& is performed on an Objective-C pointer. This
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -16765,9 +16765,15 @@
RHSDeclRef->getDecl()->getCanonicalDecl())
return;
- Diag(OpLoc, diag::warn_self_move) << LHSExpr->getType()
- << LHSExpr->getSourceRange()
- << RHSExpr->getSourceRange();
+ auto D = Diag(OpLoc, diag::warn_self_move)
+ << LHSExpr->getType() << LHSExpr->getSourceRange()
+ << RHSExpr->getSourceRange();
+ if (const FieldDecl *F =
+ getSelfAssignmentClassMemberCandidate(RHSDeclRef->getDecl()))
+ D << 1 << F
+ << FixItHint::CreateInsertion(LHSDeclRef->getBeginLoc(), "this->");
+ else
+ D << 0;
return;
}
@@ -16802,16 +16808,16 @@
RHSDeclRef->getDecl()->getCanonicalDecl())
return;
- Diag(OpLoc, diag::warn_self_move) << LHSExpr->getType()
- << LHSExpr->getSourceRange()
- << RHSExpr->getSourceRange();
+ Diag(OpLoc, diag::warn_self_move)
+ << LHSExpr->getType() << 0 << LHSExpr->getSourceRange()
+ << RHSExpr->getSourceRange();
return;
}
if (isa<CXXThisExpr>(LHSBase) && isa<CXXThisExpr>(RHSBase))
- Diag(OpLoc, diag::warn_self_move) << LHSExpr->getType()
- << LHSExpr->getSourceRange()
- << RHSExpr->getSourceRange();
+ Diag(OpLoc, diag::warn_self_move)
+ << LHSExpr->getType() << 0 << LHSExpr->getSourceRange()
+ << RHSExpr->getSourceRange();
}
//===--- Layout compatibility ----------------------------------------------//
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -5167,6 +5167,11 @@
void DiagnoseSelfMove(const Expr *LHSExpr, const Expr *RHSExpr,
SourceLocation OpLoc);
+ /// Returns a field in a CXXRecordDecl that has the same name as the decl \p
+ /// SelfAssigned when inside a CXXMethodDecl.
+ const FieldDecl *
+ getSelfAssignmentClassMemberCandidate(const ValueDecl *SelfAssigned);
+
/// Warn if we're implicitly casting from a _Nullable pointer type to a
/// _Nonnull one.
void diagnoseNullableToNonnullConversion(QualType DstType, QualType SrcType,
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6605,13 +6605,16 @@
"'%1' will be evaluated first">, InGroup<ShiftOpParentheses>;
def warn_self_assignment_builtin : Warning<
- "explicitly assigning value of variable of type %0 to itself">,
+ "explicitly assigning value of variable of type %0 to itself%select{|; did "
+ "you mean to assign to member %2?}1">,
InGroup<SelfAssignment>, DefaultIgnore;
def warn_self_assignment_overloaded : Warning<
- "explicitly assigning value of variable of type %0 to itself">,
+ "explicitly assigning value of variable of type %0 to itself%select{|; did "
+ "you mean to assign to member %2?}1">,
InGroup<SelfAssignmentOverloaded>, DefaultIgnore;
def warn_self_move : Warning<
- "explicitly moving variable of type %0 to itself">,
+ "explicitly moving variable of type %0 to itself%select{|; did you mean to "
+ "move to member %2?}1">,
InGroup<SelfMove>, DefaultIgnore;
def err_builtin_move_forward_unsupported : Error<
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``, ``-Wself-assign-overloaded`` and ``-Wself-move`` 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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
