aaronpuchert abandoned this revision.
aaronpuchert added a comment.

In D101566#2896911 <https://reviews.llvm.org/D101566#2896911>, @aaron.ballman 
wrote:

> Off-by-default warnings are generally discouraged unless there's a very 
> compelling reason to have them.

There are IMO valuable warnings that can be noisy initially and will probably 
never be on-by-default, like `-Wmissing-prototypes`. There are controversial 
warnings like `-Wshadow` (some people actually like shadowing, others do not). 
There are contradictory warnings like GCC's `-Wswitch-default` and our 
`-Wcovered-switch-default`. Warnings for compatibility with older standards. 
Warnings about certain implicit casts that are way too noisy in many code 
bases, but some might want them. Only having on-by-default warnings is just not 
realistic, there will always be many exceptions. Partly because the language 
moves on, but code needs time to catch up.

But in any event, my goal here wasn't to introduce this warning, just to fix 
it, given that it's there.

>> That was a valid reason, but now there are code bases that work with 
>> -Weverything and disable the warnings they are not interested in.
>
> -Weverything is not a particularly compelling use case IMO. We tell people 
> not to enable it and expect good things to happen over time.

That was in response to off-by-default warnings remaining unused.

> If I'm following along properly, my suggestion would be to commit the 
> uncontentious bit (silencing the diagnostic in the problematic cases) in this 
> review.

Silencing it in problematic cases means never emitting it at all. So you agree 
with @dblaikie that it should be removed.

> If still desired, we can always start a new review to see if there's 
> sentiment for the new warning behavior (under the existing diagnostic name or 
> under a new one), but hopefully the proposed new behavior would be palatable 
> for an on-by-default warning as that's more compelling than tweaks to an 
> off-by-default warning that the original author isn't sure should even exist.

Likely not.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101566/new/

https://reviews.llvm.org/D101566

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to