[cfe-commits] [PATCH] Implicit fall-through between switch labels

[David Blaikie]

On Sat, Apr 21, 2012 at 11:00 AM, Matthieu Monrocq
<matthieu.monrocq at gmail.com> wrote:
>
>
> Le 21 avril 2012 18:17, Jordy Rose <jediknil at belkadan.com> a écrit :
>
>> I feel like the common case here is not actually fall-through but a
>> missing break, though like David said there are many other ways to exit a
>> switch. I would expect something like this instead:
>>
>> warning: fall-through to subsequent switch case
>> note: use 'break' to prevent fall-through
>>  [fixit]
>> note: use 'fallthrough' attribute to silence this warning
>>  [fixit]
>>
>> If we really want to go crazy, we could look and see if 'return' is more
>> appropriate than 'break', but I think probably these would cover the common
>> cases.
>>
>> Also, the attribute fixit would have to only use square-bracket syntax if
>> it's supported, and possibly disabled altogether if the current language
>> options don't support attributes at all, i.e. non-GNU. (The warning could
>> still be disabled with pragmas, after all.)
>>
>> Jordy
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
> Two remarks:
>
>> if there is no statement between two cases, then fallthrough is most
>> probable
>> otherwise it is not possible to actually distinguish between a possible
>> fallthrough or break
>
> I don't think that providing a fixit is reasonable, because fixit should be
> provided only when they are right, taking chances is risking miscompilation.

Well in this case we're not risking miscompilation because the fixit
is suggesting the necessary change that would not affect the semantics
of the program (it's the null fixit - all it does is suppress the
warning (& document to the user that the code relies on fallthrough)).

But yes, in a codebase that generally uses this annotation correctly,
it's no longer obvious that the correct change is to suppress the
warning - if that was the case then we wouldn't want the warning
because it has too many false positives.

Like -Wparentheses, it's probably best to have the suppression in a
note afterwards, not in the original diagnostic unfortunately (I find
this annoying because it means a lot of extra verbosity - but I think
it's consistent with Clang's philosophy on when to provide (& not
provide) fixits) - perhaps others more informed than I, will be able
to speak up on this if I'm off-base.

- David

>
> -- Matthieu
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>