Fix -Wswitch warnings in ipc/chromium and dom/plugins

RESOLVED FIXED in Firefox 36

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: cpeterson, Assigned: cpeterson)

Tracking

(Blocks: 1 bug)

unspecified
mozilla36
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox35 wontfix, firefox36 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

4 years ago
Created attachment 8503014 [details] [diff] [review]
part-1-fix-message_loop-warning.patch

Bug 1060738 introduced the following -Wswitch warning:

ipc/chromium/src/base/message_loop.cc:107:11 [-Wswitch] 4 enumeration values not handled in switch: 'TYPE_DEFAULT', 'TYPE_UI', 'TYPE_IO'...
Attachment #8503014 - Flags: review?(tabraldes)
(Assignee)

Comment 1

4 years ago
Created attachment 8503016 [details] [diff] [review]
part-2-fix-npapi-warnings.patch

Part 2: Fix -Wswitch warnings in dom/plugins and enable -Werror=switch warning-as-error.

This Android gcc warnings are the last -Wswitch warnings in Gecko C++ code, so we can enable -Werror=switch warning-as-error to avoid regressions.

dom/plugins/base/nsNPAPIPlugin.cpp:2211:5: error: case value '1001' not in enumerated type 'NPNVariable' [-Werror=switch]
dom/plugins/base/nsNPAPIPlugin.cpp:2197:5: error: case value '1002' not in enumerated type 'NPNVariable' [-Werror=switch]
dom/plugins/base/nsNPAPIPlugin.cpp:2169:5: error: case value '1003' not in enumerated type 'NPNVariable' [-Werror=switch]
dom/plugins/base/nsNPAPIPlugin.cpp:2190:5: error: case value '1004' not in enumerated type 'NPNVariable' [-Werror=switch]
dom/plugins/base/nsNPAPIPlugin.cpp:2176:5: error: case value '1005' not in enumerated type 'NPNVariable' [-Werror=switch]
dom/plugins/base/nsNPAPIPlugin.cpp:2183:5: error: case value '1006' not in enumerated type 'NPNVariable' [-Werror=switch]
dom/plugins/base/nsNPAPIPlugin.cpp:2204:5: error: case value '1007' not in enumerated type 'NPNVariable' [-Werror=switch]
dom/plugins/base/nsNPAPIPlugin.cpp:2162:5: error: case value '1008' not in enumerated type 'NPNVariable' [-Werror=switch]
dom/plugins/base/nsNPAPIPlugin.cpp:2232:5: error: case value '1009' not in enumerated type 'NPNVariable' [-Werror=switch]
dom/plugins/base/nsNPAPIPlugin.cpp:2225:5: error: case value '1010' not in enumerated type 'NPNVariable' [-Werror=switch]
dom/plugins/base/nsNPAPIPlugin.cpp:2218:5: error: case value '1011' not in enumerated type 'NPNVariable' [-Werror=switch]
dom/plugins/base/nsNPAPIPlugin.cpp:2256:5: error: case value '1012' not in enumerated type 'NPNVariable' [-Werror=switch]
dom/plugins/base/nsNPAPIPlugin.cpp:2270:5: error: case value '1013' not in enumerated type 'NPNVariable' [-Werror=switch]
dom/plugins/base/nsNPAPIPlugin.cpp:2277:5: error: case value '1014' not in enumerated type 'NPNVariable' [-Werror=switch]
dom/plugins/base/nsNPAPIPlugin.cpp:2291:5: error: case value '1015' not in enumerated type 'NPNVariable' [-Werror=switch]
dom/plugins/base/nsNPAPIPlugin.cpp:2306:5: error: case value '1016' not in enumerated type 'NPNVariable' [-Werror=switch]
dom/plugins/base/nsNPAPIPlugin.cpp:2313:5: error: case value '1017' not in enumerated type 'NPNVariable' [-Werror=switch]
dom/plugins/base/nsNPAPIPlugin.cpp:2284:5: error: case value '1018' not in enumerated type 'NPNVariable' [-Werror=switch]
dom/plugins/base/nsNPAPIPlugin.cpp:2263:5: error: case value '1019' not in enumerated type 'NPNVariable' [-Werror=switch]
dom/plugins/base/nsNPAPIPlugin.cpp:2298:5: error: case value '1020' not in enumerated type 'NPNVariable' [-Werror=switch]
dom/plugins/base/nsNPAPIPlugin.cpp:2426:3: error: case value '1001' not in enumerated type 'NPPVariable' [-Werror=switch]
Attachment #8503016 - Flags: review?(benjamin)

Comment 2

4 years ago
Comment on attachment 8503016 [details] [diff] [review]
part-2-fix-npapi-warnings.patch

I don't understand why the casts are necessary in nsNPAPIPlugin.cpp. Both of those have default: cases which should suppress the warning (and are in fact important, since this is designed to be future-compatible).
Flags: needinfo?(cpeterson)
(Assignee)

Comment 3

4 years ago
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #2)
> I don't understand why the casts are necessary in nsNPAPIPlugin.cpp. Both of
> those have default: cases which should suppress the warning (and are in fact
> important, since this is designed to be future-compatible).

The warning is complaining that we are switching on a variable of enum type NPNVariable, but the switch statement includes case values that are out of range for NPNVariable. These are Android-specific values defined by Google in android_npapi.h (not our code):

https://hg.mozilla.org/mozilla-central/file/tip/dom/plugins/base/android/android_npapi.h#l111

Casting the NPNVariable to an int tells the compiler to expect any int value so it won't perform enum range check and the default case is important, as you said.
Flags: needinfo?(cpeterson)
Attachment #8503014 - Flags: review?(tabraldes) → review+
(Assignee)

Comment 4

4 years ago
Part 1: Fix -Wswitch warning in ipc/chromium. r=tabraldes

https://hg.mozilla.org/integration/mozilla-inbound/rev/73be31bb8122
status-firefox34: wontfix → ---
status-firefox35: affected → wontfix
status-firefox36: --- → affected
Keywords: leave-open

Updated

4 years ago
Attachment #8503016 - Flags: review?(benjamin) → review+
(Assignee)

Comment 6

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/19d906680583
status-firefox36: affected → fixed
Keywords: leave-open
https://hg.mozilla.org/mozilla-central/rev/19d906680583
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Depends on: 1086705

Updated

4 years ago
Depends on: 1087098
(Assignee)

Updated

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