Issue 699477 link

Starred by 5 users

Issue metadata

Status:	Fixed
Owner:	pkasting@chromium.org
Closed:	Mar 2017
Cc:	scottmg@chromium.org
Components:	Platform>DevTools
EstimatedDays:	----
NextAction:	----
OS:	Windows
Pri:	0
Type:	Bug

Windows compile fail: redefinition of blink::protocol::Response

Project Member Reported by pkasting@chromium.org, Mar 8 2017

Issue description

Sample output:

FAILED: obj/third_party/WebKit/Source/modules/modules/V8FetchEvent.obj
ninja -t msvc -e environment.x64 -- "E:\depot_tools\win_toolchain\vs_files\d3cb0e37bdd120ad0ac4650b674b09e81be45616\VC\bin\amd64/cl.exe" /nologo /showIncludes /FC @obj/third_party/WebKit/Source/modules/modules/V8FetchEvent.obj.rsp /c gen/blink/bindings/modules/v8/V8FetchEvent.cpp /Foobj/third_party/WebKit/Source/modules/modules/V8FetchEvent.obj /Fd"obj/third_party/WebKit/Source/modules/modules_cc.pdb"
e:\chrome\src\third_party\webkit\source\modules\serviceworkers\fetchevent.h(23): error C2371: 'blink::protocol::Response': redefinition; different basic types
e:\chrome\src\out\debug\gen\blink\core\inspector\protocol\forward.h(24): note: see declaration of 'blink::protocol::Response'

Possible explanation: some kind of errant "using protocol::Response" in a header, resulting in symbol clashes.  (Solution in that case: don't use such an alias in a header, use it only in the .cc file.)

Not clear why this isn't breaking the tree for everyone but multiple engineers are getting it.  Has been happening at least 9 hours.

Comment 1 by pkasting@chromium.org, Mar 8 2017

Possible workaround: remove using declaration from third_party/WebKit/Source/core/inspector/InspectorBaseAgent.h, explicitly qualify instead.  Working on testing this locally.

Comment 2 by dcheng@chromium.org, Mar 8 2017

I don't suppose there's any way to get MSVC to dump out the include chain that's pulling in the inspector header?

Comment 3 by pkasting@chromium.org, Mar 8 2017

There is, but I can't be bothered to find it ATM -- pretty sure this works, but it results in needing to explicitly qualify a lot of things in several different headers, so it's not a trivial patch to get working.  Trying to get it functional and uploaded to tryservers.

Comment 4 by pkasting@chromium.org, Mar 8 2017

Owner: pkasting@chromium.org
Status: Started (was: Untriaged)

https://codereview.chromium.org/2738453004/

Comment 5 by thakis@chromium.org, Mar 8 2017

Also happening on bots: https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.fyi%2FClangToTWin_dbg_%2F7715%2F%2B%2Frecipes%2Fsteps%2Fcompile%2F0%2Fstdout

In file included from gen/blink/bindings/modules/v8/V8FetchEvent.cpp:12:
In file included from gen/blink/bindings/modules/v8/V8FetchEvent.h:24:C:\b\rr\tmpaxgver\w\src\third_party\WebKit\Source\modules\serviceworkers\FetchEvent.h(23,1):  error: declaration conflicts with target of using declaration already in scope
class Response;
^
C:\b\rr\tmpaxgver\w\src\out\Debug\gen\blink\core\inspector\protocol\Forward.h(24,7):  note: target of using declaration
using Response = DispatchResponse;
      ^
C:\b\rr\tmpaxgver\w\src\third_party\WebKit\Source\core\inspector\InspectorBaseAgent.h(46,17):  note: using declaration
using protocol::Response;
                ^


clang also doesn't print the include chain for InspectorBaseAgent.


That bot used to be reliably green, now it's reliably red, so something broke this recently.

Comment 6 by thakis@chromium.org, Mar 8 2017

Probably the include added here: https://codereview.chromium.org/2729613008/diff/40001/third_party/WebKit/Source/core/inspector/InspectorTraceEvents.h

Comment 7 by thakis@chromium.org, Mar 8 2017

I landed a revert of that CL here: https://codereview.chromium.org/2737623006/

Please check if things look better if you sync past #455452.

Project Member

Comment 8 by bugdroid1@chromium.org, Mar 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/665bde1047fbd03398ec00158218cd56a60f1e35

commit 665bde1047fbd03398ec00158218cd56a60f1e35
Author: pkasting <pkasting@chromium.org>
Date: Wed Mar 08 16:44:24 2017

Avoid using declarations in headers which introduce symbol conflicts.

Source/modules/fetch/Response.h defines class blink::Response.
Source/core/inspector/InspectorBaseAgent.h was pulling blink::protocol::Response
into the blink namespace.  Any file transitively #including both of these
triggered a symbol redefinition error on Windows.

Instead, explicitly qualify protocol::Response in inspector headers, then add
using directives in .cpp files.

BUG= 699477 
TEST=none

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

[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/core/inspector/DOMEditor.cpp
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/core/inspector/DOMEditor.h
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/core/inspector/InspectorAnimationAgent.cpp
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/core/inspector/InspectorAnimationAgent.h
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/core/inspector/InspectorApplicationCacheAgent.cpp
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/core/inspector/InspectorApplicationCacheAgent.h
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/core/inspector/InspectorBaseAgent.h
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/core/inspector/InspectorCSSAgent.cpp
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/core/inspector/InspectorCSSAgent.h
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/core/inspector/InspectorDOMAgent.h
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/core/inspector/InspectorDOMDebuggerAgent.cpp
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/core/inspector/InspectorDOMDebuggerAgent.h
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/core/inspector/InspectorInputAgent.cpp
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/core/inspector/InspectorInputAgent.h
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/core/inspector/InspectorLayerTreeAgent.cpp
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/core/inspector/InspectorLayerTreeAgent.h
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/core/inspector/InspectorLogAgent.cpp
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/core/inspector/InspectorLogAgent.h
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/core/inspector/InspectorMemoryAgent.cpp
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/core/inspector/InspectorMemoryAgent.h
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/core/inspector/InspectorNetworkAgent.cpp
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/core/inspector/InspectorNetworkAgent.h
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/core/inspector/InspectorPageAgent.cpp
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/core/inspector/InspectorPageAgent.h
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/core/inspector/InspectorTracingAgent.cpp
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/core/inspector/InspectorTracingAgent.h
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/core/inspector/InspectorWorkerAgent.cpp
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/core/inspector/InspectorWorkerAgent.h
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/modules/accessibility/InspectorAccessibilityAgent.cpp
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/modules/accessibility/InspectorAccessibilityAgent.h
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/modules/cachestorage/InspectorCacheStorageAgent.cpp
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/modules/device_orientation/DeviceOrientationInspectorAgent.cpp
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/modules/device_orientation/DeviceOrientationInspectorAgent.h
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/modules/indexeddb/InspectorIndexedDBAgent.cpp
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/modules/indexeddb/InspectorIndexedDBAgent.h
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/modules/storage/InspectorDOMStorageAgent.cpp
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/modules/storage/InspectorDOMStorageAgent.h
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/modules/webdatabase/InspectorDatabaseAgent.cpp
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/modules/webdatabase/InspectorDatabaseAgent.h
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/web/InspectorEmulationAgent.cpp
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/web/InspectorEmulationAgent.h
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/web/InspectorRenderingAgent.cpp
[modify] https://crrev.com/665bde1047fbd03398ec00158218cd56a60f1e35/third_party/WebKit/Source/web/InspectorRenderingAgent.h

Comment 9 by scottmg@chromium.org, Mar 8 2017

Cc: scottmg@chromium.org

Comment 10 by alph@chromium.org, Mar 8 2017

Trying to reland the original patch after pkasting@ moved "using protocol::Response" out of header files. Thanks Peter!

Also added affected trybots to CQ. Will keep an eye on other trybots once it lands.

Comment 11 by pkasting@chromium.org, Mar 8 2017

Status: Fixed (was: Started)

I'm marking this fixed since my patch landed.

Comment 12 by pfeldman@chromium.org, Mar 8 2017

Thanks, Peter.

►

Issue 699477 link

Issue metadata

Windows compile fail: redefinition of blink::protocol::Response

Issue description

Comment 1 by pkasting@chromium.org, Mar 8 2017

Comment 1 Deleted

Comment 2 by dcheng@chromium.org, Mar 8 2017

Comment 2 Deleted

Comment 3 by pkasting@chromium.org, Mar 8 2017

Comment 3 Deleted

Comment 4 by pkasting@chromium.org, Mar 8 2017

Comment 4 Deleted

Comment 5 by thakis@chromium.org, Mar 8 2017

Comment 5 Deleted

Comment 6 by thakis@chromium.org, Mar 8 2017

Comment 6 Deleted

Comment 7 by thakis@chromium.org, Mar 8 2017

Comment 7 Deleted

Comment 8 by bugdroid1@chromium.org, Mar 8 2017

Comment 8 Deleted

Comment 9 by scottmg@chromium.org, Mar 8 2017

Comment 9 Deleted

Comment 10 by alph@chromium.org, Mar 8 2017

Comment 10 Deleted

Comment 11 by pkasting@chromium.org, Mar 8 2017

Comment 11 Deleted

Comment 12 by pfeldman@chromium.org, Mar 8 2017

Comment 12 Deleted

Sign in to add a comment