On Thu, Nov 03, 2016 at 01:22:11PM +0100, Bernd Schmidt wrote:
> On 11/03/2016 12:58 PM, Jakub Jelinek wrote:
> >On Thu, Nov 03, 2016 at 12:51:15PM +0100, Bernd Schmidt wrote:
> >>I'm concerned about the number of false positives for this warning, and
> >>judging by previous discussions, I'm not alone in this. This patch limits it
> >>to level 1 (any comment before the case label disables the warning) for
> >>cases where the user specified no explicit level. It'll still generate
> >>enough noise that people will be aware of it and can choose whether to use a
> >>higher level or not.
> >>
> >>Bootstrapped and tested on x86_64-linux. Ok?
> >
> >I disagree, I'm ok with changing it to 2, but 1 is too much.
>
> Well, we have data from our own sources where we had to "fix" lots of
> perfectly good code, and also from the Linux kernel. From an earlier
> discussion:
That data wasn't really convincing on this. All it proved is that most of
the cases are (likely) deliberate fall-throughs without any comment
whatsoever, the rest is in the noise. As one needs to deal with those
where comments are missing altogether, dealing with the noise is acceptable.
> Markus Trippelsdorf wrote:
> >>>I randomly looked at the differences and almost all additional
> >>>-Wimplicit-fallthrough=2 warnings are bogus (~5% are genuine).
> >>>And _all_ additional -Wimplicit-fallthrough=3 warnings appear
> >>>to be bogus.
> [...]
> >Actually looking more closely it appears that all of the 136 additional
> >warnings for level 2 are bogus, too.
>
> Also, levels above 1 enforce English as a language, which isn't something we
> should be doing, even if we could detect fallthrough comments reliably (and
> we can't).
If you use non-english comment, then you need to add lint-like comments for
this, or attributes. IMHO it is really not very high cost (compared to
all those without any comment at all) to adjust code, what takes time is to
analyze if something is intentional or invalid fallthrough. The clearer the
comments are on that, the better.
Jakub
