tracked_objects::ThreadData uses an enum name that collides with an X11 lib define. |
|||
Issue descriptionAs shown in this CL: https://codereview.chromium.org/1939683002/ (see trybot build https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_compile_dbg_ng/builds/194015) alphanumeric include ordering can cause <X11/Xlib.h> to be included before "base/tracked_objects.h". The former contains: #define Status int (... and no later #undef...) The latter contains: enum Status { ... } The compiler complains about, essentially, "enum int { ... }". The most natural solution would appear to be renaming tracked_objects::ThreadData::Status.
,
May 2 2016
We have been #undef'ing things when such conflicts happen: e.g. https://code.google.com/p/chromium/codesearch#search/&q=%22%23undef%20Status%22&sq=package:chromium&type=cs
,
May 2 2016
sadrul@ this seems like an expedient, but less-than-thorough solution.
,
May 2 2016
I would not disagree with that. But from a quick grep, there are ~130 places where an enum/class/struct is called 'Status'. I wouldn't necessarily want all of these renamed.
,
May 2 2016
In general we try to make cringe-worthy X11 headers (and win32 headers for that matter) not impact our overall codebase, by including it in as few places as possible (in particular, avoiding including it in cross-platform headers), and patching issues with #undef when necessary. What is bringing X11/Xlib.h into gpu_video_decode_accelerator.cc ?
,
May 9 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/276736a4e1f1f2431a6433208ac8a3c75f2722a4 commit 276736a4e1f1f2431a6433208ac8a3c75f2722a4 Author: markdittmer <markdittmer@chromium.org> Date: Mon May 09 20:13:23 2016 Currently, gpu_video_decode_accelerator.cc has a strange include order exception for the following reason: (1) X11/Xlib.h contains "#define Status ..." (2) base/tracked_objects.h contains "enum Status { ... }" (3) gpu_video_decode_accelerator.cc indirectly includes (10 before (2) After discussing possible fixes with danakj@, I've renamed the enum in (2). R=danakj@chromium.org BUG= 608239 Review-Url: https://codereview.chromium.org/1936093003 Cr-Commit-Position: refs/heads/master@{#392408} [modify] https://crrev.com/276736a4e1f1f2431a6433208ac8a3c75f2722a4/media/gpu/ipc/service/gpu_video_decode_accelerator.cc [modify] https://crrev.com/276736a4e1f1f2431a6433208ac8a3c75f2722a4/ui/gl/gl_bindings.h
,
May 9 2016
It turns out that this was already supposed to be #undef'd in gl_bindings.h, but the #undef was erroneously wrapped in an #if branch.
,
May 9 2016
,
May 23 2016
Bulk verified
,
May 23 2016
bulk verified |
|||
►
Sign in to add a comment |
|||
Comment 1 by fsam...@chromium.org
, May 2 2016Labels: -Pri-3 mustash1 mustash mus gpurefactor OS-All Pri-1