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

Arthur O'Dwyer via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Mar 26 10:41:04 PDT 2018


Quuxplusone added a comment.

Two more high-level comments from me, and then I'll try to butt out.

Q1. I don't understand what `field-` is doing in the name of some diagnostics. Is there a situation where `x = x;` is "less/more of an error" just because `x` is a {data member, lambda capture, structured binding} versus not? Why should the "plain old variable" case be separately toggleable? (This question pre-dates Roman's proposed patch AFAICT. I think the answer is that self-assignment specifically //of a data member// specifically //in a constructor// is considered "more of an error" because it's likely to be a very particular kind of typo-bug. I doubt this answer is quite strong enough to justify multiple warning options, though.)

Q2. I don't understand why `x &= x` (and now `x /= x`, thanks) should be treated as "more of an error" than `x & x` (and now `x / x`), or for that matter `x == x`. The sin here is not "self-assignment" per se; it's "over-complicating a tautological mathematical operation." I admit I haven't thought about it as much as you folks, but the more I think about it, the more I think the only consistent thing for Clang to do would be to back off from warning about anything beyond plain old `x = x`, and leave *all* the mathematical-tautology cases to checkers like clang-tidy and PVS-Studio.


Repository:
  rC Clang

https://reviews.llvm.org/D44883





More information about the cfe-commits mailing list