diff --git a/docs/advanced/smart_ptrs.rst b/docs/advanced/smart_ptrs.rst index 3c982136c..e4a238603 100644 --- a/docs/advanced/smart_ptrs.rst +++ b/docs/advanced/smart_ptrs.rst @@ -149,6 +149,27 @@ situation where ``true`` should be passed is when the ``T`` instances use Please take a look at the :ref:`macro_notes` before using this feature. +By default, pybind11 assumes that your custom smart pointer has a standard +interface, i.e. provides a ``.get()`` member function to access the underlying +raw pointer. If this is not the case, pybind11's ``holder_helper`` must be +specialized: + +.. code-block:: cpp + + // Always needed for custom holder types + PYBIND11_DECLARE_HOLDER_TYPE(T, SmartPtr); + + // Only needed if the type's `.get()` goes by another name + namespace pybind11 { namespace detail { + template + struct holder_helper> { // <-- specialization + static const T *get(const SmartPtr &p) { return p.getPointer(); } + }; + }} + +The above specialization informs pybind11 that the custom ``SmartPtr`` class +provides ``.get()`` functionality via ``.getPointer()``. + .. seealso:: The file :file:`tests/test_smart_ptr.cpp` contains a complete example diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 97a366014..c9312de79 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -402,6 +402,13 @@ public: make_copy_constructor(src), make_move_constructor(src)); } + static handle cast_holder(const itype *src, const void *holder) { + return type_caster_generic::cast( + src, return_value_policy::take_ownership, {}, + src ? &typeid(*src) : nullptr, &typeid(type), + nullptr, nullptr, holder); + } + template using cast_op_type = pybind11::detail::cast_op_type; operator itype*() { return (type *) value; } @@ -636,17 +643,6 @@ protected: bool success = false; }; -template class type_caster> { -public: - static handle cast(std::unique_ptr &&src, return_value_policy policy, handle parent) { - handle result = type_caster_base::cast(src.get(), policy, parent); - if (result) - src.release(); - return result; - } - static PYBIND11_DESCR name() { return type_caster_base::name(); } -}; - template <> class type_caster { public: bool load(handle src, bool) { @@ -837,8 +833,16 @@ protected: std::tuple...> value; }; +/// Helper class which abstracts away certain actions. Users can provide specializations for +/// custom holders, but it's only necessary if the type has a non-standard interface. +template +struct holder_helper { + static auto get(const T &p) -> decltype(p.get()) { return p.get(); } +}; + /// Type caster for holder types like std::shared_ptr, etc. -template class type_caster_holder : public type_caster_base { +template +struct copyable_holder_caster : public type_caster_base { public: using base = type_caster_base; using base::base; @@ -917,7 +921,7 @@ public: template ::value, int> = 0> bool try_implicit_casts(handle src, bool convert) { for (auto &cast : typeinfo->implicit_casts) { - type_caster_holder sub_caster(*cast.first); + copyable_holder_caster sub_caster(*cast.first); if (sub_caster.load(src, convert)) { value = cast.second(sub_caster.value); holder = holder_type(sub_caster.holder, (type *) value); @@ -940,10 +944,8 @@ public: #endif static handle cast(const holder_type &src, return_value_policy, handle) { - return type_caster_generic::cast( - src.get(), return_value_policy::take_ownership, handle(), - src.get() ? &typeid(*src.get()) : nullptr, &typeid(type), - nullptr, nullptr, &src); + const auto *ptr = holder_helper::get(src); + return type_caster_base::cast_holder(ptr, &src); } protected: @@ -952,7 +954,25 @@ protected: /// Specialize for the common std::shared_ptr, so users don't need to template -class type_caster> : public type_caster_holder> { }; +class type_caster> : public copyable_holder_caster> { }; + +template +struct move_only_holder_caster { + static handle cast(holder_type &&src, return_value_policy, handle) { + auto *ptr = holder_helper::get(src); + return type_caster_base::cast_holder(ptr, &src); + } + static PYBIND11_DESCR name() { return type_caster_base::name(); } +}; + +template +class type_caster> + : public move_only_holder_caster> { }; + +template +using type_caster_holder = conditional_t::value, + copyable_holder_caster, + move_only_holder_caster>; template struct always_construct_holder { static constexpr bool value = Value; }; diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 81a3d1e73..420e18e3c 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1212,24 +1212,22 @@ private: } } - /// Initialize holder object, variant 2: try to construct from existing holder object, if possible - template ::value, int> = 0> - static void init_holder_helper(instance_type *inst, const holder_type *holder_ptr, const void * /* dummy */) { - if (holder_ptr) { - new (&inst->holder) holder_type(*holder_ptr); - inst->holder_constructed = true; - } else if (inst->owned || detail::always_construct_holder::value) { - new (&inst->holder) holder_type(inst->value); - inst->holder_constructed = true; - } + static void init_holder_from_existing(instance_type *inst, const holder_type *holder_ptr, + std::true_type /*is_copy_constructible*/) { + new (&inst->holder) holder_type(*holder_ptr); } - /// Initialize holder object, variant 3: holder is not copy constructible (e.g. unique_ptr), always initialize from raw pointer - template ::value, int> = 0> - static void init_holder_helper(instance_type *inst, const holder_type * /* unused */, const void * /* dummy */) { - if (inst->owned || detail::always_construct_holder::value) { + static void init_holder_from_existing(instance_type *inst, const holder_type *holder_ptr, + std::false_type /*is_copy_constructible*/) { + new (&inst->holder) holder_type(std::move(*const_cast(holder_ptr))); + } + + /// Initialize holder object, variant 2: try to construct from existing holder object, if possible + static void init_holder_helper(instance_type *inst, const holder_type *holder_ptr, const void * /* dummy */) { + if (holder_ptr) { + init_holder_from_existing(inst, holder_ptr, std::is_copy_constructible()); + inst->holder_constructed = true; + } else if (inst->owned || detail::always_construct_holder::value) { new (&inst->holder) holder_type(inst->value); inst->holder_constructed = true; } diff --git a/tests/object.h b/tests/object.h index 753f654b2..9235f19c2 100644 --- a/tests/object.h +++ b/tests/object.h @@ -164,10 +164,10 @@ public: operator T* () { return m_ptr; } /// Return a const pointer to the referenced object - T* get() { return m_ptr; } + T* get_ptr() { return m_ptr; } /// Return a pointer to the referenced object - const T* get() const { return m_ptr; } + const T* get_ptr() const { return m_ptr; } private: T *m_ptr; }; diff --git a/tests/test_smart_ptr.cpp b/tests/test_smart_ptr.cpp index 4d1e77e32..ca28dac6b 100644 --- a/tests/test_smart_ptr.cpp +++ b/tests/test_smart_ptr.cpp @@ -90,6 +90,14 @@ PYBIND11_DECLARE_HOLDER_TYPE(T, ref, true); PYBIND11_DECLARE_HOLDER_TYPE(T, std::shared_ptr); // Not required any more for std::shared_ptr, // but it should compile without error +// Make pybind11 aware of the non-standard getter member function +namespace pybind11 { namespace detail { + template + struct holder_helper> { + static const T *get(const ref &p) { return p.get_ptr(); } + }; +}} + Object *make_object_1() { return new MyObject1(1); } ref make_object_2() { return new MyObject1(2); } @@ -206,6 +214,18 @@ struct SharedFromThisRef { std::shared_ptr shared = std::make_shared(); }; +template +class CustomUniquePtr { + std::unique_ptr impl; + +public: + CustomUniquePtr(T* p) : impl(p) { } + T* get() const { return impl.get(); } + T* release_ptr() { return impl.release(); } +}; + +PYBIND11_DECLARE_HOLDER_TYPE(T, CustomUniquePtr); + test_initializer smart_ptr_and_references([](py::module &pm) { auto m = pm.def_submodule("smart_ptr"); @@ -237,4 +257,12 @@ test_initializer smart_ptr_and_references([](py::module &pm) { py::return_value_policy::copy) .def("set_ref", [](SharedFromThisRef &, const B &) { return true; }) .def("set_holder", [](SharedFromThisRef &, std::shared_ptr) { return true; }); + + struct C { + C() { print_created(this); } + ~C() { print_destroyed(this); } + }; + + py::class_>(m, "TypeWithMoveOnlyHolder") + .def_static("make", []() { return CustomUniquePtr(new C); }); }); diff --git a/tests/test_smart_ptr.py b/tests/test_smart_ptr.py index a6867b485..6a617f753 100644 --- a/tests/test_smart_ptr.py +++ b/tests/test_smart_ptr.py @@ -201,3 +201,13 @@ def test_shared_ptr_from_this_and_references(): del ref, bad_wp, copy, holder_ref, holder_copy, s assert stats.alive() == 0 + + +def test_move_only_holder(): + from pybind11_tests.smart_ptr import TypeWithMoveOnlyHolder + + a = TypeWithMoveOnlyHolder.make() + stats = ConstructorStats.get(TypeWithMoveOnlyHolder) + assert stats.alive() == 1 + del a + assert stats.alive() == 0