std::vector<std::unique_ptr<>> causing C2280 error on Windows |
|||||
Issue descriptionOS: 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.
,
Oct 2 2017
Thanks for the bug. I'll take a look and reland with fix.
,
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
,
Oct 2 2017
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.
,
Oct 2 2017
,
Oct 2 2017
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?
,
Oct 3 2017
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;
};
,
Oct 3 2017
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?
,
Oct 3 2017
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.
,
Oct 3 2017
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!
,
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
,
Oct 3 2017
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by wfh@chromium.org
, Oct 2 2017Labels: -Pri-3 Pri-1
Owner: xhw...@chromium.org
Status: Assigned (was: Untriaged)