[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)
Brooks Moses via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Sun Apr 15 15:09:07 PDT 2018
brooksmoses added a comment.
I have noticed two things when attempting to release LLVM with this revision internally at Google:
1. It's catching real bugs, all in constructors where someone wrote "member_ = member_" when they meant "member_ = member".
2. It's catching at least as many cases of tests where people are intentionally testing that self-assignment doesn't corrupt the data values.
Thus, this seems valuable but problematic, and the problems mean that initially we're facing turning off -Wself-assign completely until this is resolved. That's definitely an issue, and at least means that this needs to be placed under its own -W option. And the real bugs it's finding seem to be very specific, and could be found by a more-focused warning.
Further, I would note that most warnings of this sort have some canonical way of arranging the code to avoid the warning -- for instance, casting an unused variable to "void" to create a no-op expression and thereby avoid the "unused variable" warning. This warning doesn't seem to have one; "var = (var)", for instance, should IMO turn it off but doesn't.
(To add to that, we have a source file that does a lot of "var = var" to silence unused-variable warnings, so this breaks that on top of not having its own source "workaround". Ick.)
Repository:
rC Clang
https://reviews.llvm.org/D44883
More information about the cfe-commits
mailing list