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

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D146897#4249300 <https://reviews.llvm.org/D146897#4249300>, @rjmccall wrote:

> 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.

Yeah, good point; I'm convinced.

> (I'd draw the line at statement boundaries, though, because requiring function-wide consistency could be really noisy.)

At least, I don't think we should warn if both `a` and `this->a` are used in different statements in the same function, and `a` is shadowed by a local declaration wherever `this->a` is used, and I think it might be reasonable to not warn on a member `a` if the name `a` is ever declared in the function regardless of where it's in scope. For the remaining cases, where `a` is never declared locally, I suspect the false positive rate for warning if the function uses both `a` and `this->a` would be lower, but we could probably get some data on that pretty easily. Speaking of data, it'd be nice to get some evidence that this is a mistake that actually happens before landing a dedicated warning for it :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146897