lebedev.ri updated this revision to Diff 140547.
lebedev.ri added a comment.
- Rebased
- Also test that `*&` contraption silences the warning.
Repository:
rC Clang
https://reviews.llvm.org/D44883
Files:
docs/ReleaseNotes.rst
lib/Sema/SemaExpr.cpp
test/SemaCXX/implicit-exception-spec.cpp
test/SemaCXX/member-init.cpp
test/SemaCXX/warn-self-assign-builtin.cpp
test/SemaCXX/warn-self-assign-field-builtin.cpp
test/SemaCXX/warn-self-assign-field-overloaded.cpp
test/SemaCXX/warn-self-assign-overloaded.cpp
test/SemaCXX/warn-self-assign.cpp
Index: test/SemaCXX/warn-self-assign-overloaded.cpp
===================================================================
--- /dev/null
+++ test/SemaCXX/warn-self-assign-overloaded.cpp
@@ -0,0 +1,97 @@
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign -DDUMMY -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign -DV0 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign -DV1 -verify %s
+// 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
+
+#ifdef DUMMY
+struct S {};
+#else
+struct S {
+#if defined(V0)
+ S() = default;
+#elif defined(V1)
+ S &operator=(const S &) = default;
+#elif defined(V2)
+ S &operator=(S &) = default;
+#elif defined(V3)
+ S &operator=(const S &);
+#elif defined(V4)
+ S &operator=(S &);
+#else
+#error Define something!
+#endif
+ S &operator*=(const S &);
+ S &operator/=(const S &);
+ S &operator%=(const S &);
+ S &operator+=(const S &);
+ S &operator-=(const S &);
+ S &operator<<=(const S &);
+ S &operator>>=(const S &);
+ 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;
+ a /= a; // expected-warning {{explicitly assigning}}
+ a %= a; // expected-warning {{explicitly assigning}}
+ a += a;
+ a -= a; // expected-warning {{explicitly assigning}}
+ a <<= a;
+ a >>= a;
+ a &= a; // expected-warning {{explicitly assigning}}
+ a |= a; // expected-warning {{explicitly assigning}}
+ a ^= a; // expected-warning {{explicitly assigning}}
+#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
+
+ // Ways to silence the warning.
+ a = *&a;
+ a = (S &)a;
+
+#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 <typename T>
+void g() {
+ T a;
+ a = a; // expected-warning{{explicitly assigning}}
+}
+void instantiate() {
+ g<int>();
+ g<S>();
+}
Index: test/SemaCXX/warn-self-assign-field-overloaded.cpp
===================================================================
--- /dev/null
+++ test/SemaCXX/warn-self-assign-field-overloaded.cpp
@@ -0,0 +1,146 @@
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -DDUMMY -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -DV0 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -DV1 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -DV2 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -DV3 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -DV4 -verify %s
+
+#ifdef DUMMY
+struct S {};
+#else
+struct S {
+#if defined(V0)
+ S() = default;
+#elif defined(V1)
+ S &operator=(const S &) = default;
+#elif defined(V2)
+ S &operator=(S &) = default;
+#elif defined(V3)
+ S &operator=(const S &);
+#elif defined(V4)
+ S &operator=(S &);
+#else
+#error Define something!
+#endif
+ S &operator*=(const S &);
+ S &operator/=(const S &);
+ S &operator%=(const S &);
+ S &operator+=(const S &);
+ S &operator-=(const S &);
+ S &operator<<=(const S &);
+ S &operator>>=(const S &);
+ S &operator&=(const S &);
+ S &operator|=(const S &);
+ S &operator^=(const S &);
+ S &operator=(const volatile S &) volatile;
+};
+#endif
+struct C {
+ S a;
+ S b;
+
+ void f() {
+ a = a; // expected-warning {{assigning field to itself}}
+ b = b; // expected-warning {{assigning field to itself}}
+ a = b;
+
+ this->a = a; // expected-warning {{assigning field to itself}}
+ this->b = b; // expected-warning {{assigning field to itself}}
+ a = this->a; // expected-warning {{assigning field to itself}}
+ b = this->b; // expected-warning {{assigning field to itself}}
+ this->a = this->a; // expected-warning {{assigning field to itself}}
+ this->b = this->b; // expected-warning {{assigning field to itself}}
+
+ a = b;
+ a = this->b;
+ this->a = b;
+ this->a = this->b;
+
+#ifndef DUMMY
+ a *= a;
+ a /= a; // expected-warning {{assigning field to itself}}
+ a %= a; // expected-warning {{assigning field to itself}}
+ a += a;
+ a -= a; // expected-warning {{assigning field to itself}}
+ a <<= a;
+ a >>= a;
+ a &= a; // expected-warning {{assigning field to itself}}
+ a |= a; // expected-warning {{assigning field to itself}}
+ a ^= a; // expected-warning {{assigning field to itself}}
+#endif
+ }
+
+ void false_positives() {
+#define OP =
+#define LHS a
+#define RHS a
+ // These shouldn't warn due to the use of the preprocessor.
+ a OP a; // expected-warning {{assigning field to itself}}
+ LHS = a; // expected-warning {{assigning field to itself}}
+ a = RHS; // expected-warning {{assigning field to itself}}
+ LHS OP RHS; // expected-warning {{assigning field to itself}}
+#undef OP
+#undef LHS
+#undef RHS
+
+ // Ways to silence the warning.
+ a = *&a;
+ a = (S &)a;
+ }
+
+#ifndef DUMMY
+ volatile S vol_a;
+ void vol_test() {
+ // Volatile stores aren't side-effect free.
+ vol_a = vol_a; // expected-warning {{assigning field to itself}}
+ volatile S &vol_a_ref = vol_a;
+ vol_a_ref = vol_a_ref;
+ }
+#endif
+};
+
+template <typename T>
+struct TemplateClass {
+ T var;
+ void f() {
+ var = var; // expected-warning 3 {{assigning field to itself}}
+ }
+};
+void instantiate() {
+ {
+ TemplateClass<int> c;
+ c.f(); // expected-note {{in instantiation of member function}}
+ }
+ {
+ TemplateClass<S> c;
+ c.f(); // expected-note {{in instantiation of member function}}
+ }
+}
+
+// It may make sense not to warn on the rest of the tests.
+// It may be a valid use-case to self-assign to tell the compiler that
+// it is ok to vectorize the store.
+
+void f0(C *s, C *t) {
+ s->a = s->a;
+ t->a = s->a;
+}
+
+void f1(C &s, C &t) {
+ s.a = s.a;
+ t.a = s.a;
+}
+
+struct T {
+ C *s;
+};
+
+void f2(T *t, T *t2) {
+ t->s->a = t->s->a;
+ t2->s->a = t->s->a;
+}
+
+void f3(T &t, T &t2) {
+ t.s->a = t.s->a;
+ t2.s->a = t.s->a;
+}
Index: test/SemaCXX/warn-self-assign-field-builtin.cpp
===================================================================
--- /dev/null
+++ test/SemaCXX/warn-self-assign-field-builtin.cpp
@@ -0,0 +1,109 @@
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign-field -verify %s
+
+struct C {
+ int a;
+ int b;
+
+ void f() {
+ a = a; // expected-warning {{assigning field to itself}}
+ b = b; // expected-warning {{assigning field to itself}}
+ a = b;
+
+ this->a = a; // expected-warning {{assigning field to itself}}
+ this->b = b; // expected-warning {{assigning field to itself}}
+ a = this->a; // expected-warning {{assigning field to itself}}
+ b = this->b; // expected-warning {{assigning field to itself}}
+ this->a = this->a; // expected-warning {{assigning field to itself}}
+ this->b = this->b; // expected-warning {{assigning field to itself}}
+
+ a = b;
+ a = this->b;
+ this->a = b;
+ this->a = this->b;
+
+ a *= a;
+ a /= a;
+ a %= a;
+ a += a;
+ a -= a;
+ a <<= a;
+ a >>= a;
+ a &= a;
+ a |= a;
+ a ^= a;
+ }
+
+ void false_positives() {
+#define OP =
+#define LHS a
+#define RHS a
+ // These shouldn't warn due to the use of the preprocessor.
+ a OP a; // expected-warning {{assigning field to itself}}
+ LHS = a; // expected-warning {{assigning field to itself}}
+ a = RHS; // expected-warning {{assigning field to itself}}
+ LHS OP RHS; // expected-warning {{assigning field to itself}}
+#undef OP
+#undef LHS
+#undef RHS
+
+ // A way to silence the warning.
+ a = (int &)a;
+ }
+
+ volatile int vol_a;
+ void vol_test() {
+ // Volatile stores aren't side-effect free.
+ vol_a = vol_a; // expected-warning {{assigning field to itself}}
+ volatile int &vol_a_ref = vol_a;
+ vol_a_ref = vol_a_ref;
+ }
+};
+
+// Dummy type.
+struct Dummy {};
+
+template <typename T>
+struct TemplateClass {
+ T var;
+ void f() {
+ var = var; // expected-warning 3 {{assigning field to itself}}
+ }
+};
+void instantiate() {
+ {
+ TemplateClass<int> c;
+ c.f(); // expected-note {{in instantiation of member function}}
+ }
+ {
+ TemplateClass<Dummy> c;
+ c.f(); // expected-note {{in instantiation of member function}}
+ }
+}
+
+// It may make sense not to warn on the rest of the tests.
+// It may be a valid use-case to self-assign to tell the compiler that
+// it is ok to vectorize the store.
+
+void f0(C *s, C *t) {
+ s->a = s->a;
+ t->a = s->a;
+}
+
+void f1(C &s, C &t) {
+ s.a = s.a;
+ t.a = s.a;
+}
+
+struct T {
+ C *s;
+};
+
+void f2(T *t, T *t2) {
+ t->s->a = t->s->a;
+ t2->s->a = t->s->a;
+}
+
+void f3(T &t, T &t2) {
+ t.s->a = t.s->a;
+ t2.s->a = t.s->a;
+}
Index: test/SemaCXX/warn-self-assign-builtin.cpp
===================================================================
--- test/SemaCXX/warn-self-assign-builtin.cpp
+++ test/SemaCXX/warn-self-assign-builtin.cpp
@@ -8,8 +8,16 @@
b = a = b;
a = a = a; // expected-warning{{explicitly assigning}}
a = b = b = a;
- a &= a; // expected-warning{{explicitly assigning}}
- a |= a; // expected-warning{{explicitly assigning}}
+
+ a *= a;
+ a /= a;
+ a %= a;
+ a += a;
+ a -= a;
+ a <<= a;
+ a >>= a;
+ a &= a; // expected-warning {{explicitly assigning}}
+ a |= a; // expected-warning {{explicitly assigning}}
a ^= a;
}
@@ -30,19 +38,20 @@
#undef LHS
#undef RHS
- S s;
- s = s; // Not a builtin assignment operator, no warning.
+ // A way to silence the warning.
+ a = (int &)a;
// Volatile stores aren't side-effect free.
volatile int vol_a;
vol_a = vol_a;
volatile int &vol_a_ref = vol_a;
vol_a_ref = vol_a_ref;
}
-template <typename T> void g() {
+template <typename T>
+void g() {
T a;
- a = a; // May or may not be a builtin assignment operator, no warning.
+ a = a; // expected-warning{{explicitly assigning}}
}
void instantiate() {
g<int>();
Index: test/SemaCXX/member-init.cpp
===================================================================
--- test/SemaCXX/member-init.cpp
+++ test/SemaCXX/member-init.cpp
@@ -101,7 +101,7 @@
struct Sprite {
Point location = Point(0,0); // expected-error {{no matching constructor for initialization of 'rdar14084171::Point'}}
};
- void f(Sprite& x) { x = x; }
+ void f(Sprite& x) { x = x; } // expected-warning {{explicitly assigning value of variable}}
}
namespace PR18560 {
Index: test/SemaCXX/implicit-exception-spec.cpp
===================================================================
--- test/SemaCXX/implicit-exception-spec.cpp
+++ test/SemaCXX/implicit-exception-spec.cpp
@@ -119,7 +119,7 @@
template<typename T, bool A, bool B, bool C, bool D, bool E, bool F> void check() {
T *p = nullptr;
T &a = *p;
- static_assert(noexcept(a = a) == D, "");
+ static_assert(noexcept(a = a) == D, ""); // expected-warning {{explicitly assigning value of variable}}
static_assert(noexcept(a = static_cast<T&&>(a)) == E, "");
static_assert(noexcept(delete &a) == F, "");
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -11471,8 +11471,7 @@
}
/// 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) {
if (S.inTemplateInstantiation())
@@ -12091,6 +12090,21 @@
static ExprResult BuildOverloadedBinOp(Sema &S, Scope *Sc, SourceLocation OpLoc,
BinaryOperatorKind Opc,
Expr *LHS, Expr *RHS) {
+ switch (Opc) {
+ case BO_Assign:
+ case BO_DivAssign:
+ case BO_RemAssign:
+ case BO_SubAssign:
+ case BO_AndAssign:
+ case BO_OrAssign:
+ case BO_XorAssign:
+ DiagnoseSelfAssignment(S, LHS, RHS, OpLoc);
+ CheckIdentityFieldAssignment(LHS, RHS, OpLoc, S);
+ 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: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -59,6 +59,9 @@
``-Wno-c++98-compat-extra-semi``, so if you want that diagnostic, you need
to explicitly re-enable it (e.g. by appending ``-Wextra-semi``).
+- ``-Wself-assign`` and ``-Wself-assign-field`` were extended to diagnose
+ self-assignment operations using overloaded operators (i.e. classes)
+
Non-comprehensive list of changes in this release
-------------------------------------------------
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
