[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

rsmith added a comment. I would feel a lot more comfortable adding a `-wtest` flag if we had more than one warning that it would control. If there's really only one warning out of >600 that qualifies for this treatment, I really don't think we have a good argument to introduce a new feature

[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

lebedev.ri added a comment. @brooksmoses The simple `-Wno-self-assign-overloaded` variant (https://reviews.llvm.org/D45766) has landed, so the issue should be resolved? Not sure what to do with this differential, should i abandon it until there is more interest for such functionality?

[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

brooksmoses added a comment. Thanks for the summary, John. To confirm, I found two examples of bugs involving local variables, as well as the field-based examples. And, indeed, all of the false positives were in unit tests. Repository: rC Clang https://reviews.llvm.org/D45685

[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

rjmccall added a comment. Let me try to summarize where I think we are. 1. I think it’s now generally agreed that this is a useful warning — certainly for field assignments, but we also have (at least?) one example with a local variable. 2. It’s also generally agreed that this warning has a

Re: [PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

Let me try to summarize where I think we are. 1. I think it’s now generally agreed that this is a useful warning — certainly for field assignments, but we also have (at least?) one example with a local variable. 2. It’s also generally agreed that this warning has a problem with unit tests and

[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

rjmccall added a comment. I'm sorry, that warning *is* in self-assign, although I'm not sure it should be. Repository: rC Clang https://reviews.llvm.org/D45685 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

rjmccall added a comment. The goal of having a unified option is that you can uniformly suppress warnings that don't always make sense in unit tests. It's future-proofing against the addition of other warnings (probably C++ warnings) that might not make sense for unit tests, like extending

[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

thakis added a comment. +1 to just adding a dedicated Wno flag for the new warning instead of coming up with this new spelling. Repository: rC Clang https://reviews.llvm.org/D45685 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

+1 to just adding a dedicated Wno flag for the new warning instead of coming up with this new spelling. On Mon, Apr 16, 2018, 3:41 PM Roman Lebedev via Phabricator via cfe-commits wrote: > lebedev.ri added a comment. > > Uuuh, the fact that phab posts the

[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

lebedev.ri added a comment. Uuuh, the fact that phab posts the top-postings, but silently ignores inline replies is annoying. >> lebedev.ri added a comment. >> >> In https://reviews.llvm.org/D45685#1069040, @dblaikie wrote: >> >>> I'm not sure this is a practical direction to pursue - though

Re: [PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

On Mon, Apr 16, 2018 at 12:08 PM Roman Lebedev via Phabricator < revi...@reviews.llvm.org> wrote: > lebedev.ri added a comment. > > In https://reviews.llvm.org/D45685#1069040, @dblaikie wrote: > > > I'm not sure this is a practical direction to pursue - though perhaps > > others disagree. > > >

[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

lebedev.ri added a comment. In https://reviews.llvm.org/D45685#1069040, @dblaikie wrote: > I'm not sure this is a practical direction to pursue - though perhaps > others disagree. > It's likely non-trivial to plumb a flag through most build systems to be > applied only to test code I'm

Re: [PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

I'm not sure this is a practical direction to pursue - though perhaps others disagree. It's likely non-trivial to plumb a flag through most build systems to be applied only to test code (& likely would suppress the warning in headers only included in test code - so for example, in a header-only

[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

dblaikie added a subscriber: lebedev.ri. dblaikie added a comment. I'm not sure this is a practical direction to pursue - though perhaps others disagree. It's likely non-trivial to plumb a flag through most build systems to be applied only to test code (& likely would suppress the warning in

[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

lebedev.ri added a comment. In https://reviews.llvm.org/D45685#1068981, @Quuxplusone wrote: > I don't understand the spelling of this option. You spell it `-wtest` > (because rjmccall suggested that spelling); but for my money it should be > spelled `-Wno-self-assign-nonfield`. You did read

[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

Quuxplusone added a comment. I don't understand the spelling of this option. You spell it `-wtest` (because rjmccall suggested that spelling); but for my money it should be spelled `-Wno-self-assign-nonfield`. Also, if you go this route, you miss the true positive reported by someone on the

[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

lebedev.ri updated this revision to Diff 142636. lebedev.ri retitled this revision from "[Sema] Add -Wtest global flag that silences -Wself-assign for overloaded operators." to "[Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.". lebedev.ri edited the summary of

[PATCH] D45685: [Sema] Add -Wtest global flag that silences -Wself-assign for overloaded operators.

lebedev.ri updated this revision to Diff 142627. lebedev.ri added a comment. - Don't mis-spell the name of the flag. FIXME: So do we want it to be `-Wtests`, `-Wtest`, or we really want it to be `-wtest` ? Repository: rC Clang https://reviews.llvm.org/D45685 Files: docs/ReleaseNotes.rst

[PATCH] D45685: [Sema] Add -Wtest global flag that silences -Wself-assign for overloaded operators.

lebedev.ri created this revision. lebedev.ri added reviewers: rjmccall, aaron.ballman, dblaikie, thakis, rsmith, chandlerc, brooksmoses. Herald added a subscriber: klimek. Credit for the idea goes to @rjmccall. I'm not quite sure how to do it with `-wtest` (lowercase). so i just modelled it