This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Implement -Wimplicit-fallthrough (version 9)


Hi,

On Tue, 27 Sep 2016, Tom Tromey wrote:

> The point of the warning is to make code more robust.  But accepting any 
> comment like "Don't fall through" is not more robust, but rather an 
> error waiting to happen; as IIUC the user has no way to detect this 
> case.
> 
> I think it's better for the comment-scanning feature to be very picky 
> (or even just not exist at all) -- that way you know exactly what you 
> are getting.  Lint was traditionally picky IIRC.  And, this is a warning 
> that isn't default and can also be disabled, so it's not as if users 
> have no recourse.

Not accepting
  /* And here we intentionally fall through because ... */
and forcing users to replace this by:
  /* fallthrough */
is not robust either.  It's actually actively lowering robustness of code, 
it creates work for programmers that will be regarded as pointless (and 
rightly so) and will merely lead to everybody disabling the warning (see 
our generated files) at which point we could just as well not have 
implemented it (which would be a shame because I think it's genuinely 
useful).

The point of warnings is to make code robust under the condition of not 
being a pain by giving zillions of false positives.  In this specific case 
the chance of giving false positives by being picky in how to disable the 
warning is very high.  On the other hand the chance of unintentionally 
disabling the warning by a negative comment like "Don't fall through here" 
is low, because presumably the one adding that comment (and hence thinking 
about that part of the code) also in fact put in the "break;" afterwards.

The argument with lint being picky would apply only if GCC would have 
added this warning maybe 20 years ago, not now where nearly nobody even 
knows what lint is, which lead to a large existing code base not having 
comments that would be accepted by lint but comments that do specify the 
intent of falling through.


Ciao,
Michael.
P.S.: Initially I even wanted to argue that the mere existence of _any_ 
comment before a case label would disable the warning.  I don't have the 
numbers but I bet even that version would have found the very same bugs 
that the picky version has.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]