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

[Richard Smith via llvm-commits]

On 23 April 2018 at 16:23, David Blaikie via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> 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.
>

This is not the only warning group that has the property that only one of
-Wfoo and -Wno-foo seems useful. There are, for instance, plenty of
diagnostic groups that make sense to turn on, but not to turn off (eg,
-Wno-all, -Wno-extra are largely meaningless), and some that make sense to
turn off, but not to turn on (eg, -Wnon-gcc is only intended to be turned
off). So I don't think that's as special a property as is being suggested.
If someone uses -Wnoisy-in-tests and it turns on all warnings that are
noisy in tests, I think they got what they asked for.

This is also not the only warning group for which we want diagnostics to be
in this group and in some other group or groups. I think the idea to
include -Wnoisy-in-tests in the diagnostic output is very interesting, but
as a general feature not as a -Wnoisy-in-tests special case -- for example,
if I use a binary literal in C++98 mode, I'd like the warning to be tagged
with [-Wbinary-literal,-Wc++14-extensions,-Wgnu], rather than the current
[-Wc++14-binary-literal] (an artificial warning group that only exists to
work around the inability of our infrastructure to properly represent
warnings that are in multiple groups at once).

As a simple way to get this effect, perhaps we could allow warning groups
to be tagged as artificial, and for such warning groups, recurse to the
warning groups including them to find the name to use for diagnostics.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180423/00f6cdaa/attachment.html>