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