1235277 - (MOZ_FALLTHROUGH_ASSERT) Define MOZ_FALLTHROUGH_ASSERT to workaround -Wunreachable-code warnings about MOZ_FALLTHROUGH in debug builds

Status: RESOLVED FIXED

(Core :: MFBT, defect, P3)

Description

[Chris Peterson [:cpeterson]]

Attachment: MOZ_FALLTHROUGH_ASSERT.patch

MOZ_FALLTHROUGH (bug 1215411) is an annotation to suppress -Wimplicit-fallthrough warnings about switch cases that fall through without a break or return statement.

MOZ_FALLTHROUGH_ASSERT will suppress -Wimplicit-fallthrough warnings about switch cases that MOZ_ASSERT(false) (or its alias MOZ_ASSERT_UNREACHABLE) in debug builds, but intentionally fall through in release builds. The problem with this common case is that, in release builds, the MOZ_ASSERT(false) will expand to `do { } while (0)`, requiring a MOZ_FALLTHROUGH annotation to suppress a -Wimplicit-fallthrough warning. In debug builds, the MOZ_ASSERT(false) will expand to something like `if (true) { MOZ_CRASH(); }` and the MOZ_FALLTHROUGH annotation will cause a -Wunreachable-code warning. The MOZ_FALLTHROUGH_ASSERT macro breaks this warning stalemate.

BAD EXAMPLE:

  switch (foo) {
    default:
      // This case wants to assert in debug builds, fall through in release.
      MOZ_ASSERT(false); // -Wimplicit-fallthrough warning in release builds!
      MOZ_FALLTHROUGH;   // but -Wunreachable-code warning in debug builds!
    case 5:
      return 5;
  }

GOOD EXAMPLE:

  switch (foo) {
    default:
      // This case asserts in debug builds, falls through in release.
      MOZ_FALLTHROUGH_ASSERT("Unexpected foo value?!");
    case 5:
      return 5;
  }

Unfortunately, the MOZ_FALLTHROUGH definition and comments are in Attributes.h, but MOZ_FALLTHROUGH_ASSERT definition and comments are in Assertions.h. These macro definitions depend on other macro definitions in their respective header files.

Comment 1

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d0a8d632dce5

Comment 2

[Chris Peterson [:cpeterson]]

I filed a clang feature request to make this MOZ_FALLTHROUGH_ASSERT() macro unnecessary:

https://llvm.org/bugs/show_bug.cgi?id=25966

clang could allow a [[clang::fallthrough]] annotation after a call to a noreturn function like abort(), but still warn about unreachable [[clang::fallthrough]] annotations after an early break or return.

Comment 3

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d0a8d632dce5