Warn on self-assignment to member variables. PR13104.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@159394 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td
index a99eb1d..ab2cd1b 100644
--- a/include/clang/Basic/DiagnosticGroups.td
+++ b/include/clang/Basic/DiagnosticGroups.td
@@ -169,7 +169,8 @@
def ReturnType : DiagGroup<"return-type", [ReturnTypeCLinkage]>;
def BindToTemporaryCopy : DiagGroup<"bind-to-temporary-copy",
[CXX98CompatBindToTemporaryCopy]>;
-def SelfAssignment : DiagGroup<"self-assign">;
+def SelfAssignmentMemvar : DiagGroup<"self-assign-memvar">;
+def SelfAssignment : DiagGroup<"self-assign", [SelfAssignmentMemvar]>;
def SemiBeforeMethodBody : DiagGroup<"semicolon-before-method-body">;
def Sentinel : DiagGroup<"sentinel">;
def MissingMethodReturnType : DiagGroup<"missing-method-return-type">;
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 91f90ca..0b3a8ed 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5274,6 +5274,10 @@
"unspecified (use strncmp instead)">,
InGroup<DiagGroup<"string-compare">>;
+def warn_identity_memvar_assign : Warning<
+ "assigning %select{member variable|instance variable}0 to itself">,
+ InGroup<SelfAssignmentMemvar>;
+
// Generic selections.
def err_assoc_type_incomplete : Error<
"type %0 in generic association incomplete">;
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index a6051d6..cd3c93b 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -7558,7 +7558,27 @@
return true;
}
+static void CheckIdentityMemvarAssignment(Expr *LHSExpr, Expr *RHSExpr,
+ SourceLocation Loc,
+ Sema &Sema) {
+ // C / C++ memvars
+ MemberExpr *ML = dyn_cast<MemberExpr>(LHSExpr);
+ MemberExpr *MR = dyn_cast<MemberExpr>(RHSExpr);
+ if (ML && MR && ML->getMemberDecl() == MR->getMemberDecl()) {
+ if (isa<CXXThisExpr>(ML->getBase()) && isa<CXXThisExpr>(MR->getBase()))
+ Sema.Diag(Loc, diag::warn_identity_memvar_assign) << 0;
+ }
+ // Objective-C memvars
+ ObjCIvarRefExpr *OL = dyn_cast<ObjCIvarRefExpr>(LHSExpr);
+ ObjCIvarRefExpr *OR = dyn_cast<ObjCIvarRefExpr>(RHSExpr);
+ if (OL && OR && OL->getDecl() == OR->getDecl()) {
+ DeclRefExpr *RL = dyn_cast<DeclRefExpr>(OL->getBase()->IgnoreImpCasts());
+ DeclRefExpr *RR = dyn_cast<DeclRefExpr>(OR->getBase()->IgnoreImpCasts());
+ if (RL && RR && RL->getDecl() == RR->getDecl())
+ Sema.Diag(Loc, diag::warn_identity_memvar_assign) << 1;
+ }
+}
// C99 6.5.16.1
QualType Sema::CheckAssignmentOperands(Expr *LHSExpr, ExprResult &RHS,
@@ -7575,6 +7595,10 @@
CompoundType;
AssignConvertType ConvTy;
if (CompoundType.isNull()) {
+ Expr *RHSCheck = RHS.get();
+
+ CheckIdentityMemvarAssignment(LHSExpr, RHSCheck, Loc, *this);
+
QualType LHSTy(LHSType);
ConvTy = CheckSingleAssignmentConstraints(LHSTy, RHS);
if (RHS.isInvalid())
@@ -7595,7 +7619,6 @@
// If the RHS is a unary plus or minus, check to see if they = and + are
// right next to each other. If so, the user may have typo'd "x =+ 4"
// instead of "x += 4".
- Expr *RHSCheck = RHS.get();
if (ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(RHSCheck))
RHSCheck = ICE->getSubExpr();
if (UnaryOperator *UO = dyn_cast<UnaryOperator>(RHSCheck)) {
diff --git a/test/Sema/warn-self-assign-memvar.mm b/test/Sema/warn-self-assign-memvar.mm
new file mode 100644
index 0000000..8c6fb0a
--- /dev/null
+++ b/test/Sema/warn-self-assign-memvar.mm
@@ -0,0 +1,66 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+class S {
+ public:
+ int a_;
+ void s(int a) {
+ a_ = a_; // expected-warning {{assigning member variable to itself}}
+
+ // Don't really care about this one either way.
+ this->a_ = a_; // expected-warning {{assigning member variable to itself}}
+
+ a_ += a_; // Shouldn't warn.
+ }
+};
+
+void f0(S* s) {
+ // Would be nice to have, but not important.
+ s->a_ = s->a_;
+}
+
+void f1(S* s, S* t) {
+ // Shouldn't warn.
+ t->a_ = s->a_;
+}
+
+struct T {
+ S* s_;
+};
+
+void f2(T* t) {
+ // Would be nice to have, but even less important.
+ t->s_->a_ = t->s_->a_;
+}
+
+void f3(T* t, T* t2) {
+ // Shouldn't warn.
+ t2->s_->a_ = t->s_->a_;
+}
+
+void f4(int i) {
+ // This is a common pattern to silence "parameter unused". Shouldn't warn.
+ i = i;
+
+ int j = 0;
+ j = j; // Likewise.
+}
+
+@interface I {
+ int a_;
+}
+@end
+
+@implementation I
+- (void)setA:(int)a {
+ a_ = a_; // expected-warning {{assigning instance variable to itself}}
+}
+
+- (void)foo:(I*)i {
+ // Don't care much about this warning.
+ i->a_ = i->a_; // expected-warning {{assigning instance variable to itself}}
+
+ // Shouldn't warn.
+ a_ = i->a_;
+ i->a_ = a_;
+}
+@end
diff --git a/test/SemaObjC/provisional-ivar-lookup.m b/test/SemaObjC/provisional-ivar-lookup.m
index 2ec23a5..7460fc2 100644
--- a/test/SemaObjC/provisional-ivar-lookup.m
+++ b/test/SemaObjC/provisional-ivar-lookup.m
@@ -36,7 +36,7 @@
@synthesize PROP=PROP;
- (void)setPROP:(int)value {
- PROP = PROP; // OK
+ PROP = value; // OK
}
@end