Annotate intentional switch fallthroughs to suppress -Wimplicit-fallthrough warnings in gfx/

RESOLVED FIXED

Status

()

Core
Graphics
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: cpeterson, Assigned: cpeterson)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

3 years ago
Created attachment 8703398 [details] [diff] [review]
gfx_part-1_MOZ_FALLTHROUGH.patch

clang's -Wimplicit-fallthrough warnings (not yet enabled in mozilla-central) warn about switch cases that fall through without a break or return statement. MOZ_FALLTHROUGH (bug 1215411) is an annotation to suppress -Wimplicit-fallthrough warnings about switch cases that intentionally fall through without a break or return statement.  MOZ_FALLTHROUGH is only needed on cases that have code.

MOZ_FALLTHROUGH_ASSERT (bug 1235277) 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.

Why do we need MOZ_FALLTHROUGH_ASSERT in addition to MOZ_FALLTHROUGH? 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.

gfx/thebes/gfxFcPlatformFontList.cpp:706:9: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
gfx/thebes/gfxFontconfigFonts.cpp:2183:9: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
gfx/2d/DrawTargetCG.cpp:812:5: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
gfx/2d/DrawTargetCairo.cpp:636:5: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
Attachment #8703398 - Flags: review?(nical.bugzilla)
(Assignee)

Comment 1

3 years ago
Created attachment 8703399 [details] [diff] [review]
gfx_part-2_Wno-implicit-fallthrough.patch

Part 2: Suppress -Wimplicit-fallthrough warnings in gfx/ from Skia header files. I will try to submit upstream fixes for these Skia warnings. Then I can remove this -Wno-implicit-fallthrough change from mozilla-central.

gfx/skia/skia/include/gpu/GrTestUtils.h:94:9 [-Wimplicit-fallthrough] unannotated fall-through between switch labels
gfx/skia/skia/include/gpu/GrTypesPriv.h:181:9 [-Wimplicit-fallthrough] unannotated fall-through between switch labels
Attachment #8703399 - Flags: review?(nical.bugzilla)
(Assignee)

Updated

3 years ago
Depends on: 1215411

Updated

3 years ago
Attachment #8703398 - Flags: review?(nical.bugzilla) → review+

Updated

3 years ago
Attachment #8703399 - Flags: review?(nical.bugzilla) → review+
(Assignee)

Comment 3

3 years ago
I backed out part 2 for introducing -Wunreachable-code warnings-as-errors, which I have a fix waiting for review in bug 1237352.
Depends on: 1237352
Keywords: leave-open
(Assignee)

Updated

3 years ago
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Keywords: leave-open
Resolution: --- → FIXED
(Assignee)

Updated

2 years ago
Blocks: 1253170
You need to log in before you can comment on or make changes to this bug.