Closed
Bug 1333686
Opened 7 years ago
Closed 7 years ago
Fix compiler warnings and enable warnings-as-errors in webrtc/signaling
Categories
(Core :: WebRTC, defect, P2)
Tracking
()
RESOLVED
FIXED
mozilla54
People
(Reporter: cpeterson, Assigned: cpeterson)
References
(Blocks 1 open bug)
Details
Attachments
(4 files)
media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp:314:15 [-Wswitch] enumeration values 'kText', 'kApplication', and 'kMessage' not handled in switch media/webrtc/signaling/src/sdp/sipcc/sdp_attr.c(584): warning C4146: unary minus operator applied to unsigned type, result still unsigned media/webrtc/signaling/src/sdp/sipcc/sdp_attr.c(585): warning C4146: unary minus operator applied to unsigned type, result still unsigned media/webrtc/signaling/src/sdp/sipcc/sdp_attr.c(586): warning C4146: unary minus operator applied to unsigned type, result still unsigned Warning: -Wmacro-redefined in ipc/chromium/src/base/compiler_specific.h: 'WARN_UNUSED_RESULT' macro redefined ipc/chromium/src/base/compiler_specific.h:73:9: warning: 'WARN_UNUSED_RESULT' macro redefined [-Wmacro-redefined] #define WARN_UNUSED_RESULT __attribute__((warn_unused_result)) media/webrtc/trunk/webrtc/voice_engine/voe_external_media_impl.h:29:15 [-Winconsistent-missing-override] 'SetExternalRecordingStatus' overrides a member function but is not marked 'override' media/webrtc/trunk/webrtc/voice_engine/voe_external_media_impl.h:31:15 [-Winconsistent-missing-override] 'SetExternalPlayoutStatus' overrides a member function but is not marked 'override' media/webrtc/trunk/webrtc/voice_engine/voe_external_media_impl.h:33:15 [-Winconsistent-missing-override] 'ExternalRecordingInsertData' overrides a member function but is not marked 'override' media/webrtc/trunk/webrtc/voice_engine/voe_external_media_impl.h:40:15 [-Winconsistent-missing-override] 'ExternalPlayoutData' overrides a member function but is not marked 'override' media/webrtc/trunk/webrtc/voice_engine/voe_external_media_impl.h:47:15 [-Winconsistent-missing-override] 'ExternalPlayoutGetData' overrides a member function but is not marked 'override'
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
Assignee | |
Updated•7 years ago
|
Attachment #8830175 -
Flags: review?(docfaraday)
|
||
Comment 5•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8830173 [details] Bug 1333686 - Part 1: Fix -Wswitch warning in webrtc/signaling. https://reviewboard.mozilla.org/r/107066/#review108368
Attachment #8830173 -
Flags: review?(docfaraday) → review+
|
|
||
Comment 6•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8830174 [details] Bug 1333686 - Part 2: Fix "unary minus operator applied to unsigned type" MSVC warnings in webrtc/signaling. https://reviewboard.mozilla.org/r/107068/#review108370
Attachment #8830174 -
Flags: review?(docfaraday) → review+
|
|
||
Comment 7•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8830176 [details] Bug 1333686 - Part 4: Enable warnings-as-errors in webrtc/signaling. https://reviewboard.mozilla.org/r/107072/#review108372
Attachment #8830176 -
Flags: review?(docfaraday) → review+
|
|
Assignee | |
Comment 8•7 years ago
|
||
I removed the review request for the -Wmacro-redefined warnings because I have a cleaner solution. The warning comes from an ipc/chromium header file that we hard forked from Google, so I can fix the warning in our copy.
|
||
Updated•7 years ago
|
Rank: 25
Priority: -- → P2
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
||
Comment 11•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8830175 [details] Bug 1333686 - Part 3: Fix -Wmacro-redefined warnings in ipc/chromium. https://reviewboard.mozilla.org/r/107070/#review108414
Attachment #8830175 -
Flags: review?(jld) → review+
|
||
Comment 12•7 years ago
|
||
Pushed by cpeterson@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5c610ccf76de Part 1: Fix -Wswitch warning in webrtc/signaling. r=bwc https://hg.mozilla.org/integration/autoland/rev/313a9604211f Part 2: Fix "unary minus operator applied to unsigned type" MSVC warnings in webrtc/signaling. r=bwc https://hg.mozilla.org/integration/autoland/rev/514fdfd43d63 Part 3: Fix -Wmacro-redefined warnings in ipc/chromium. r=jld https://hg.mozilla.org/integration/autoland/rev/154ee971feb7 Part 4: Enable warnings-as-errors in webrtc/signaling. r=bwc
|
||
Comment 13•7 years ago
|
||
Backed out in https://hg.mozilla.org/integration/autoland/rev/98ab69f5f624 for the odd bedfellows of ASan and (at least Linux, so far) static analysis agreeing about https://treeherder.mozilla.org/logviewer.html#?job_id=72117093&repo=autoland
|
|
Assignee | |
Comment 14•7 years ago
|
||
gps, do you know why gyp doesn't seem to recognize the Linux address-sanitizer or static-analysis builds as using clang? On the Linux builders, gyp doesn't match my 'clang == 1' condition in media/webrtc/signaling/signaling.gyp for the address-sanitizer or static-analysis builds. This causes warnings-as-errors breakage because my -Wno-inconsistent-missing-override flag is not added to cflags_mozilla. On my local Mac ASan builds, however, gyp correctly matches the 'clang == 1' condition. I'm now testing a patch to simply not enable warnings-as-errors for webrtc/signaling when compiling with clang on Linux. Debugging the root cause of gyp not recognizing address-sanitizer or static-analysis builds as clang doesn't seem worth the extra effort. <:)
Flags: needinfo?(gps)
|
|
||
Comment 15•7 years ago
|
||
Pushed by cpeterson@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/7513b8053749 Part 2: Fix "unary minus operator applied to unsigned type" MSVC warnings in webrtc/signaling. r=bwc https://hg.mozilla.org/integration/mozilla-inbound/rev/fb2f6cf07488 Part 3: Fix -Wmacro-redefined warnings in ipc/chromium. r=jld https://hg.mozilla.org/integration/mozilla-inbound/rev/af51cb06905b Part 4: Enable warnings-as-errors in webrtc/signaling. r=bwc
|
|
Assignee | |
Comment 16•7 years ago
|
||
I relanded my changes, but kept ALLOW_COMPILER_WARNINGS when compiling with clang on Linux and dropped the -Wswitch warning fix (because it has fixed in bug 1160558 since my backout).
Status: NEW → ASSIGNED
Flags: needinfo?(gps)
|
||
Comment 17•7 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/7513b8053749 https://hg.mozilla.org/mozilla-central/rev/fb2f6cf07488 https://hg.mozilla.org/mozilla-central/rev/af51cb06905b
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in
before you can comment on or make changes to this bug.
Description
•