Skip to content

Commit

Permalink
[Sema] Add -Wno-self-assign-overloaded
Browse files Browse the repository at this point in the history
Summary:
It seems there isn't much enthusiasm for `-wtest` D45685.

This is more conservative version, which i had in the very first
revision of D44883, but that 'erroneously' got removed because of the review.

**Based on some [irc] discussions, it must really be documented that
we want all the new diagnostics to have their own flags, to ease
rollouts, transitions, etc.**

Please do note that i'm only adding `-Wno-self-assign-overloaded`,
but not `-Wno-self-assign-field-overloaded`, because i'm honestly
not aware of any false-positives from the `-field` variant,
but i can just as easily add it if wanted.
https://reviews.llvm.org/D44883#1068561

Reviewers: dblaikie, aaron.ballman, thakis, rjmccall, rsmith

Reviewed By: dblaikie

Subscribers: Quuxplusone, chandlerc, cfe-commits

Differential Revision: https://reviews.llvm.org/D45766

llvm-svn: 330651
  • Loading branch information
LebedevRI committed Apr 23, 2018
1 parent d6af13e commit 6ed0fad
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 11 deletions.
5 changes: 3 additions & 2 deletions clang/docs/ReleaseNotes.rst
Expand Up @@ -62,8 +62,9 @@ Improvements to Clang's diagnostics
- ``-Wself-assign`` and ``-Wself-assign-field`` were extended to diagnose
self-assignment operations using overloaded operators (i.e. classes).
If you are doing such an assignment intentionally, e.g. in a unit test for
a data structure, the warning can be suppressed by adding ``*&`` to the
right-hand side or casting it to the appropriate reference type.
a data structure, the first warning can be disabled by passing
``-Wno-self-assign-overloaded``, also the warning can be suppressed by adding
``*&`` to the right-hand side or casting it to the appropriate reference type.

Non-comprehensive list of changes in this release
-------------------------------------------------
Expand Down
3 changes: 2 additions & 1 deletion clang/include/clang/Basic/DiagnosticGroups.td
Expand Up @@ -404,7 +404,8 @@ 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 SelfAssignment : DiagGroup<"self-assign", [SelfAssignmentOverloaded, SelfAssignmentField]>;
def SelfMove : DiagGroup<"self-move">;
def SemiBeforeMethodBody : DiagGroup<"semicolon-before-method-body">;
def Sentinel : DiagGroup<"sentinel">;
Expand Down
5 changes: 4 additions & 1 deletion clang/include/clang/Basic/DiagnosticSemaKinds.td
Expand Up @@ -5612,9 +5612,12 @@ def warn_addition_in_bitshift : Warning<
"operator '%0' has lower precedence than '%1'; "
"'%1' will be evaluated first">, InGroup<ShiftOpParentheses>;

def warn_self_assignment : Warning<
def warn_self_assignment_builtin : Warning<
"explicitly assigning value of variable of type %0 to itself">,
InGroup<SelfAssignment>, DefaultIgnore;
def warn_self_assignment_overloaded : Warning<
"explicitly assigning value of variable of type %0 to itself">,
InGroup<SelfAssignmentOverloaded>, DefaultIgnore;
def warn_self_move : Warning<
"explicitly moving variable of type %0 to itself">,
InGroup<SelfMove>, DefaultIgnore;
Expand Down
15 changes: 8 additions & 7 deletions clang/lib/Sema/SemaExpr.cpp
Expand Up @@ -11497,7 +11497,7 @@ static inline UnaryOperatorKind ConvertTokenKindToUnaryOpcode(
/// 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,
SourceLocation OpLoc) {
SourceLocation OpLoc, bool IsBuiltin) {
if (S.inTemplateInstantiation())
return;
if (S.isUnevaluatedContext())
Expand All @@ -11524,9 +11524,10 @@ static void DiagnoseSelfAssignment(Sema &S, Expr *LHSExpr, Expr *RHSExpr,
if (RefTy->getPointeeType().isVolatileQualified())
return;

S.Diag(OpLoc, diag::warn_self_assignment)
<< LHSDeclRef->getType()
<< LHSExpr->getSourceRange() << RHSExpr->getSourceRange();
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
Expand Down Expand Up @@ -11719,7 +11720,7 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc,
OK = LHS.get()->getObjectKind();
}
if (!ResultTy.isNull()) {
DiagnoseSelfAssignment(*this, LHS.get(), RHS.get(), OpLoc);
DiagnoseSelfAssignment(*this, LHS.get(), RHS.get(), OpLoc, true);
DiagnoseSelfMove(LHS.get(), RHS.get(), OpLoc);
}
RecordModifiableNonNullParam(*this, LHS.get());
Expand Down Expand Up @@ -11817,7 +11818,7 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc,
break;
case BO_AndAssign:
case BO_OrAssign: // fallthrough
DiagnoseSelfAssignment(*this, LHS.get(), RHS.get(), OpLoc);
DiagnoseSelfAssignment(*this, LHS.get(), RHS.get(), OpLoc, true);
LLVM_FALLTHROUGH;
case BO_XorAssign:
CompResultTy = CheckBitwiseOperands(LHS, RHS, OpLoc, Opc);
Expand Down Expand Up @@ -12124,7 +12125,7 @@ static ExprResult BuildOverloadedBinOp(Sema &S, Scope *Sc, SourceLocation OpLoc,
case BO_AndAssign:
case BO_OrAssign:
case BO_XorAssign:
DiagnoseSelfAssignment(S, LHS, RHS, OpLoc);
DiagnoseSelfAssignment(S, LHS, RHS, OpLoc, false);
CheckIdentityFieldAssignment(LHS, RHS, OpLoc, S);
break;
default:
Expand Down
18 changes: 18 additions & 0 deletions clang/test/SemaCXX/warn-self-assign-overloaded-disable.cpp
@@ -0,0 +1,18 @@
// RUN: %clang_cc1 -fsyntax-only -Wall -verify %s
// RUN: %clang_cc1 -fsyntax-only -Wself-assign -verify %s
// RUN: %clang_cc1 -fsyntax-only -Wself-assign-overloaded -verify %s
// RUN: %clang_cc1 -fsyntax-only -Wall -Wno-self-assign-overloaded -DSILENCE -verify %s
// RUN: %clang_cc1 -fsyntax-only -Wself-assign -Wno-self-assign-overloaded -DSILENCE -verify %s
// RUN: %clang_cc1 -fsyntax-only -Wself-assign-overloaded -Wno-self-assign-overloaded -DSILENCE -verify %s

struct S {};

void f() {
S a;
#ifndef SILENCE
a = a; // expected-warning{{explicitly assigning}}
#else
// expected-no-diagnostics
a = a;
#endif
}
6 changes: 6 additions & 0 deletions clang/test/SemaCXX/warn-self-assign-overloaded.cpp
Expand Up @@ -4,6 +4,12 @@
// RUN: %clang_cc1 -fsyntax-only -Wself-assign -DV2 -verify %s
// RUN: %clang_cc1 -fsyntax-only -Wself-assign -DV3 -verify %s
// RUN: %clang_cc1 -fsyntax-only -Wself-assign -DV4 -verify %s
// RUN: %clang_cc1 -fsyntax-only -Wno-self-assign -Wself-assign-overloaded -DDUMMY -verify %s
// RUN: %clang_cc1 -fsyntax-only -Wno-self-assign -Wself-assign-overloaded -DV0 -verify %s
// RUN: %clang_cc1 -fsyntax-only -Wno-self-assign -Wself-assign-overloaded -DV1 -verify %s
// RUN: %clang_cc1 -fsyntax-only -Wno-self-assign -Wself-assign-overloaded -DV2 -verify %s
// RUN: %clang_cc1 -fsyntax-only -Wno-self-assign -Wself-assign-overloaded -DV3 -verify %s
// RUN: %clang_cc1 -fsyntax-only -Wno-self-assign -Wself-assign-overloaded -DV4 -verify %s

#ifdef DUMMY
struct S {};
Expand Down

0 comments on commit 6ed0fad

Please sign in to comment.