python3kgae updated this revision to Diff 511795. python3kgae added a comment.
Update the comment done by rjmccall. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146897/new/ https://reviews.llvm.org/D146897 Files: clang/lib/Sema/SemaExpr.cpp clang/test/SemaCXX/warn-self-assign-overloaded.cpp Index: clang/test/SemaCXX/warn-self-assign-overloaded.cpp =================================================================== --- clang/test/SemaCXX/warn-self-assign-overloaded.cpp +++ clang/test/SemaCXX/warn-self-assign-overloaded.cpp @@ -53,15 +53,15 @@ #ifndef DUMMY a *= a; - a /= a; // expected-warning {{explicitly assigning}} - a %= a; // expected-warning {{explicitly assigning}} + a /= a; + a %= a; a += a; - a -= a; // expected-warning {{explicitly assigning}} + a -= a; a <<= a; a >>= a; - a &= a; // expected-warning {{explicitly assigning}} - a |= a; // expected-warning {{explicitly assigning}} - a ^= a; // expected-warning {{explicitly assigning}} + a &= a; + a |= a; + a ^= a; #endif } Index: clang/lib/Sema/SemaExpr.cpp =================================================================== --- clang/lib/Sema/SemaExpr.cpp +++ clang/lib/Sema/SemaExpr.cpp @@ -15657,13 +15657,22 @@ Expr *LHS, Expr *RHS) { switch (Opc) { case BO_Assign: + // In the non-overloaded case, we warn about self-assignment (x = x) for + // both simple assignment and certain compound assignments where algebra + // tells us the operation yields a constant result. When the operator is + // overloaded, we can't do the latter because we don't want to assume that + // those algebraic identities still apply; for example, a path-building + // library might use operator/= to append paths. But it's still reasonable + // to assume that simple assignment is just moving/copying values around + // and so self-assignment is likely a bug. + DiagnoseSelfAssignment(S, LHS, RHS, OpLoc, false); + [[fallthrough]]; case BO_DivAssign: case BO_RemAssign: case BO_SubAssign: case BO_AndAssign: case BO_OrAssign: case BO_XorAssign: - DiagnoseSelfAssignment(S, LHS, RHS, OpLoc, false); CheckIdentityFieldAssignment(LHS, RHS, OpLoc, S); break; default:
Index: clang/test/SemaCXX/warn-self-assign-overloaded.cpp =================================================================== --- clang/test/SemaCXX/warn-self-assign-overloaded.cpp +++ clang/test/SemaCXX/warn-self-assign-overloaded.cpp @@ -53,15 +53,15 @@ #ifndef DUMMY a *= a; - a /= a; // expected-warning {{explicitly assigning}} - a %= a; // expected-warning {{explicitly assigning}} + a /= a; + a %= a; a += a; - a -= a; // expected-warning {{explicitly assigning}} + a -= a; a <<= a; a >>= a; - a &= a; // expected-warning {{explicitly assigning}} - a |= a; // expected-warning {{explicitly assigning}} - a ^= a; // expected-warning {{explicitly assigning}} + a &= a; + a |= a; + a ^= a; #endif } Index: clang/lib/Sema/SemaExpr.cpp =================================================================== --- clang/lib/Sema/SemaExpr.cpp +++ clang/lib/Sema/SemaExpr.cpp @@ -15657,13 +15657,22 @@ Expr *LHS, Expr *RHS) { switch (Opc) { case BO_Assign: + // In the non-overloaded case, we warn about self-assignment (x = x) for + // both simple assignment and certain compound assignments where algebra + // tells us the operation yields a constant result. When the operator is + // overloaded, we can't do the latter because we don't want to assume that + // those algebraic identities still apply; for example, a path-building + // library might use operator/= to append paths. But it's still reasonable + // to assume that simple assignment is just moving/copying values around + // and so self-assignment is likely a bug. + DiagnoseSelfAssignment(S, LHS, RHS, OpLoc, false); + [[fallthrough]]; case BO_DivAssign: case BO_RemAssign: case BO_SubAssign: case BO_AndAssign: case BO_OrAssign: case BO_XorAssign: - DiagnoseSelfAssignment(S, LHS, RHS, OpLoc, false); CheckIdentityFieldAssignment(LHS, RHS, OpLoc, S); break; default:
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits