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

[Matthieu Monrocq]

Le 8 février 2012 19:34, Nico Weber <thakis at chromium.org> a écrit :

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

I don't quite see the need to move a warning *away* from -Wall.

I understand that fixing large code bases is unpractical, I have some such
medium large (and oldish) code bases to deal with. However simply tuning
the Makefile (-Wno-covered-switch-default) is so simple that I don't quite
understand the reluctance here.

It may require a (slight) change of mentality, but the obvious advantage of
blacklisting some warnings, is that at least you make explicit what is not
checked.

I don't quite see why Open Source projects would reject a patch to add a
few -Wno-XXXX to their makefiles. Certainly this seems trivial and harmless.

--Matthieu
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120208/d23d462c/attachment.html>