The switch fallthrough has been widely considered a design defect in C, a
misfeature or, to use Marshall Cline's definition, evil. The overwhelming
majority of the time you don't want to fall through to the next case, but it is
easy to forget to "break" at the end of the case, making this far too error
prone. Yet GCC (and probably other compilers, too) doesn't have the ability to
warn in this case. A feature request for such warning was opened back in 2002,
but it's mostly been untouched since. But then the [[fallthrough]] attribute
was
approved for C++17 [1], and that's what has got me to do all this.
The following series attempts to implement the fabled -Wswitch-fallthrough
warning. The warning would be quite easy to implement were it not for control
statements, nested scopes, gotos, and such. I took great pains to try to
handle all of that before I succeeded in getting all the pieces in place. So
GCC ought to do the right thing for e.g.:
switch (n)
{
case 1:
if (n > 20)
break;
else if (n > 10)
{
foo (9);
__builtin_fallthrough ();
}
else
foo (8); // warn here
case 2:;
}
Since approximately 3% of fall throughs are desirable, I added a new builtin,
__builtin_fallthrough, that prevents the warning from occurring. It can only
be used in a switch; the compiler will issue an error otherwise. The new C++
attribute can then be implemented in terms of this builtin. There was an idea
floating around about not warning for /* FALLTHRU */ etc. comments but I
rejected this after I'd spent time thinking about it: the preprocessor can't
know that we're in a switch statement so we might reject valid programs, the
"falls through" comments vary wildly, occur even in if-statements, etc. The
warning doesn't warn when the block is empty, e.g. on case 1: case 2: ... The
warning does, inevitably, trigger on Duff's devices. Too bad. I guess I'd
suggest using #pragma GCC warning then. (I think Perl uses the "continue"
keyword for exactly this purpose -- an interesting idea...)
After I'd completed the warning, I kicked off a bootstrap so as to add various
gcc_fallthrough calls. There were a good amount of them, as expected. I also
grepped various FALLTHRU/Falls through/...fall thru.../... comments in config/
and added gcc_fallthroughs to make it easier for people. Wherever I wasn't
sure that the gcc_fallthrough was appropriate, I added an 'XXX' comment. This
must not be relied upon as I don't know most of the compiler code. The same
goes for relevant libraries where I introduced various gomp_fallthrough
macros conditional on __GNUC__ (I'd tried to use a configure check but then
decided I won't put up with all the vagaries of autoconf).
This patch is accompanied by more than 2000 lines of new tests to get the
warning covered though I'm sure folks will come up with other test cases
that I hadn't considered (hi Martin S. ;).
This warning is enabled by default for C/C++. I was more inclined to put this
into -Wall, but our common.opt machinery doesn't seem to allow that (ugh!).
The clang version I have installed doesn't have this warning so I couldn't
compare my implementation with others.
I plan to plunge into the C++ [[fallthrough]] thingie after this core part is
dealt with.
Since the patch is too large, I split it into various parts, the core and then
various __builtin_fallthrough additions.
This patch has been tested on powerpc64le-unknown-linux-gnu, aarch64-linux-gnu,
x86_64-redhat-linux, and also on powerpc-ibm-aix7.1.0.0, though due to PR71816
I was unable to bootstrap.
Thanks,
[1] http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0188r0.pdf
Marek
