Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2834753002:
Remove unneeded enum forward declaration (Closed)
|
Created:
3 years, 8 months ago by brucedawson Modified:
3 years, 5 months ago CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com, zhuangzesen_agora.io, stefan-webrtc, mflodman Target Ref:
refs/heads/master Project:
webrtc Visibility:
Public. |
DescriptionRemove unneeded enum forward declaration
While building Chrome with the VC++ 2017 /permissive- flag I got a
warning about a forward declaration of enum RateControlRegion. Untyped
forward declarations of enums are illegal because the compiler doesn't
know what size to make them. The only reason this forward declaration is
legal is because it isn't needed (the type is already defined).
This was found because /permissive- (or, equivalently for this purpose,
/w14471) incorrectly fires on this forward declaration even though it is
legal.
BUG=chromium:736059
Review-Url: https://codereview.webrtc.org/2834753002
Cr-Commit-Position: refs/heads/master@{#18741}
Committed: https://chromium.googlesource.com/external/webrtc/+/5e5f7e14b23ee93fb163d64229da5ec5708f7cf9
Patch Set 1 #Patch Set 2 : Update definition as well #Patch Set 3 : Remove forward declaration #Messages
Total messages: 45 (32 generated)
The CQ bit was checked by brucedawson@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_clang_rel on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/win_clang_rel/builds/12958) win_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/win_dbg/builds/18356) win_rel on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/win_rel/builds/25463) win_x64_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/win_x64_dbg/builds/8436) win_x64_rel on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/win_x64_rel/builds/24878)
The CQ bit was checked by brucedawson@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
Description was changed from ========== Fix bug found by VC++ 2017's /permissive- flag The VC++ 2017 /permissive- flag tells the compiler to look for standards violations: https://blogs.msdn.microsoft.com/vcblog/2016/11/16/permissive-switch/ Compiling with this flag found an enum that was forward declared without specifying a size. ========== to ========== Fix bug found by VC++ 2017's /permissive- flag The VC++ 2017 /permissive- flag tells the compiler to look for standards violations: https://blogs.msdn.microsoft.com/vcblog/2016/11/16/permissive-switch/ Compiling with this flag found an enum that was forward declared without specifying a size. Specifying a size for the declaration is enough to make VC++ happy, but it makes gcc/clang unhappy - they insist that if you specify the size for the declaration you must also specify it for the definition, so this change does that. ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_clang_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/win_clang_dbg/builds/13102) win_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/win_dbg/builds/18357) win_x64_clang_rel on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/win_x64_clang_rel/build...)
The CQ bit was checked by brucedawson@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: presubmit on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/16321)
The CQ bit was checked by brucedawson@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
brucedawson@chromium.org changed reviewers: + stefan@webrtc.org
Description was changed from ========== Fix bug found by VC++ 2017's /permissive- flag The VC++ 2017 /permissive- flag tells the compiler to look for standards violations: https://blogs.msdn.microsoft.com/vcblog/2016/11/16/permissive-switch/ Compiling with this flag found an enum that was forward declared without specifying a size. Specifying a size for the declaration is enough to make VC++ happy, but it makes gcc/clang unhappy - they insist that if you specify the size for the declaration you must also specify it for the definition, so this change does that. ========== to ========== Remove unneeded enum forward declare While building Chrome with the VC++ 2017 /permissive- flag I got a warning about a forward declaration of enum RateControlRegion. Untyped forward declarations of enums are illegal because the compiler doesn't know what size to make them. The only reason this forward declaration is legal is because it isn't needed (the type is already defined). This was found because /permissive- (or, equivalently for this purpose, /w44471) incorrectly fires on this forward declaration even though it is legal. ==========
Description was changed from ========== Remove unneeded enum forward declare While building Chrome with the VC++ 2017 /permissive- flag I got a warning about a forward declaration of enum RateControlRegion. Untyped forward declarations of enums are illegal because the compiler doesn't know what size to make them. The only reason this forward declaration is legal is because it isn't needed (the type is already defined). This was found because /permissive- (or, equivalently for this purpose, /w44471) incorrectly fires on this forward declaration even though it is legal. ========== to ========== Remove unneeded enum forward declaration While building Chrome with the VC++ 2017 /permissive- flag I got a warning about a forward declaration of enum RateControlRegion. Untyped forward declarations of enums are illegal because the compiler doesn't know what size to make them. The only reason this forward declaration is legal is because it isn't needed (the type is already defined). This was found because /permissive- (or, equivalently for this purpose, /w44471) incorrectly fires on this forward declaration even though it is legal. ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: presubmit on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/16350)
Minor tweak. PTAL.
Ping - PTAL. Tiny change to remove an unneeded/misleading forward declaration.
brucedawson@chromium.org changed reviewers: + stefan-webrtc. henrika@webrtc.org - stefan@webrtc.org
Ping, with another reviewer added also. Trivial change.
brucedawson@chromium.org changed reviewers: + henrik.lundin@webrtc.org - stefan-webrtc. henrika@webrtc.org
PTAL - minor code cleanup that has languished for a bit.
lgtm
The CQ bit was checked by brucedawson@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: presubmit on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/18500)
Description was changed from ========== Remove unneeded enum forward declaration While building Chrome with the VC++ 2017 /permissive- flag I got a warning about a forward declaration of enum RateControlRegion. Untyped forward declarations of enums are illegal because the compiler doesn't know what size to make them. The only reason this forward declaration is legal is because it isn't needed (the type is already defined). This was found because /permissive- (or, equivalently for this purpose, /w44471) incorrectly fires on this forward declaration even though it is legal. ========== to ========== Remove unneeded enum forward declaration While building Chrome with the VC++ 2017 /permissive- flag I got a warning about a forward declaration of enum RateControlRegion. Untyped forward declarations of enums are illegal because the compiler doesn't know what size to make them. The only reason this forward declaration is legal is because it isn't needed (the type is already defined). This was found because /permissive- (or, equivalently for this purpose, /w14471) incorrectly fires on this forward declaration even though it is legal. BUG=chromium:736059 ==========
The CQ bit was checked by brucedawson@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: presubmit on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/18504)
brucedawson@chromium.org changed reviewers: + stefan@webrtc.org - henrik.lundin@webrtc.org
Sigh... Apparently hlundin isn't an owner. PTAL
tommi@webrtc.org changed reviewers: + tommi@webrtc.org
The CQ bit was checked by tommi@webrtc.org
lgtm
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1498333128002200,
"parent_rev": "47bfa3902d9651ee64f465c559dd7418f8a0b76c", "commit_rev":
"5e5f7e14b23ee93fb163d64229da5ec5708f7cf9"}
Message was sent while issue was closed.
Description was changed from ========== Remove unneeded enum forward declaration While building Chrome with the VC++ 2017 /permissive- flag I got a warning about a forward declaration of enum RateControlRegion. Untyped forward declarations of enums are illegal because the compiler doesn't know what size to make them. The only reason this forward declaration is legal is because it isn't needed (the type is already defined). This was found because /permissive- (or, equivalently for this purpose, /w14471) incorrectly fires on this forward declaration even though it is legal. BUG=chromium:736059 ========== to ========== Remove unneeded enum forward declaration While building Chrome with the VC++ 2017 /permissive- flag I got a warning about a forward declaration of enum RateControlRegion. Untyped forward declarations of enums are illegal because the compiler doesn't know what size to make them. The only reason this forward declaration is legal is because it isn't needed (the type is already defined). This was found because /permissive- (or, equivalently for this purpose, /w14471) incorrectly fires on this forward declaration even though it is legal. BUG=chromium:736059 Review-Url: https://codereview.webrtc.org/2834753002 Cr-Commit-Position: refs/heads/master@{#18741} Committed: https://chromium.googlesource.com/external/webrtc/+/5e5f7e14b23ee93fb163d6422... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/external/webrtc/+/5e5f7e14b23ee93fb163d6422... |
