[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D44883#1063003, @thakis wrote:

> This landing made our clang trunk bots do an evaluation of this warning :-P It fired 8 times, all false positives, and all from unit tests testing that operator= works for self-assignment. (https://chromium-review.googlesource.com/c/chromium/src/+/1000856 has the exact details) It looks like the same issue exists in LLVM itself too, https://reviews.llvm.org/D45082


Right, i guess i only built the chrome binary itself, not the tests, good to know...

> Now tests often need warning suppressions for things like this, and this in itself doesn't seem horrible. However, this change takes a warning that was previously 100% noise-free in practice and makes it pretty noisy – without a big benefit in practice. I get that it's beneficial in theory, but that's true of many warnings.
> 
> Based on how this warning does in practice, I think it might be better for the static analyzer, which has a lower bar for false positives.

Noisy in the sense that it correctly diagnoses a self-assignment where one **intended** to have self-assignment.
And unsurprisingly, it happened in the unit-tests, as was expected ^ in previous comments.
**So far** there are no truly false-positives noise (at least no reports of it).

We could help workaround that the way i initially suggested, by keeping this new part of the diag under it's own sub-flag,
and suggest to disable it for tests. But yes, that


Repository:
  rC Clang

https://reviews.llvm.org/D44883