Added "-Wno-signed-enum-bitfield" flag to not trigger Clang warning

Added "-Wno-signed-enum-bitfield" flag to not trigger Clang warning

Added "-Wno-signed-enum-bitfield" flag to not trigger upcoming Clang
warning regarding non-unsigned enum bitfields producing non-portable
code.

For more details, see the patch here: https://reviews.llvm.org/D24289

BUG=648462
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2354453002

[sashab, 2016-09-19 07:11:31]

Description was changed from

==========
Fix bitfield enums to not trigger upcoming Clang warning

Fix bitfield enums to not trigger upcoming Clang warning regarding
non-unsigned enum bitfields producing non-portable code.

For more details, see the patch here: https://reviews.llvm.org/D24289
==========

to

==========
Fix bitfield enums to not trigger upcoming Clang warning

Fix bitfield enums to not trigger upcoming Clang warning regarding
non-unsigned enum bitfields producing non-portable code.

For more details, see the patch here: https://reviews.llvm.org/D24289
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2354453002
==========

[sashab, 2016-09-20 00:00:00]

sashab@chromium.org changed reviewers:
+ bsalomon@chromium.org, reed@chromium.org

[sashab, 2016-09-20 00:04:02]

Description was changed from

==========
Fix bitfield enums to not trigger upcoming Clang warning

Fix bitfield enums to not trigger upcoming Clang warning regarding
non-unsigned enum bitfields producing non-portable code.

For more details, see the patch here: https://reviews.llvm.org/D24289
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2354453002
==========

to

==========
Fix bitfield enums to not trigger upcoming Clang warning

Fix bitfield enums to not trigger upcoming Clang warning regarding
non-unsigned enum bitfields producing non-portable code.

For more details, see the patch here: https://reviews.llvm.org/D24289

BUG=648462
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2354453002
==========

[reed1, 2016-09-20 17:11:47]

reed@google.com changed reviewers:
+ bungeman@google.com, mtklein@google.com, reed@google.com

[reed1, 2016-09-20 17:11:47]

How do we repro the warnings / test the fixes?

[mtklein_C, 2016-09-20 17:23:15]

mtklein@chromium.org changed reviewers:
+ mtklein@chromium.org

[mtklein_C, 2016-09-20 17:23:15]

I don't think we can do this until the GCC issue is resolved.  Skia builds with
GCC too, and it'd be pretty dangerous for us to turn of -Werror (and annoying to
see those warnings all the time).  Here's the test program I used to determine
we can't do this:

#include <stdio.h>

enum Foo : unsigned { A, B };

struct Bar {
    unsigned n : 31;
    Foo    foo :  1;
};

int main(int, char**) {
    printf("%zu\n", sizeof(Bar));

    return 0;
}

$ clang++ -std=c++11 -Werror test.cc && ./a.out
4
$ g++ -std=c++11 -Werror test.cc && ./a.out
test.cc:7:19: error: ‘Bar::foo’ is too small to hold all values of ‘enum Foo’
[-Werror]
     Foo    foo :  1;

[mtklein_C, 2016-09-20 17:26:20]

On 2016/09/20 at 17:23:15, mtklein_C wrote:
> I don't think we can do this until the GCC issue is resolved.  Skia builds
with GCC too, and it'd be pretty dangerous for us to turn of -Werror (and
annoying to see those warnings all the time).  Here's the test program I used to
determine we can't do this:
> 
> #include <stdio.h>
> 
> enum Foo : unsigned { A, B };
> 
> struct Bar {
>     unsigned n : 31;
>     Foo    foo :  1;
> };
> 
> int main(int, char**) {
>     printf("%zu\n", sizeof(Bar));
> 
>     return 0;
> }
> 
> $ clang++ -std=c++11 -Werror test.cc && ./a.out
> 4
> $ g++ -std=c++11 -Werror test.cc && ./a.out
> test.cc:7:19: error: ‘Bar::foo’ is too small to hold all values of ‘enum Foo’
[-Werror]
>      Foo    foo :  1;

Forgot to mention, that's Clang 3.8.1 and GCC 6.2.1.

[sashab, 2016-09-23 01:16:15]

On 2016/09/20 at 17:26:20, mtklein wrote:
> On 2016/09/20 at 17:23:15, mtklein_C wrote:
> > I don't think we can do this until the GCC issue is resolved.  Skia builds
with GCC too, and it'd be pretty dangerous for us to turn of -Werror (and
annoying to see those warnings all the time).  Here's the test program I used to
determine we can't do this:
> > 
> > #include <stdio.h>
> > 
> > enum Foo : unsigned { A, B };
> > 
> > struct Bar {
> >     unsigned n : 31;
> >     Foo    foo :  1;
> > };
> > 
> > int main(int, char**) {
> >     printf("%zu\n", sizeof(Bar));
> > 
> >     return 0;
> > }
> > 
> > $ clang++ -std=c++11 -Werror test.cc && ./a.out
> > 4
> > $ g++ -std=c++11 -Werror test.cc && ./a.out
> > test.cc:7:19: error: ‘Bar::foo’ is too small to hold all values of ‘enum
Foo’ [-Werror]
> >      Foo    foo :  1;
> 
> Forgot to mention, that's Clang 3.8.1 and GCC 6.2.1.

I've replied to the bug on GCC
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51242#c28), but it could be years
before that's fixed.

Would building with -Wno-signed-enum-bitfield be a good alternative to this
patch, for now? :)

[mtklein, 2016-09-23 11:39:41]

> Would building with -Wno-signed-enum-bitfield be a good alternative to this
> patch, for now? :)

Do you mean, building Skia in Chrome with -Wno-signed-enum-bitfield?  Sounds
fine... that's the status quo.

[sashab, 2016-11-16 02:26:12]

Description was changed from

==========
Fix bitfield enums to not trigger upcoming Clang warning

Fix bitfield enums to not trigger upcoming Clang warning regarding
non-unsigned enum bitfields producing non-portable code.

For more details, see the patch here: https://reviews.llvm.org/D24289

BUG=648462
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2354453002
==========

to

==========
Added "-Wno-signed-enum-bitfield" flag to not trigger Clang warning

Added "-Wno-signed-enum-bitfield" flag to not trigger upcoming Clang
warning regarding non-unsigned enum bitfields producing non-portable
code.

For more details, see the patch here: https://reviews.llvm.org/D24289

BUG=648462
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2354453002
==========

[sashab, 2016-11-16 02:26:24]

Great, done! Thanks :) Sorry for the lack of activity for a while

[mtklein_C, 2016-11-16 13:29:52]

On 2016/11/16 at 02:26:24, sashab wrote:
> Great, done! Thanks :) Sorry for the lack of activity for a while

What you've done here is disable this warning in Skia's standalone builds. 
Which is nice, but I think it's probably better to put this off until we start
using a Clang where this actually can trigger.

gn/BUILD.gn doesn't affect Chrome.  I think you'll want to open a new CL that
makes this change to src/skia/BUILD.gn in the Chromium repo.

[sashab, 2016-11-17 03:17:06]

Ahh I see, sorry about that. PTAL now :)