On Wed, Mar 30, 2016 at 9:23 AM, Alexander Kornienko <ale...@google.com> wrote: > alexfh added a comment. > > In http://reviews.llvm.org/D18265#386720, @baloghadamsoftware wrote: > >> Actually, there was nothing wrong with assign operator signatures per se >> either although the original name of the checker was >> AssignOperatorSignature. The only change here is that it does not check the >> signature only anymore, but also the body (if present). > > > Maybe the old check name should have been > `misc-unconventional-assign-operator-signature` or something like that, but > even the old name limited the scope enough to make it easy to guess about the > possible issues it flags. However, further expanding the scope makes the name > even less informative. There are just too many things that could be wrong > with assignment operators. If/when we add a check for another bug-prone > pattern related to assignment operators, the name `misc-assign-operator` > could be broad enough to cover this hypothetical new check as well. This will > inevitably lead to confusion.
I agree with your point; that's why my slight preference is for leaving them split into multiple checks. Either we want this to be the catch-all for operator assignment checks (and plan to use config options to control behavior for additional checks), at which point misc-operator-assign is a reasonable enough name, or we want a clear name for a check that checks two separate-but-related things (one checks the signature, the other checks the return expression value). I'm not certain we'll get a particularly *clear* name for a check that diagnoses fairly separate issues. ~Aaron _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
