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

[David Blaikie]

On Wed, Feb 8, 2012 at 12:58 PM, Ted Kremenek <kremenek at apple.com> wrote:
> On Feb 8, 2012, at 10:17 AM, David Blaikie wrote:
>
> (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)
>
>
> I think the "right answer" is that there isn't one clear policy.  Warnings
> are warnings.  Some people like them, some people don't.  Deciding what goes
> in -Wall, what's on by default, etc., all comes down to a nuanced discussion
> of how a *particular* warning impacts existing code, whether it has benefit,
> etc.

No doubt - I do agree with that & I'm by no means clearly on the side
of "this should be in -Wall". I guess I'm just rambling on about a
bunch of issues surrounding this in some attempt to make a coherent
logic for myself or others.

I don't want to argue for hard line rules that result in less than
stellar user experiences but I do have a lot of questions about what
tradeoffs we're making.

I suppose primarily my concerns are:

1) splitting, by default, two halves of the same warning - this is not
unjustifiable (clearly half the warning may be relatively unobtrusive
(or has existed for so long under a common flag as to have become so))
but a little bit of a pity if it leaves the other half totally out in
the cold in terms of discoverability.

If I didn't know the warning existed & I was an LLVM code reviewer
providing that kind of feedback to submitters - I wouldn't really know
the warning existed & be able to suggest turning it on. The existence
of -Wswitch's behavior is pretty much a cultural one (which is why
adding this warning seems valuable - to elevate that knowledge "hey,
there's this warning thath could be helping you if you coded to this
style, but you aren't so we can't").

The previous, similar style, warning I'd suggested & Doug had rejected
(which would admittedly be /much/ noisier) was to warn on null pointer
literals that were not NULL or nullptr because, again, we have the
inverse (-Wconversion-null) but common teaching (even Stroustrup) says
to use 0 because NULL isn't really giving you anything & just hiding
the inherent danger/ambiguity of a zero literal. But that's not true -
compilers can/do provide value here & users don't use it because
they're not aware.

2) new code bases could have a much higher bar than we currently
provide under common flags (-Wall, etc) but -Weverything has lots of
noise (-Wglobal-constructors, to take another current topic -
especially relevant to some users, completely irrelevant to others (&
if you have to go through & disable /all/ of those kinds of warnings
it might be a slow project start-up)) so how do we provide a nice,
usable but high bar for new projects without drowning existing
projects in new warnings with every release?

- David

[Sorry if this is all a bit rambly & derailing a minor issue into a
philosophical/political/metaphysical one. I hope this is still
constructive - please let me know if it isn't.]