TL;DR?
Annotate intentional switch fallthroughs with `MOZ_FALLTHROUGH;` or
`MOZ_FALLTHROUGH_ASSERT();` instead of `// fall through` comments.
The details:
clang's -Wimplicit-fallthrough warning (currently off) reports switch
cases that fall through without a break or return statement.
MOZ_FALLTHROUGH (bug 1215411) is an annotation macro to suppress the
warning when the fallthrough is intentional. All -Wimplicit-fallthrough
warnings in mozilla-central have been fixed and I would like to enable
-Wimplicit-fallthrough soon (bug 1253170).
We currently have 287 intentional fallthroughs with MOZ_FALLTHROUGH
annotations. I found 6 unintentional fallthroughs, which is a small
number but it means 2% of fallthroughs were bugs!
MOZ_FALLTHROUGH is only needed on fallthrough cases that have code:
switch (foo) {
case 1: // cases without code, so MOZ_FALLTHROUGH is not needed.
case 2:
case 3:
case 4:
DoSomething(); // case with code, so MOZ_FALLTHROUGH is needed!
MOZ_FALLTHROUGH;
default:
return foo;
}
For clang, MOZ_FALLTHROUGH is #defined as `[[clang::fallthrough]]`,
which is only available in C++11 code. For MSVC, MOZ_FALLTHROUGH is
#defined as `__fallthrough`, an annotation checked by MSVC's optional
"Code Analysis" (/analyze), though I haven't tested this. gcc does not
have a similar warning or annotation, so MOZ_FALLTHROUGH is a no-op.
Unfortunately, a second annotation macro, MOZ_FALLTHROUGH_ASSERT, is
needed for switch cases that previously called MOZ_ASSERT(false) to
assert in debug builds, but intentionally fall through in release builds:
switch (foo) {
default:
// This case asserts in debug builds and falls through in release.
MOZ_FALLTHROUGH_ASSERT("Unexpected foo value?!");
case 5:
DoSomething();
}
The problem is that, in release builds, MOZ_ASSERT(false) expands to `do
{ } while (0)`, requiring a MOZ_FALLTHROUGH annotation to suppress a
-Wimplicit-fallthrough warning because the case has code. But in debug
builds, MOZ_ASSERT(false) expands to something like `if (true) {
MOZ_CRASH(); }`and a MOZ_FALLTHROUGH annotation after MOZ_ASSERT(false)
will cause an unreachable code warning because the program aborts before
the fallthrough is reached. The MOZ_FALLTHROUGH_ASSERT macro breaks this
stalemate. I filed a clang bug suggesting they remove the unreachable
annotation warning:
https://llvm.org/bugs/show_bug.cgi?id=25966
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
