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

Nico Weber thakis at chromium.org
Wed Feb 8 10:34:06 PST 2012


On Wed, Feb 8, 2012 at 10:17 AM, David Blaikie <dblaikie at gmail.com> wrote:
>> 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.
>
> I could believe it (& I know you mentioned as much when you added
> -Weverything) - but I'm concerned that we end up then catering to the
> lowest common denominator & harm Clang's usefulness on new code bases
> where, if these warnings were adopted from the start, they could be
> quite helpful.
>
> (as I mentioned in my reply to Bob - many of the warnings in -Wall are
> prone to the same criticism as -Wcovered-switch-default - they would
> produce a bunch of false positives/violations in any existing code
> base that hadn't been built with the warning all along (-Wswitch
> itself, or -Wparentheses, etc). So how did they get into -Wall to
> begin with? Is the right answer that -Wall is immutable as soon as it
> shipped? I'd imagine that there are users who expect that to be true
> (as you've mentioned) & also users who would be rather surprised by
> this & expect the opposite)
>
>> 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.
>
> No more stylistic than many other warnings under -Wall (which have the
> luxury of having been there from the start, I know)
>
>> 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.
>
> Yep - I've been playing with that for some time now trying to use it
> as a way to raise the warning bar in LLVM and to improve the quality
> of warnings (-Wweak-vtable and -Wunreachable-code are a couple you've
> seen me play with - I haven't quite managed to get LLVM totally clean
> (by code fixes & improving the warning quality itself) on either of
> those yet, but WIP)
>
>> These issues aside, I don't think -Wcovered-switch-default could be under
>> -Wall either.
>
> Fair enough. Don't get me wrong - I do see where you're coming from &
> understand that a 'half closed' warning (warning for the affirmative
> case, but not the negative) is sometimes necessary given the
> limitations of users, etc.
>
> I'm also reminded of Chandler's talk at GoingNative last week when he
> mentioned that the -Wparentheses warning didn't used to have the
> inverse (checking that you /don't/ use extra () when you're just doing
> an equality not an assignment) & this was added. But I know it's not a
> perfect analogy because the extra () were already
> deliberately/habitually for -Wparentheses in the first place. Good
> that we could have negative & positive warnings on at the same time by
> default in that case.
>
> So to ask a clear question: Should we move -Wswitch from -Wmost to -Wall?

FWIW, I'm for moving -Wswitch to -Wall. This warning produces hundreds
of warnings in ffmpeg and icu (I think), and people who care about
warnings like this are likely to build with -Wall anyway.

>
>> > 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.
>
> Good to know - I felt a little alone when I had this discussion with
> Doug on the list about 6 months ago.
>
> Thanks,
> - David
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits




More information about the cfe-commits mailing list