New issue
Advanced search Search tips

Issue 747371 link

Starred by 1 user

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

STATIC_ASSERT_ENUM is defined 11 times, once should be enough

Project Member Reported by brat...@opera.com, Jul 21 2017

Issue description

There is a macro to compare two enum values to see that they have the same numerical value called STATIC_ASSERT_ENUM. It is so popular that it exists 12 different places. Preferably everyone should use the one in third_party/WebKit/Source/platform/wtf/Assertions.h but reasons.

bratell@bratell-linux:~/src/chromium/src$ git grep "#define STATIC_ASSERT_ENUM"
components/printing/renderer/print_web_view_helper.cc:#define STATIC_ASSERT_ENUM(a, b)                            \
content/browser/web_contents/web_contents_view_mac.mm:#define STATIC_ASSERT_ENUM(a, b)                            \
content/child/assert_matching_enums.cc:#define STATIC_ASSERT_ENUM(a, b)                            \
content/child/web_url_request_util.cc:#define STATIC_ASSERT_ENUM(a, b)                            \
content/public/common/web_preferences.cc:#define STATIC_ASSERT_ENUM(a, b)                            \
content/renderer/pepper/pepper_plugin_instance_impl.cc:#define STATIC_ASSERT_ENUM(a, b)                            \
media/blink/webmediaplayer_impl.cc:#define STATIC_ASSERT_ENUM(a, b)                            \
media/gpu/gpu_video_accelerator_util.cc:#define STATIC_ASSERT_ENUM_MATCH(name)                             \
pdf/pdfium/pdfium_assert_matching_enums.cc:#define STATIC_ASSERT_ENUM(a, b)                            \
ppapi/thunk/ppb_text_input_thunk.cc:#define STATIC_ASSERT_ENUM(a, b)                            \
third_party/WebKit/Source/platform/wtf/Assertions.h:#define STATIC_ASSERT_ENUM(a, b)                            \
third_party/WebKit/public/web/ConsoleMessageStructTraits.cpp:#define STATIC_ASSERT_ENUM(a, b)                            \

I noticed it while doing jumbo but this is really just a generic code issue. Adding people that I know care about the code to CC.
 

Comment 1 by brat...@opera.com, Jul 21 2017

Summary: STATIC_ASSERT_ENUM is defined 11 times, once should be enough (was: STATIC_ASSERT_ENUM is defined 12 times, once should be enough)
Ok, the one in gpu is different, so make it 11.

Comment 2 by danakj@chromium.org, Jul 21 2017

This is macro for a static assert with static casts. I agree it could be defined once, though I'm not sure it's worth spending time on really.. I guess the cost of this macro seems very low to me.

Comment 3 by brat...@opera.com, Jul 21 2017

I encountered it because the one in content/renderer/effective_connection_type_helper.cc (which isn't in the list above because I had changed that code, my mistake) collided with the one in third_party/WebKit/Source/platform/wtf/Assertions.h because some code in content/renderer indirectly included that header (no idea how) so even if it is really not important in itself, jumbo might make it necessary to do something.

It's not urgent (jumbo is not even enabled by default, and is not supported in content yet anyway) but I wonder what the right way to solve it should be.

Sign in to add a comment