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

[David Blaikie]

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

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