New issue
Advanced search Search tips

Issue 770771 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

std::vector<std::unique_ptr<>> causing C2280 error on Windows

Project Member Reported by kylixrd@chromium.org, Oct 2 2017

Issue description

OS: Windows

Encountering the following error:

[345/18457] CXX obj/media/cdm/cdm/cdm_host_files.obj
FAILED: obj/media/cdm/cdm/cdm_host_files.obj
c:\goma\goma-win64/gomacc.exe ../../third_party/llvm-build/Release+Asserts/bin/clang-cl.exe /nologo /showIncludes  @obj/media/cdm/cdm/cdm_host_files.obj.rsp /c ../../media/cdm/cdm_host_files.cc /Foobj/media/cdm/cdm/cdm_host_files.obj /Fd"obj/media/cdm/cdm_cc.pdb"
In file included from ../../media/cdm/cdm_host_files.cc:5:
In file included from ../..\media/cdm/cdm_host_files.h:8:
In file included from c:\builds\chromium\src\third_party\depot_tools\win_toolchain\vs_files\9bc7ccbf9f4bd50d4a3bd185e8ca94ff1618de0b\win_sdk\bin\..\..\vc\tools\msvc\14.11.25503\include\map:9:
In file included from c:\builds\chromium\src\third_party\depot_tools\win_toolchain\vs_files\9bc7ccbf9f4bd50d4a3bd185e8ca94ff1618de0b\win_sdk\bin\..\..\vc\tools\msvc\14.11.25503\include\xtree:6:
In file included from c:\builds\chromium\src\third_party\depot_tools\win_toolchain\vs_files\9bc7ccbf9f4bd50d4a3bd185e8ca94ff1618de0b\win_sdk\bin\..\..\vc\tools\msvc\14.11.25503\include\xmemory:6:
c:\builds\chromium\src\third_party\depot_tools\win_toolchain\vs_files\9bc7ccbf9f4bd50d4a3bd185e8ca94ff1618de0b\win_sdk\bin\..\..\vc\tools\msvc\14.11.25503\include\xmemory0(856,4):  error: call to deleted constructor of 'std::unique_ptr<media::CdmHostFile, std::default_delete<media::CdmHostFile> >'
                       _Objty(_STD forward<_Types>(_Args)...);
                       ^      ~~~~~~~~~~~~~~~~~~~~~~~~~~~

Note a similar error happens when using MSVC.
 

Comment 1 by wfh@chromium.org, Oct 2 2017

Cc: thestig@chromium.org
Labels: -Pri-3 Pri-1
Owner: xhw...@chromium.org
Status: Assigned (was: Untriaged)
only happens for is_chrome_branded = true since https://chromium.googlesource.com/chromium/src/+/87abc8da6d088f6e4538673dae529e28b42cf81f so I'll just go ahead and revert this CL.
Status: Started (was: Assigned)
Thanks for the bug. I'll take a look and reland with fix.
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 2 2017

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

commit 38b2f12c1d36f379a00b7bdbb0802b057294b13d
Author: Will Harris <wfh@chromium.org>
Date: Mon Oct 02 18:40:14 2017

Revert "media: Move CdmHostFile(s) from content/ to media/"

This reverts commit 87abc8da6d088f6e4538673dae529e28b42cf81f.

Reason for revert: breaks component builds for is_chrome_branded=true see  crbug.com/770771 

Original change's description:
> media: Move CdmHostFile(s) from content/ to media/
> 
> These classes have very little dependency on content/ and will be needed
> by the CdmModule in media/. Hence, replace the content/ dependency with
> an injected callback, and move these files to media/.
> 
> TBR=thestig@chromium.org
> BUG=730770
> TEST=No functionality change
> 
> Change-Id: I034608ce708f45c1c2fb139fe5b578033a7f7a7a
> Reviewed-on: https://chromium-review.googlesource.com/689858
> Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
> Reviewed-by: John Rummell <jrummell@chromium.org>
> Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#505564}

TBR=thestig@chromium.org,xhwang@chromium.org,jrummell@chromium.org,alexmos@chromium.org

Change-Id: Ifb155b85a995b365d33f9ee3ab6880abd2d56c3d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 730770,  770771 
Reviewed-on: https://chromium-review.googlesource.com/695783
Commit-Queue: Will Harris <wfh@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505695}
[modify] https://crrev.com/38b2f12c1d36f379a00b7bdbb0802b057294b13d/chrome/common/chrome_content_client.cc
[modify] https://crrev.com/38b2f12c1d36f379a00b7bdbb0802b057294b13d/chrome/common/chrome_content_client.h
[modify] https://crrev.com/38b2f12c1d36f379a00b7bdbb0802b057294b13d/chrome/common/media/cdm_host_file_path.cc
[modify] https://crrev.com/38b2f12c1d36f379a00b7bdbb0802b057294b13d/chrome/common/media/cdm_host_file_path.h
[modify] https://crrev.com/38b2f12c1d36f379a00b7bdbb0802b057294b13d/content/common/BUILD.gn
[rename] https://crrev.com/38b2f12c1d36f379a00b7bdbb0802b057294b13d/content/common/media/cdm_host_file.cc
[rename] https://crrev.com/38b2f12c1d36f379a00b7bdbb0802b057294b13d/content/common/media/cdm_host_file.h
[rename] https://crrev.com/38b2f12c1d36f379a00b7bdbb0802b057294b13d/content/common/media/cdm_host_files.cc
[rename] https://crrev.com/38b2f12c1d36f379a00b7bdbb0802b057294b13d/content/common/media/cdm_host_files.h
[modify] https://crrev.com/38b2f12c1d36f379a00b7bdbb0802b057294b13d/content/common/media/cdm_info.cc
[modify] https://crrev.com/38b2f12c1d36f379a00b7bdbb0802b057294b13d/content/ppapi_plugin/ppapi_thread.cc
[modify] https://crrev.com/38b2f12c1d36f379a00b7bdbb0802b057294b13d/content/public/common/cdm_info.h
[modify] https://crrev.com/38b2f12c1d36f379a00b7bdbb0802b057294b13d/content/public/common/content_client.h
[modify] https://crrev.com/38b2f12c1d36f379a00b7bdbb0802b057294b13d/media/cdm/BUILD.gn

It seems to me this is either a compiler bug, or somehow it's related to how __declspec(dllexport) (MEDIA_EXPORT) interferes with the compiler.

Here's a minimal repro case for this compile error:
https://chromium-review.googlesource.com/#/c/chromium/src/+/695713

If I remove the MEDIA_EXPORT part in foo.h, it compiles.

Here's the exact error message that I got using this minimal repro case.

C:\src\chrome\src>ninja -C out\GN -j1000 chrome
ninja: Entering directory `out\GN'
[1/493] CXX obj/media/cdm/cdm/cdm_adapter.obj
FAILED: obj/media/cdm/cdm/cdm_adapter.obj
ninja -t msvc -e environment.x64 -- C:\goma\goma-win64/gomacc.exe "c:\src\chrome\src\third_party\depot_tools\win_toolchain\vs_files\9bc7ccbf9f4bd50d4a3bd185e8ca94ff1618de0b\vc\tools\msvc\14.11.25503\bin\hostx64\x64/cl.exe" /nologo /showIncludes  @obj/media/cdm/cdm/cdm_adapter.obj.rsp /c ../../media/cdm/cdm_adapter.cc /Foobj/media/cdm/cdm/cdm_adapter.obj /Fd"obj/media/cdm/cdm_cc.pdb"
c:\src\chrome\src\third_party\depot_tools\win_toolchain\vs_files\9bc7ccbf9f4bd50d4a3bd185e8ca94ff1618de0b\win_sdk\bin\..\..\vc\tools\msvc\14.11.25503\include\xutility(2254): error C2280: 'std::unique_ptr<int,std::default_delete<_Ty>> &std::unique_ptr<_Ty,std::default_delete<_Ty>>::operator =(const std::unique_ptr<_Ty,std::default_delete<_Ty>> &)': attempting to reference a deleted function
        with
        [
            _Ty=int
        ]
c:\src\chrome\src\third_party\depot_tools\win_toolchain\vs_files\9bc7ccbf9f4bd50d4a3bd185e8ca94ff1618de0b\win_sdk\bin\..\..\vc\tools\msvc\14.11.25503\include\memory(2242): note: see declaration of 'std::unique_ptr<int,std::default_delete<_Ty>>::operator ='
        with
        [
            _Ty=int
        ]
c:\src\chrome\src\third_party\depot_tools\win_toolchain\vs_files\9bc7ccbf9f4bd50d4a3bd185e8ca94ff1618de0b\win_sdk\bin\..\..\vc\tools\msvc\14.11.25503\include\xutility(2273): note: see reference to function template instantiation '_OutIt std::_Copy_unchecked1<_InIt,_OutIt>(_InIt,_InIt,_OutIt,std::_General_ptr_iterator_tag)' being compiled
        with
        [
            _OutIt=std::unique_ptr<int,std::default_delete<int>> *,
            _InIt=std::unique_ptr<int,std::default_delete<int>> *
        ]
c:\src\chrome\src\third_party\depot_tools\win_toolchain\vs_files\9bc7ccbf9f4bd50d4a3bd185e8ca94ff1618de0b\win_sdk\bin\..\..\vc\tools\msvc\14.11.25503\include\vector(1422): note: see reference to function template instantiation '_OutIt *std::_Copy_unchecked<_Iter,std::unique_ptr<int,std::default_delete<_Ty>>*>(_InIt,_InIt,_OutIt)' being compiled
        with
        [
            _OutIt=std::unique_ptr<int,std::default_delete<int>> *,
            _Iter=std::unique_ptr<int,std::default_delete<int>> *,
            _Ty=int,
            _InIt=std::unique_ptr<int,std::default_delete<int>> *
        ]
c:\src\chrome\src\third_party\depot_tools\win_toolchain\vs_files\9bc7ccbf9f4bd50d4a3bd185e8ca94ff1618de0b\win_sdk\bin\..\..\vc\tools\msvc\14.11.25503\include\vector(1440): note: see reference to function template instantiation 'void std::vector<std::unique_ptr<int,std::default_delete<_Ty>>,std::allocator<std::unique_ptr<_Ty,std::default_delete<_Ty>>>>::_Assign_range<_Iter>(_Iter,_Iter,std::forward_iterator_tag)' being compiled
        with
        [
            _Ty=int,
            _Iter=std::unique_ptr<int,std::default_delete<int>> *
        ]
c:\src\chrome\src\third_party\depot_tools\win_toolchain\vs_files\9bc7ccbf9f4bd50d4a3bd185e8ca94ff1618de0b\win_sdk\bin\..\..\vc\tools\msvc\14.11.25503\include\vector(1440): note: see reference to function template instantiation 'void std::vector<std::unique_ptr<int,std::default_delete<_Ty>>,std::allocator<std::unique_ptr<_Ty,std::default_delete<_Ty>>>>::_Assign_range<_Iter>(_Iter,_Iter,std::forward_iterator_tag)' being compiled
        with
        [
            _Ty=int,
            _Iter=std::unique_ptr<int,std::default_delete<int>> *
        ]
c:\src\chrome\src\third_party\depot_tools\win_toolchain\vs_files\9bc7ccbf9f4bd50d4a3bd185e8ca94ff1618de0b\win_sdk\bin\..\..\vc\tools\msvc\14.11.25503\include\vector(1463): note: see reference to function template instantiation 'void std::vector<std::unique_ptr<int,std::default_delete<_Ty>>,std::allocator<std::unique_ptr<_Ty,std::default_delete<_Ty>>>>::assign<std::unique_ptr<_Ty,std::default_delete<_Ty>>*,void>(_Iter,_Iter)' being compiled
        with
        [
            _Ty=int,
            _Iter=std::unique_ptr<int,std::default_delete<int>> *
        ]
c:\src\chrome\src\third_party\depot_tools\win_toolchain\vs_files\9bc7ccbf9f4bd50d4a3bd185e8ca94ff1618de0b\win_sdk\bin\..\..\vc\tools\msvc\14.11.25503\include\vector(1463): note: see reference to function template instantiation 'void std::vector<std::unique_ptr<int,std::default_delete<_Ty>>,std::allocator<std::unique_ptr<_Ty,std::default_delete<_Ty>>>>::assign<std::unique_ptr<_Ty,std::default_delete<_Ty>>*,void>(_Iter,_Iter)' being compiled
        with
        [
            _Ty=int,
            _Iter=std::unique_ptr<int,std::default_delete<int>> *
        ]
c:\src\chrome\src\third_party\depot_tools\win_toolchain\vs_files\9bc7ccbf9f4bd50d4a3bd185e8ca94ff1618de0b\win_sdk\bin\..\..\vc\tools\msvc\14.11.25503\include\vector(1449): note: while compiling class template member function 'std::vector<std::unique_ptr<int,std::default_delete<_Ty>>,std::allocator<std::unique_ptr<_Ty,std::default_delete<_Ty>>>> &std::vector<std::unique_ptr<_Ty,std::default_delete<_Ty>>,std::allocator<std::unique_ptr<_Ty,std::default_delete<_Ty>>>>::operator =(const std::vector<std::unique_ptr<_Ty,std::default_delete<_Ty>>,std::allocator<std::unique_ptr<_Ty,std::default_delete<_Ty>>>> &)'
        with
        [
            _Ty=int
        ]
../..\media/cdm/foo.h(18): note: see reference to function template instantiation 'std::vector<std::unique_ptr<int,std::default_delete<_Ty>>,std::allocator<std::unique_ptr<_Ty,std::default_delete<_Ty>>>> &std::vector<std::unique_ptr<_Ty,std::default_delete<_Ty>>,std::allocator<std::unique_ptr<_Ty,std::default_delete<_Ty>>>>::operator =(const std::vector<std::unique_ptr<_Ty,std::default_delete<_Ty>>,std::allocator<std::unique_ptr<_Ty,std::default_delete<_Ty>>>> &)' being compiled
        with
        [
            _Ty=int
        ]
../..\media/cdm/foo.h(17): note: see reference to class template instantiation 'std::vector<std::unique_ptr<int,std::default_delete<_Ty>>,std::allocator<std::unique_ptr<_Ty,std::default_delete<_Ty>>>>' being compiled
        with
        [
            _Ty=int
        ]
c:\src\chrome\src\third_party\depot_tools\win_toolchain\vs_files\9bc7ccbf9f4bd50d4a3bd185e8ca94ff1618de0b\win_sdk\bin\..\..\vc\tools\msvc\14.11.25503\include\memory(2242): note: 'std::unique_ptr<int,std::default_delete<_Ty>> &std::unique_ptr<_Ty,std::default_delete<_Ty>>::operator =(const std::unique_ptr<_Ty,std::default_delete<_Ty>> &)': function was explicitly deleted
        with
        [
            _Ty=int
        ]
[2/493] ACTION //components/resources:about_credits(//build/toolchain/win:x64)
ninja: build stopped: subcommand failed.
Cc: brucedaw...@chromium.org
Summary: std::vector<std::unique_ptr<>> causing C2280 error on Windows (was: CL 689858 seems to be causing build failures on Windows)
Cc: jrumm...@chromium.org
brucedawson: This is the issue I mentioned to you. Could you please take a look and see whether it's a compiler issue or I am doing something wrong?
Thanks for the minimal repro. I'm pretty sure I understand the issue. You are exporting Foo and the compiler is trying to generate this function in order to complete the export:

    std::vector<std::unique_ptr<>>::operator =

It can't find that. That's because operator= returns a copy of the object and that isn't allowed with unique_ptr or with vector<unique_ptr>. So, you need to explicitly delete the operator= method of the class that you export, and maybe prohibit copy construction as well, although that isn't necessary for this issue.

To put it in a different way, exporting a class requires generating and exporting a full set of member functions, including the default constructors, destructors, and assignment operators. Since one of these functions cannot be generated it gives a delightfully verbose and cryptic error.

Testing shows that you have to delete two functions, but then your minimal repro builds:


diff --git a/media/cdm/foo.h b/media/cdm/foo.h
index 14b25898c5a4..48b60299ffb8 100644
--- a/media/cdm/foo.h
+++ b/media/cdm/foo.h
@@ -13,6 +13,9 @@
 namespace media {

 class MEDIA_EXPORT Foo {
+  Foo& operator=(const Foo&) = delete;
+  Foo(const Foo&) = delete;
+
  private:
   std::vector<std::unique_ptr<int>> i;
 };
Wow, that's great! Thank you soooooo much for the help!

Just OOC, why I don't see this issue on other platforms? Is this required by the C++ standard? Or put it another way, is this a limitation on the Windows compiler?
Exporting of classes is a highly platform dependent feature. I think there is a good case for the Windows behavior being correct, because it would be weird to have those functions not available even though they haven't been deleted.

On the other hand, one could make a case that the importing DLL could generate those functions as inline functions when it needs them, which I'd guess is what Linux does.

No matter. Given that the class cannot be assigned to or copy-constructed it is best to explicitly disallow those options rather than relying on subtle prohibitions. I just wish the error message was more readable.

That totally makes sense. I actually forget to use DISALLOW_COPY_AND_ASSIGN in my class, which caused this problem in the first place. But the fact that the error message is hard to read, and it only happens on Windows, somehow misguided me.

Thank you again for the help!
Project Member

Comment 11 by bugdroid1@chromium.org, Oct 3 2017

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

commit 5591646b86cd3526357b2775b850bcdb2189a5b4
Author: Xiaohan Wang <xhwang@chromium.org>
Date: Tue Oct 03 21:25:45 2017

(reland) media: Move CdmHostFile(s) from content/ to media/

This reverts commit 38b2f12c1d36f379a00b7bdbb0802b057294b13d.

Original CL description:

These classes have very little dependency on content/ and will be needed
by the CdmModule in media/. Hence, replace the content/ dependency with
an injected callback, and move these files to media/.

TBR=thestig@chromium.org,alexmos@chromium.org,jrummell@chromium.org
BUG=730770, 770771 
TEST=No functionality change

Change-Id: I416fd3162245317ceaaf92a73be640f55d8a7054
Reviewed-on: https://chromium-review.googlesource.com/695909
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506179}
[modify] https://crrev.com/5591646b86cd3526357b2775b850bcdb2189a5b4/chrome/common/chrome_content_client.cc
[modify] https://crrev.com/5591646b86cd3526357b2775b850bcdb2189a5b4/chrome/common/chrome_content_client.h
[modify] https://crrev.com/5591646b86cd3526357b2775b850bcdb2189a5b4/chrome/common/media/cdm_host_file_path.cc
[modify] https://crrev.com/5591646b86cd3526357b2775b850bcdb2189a5b4/chrome/common/media/cdm_host_file_path.h
[modify] https://crrev.com/5591646b86cd3526357b2775b850bcdb2189a5b4/content/common/BUILD.gn
[modify] https://crrev.com/5591646b86cd3526357b2775b850bcdb2189a5b4/content/common/media/cdm_info.cc
[modify] https://crrev.com/5591646b86cd3526357b2775b850bcdb2189a5b4/content/ppapi_plugin/ppapi_thread.cc
[modify] https://crrev.com/5591646b86cd3526357b2775b850bcdb2189a5b4/content/public/common/cdm_info.h
[modify] https://crrev.com/5591646b86cd3526357b2775b850bcdb2189a5b4/content/public/common/content_client.h
[modify] https://crrev.com/5591646b86cd3526357b2775b850bcdb2189a5b4/media/cdm/BUILD.gn
[rename] https://crrev.com/5591646b86cd3526357b2775b850bcdb2189a5b4/media/cdm/cdm_host_file.cc
[rename] https://crrev.com/5591646b86cd3526357b2775b850bcdb2189a5b4/media/cdm/cdm_host_file.h
[rename] https://crrev.com/5591646b86cd3526357b2775b850bcdb2189a5b4/media/cdm/cdm_host_files.cc
[rename] https://crrev.com/5591646b86cd3526357b2775b850bcdb2189a5b4/media/cdm/cdm_host_files.h

Status: Fixed (was: Started)

Sign in to add a comment