[libcxx-commits] [PATCH] D119159: [libc++] Make shared_ptr move unique_ptr's deleter

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik requested changes to this revision.
philnik added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/include/__memory/shared_ptr.h:656
+         is_convertible<typename unique_ptr<_Yp, _Dp>::pointer, element_type*>::value &&
+         is_move_constructible<_Dp>::value
     > >
----------------
ashermancinelli wrote:
> Should the requirement be added here, or should we reuse `__shared_ptr_deleter_ctor_reqs`? libstdc++ has a similar alias template used here that sfinaes away without `move_constructible`. That might be more readable, even though the alias template would only be used here iiuc. Also, would it be nicer to use `_And`?
I think it's OK the way it is. Don't use `_And` if there is no good reason.


================
Comment at: libcxx/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.const/unique_ptr.pass.cpp:87
+struct MoveOnlyDeleter {
+  MoveOnlyDeleter() {}
+  MoveOnlyDeleter(MoveOnlyDeleter&&) {}
----------------
Could you `= delete` the default constructor? That ensures the deleter is actually moved properly. Also count how many moves have been made and ensure it's exactly one.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119159/new/

https://reviews.llvm.org/D119159