New issue
Advanced search Search tips

Issue 747081 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocking:
issue 757374



Sign in to add a comment

Fix url::CanonOutputT template export

Project Member Reported by mlamouri@chromium.org, Jul 20 2017

Issue description

The template instantiations seems to cause double definitions because of virtual methods. This has been creating some issues to land https://chromium-review.googlesource.com/c/525692/. A quick fix is to EXPORT_TEMPLATE*. A longer fix would be to remove the virtual-ness.
 
This build log (while it lasts) give an idea of the error:
https://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_compile_dbg_ng/builds/459372

Snippet:
```
[8969/11794] LINK(DLL) blink_platform.dll blink_platform.dll.lib blink_platform.dll.pdb
FAILED: blink_platform.dll blink_platform.dll.lib blink_platform.dll.pdb 
E:/b/depot_tools/win_tools-2_7_6_bin/python/bin/python.exe ../../build/toolchain/win/tool_wrapper.py link-wrapper environment.x86 False link.exe /nologo /IMPLIB:./blink_platform.dll.lib /DLL /OUT:./blink_platform.dll /PDB:./blink_platform.dll.pdb @./blink_platform.dll.rsp
url_lib.dll.lib(url_lib.dll) : error LNK2005: "public: __thiscall url::CanonOutputT<char>::CanonOutputT<char>(void)" (??0?$CanonOutputT@D@url@@QAE@XZ) already defined in LinkHash.obj
url_lib.dll.lib(url_lib.dll) : error LNK2005: "public: virtual __thiscall url::CanonOutputT<char>::~CanonOutputT<char>(void)" (??1?$CanonOutputT@D@url@@UAE@XZ) already defined in LinkHash.obj
url_lib.dll.lib(url_lib.dll) : error LNK2005: "public: int __thiscall url::CanonOutputT<char>::length(void)const " (?length@?$CanonOutputT@D@url@@QBEHXZ) already defined in LinkHash.obj
url_lib.dll.lib(url_lib.dll) : error LNK2005: "public: char * __thiscall url::CanonOutputT<char>::data(void)" (?data@?$CanonOutputT@D@url@@QAEPADXZ) already defined in LinkHash.obj
./blink_platform.dll : fatal error LNK1169: one or more multiply defined symbols found
```
Blocking: 757374
Cc: hajimehoshi@chromium.org tasak@google.com
+tasak

#1: IIUC, the errors (warnings) happen that the keyword 'extern' and '__declspec(dllexport)' are exclusive and the only one of them can exist. I was trying to suppress the warnings at https://chromium-review.googlesource.com/c/chromium/src/+/647050, which was reverted due to breaking some bots.

I was wondering how https://chromium-review.googlesource.com/c/chromium/src/+/579211 was going to solve that.
Owner: mlamouri@chromium.org
Status: Assigned (was: Available)
Cc: erikc...@chromium.org
+erikchen

What's going on this?

This has been a blocker on crbug/757374 ...
mlamouri: I think we can move forward with this.

I read through: https://chromium-review.googlesource.com/c/chromium/src/+/579211

My understanding is that thakis@ believes that there's a better way of solving the problem, but doesn't have the time to look. Everyone else who's looked [including dcheng, another base owner], believes this is the best short term solution. 

jam@, a top level owner, has decided it's okay to move forward: https://chromium-review.googlesource.com/c/chromium/src/+/579211#message-4e18287222fd612f8c2e24a9c50b2118a921e385. If at some point thakis@ finds a better solution, we can move forward with that.


Project Member

Comment 7 by bugdroid1@chromium.org, Oct 13 2017

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

commit 130ad75c1b9408a91a8f86c320240e4137f35143
Author: Mounir Lamouri <mlamouri@chromium.org>
Date: Fri Oct 13 17:07:15 2017

Export CanonOutputT template instantiations to avoid double definition.

The double instantiations seem to happen because of virtual methods. If
the class could lose be non-virtual, this could be fixed. However, this
is blocking another CL and this fix is the optimal one with regards to
time constraints.

Bug:  747081 
Change-Id: I5c2d2cc7a9e49266b25d2cd3d9835d0647e5438a
Reviewed-on: https://chromium-review.googlesource.com/579211
Commit-Queue: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508734}
[modify] https://crrev.com/130ad75c1b9408a91a8f86c320240e4137f35143/base/BUILD.gn
[rename] https://crrev.com/130ad75c1b9408a91a8f86c320240e4137f35143/base/export_template.h
[modify] https://crrev.com/130ad75c1b9408a91a8f86c320240e4137f35143/ipc/BUILD.gn
[modify] https://crrev.com/130ad75c1b9408a91a8f86c320240e4137f35143/ipc/ipc_message_macros.h
[modify] https://crrev.com/130ad75c1b9408a91a8f86c320240e4137f35143/url/BUILD.gn
[add] https://crrev.com/130ad75c1b9408a91a8f86c320240e4137f35143/url/url_canon.cc
[modify] https://crrev.com/130ad75c1b9408a91a8f86c320240e4137f35143/url/url_canon.h

Status: Fixed (was: Assigned)

Sign in to add a comment