[cfe-commits] r150055 - in /cfe/trunk: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td test/Sema/switch.c

[Ted Kremenek]

On Feb 7, 2012, at 10:50 PM, David Blaikie <dblaikie at gmail.com> wrote:

> I think it's a real pity to separate the two warnings, but I can
> understand the limitation/problem. Having this flag under /no/ group
> at all does leave me a bit concerned that it'll never be found/used,
> though.

The rollout of new warnings is largely political.  Believe it or not, many users have complained to use when we roll out new warnings under -Wall.  Such users expect -Wall to have consistent behavior between releases.

In our case, we observed that many projects we built at Apple violated this warning.  We often push for projects to fix their code, but this was not one that would be worth it.  It's very stylistic.

We do have a warning group: -Weverything.  That warning group was created so that everyone could opt in to the entire set of warnings, and then turn off the ones they want.

In reality, -Wall isn't what it's name implies, and we can't just add any warning we want to it.  When the fallout of a warning is high, we have to assess the tradeoff of that cost of increased warnings versus the payoff.  I don't think this warning made that cut.

> 
> As a side note (& I wouldn't've minded a more all-party discussion of
> this - though I know it came up once or twice on IRC) Clang's -Wswitch
> is under -Wmost (on by default) whereas GCC's -Wswitch is under -Wall
> (off by default). One of the possible solutions to this issue (of
> -Wcovered-switch-default) I'd considered was to fix this was to make
> Clang's -Wswitch behavior consistent with GCC's (thus turning it, and
> -Wcovered-switch-default, off by default). Yes, this would still put
> -Wcovered-switch-default would be under -Wall & thus, perhaps, still a
> bit noisy but it's more likely someone actually wants -Wswitch if they
> turn on -Wall & it'd be helpful if they got the other half of that
> warning (-Wcovered-switch-default) so -Wswitch was reliable.

These issues aside, I don't think -Wcovered-switch-default could be under -Wall either.

> 
>> This is a great warning, but it was observed that a ton of real world code violates
>> it all the time for (semi-)legitimate reasons.
> 
> Last time I brought up any off-by-default warnings Doug seemed to veto
> them. (I'm not sure if he's even seen this warning/has an opinion on
> it - I'm a bit concerned that he won't like it once he sees it)

I don't see the rationale for that.  We have plenty of off-by-default warnings.

> 
>>  This warnings is fairly pedantic, which is good,
>> but not for everyone.
> 
> Could we turn it on in the LLVM Makefiles (llvm/Makefile.rules ~ line
> 650 - I'm not sure if we have to do something else for the cmake
> build)? Do we have a way to turn on certain flags only in the presence
> of Clang?

I have no problem with doing that.  It's a great warning; it's just not for everybody.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120208/16ec68f6/attachment.html>