lebedev.ri added a comment. In https://reviews.llvm.org/D44883#1048010, @rjmccall wrote:
> I'm not sure you really need to put these in their own warning sub-group just > because they're user-defined operators. That's especially true because it > appears we already have divisions in the warning group based on the form of > the l-value; we don't want this to go combinatorial. Several reasons: - The initial `-Wself-assign` was intentionally implemented not to warn on overloaded operators. https://github.com/llvm-mirror/clang/commit/9f7a6eeee441bcbb1b17208cb3abd65a0017525a#diff-e0deb7b32f28507a3044a6bf9a63b515R31 (https://reviews.llvm.org/rL122804) - While it is an obvious bug when self-operation happens with builtin operators, i'm less certain of that with overloaded operators. If you happen to be routinely using self-assignment via oh-so-very-special overloaded operator=, and you don't like to have this diagnostic, you could just disable it, and not loose the coverage of the `-Wself-assign-builtin`. If it is all in one group, you can't do that... - Based on previous expirience, separate diag groups are good, see e.g https://reviews.llvm.org/D37620, https://reviews.llvm.org/D37629 - I'm failing to find the original quote, but i **think** @rsmith said something along the "diag groups are cheap, use them". But i may as well be mis-remembering/having false memories here, sorry. TLDR: if you insist, sure, i can just cram it into the already-existing `-Wself-assign`, but i believe that is the opposite of what should be done, and is against the way it was done previously. ================ Comment at: lib/Sema/SemaExpr.cpp:12087 + case BO_AndAssign: + case BO_OrAssign: + DiagnoseSelfAssignment(S, LHS, RHS, OpLoc, /*IsBuiltin=*/false); ---------------- rjmccall wrote: > Quuxplusone wrote: > > I understand why `x &= x` and `x |= x` are mathematically special for the > > built-in types, but surely `x -= x` and `x ^= x` and `x /= x` are just as > > likely to indicate programmer error. I would be happy if Clang either took > > the philosophical stance "We will diagnose `x = x` but uniformly //never// > > `x op= x`," or else took the pragmatic stance "We will diagnose any `x op= > > x` or `x op x` that seems likely to be a programming bug." This "middle > > way" of warning only for `&=` and `|=` is bothersome to me. > I think "we want to diagnose anything that seems likely to be a programming > bug" is already our policy here. It's inevitable that we'll overlook > examples of that. I agree that we should apply this warning to at least -=, > ^=, and /=. Ok, will extend. Repository: rC Clang https://reviews.llvm.org/D44883 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
