Fix bitfield enums to not trigger upcoming Clang warning |
|||||
Issue descriptionFix bitfield enums to not trigger upcoming Clang warning regarding non-unsigned enum bitfields producing non-portable code. This usually just involves adding ': unsigned' to the enum description whenever an enum is used in a bitfield. The flag -Wsigned-enum-bitfield enables a check for enums used in bitfields. Enum bitfields should use unsigned as the underlying type to prevent strange behavior in MSVC. Design Doc (in context of ComputedStyle): https://docs.google.com/document/d/1BfjMEXIzvY1WZEn_nESBxyqkazkfa0sBSEvm-F89YwM/edit Discussion on style-dev: https://www.google.com/url?hl=en&q=https://groups.google.com/a/chromium.org/d/msgid/style-dev/CADAgNurPp47TyFcws6LAyY%253DAxT5RpRb0MkAWJ_G3J2oL0Uj-mg%2540mail.gmail.com?utm_medium%3Demail%26utm_source%3Dfooter&source=gmail&ust=1471065087985000&usg=AFQjCNGjo6xUygkhjxGo90tVmjGrlm6MiA). Allowing enums in bitfields has the advantage of saving space while not having to use static casts when reading from/writing to the bitfield, as well as not having to match the correct underlying type (signed/unsigned) when deciding the field to store. For more details, see the patch here: https://reviews.llvm.org/D24289
,
Sep 20 2016
Hmm, there's a new caveat with this patch. See the discussions: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51242 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61414 Basically, enums with an explicit underlying type (or enum classes, although the latter is to be fixed) are given the size of their underlying type. For example: enum Foo { A, B }; can be stored in a bitfield of size 1, but: enum Foo : unsigned char { A, B }; will error in a bitfield of any size smaller than 8. I think it's worth filing a bug against GCC on this, or reviving the old one.
,
Sep 20 2016
,
Sep 20 2016
Commented on the GCC bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51242#c28 Hopefully they will look at this, despite it not having been fixed in over 3 years.
,
Sep 20 2016
The following revision refers to this bug: https://chromium.googlesource.com/angle/angle/+/10f563d5ab96e280d85df9c99384e79eee9dec9f commit 10f563d5ab96e280d85df9c99384e79eee9dec9f Author: Sasha Bermeister <sashab@chromium.org> Date: Mon Sep 19 07:02:36 2016 Fix base/numerics to not trigger upcoming Clang warning Fix base/numerics to not trigger upcoming Clang warning regarding unsigned enum bitfields producing non-portable code. For more details, see the patch here: https://reviews.llvm.org/D24289 BUG=648462 Change-Id: I9aaa01e99f734ff927b5b1171706b2f49b437d0b Reviewed-on: https://chromium-review.googlesource.com/386887 Commit-Queue: Jamie Madill <jmadill@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org> [modify] https://crrev.com/10f563d5ab96e280d85df9c99384e79eee9dec9f/src/common/third_party/numerics/base/numerics/safe_conversions_impl.h
,
Sep 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7265052d46cd24c7f9665bd298f8d878ef12274e commit 7265052d46cd24c7f9665bd298f8d878ef12274e Author: cwallez <cwallez@chromium.org> Date: Thu Sep 22 03:52:12 2016 Roll ANGLE 8b28a8b..c287ea6 https://chromium.googlesource.com/angle/angle.git/+log/8b28a8b..c287ea6 BUG= chromium:644033 , chromium:637050 ,648462, chromium:647807 TBR=geofflang@chromium.org TEST=bots CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2357933002 Cr-Commit-Position: refs/heads/master@{#420258} [modify] https://crrev.com/7265052d46cd24c7f9665bd298f8d878ef12274e/DEPS
,
Nov 17 2016
,
Feb 2 2018
Shifting focus to work on ChromeOS.
,
Jan 11
Available, but no owner or component? Please find a component, as no one will ever find this without one. |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by sashab@chromium.org
, Sep 20 2016