[PATCH] Warn on empty switch statements

[Aaron Ballman]

Ping?

I've given this some more thought, and I feel like Reid and Daniel (in
the PR) hit the nail on the head: it is really nice to be able to
write code in clang and not have to worry that I've introduced a
warning in MSVC, especially since I try to compile everything with
warnings as errors.  I understand the fear of trying to match
diagnostics with other compilers, but where's the harm if it's not
overly chatty and it can catch bugs?

Also, I think the warning adds value over the empty statement warning
because the situations are different.  Eg)

switch (expr);  // Should warn because the ; is suspicious
{
  /* stuff */
}

// vs

switch (expr) {
  /* ToDo: finish this */
}  // Should warn because the emptiness of the block is suspicious


In the case where the user really does want an empty switch statement,
they can selectively turn the diagnostic off.  This really shouldn't
be a common case, after all.

~Aaron

On Wed, May 29, 2013 at 4:10 PM, Reid Kleckner <rnk at google.com> wrote:
> On Wed, May 29, 2013 at 3:49 PM, Richard Smith <richard at metafoo.co.uk>
> wrote:
>>
>> On Wed, May 29, 2013 at 7:35 AM, Aaron Ballman <aaron at aaronballman.com>
>> wrote:
>>>
>>> -Wunreachable-code doesn't warn on empty statement bodies for the
>>> switch.  So, for instance:
>>>
>>> int main() {
>>>   int i = 0;
>>>   switch(i) {}
>>> }
>>>
>>> yields no warnings in clang with -Wunreachable-code (or -Wall), but at
>>> /W3 in MSVC you get: warning C4060: switch statement contains no
>>> 'case' or 'default' labels.  Will it catch lots of bugs?  Probably
>>> not.  But it also should have basically no false positives either.  It
>>> would be nice for people who want their code to also be warning-free
>>> under MSVC.
>>
>>
>> I think we need more justification for adding the warning than this.
>>
>> It doesn't seem reasonable to expect us to implement the union of all
>> warnings which every existent compiler produces. Some of those warnings are
>> really dumb. For instance, /W3 also enables C4800, which I don't think we
>> will ever want to implement.
>
>
> I agree that warning (forcing value to bool) is really, really bad, and I
> hope most serious projects turn it off.
>
> However, I can say it's pretty annoying if you're working on cross-platform
> project and have to push to another platform to build just to get some
> compiler warnings for a compiler which you're expected to keep warning
> clean, value judgements about its diagnostic quality aside.  That cycle is
> long and painful, and if you skip it people often yell at you.
>
> While clang will never be able to paper over the differences between
> platforms, having some off-by-default diagnostics like this could save users
> some time and increase the likelihood that their changes Just Work on the
> first push.