Botond Ballo [:botond] Reporter
	Description • 8 years ago

std::unique_ptr has a feature where a custom deleter can customize the unique_ptr's pointer type (that is, the typedef 'unique_ptr<T, D>::pointer' which the constructos of unique_ptr<T, D> take as their first argument), by defining a typedef named 'pointer' inside a deleter.

(This is specified in [unique.ptr.single] p3.)

mozilla::UniquePtr does not support this, it instead makes 'UniquePtr<T, D>::pointer' unconditionally be 'T*'.

This causes problems when we substitute mozilla::UniquePtr for std::unique_ptr in code that expects all the features of std::unique_ptr, including this one.

	(no longer active)
	Comment 1 • 8 years ago

I'm currently hitting this issue caused by skia's #define'ing unique_ptr to UniquePtr:

 0:16.01 In file included from /Users/ehsan/moz/src/obj-ff-clang-plugin.noindex/gfx/layers/Unified_cpp_gfx_layers3.cpp:11:
 0:16.01 In file included from /Users/ehsan/moz/src/gfx/layers/basic/BasicLayersImpl.cpp:13:
 0:16.01 In file included from /Users/ehsan/moz/src/obj-ff-clang-plugin.noindex/dist/include/mozilla/layers/ISurfaceAllocator.h:17:
 0:16.01 In file included from /Users/ehsan/moz/src/obj-ff-clang-plugin.noindex/ipc/ipdl/_ipdlheaders/mozilla/layers/LayersMessages.h:16:
 0:16.01 In file included from /Users/ehsan/moz/src/obj-ff-clang-plugin.noindex/dist/include/mozilla/ipc/ProtocolUtils.h:21:
 0:16.01 In file included from /Users/ehsan/moz/src/obj-ff-clang-plugin.noindex/dist/include/mozilla/ipc/Transport.h:12:
 0:16.01 In file included from /Users/ehsan/moz/src/ipc/chromium/src/chrome/common/ipc_channel.h:10:
 0:16.01 In file included from /Users/ehsan/Downloads/clang+llvm-3.7.0-x86_64-apple-darwin/bin/../include/c++/v1/queue:169:
 0:16.01 /Users/ehsan/Downloads/clang+llvm-3.7.0-x86_64-apple-darwin/bin/../include/c++/v1/deque:2415:59: error: no matching constructor for initialization of 'UniquePtr<typename __alloc_traits::pointer, _Dp>'
 0:16.01         unique_ptr<typename __alloc_traits::pointer, _Dp> __hold(
 0:16.01                                                           ^
 0:16.01 /Users/ehsan/Downloads/clang+llvm-3.7.0-x86_64-apple-darwin/bin/../include/c++/v1/deque:1768:9: note: in instantiation of member function 'std::__1::deque<mozilla::gl::GLContext::LocalErrorScope *, std::__1::allocator<mozilla::gl::GLContext::LocalErrorScope *> >::__add_back_capacity' requested here
 0:16.01         __add_back_capacity();
 0:16.01         ^
 0:16.01 /Users/ehsan/Downloads/clang+llvm-3.7.0-x86_64-apple-darwin/bin/../include/c++/v1/stack:197:36: note: in instantiation of member function 'std::__1::deque<mozilla::gl::GLContext::LocalErrorScope *, std::__1::allocator<mozilla::gl::GLContext::LocalErrorScope *> >::push_back' requested here
 0:16.01     void push(value_type&& __v) {c.push_back(_VSTD::move(__v));}
 0:16.01                                    ^
 0:16.02 /Users/ehsan/moz/src/obj-ff-clang-plugin.noindex/dist/include/GLContext.h:647:39: note: in instantiation of member function 'std::__1::stack<mozilla::gl::GLContext::LocalErrorScope *, std::__1::deque<mozilla::gl::GLContext::LocalErrorScope *, std::__1::allocator<mozilla::gl::GLContext::LocalErrorScope *> > >::push' requested here
 0:16.02             mGL.mLocalErrorScopeStack.push(this);
 0:16.08                                       ^
 0:16.08 /Users/ehsan/moz/src/gfx/layers/../../mfbt/UniquePtr.h:220:3: note: candidate constructor not viable: no known conversion from 'pointer' (aka 'mozilla::gl::GLContext::LocalErrorScope **') to 'Pointer' (aka 'mozilla::gl::GLContext::LocalErrorScope ***') for 1st argument
 0:16.08   UniquePtr(Pointer aPtr,
 0:16.08   ^
 0:16.08 /Users/ehsan/moz/src/gfx/layers/../../mfbt/UniquePtr.h:191:3: note: candidate constructor not viable: no known conversion from 'pointer' (aka 'mozilla::gl::GLContext::LocalErrorScope **') to 'Pointer' (aka 'mozilla::gl::GLContext::LocalErrorScope ***') for 1st argument
 0:16.08   UniquePtr(Pointer aPtr,
 0:16.08   ^
 0:16.08 /Users/ehsan/moz/src/gfx/layers/../../mfbt/UniquePtr.h:242:3: note: candidate template ignored: could not match 'UniquePtr<type-parameter-0-0, type-parameter-0-1>' against 'mozilla::gl::GLContext::LocalErrorScope **'
 0:16.08   UniquePtr(UniquePtr<U, E>&& aOther,
 0:16.08   ^
 0:16.08 /Users/ehsan/moz/src/gfx/layers/../../mfbt/UniquePtr.h:184:12: note: candidate constructor not viable: requires single argument 'aPtr', but 2 arguments were provided
 0:16.08   explicit UniquePtr(Pointer aPtr)
 0:16.08            ^
 0:16.08 /Users/ehsan/moz/src/gfx/layers/../../mfbt/UniquePtr.h:228:3: note: candidate constructor not viable: requires single argument 'aOther', but 2 arguments were provided
 0:16.08   UniquePtr(UniquePtr&& aOther)
 0:16.08   ^
 0:16.08 /Users/ehsan/moz/src/gfx/layers/../../mfbt/UniquePtr.h:233:3: note: candidate constructor not viable: requires 1 argument, but 2 were provided
 0:16.08   UniquePtr(decltype(nullptr))
 0:16.08   ^
 0:16.08 /Users/ehsan/moz/src/gfx/layers/../../mfbt/UniquePtr.h:318:3: note: candidate constructor not viable: requires single argument 'aOther', but 2 arguments were provided
 0:16.08   UniquePtr(const UniquePtr& aOther) = delete; // construct using Move()!
 0:16.08   ^
 0:16.09 /Users/ehsan/moz/src/gfx/layers/../../mfbt/UniquePtr.h:174:17: note: candidate constructor not viable: requires 0 arguments, but 2 were provided
 0:16.09   MOZ_CONSTEXPR UniquePtr()
 0:16.09                 ^
 0:18.44 1 error generated.
 0:18.50

	Nathan Froyd [:froydnj]
	Comment 3 • 8 years ago

Comment on attachment 8729669 [details] [diff] [review]
Allow mozilla::UniquePtr's deleter template argument to customize the pointer type

Review of attachment 8729669 [details] [diff] [review]:
-----------------------------------------------------------------

Ah, this is what I was too dense to figure out from lsalzman's earlier compiler errors.

	Pulsebot
	Comment 4 • 8 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/256bbc9c278e

	Ryan VanderMeulen [:RyanVM]
	Comment 5 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/256bbc9c278e