New issue
Advanced search Search tips

Issue 736059 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

We should remove unneeded and illegal enum forward declarations

Project Member Reported by brucedaw...@chromium.org, Jun 22 2017

Issue description

Forward declaring of enums without specifying a size is prohibited in C++. However VC++ allowed it for a long time so some crept in.

Some of them (those that are also compiled for other platforms) are just unnecessary and can be deleted, to avoid confusion.

Others (Windows only code) are wrong and need fixing.

 
Some fixes that have been done already:

commit 0ce0ee1aa6d9d2db1a7cdc7f000cfe451378d6f2
Author: brucedawson <brucedawson@chromium.org>
Date:   Tue Apr 25 17:44:25 2017 -0700

    Remove unneeded enum forward declaration

    Forward declaring of enums without specifying a type is illegal. This
    forward declaration only worked because it wasn't needed. It was
    found by VC++'s overzealous C4471 warning.

    R=raymes

    Review-Url: https://codereview.chromium.org/2836913003
    Cr-Commit-Position: refs/heads/master@{#467182}


commit a85fa6ea57acf1e6d8b3a13f1dc9b9754c2ec89b
Author: brucedawson <brucedawson@chromium.org>
Date:   Mon Apr 24 07:21:12 2017 -0700

    Remove some illegal enum forward declarations

    C++ forbids the forward declaring of enums without specifying a size
    because the size is not known. Specifying the size in multiple places is
    dangerous. So, this change removes the forward declarations and instead
    includes the required header files, except where one case where the
    forward declaration is unavoidable.

    These problems were found with the C4471 option, enabled with /w14471.

    R=jschuh@chromium.org
    CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng

    Review-Url: https://codereview.chromium.org/2829853010
    Cr-Commit-Position: refs/heads/master@{#466628}


commit b882d76143eaa993f8f1d0c6393d4d52d836838e
Author: brucedawson <brucedawson@chromium.org>
Date:   Mon Apr 24 15:10:31 2017 -0700

    Remove unneeded WebKit enum forward declarations

    Forward declaring of enums without specifying a type is illegal. These
    forward declarations only worked because they weren't needed. They were
    found by VC++'s overzealous C4471 warning.

    R=chrishtr@chromium.org

    Review-Url: https://codereview.chromium.org/2837113002
    Cr-Commit-Position: refs/heads/master@{#466789}

Project Member

Comment 2 by bugdroid1@chromium.org, Jun 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/external/webrtc.git/+/5e5f7e14b23ee93fb163d64229da5ec5708f7cf9

commit 5e5f7e14b23ee93fb163d64229da5ec5708f7cf9
Author: brucedawson <brucedawson@chromium.org>
Date: Sat Jun 24 20:04:29 2017

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}

[modify] https://crrev.com/5e5f7e14b23ee93fb163d64229da5ec5708f7cf9/webrtc/modules/remote_bitrate_estimator/overuse_detector.h

Status: Fixed (was: Started)
I'm not aware of anymore and we've landed a clang-cl warning (crrev.com/2955643002) that should keep things clean.

Sign in to add a comment