[PATCH] D146897: [clang:diagnostics] Turning off warn_self_assignment_overloaded for user-defined compound assignments

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 6 10:43:52 PDT 2023


rjmccall added a comment.

Yeah, I feel there are three ideas here:

- Clang should warn about self simple assignment in all cases, because it's probably a mistake.  We can assume it's a mistake because it's reasonable to assume that the simple assignment operator behaves like a value assignment, even if it's user-defined, and overwriting a value with itself is a pointless operation.
- Clang should warn about self compound assignment when the type is arithmetic for these specific operators where algebraically `x OP x` would yield a constant value, because it's probably a mistake.  This is because we know the exact behavior of the operator, and producing a constant value by cancellation is a pointless operation.
- Clang should warn about inconsistent use of `this` on self compound assignment for all operators, because it's probably a mistake.  This is because the implication of writing the member reference two different ways is that there are two different variables in play, but in fact there are not.

The third idea seems like a valuable warning, but it's basically a *different* warning and shouldn't be lumped into this one.  I understand the instinct to carve it out here so that we don't regress on warning about it, but I think we should just do it separately.  And we should do it much more generally, because there's no reason that logic is limited to just these specific compound operators, or in fact to any individual operator; we should probably warn about all inconsistent references to the same variable within a single statement. (I'd draw the line at statement boundaries, though, because requiring function-wide consistency could be really noisy.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146897/new/

https://reviews.llvm.org/D146897



More information about the cfe-commits mailing list