Index: include/clang/Basic/DiagnosticGroups.td =================================================================== --- include/clang/Basic/DiagnosticGroups.td +++ include/clang/Basic/DiagnosticGroups.td @@ -399,8 +399,11 @@ def ReturnType : DiagGroup<"return-type", [ReturnTypeCLinkage]>; def BindToTemporaryCopy : DiagGroup<"bind-to-temporary-copy", [CXX98CompatBindToTemporaryCopy]>; -def SelfAssignmentField : DiagGroup<"self-assign-field">; -def SelfAssignment : DiagGroup<"self-assign", [SelfAssignmentField]>; +def SelfAssignmentOverloaded : DiagGroup<"self-assign-overloaded">; +def SelfAssignmentBuiltinField : DiagGroup<"self-assign-field">; +def SelfAssignmentBuiltin : DiagGroup<"self-assign-builtin", [SelfAssignmentBuiltinField]>; +def SelfAssignment : DiagGroup<"self-assign", [SelfAssignmentBuiltin, + SelfAssignmentOverloaded]>; def SelfMove : DiagGroup<"self-move">; def SemiBeforeMethodBody : DiagGroup<"semicolon-before-method-body">; def Sentinel : DiagGroup<"sentinel">; @@ -729,6 +732,7 @@ MissingFieldInitializers, IgnoredQualifiers, InitializerOverrides, + SelfAssignmentOverloaded, SemiBeforeMethodBody, MissingMethodReturnType, SignCompare, @@ -750,7 +754,7 @@ MultiChar, Reorder, ReturnType, - SelfAssignment, + SelfAssignmentBuiltin, SelfMove, SizeofArrayArgument, SizeofArrayDecay, Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -5601,9 +5601,12 @@ "operator '%0' has lower precedence than '%1'; " "'%1' will be evaluated first">, InGroup; -def warn_self_assignment : Warning< +def warn_self_assignment_overloaded : Warning< + "explicitly assigning value of variable of class type %0 to itself">, + InGroup, DefaultIgnore; +def warn_self_assignment_builtin : Warning< "explicitly assigning value of variable of type %0 to itself">, - InGroup, DefaultIgnore; + InGroup, DefaultIgnore; def warn_self_move : Warning< "explicitly moving variable of type %0 to itself">, InGroup, DefaultIgnore; @@ -7925,9 +7928,9 @@ "unspecified (use strncmp instead)">, InGroup; -def warn_identity_field_assign : Warning< +def warn_identity_field_assign_builtin : Warning< "assigning %select{field|instance variable}0 to itself">, - InGroup; + InGroup; // Type safety attributes def err_type_tag_for_datatype_not_ice : Error< Index: lib/Sema/SemaExpr.cpp =================================================================== --- lib/Sema/SemaExpr.cpp +++ lib/Sema/SemaExpr.cpp @@ -10698,14 +10698,13 @@ } static void CheckIdentityFieldAssignment(Expr *LHSExpr, Expr *RHSExpr, - SourceLocation Loc, - Sema &Sema) { + SourceLocation Loc, Sema &Sema) { // C / C++ fields MemberExpr *ML = dyn_cast(LHSExpr); MemberExpr *MR = dyn_cast(RHSExpr); if (ML && MR && ML->getMemberDecl() == MR->getMemberDecl()) { if (isa(ML->getBase()) && isa(MR->getBase())) - Sema.Diag(Loc, diag::warn_identity_field_assign) << 0; + Sema.Diag(Loc, diag::warn_identity_field_assign_builtin) << 0; } // Objective-C instance variables @@ -10715,7 +10714,7 @@ DeclRefExpr *RL = dyn_cast(OL->getBase()->IgnoreImpCasts()); DeclRefExpr *RR = dyn_cast(OR->getBase()->IgnoreImpCasts()); if (RL && RR && RL->getDecl() == RR->getDecl()) - Sema.Diag(Loc, diag::warn_identity_field_assign) << 1; + Sema.Diag(Loc, diag::warn_identity_field_assign_builtin) << 1; } } @@ -11459,10 +11458,9 @@ } /// DiagnoseSelfAssignment - Emits a warning if a value is assigned to itself. -/// This warning is only emitted for builtin assignment operations. It is also -/// suppressed in the event of macro expansions. +/// This warning suppressed in the event of macro expansions. static void DiagnoseSelfAssignment(Sema &S, Expr *LHSExpr, Expr *RHSExpr, - SourceLocation OpLoc) { + SourceLocation OpLoc, bool IsBuiltin) { if (S.inTemplateInstantiation()) return; if (OpLoc.isInvalid() || OpLoc.isMacroID()) @@ -11487,9 +11485,11 @@ if (RefTy->getPointeeType().isVolatileQualified()) return; - S.Diag(OpLoc, diag::warn_self_assignment) - << LHSDeclRef->getType() - << LHSExpr->getSourceRange() << RHSExpr->getSourceRange(); + // FIXME: what do we want to do with trivial operator=()? treat it as builtin? + S.Diag(OpLoc, IsBuiltin ? diag::warn_self_assignment_builtin + : diag::warn_self_assignment_overloaded) + << LHSDeclRef->getType() << LHSExpr->getSourceRange() + << RHSExpr->getSourceRange(); } /// Check if a bitwise-& is performed on an Objective-C pointer. This @@ -11682,7 +11682,8 @@ OK = LHS.get()->getObjectKind(); } if (!ResultTy.isNull()) { - DiagnoseSelfAssignment(*this, LHS.get(), RHS.get(), OpLoc); + DiagnoseSelfAssignment(*this, LHS.get(), RHS.get(), OpLoc, + /*IsBuiltin=*/true); DiagnoseSelfMove(LHS.get(), RHS.get(), OpLoc); } RecordModifiableNonNullParam(*this, LHS.get()); @@ -11780,7 +11781,8 @@ break; case BO_AndAssign: case BO_OrAssign: // fallthrough - DiagnoseSelfAssignment(*this, LHS.get(), RHS.get(), OpLoc); + DiagnoseSelfAssignment(*this, LHS.get(), RHS.get(), OpLoc, + /*IsBuiltin=*/true); LLVM_FALLTHROUGH; case BO_XorAssign: CompResultTy = CheckBitwiseOperands(LHS, RHS, OpLoc, Opc); @@ -12079,6 +12081,16 @@ static ExprResult BuildOverloadedBinOp(Sema &S, Scope *Sc, SourceLocation OpLoc, BinaryOperatorKind Opc, Expr *LHS, Expr *RHS) { + switch (Opc) { + case BO_Assign: + case BO_AndAssign: + case BO_OrAssign: + DiagnoseSelfAssignment(S, LHS, RHS, OpLoc, /*IsBuiltin=*/false); + break; + default: + break; + } + // Find all of the overloaded operators visible from this // point. We perform both an operator-name lookup from the local // scope and an argument-dependent lookup based on the types of Index: test/SemaCXX/warn-self-assign-builtin.cpp =================================================================== --- test/SemaCXX/warn-self-assign-builtin.cpp +++ test/SemaCXX/warn-self-assign-builtin.cpp @@ -1,4 +1,5 @@ -// RUN: %clang_cc1 -fsyntax-only -Wself-assign -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wself-assign -Wno-self-assign-overloaded -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wself-assign-builtin -verify %s void f() { int a = 42, b = 42; @@ -40,7 +41,8 @@ vol_a_ref = vol_a_ref; } -template void g() { +template +void g() { T a; a = a; // May or may not be a builtin assignment operator, no warning. } Index: test/SemaCXX/warn-self-assign-overloaded.cpp =================================================================== --- /dev/null +++ test/SemaCXX/warn-self-assign-overloaded.cpp @@ -0,0 +1,80 @@ +// RUN: %clang_cc1 -fsyntax-only -Wself-assign -Wno-self-assign-builtin -DDUMMY -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wself-assign-overloaded -DDUMMY -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wself-assign -Wno-self-assign-builtin -DV0 -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wself-assign-overloaded -DV0 -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wself-assign -Wno-self-assign-builtin -DV1 -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wself-assign-overloaded -DV1 -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wself-assign -Wno-self-assign-builtin -DV2 -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wself-assign-overloaded -DV2 -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wself-assign -Wno-self-assign-builtin -DV3 -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wself-assign-overloaded -DV3 -verify %s + +#ifdef DUMMY +struct S {}; +#else +struct S { +#if defined(V0) + S &operator=(const S &) = default; +#elif defined(V1) + S &operator=(S &) = default; +#elif defined(V2) + S &operator=(const S &); +#elif defined(V3) + S &operator=(S &); +#else +#error Define something! +#endif + S &operator&=(const S &); + S &operator|=(const S &); + S &operator^=(const S &); + S &operator=(const volatile S &) volatile; +}; +#endif + +void f() { + S a, b; + a = a; // expected-warning{{explicitly assigning}} + b = b; // expected-warning{{explicitly assigning}} + a = b; + b = a = b; + a = a = a; // expected-warning{{explicitly assigning}} + a = b = b = a; +#ifndef DUMMY + a &= a; // expected-warning{{explicitly assigning}} + a |= a; // expected-warning{{explicitly assigning}} + a ^= a; +#endif +} + +void false_positives() { +#define OP = +#define LHS a +#define RHS a + S a; + // These shouldn't warn due to the use of the preprocessor. + a OP a; + LHS = a; + a = RHS; + LHS OP RHS; +#undef OP +#undef LHS +#undef RHS + +#ifndef DUMMY + // Volatile stores aren't side-effect free. + volatile S vol_a; + vol_a = vol_a; + volatile S &vol_a_ref = vol_a; + vol_a_ref = vol_a_ref; +#endif +} + +template +void g() { + T a; + a = a; // expected-warning{{explicitly assigning}} +} +void instantiate() { + g(); + g(); +}