[PATCH] D45766: [Sema] Add -Wno-self-assign-overloaded

[David Blaikie via cfe-commits]

On Mon, Apr 23, 2018 at 4:12 PM John McCall <rjmccall at gmail.com> wrote:

> On Mon, Apr 23, 2018 at 6:32 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>> On Mon, Apr 23, 2018 at 3:29 PM John McCall via Phabricator <
>> reviews at reviews.llvm.org> wrote:
>>
>>> rjmccall added a comment.
>>>
>>> In https://reviews.llvm.org/D45766#1076176, @dblaikie wrote:
>>>
>>> > Is there anything else in the "-w" namespace other than the literal
>>> "-w" so
>>> >  far?
>>>
>>>
>>> No. This would be novel.
>>>
>>
>> Ah, I see.
>>
>>
>>> > I mean, I could imagine it might make more sense to default these
>>> warnings
>>> >  off & users can turn them on for non-test code, potentially? So
>>> >  "-Wnon-test" might make sense.
>>>
>>> That's an interesting idea, but it's still not a warning group, because
>>> you shouldn't get the self-assign warnings unless `-Wself-assign` is
>>> enabled.
>>>
>>
>> You shouldn't?
>>
>
> I wouldn't think so.  Remember that the goal of the option is to be a
> single thing that users can add to their unit-test CFLAGS to disable these
> noisy-in-tests cases.  So if we add an opt-in/experimental
> `-Wunpredictable-foozits` warning, and it has a unit-test carve-out,
> passing `-wtest -wno-test` or whatever shouldn't turn on the carved-out
> special case of `-Wunpredictable-foozits`.
>
> It's probably not the worst thing to just use a `-W` spelling anyway; not
> everything in that namespace is (e.g. `-Werror`).  It could be
> `-Wnoisy-in-tests` and `-Wno-noisy-in-tests`, with a documentation note
> that `-Wnoisy-in-tests` is just a cancellation of `-Wno-noisy-in-tests` and
> doesn't actually enable any warnings by itself.  We could have the
> diagnostic printer add `-Wnoisy-in-tests` to the diagnostic-group
> annotation for diagnostics that would be suppressed under
> `-Wno-noisy-in-tests`, analogously to how it adds `-Werror` for diagnostics
> that have been promoted to an error.
>

That sort of sounds pretty plausible to me. Poked Richard about his opinion
here too.


>
> John.
>
>
>> But yeah, it's tricky either way - either you get them all, then opt out
>> of all the warnings for test code you don't generally want.
>>
>> I'll leave it to you, then - don't feel too strongly. Maybe worth seeing
>> if Richard has an opinion, but up to you.
>>
>> - Dave
>>
>>
>>>
>>>
>>> Repository:
>>>   rL LLVM
>>>
>>> https://reviews.llvm.org/D45766
>>>
>>>
>>>
>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180423/cb2c7899/attachment.html>