Closed Bug 1235277 (MOZ_FALLTHROUGH_ASSERT) Opened 4 years ago Closed 4 years ago

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

Categories

(Core :: MFBT, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox46 --- fixed

People

(Reporter: cpeterson, Assigned: cpeterson)

References

Details

Attachments

(1 file)

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.
Attachment #8702153 - Flags: review?(botond)
Attachment #8702153 - Flags: review?(botond) → review+
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.
https://hg.mozilla.org/mozilla-central/rev/d0a8d632dce5
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Alias: MOZ_FALLTHROUGH_ASSERT
Priority: -- → P3
See Also: → fallthrough
You need to log in before you can comment on or make changes to this bug.