Enabling casting to unique_ptr<T>.

Author: EricCousineau-TRI

docs/advanced/smart_ptrs.rst

@@ -29,18 +29,31 @@ instances wrapped in C++11 unique pointers, like so
 
     m.def("create_example", &create_example);
 
-In other words, there is nothing special that needs to be done. While returning
-unique pointers in this way is allowed, it is *illegal* to use them as function
-arguments. For instance, the following function signature cannot be processed
-by pybind11.
+In other words, there is nothing special that needs to be done. Also note that
+you may use ``std::unique_ptr`` as an argument to a function (or as a type in
+``py::move`` / ``py::cast``):
 
 .. code-block:: cpp
 
     void do_something_with_example(std::unique_ptr<Example> ex) { ... }
 
-The above signature would imply that Python needs to give up ownership of an
-object that is passed to this function, which is generally not possible (for
-instance, the object might be referenced elsewhere).
+When a pybind object is passed to this function signature, please note that
+pybind will no longer have ownership of this object (meaning C++ may destroy
+the object while there are still existing Python references). Care must be
+taken, the same as what is done for bare pointers.
+
+In the above function, note that the lifetime of this object is *terminal*,
+meaning that Python should *not* refer to the object after the function is done
+calling. You *may* return ownership back to pybind by casting the object, as so:
+
+.. code-block:: cpp
+
+    void do_something_with_example(std::unique_ptr<Example> ex) {
+        // ... operations...
+        py::cast(std::move(ex));  // This gives pybind back ownership.
+    }
+
+If this is done, then you may continue referencing the object in Python.
 
 std::shared_ptr
 ===============

include/pybind11/attr.h

@@ -250,6 +250,9 @@ struct type_record {
     /// Is the class definition local to the module shared object?
     bool module_local : 1;
 
+    /* See `type_info::holder_info_t` for more information.) */
+    type_info::holder_info_t holder_info;
+
     PYBIND11_NOINLINE void add_base(const std::type_info &base, void *(*caster)(void *)) {
         auto base_info = detail::get_type_info(base, false);
         if (!base_info) {

include/pybind11/cast.h

@@ -479,6 +479,20 @@ inline PyThreadState *get_thread_state_unchecked() {
 inline void keep_alive_impl(handle nurse, handle patient);
 inline PyObject *make_new_instance(PyTypeObject *type);
 
+inline bool reclaim_existing_if_needed(
+        instance *inst, const detail::type_info *tinfo, const void *existing_holder) {
+    // Only reclaim if (a) we have an existing holder and (b) if it's a move-only holder.
+    // TODO: Remove `default_holder`, store more descriptive holder information.
+    if (existing_holder && tinfo->default_holder) {
+        // Requesting reclaim from C++.
+        value_and_holder v_h = inst->get_value_and_holder(tinfo);
+        // TODO(eric.cousineau): Add `holder_type_erased` to avoid need for `const_cast`.
+        tinfo->holder_info.reclaim(v_h, const_cast<void*>(existing_holder));
+        return true;
+    }
+    return false;
+}
+
 class type_caster_generic {
 public:
     PYBIND11_NOINLINE type_caster_generic(const std::type_info &type_info)
@@ -506,8 +520,12 @@ class type_caster_generic {
         auto it_instances = get_internals().registered_instances.equal_range(src);
         for (auto it_i = it_instances.first; it_i != it_instances.second; ++it_i) {
             for (auto instance_type : detail::all_type_info(Py_TYPE(it_i->second))) {
-                if (instance_type && same_type(*instance_type->cpptype, *tinfo->cpptype))
-                    return handle((PyObject *) it_i->second).inc_ref();
+                if (instance_type && same_type(*instance_type->cpptype, *tinfo->cpptype)) {
+                    // Casting for an already registered type. Return existing reference.
+                    instance *inst = it_i->second;
+                    reclaim_existing_if_needed(inst, tinfo, existing_holder);
+                    return handle((PyObject *) inst).inc_ref();
+                }
             }
         }
 
@@ -1373,6 +1391,16 @@ struct holder_helper {
     static auto get(const T &p) -> decltype(p.get()) { return p.get(); }
 };
 
+template <typename holder_type>
+cast_error cast_error_holder_unheld() {
+    return cast_error("Unable to cast from non-held to held instance (T& to Holder<T>) "
+#if defined(NDEBUG)
+                     "(compile in debug mode for type information)");
+#else
+                     "of type '" + type_id<holder_type>() + "''");
+#endif
+}
+
 /// Type caster for holder types like std::shared_ptr, etc.
 template <typename type, typename holder_type>
 struct copyable_holder_caster : public type_caster_base<type> {
@@ -1419,12 +1447,7 @@ struct copyable_holder_caster : public type_caster_base<type> {
             holder = v_h.template holder<holder_type>();
             return true;
         } else {
-            throw cast_error("Unable to cast from non-held to held instance (T& to Holder<T>) "
-#if defined(NDEBUG)
-                             "(compile in debug mode for type information)");
-#else
-                             "of type '" + type_id<holder_type>() + "''");
-#endif
+            throw cast_error_holder_unheld<holder_type>();
         }
     }
 
@@ -1446,7 +1469,7 @@ struct copyable_holder_caster : public type_caster_base<type> {
 
     static bool try_direct_conversions(handle) { return false; }
 
-
+private:
     holder_type holder;
 };
 
@@ -1455,24 +1478,56 @@ template <typename T>
 class type_caster<std::shared_ptr<T>> : public copyable_holder_caster<T, std::shared_ptr<T>> { };
 
 template <typename type, typename holder_type>
-struct move_only_holder_caster {
-    static_assert(std::is_base_of<type_caster_base<type>, type_caster<type>>::value,
+struct move_only_holder_caster : type_caster_base<type> {
+    using base = type_caster_base<type>;
+    static_assert(std::is_base_of<base, type_caster<type>>::value,
             "Holder classes are only supported for custom types");
+    using base::base;
+    using base::cast;
+    using base::typeinfo;
+    using base::value;
 
-    static handle cast(holder_type &&src, return_value_policy, handle) {
-        auto *ptr = holder_helper<holder_type>::get(src);
-        return type_caster_base<type>::cast_holder(ptr, &src);
+    bool load(handle src, bool convert) {
+        return base::template load_impl<move_only_holder_caster<type, holder_type>>(src, convert);
     }
 
     // Force rvalue.
     template <typename T>
     using cast_op_type = holder_type&&;
 
     operator holder_type&&() {
-        throw std::runtime_error("Currently unsupported");
+        return std::move(holder);
+    }
+
+    static handle cast(holder_type &&src, return_value_policy, handle) {
+        auto *ptr = holder_helper<holder_type>::get(src);
+        handle h = type_caster_base<type>::cast_holder(ptr, &src);
+        assert(src.get() == nullptr);
+        return h;
+    }
+
+protected:
+    friend class type_caster_generic;
+    void check_holder_compat() {}
+
+    bool load_value(value_and_holder &&v_h) {
+        if (v_h.holder_constructed()) {
+            // Do NOT use `v_h.type`.
+            typeinfo->holder_info.release(v_h, &holder);
+            assert(v_h.holder<holder_type>().get() == nullptr);
+            return true;
+        } else {
+            throw cast_error_holder_unheld<holder_type>();
+        }
     }
 
-    static constexpr auto name = type_caster_base<type>::name;
+    // TODO(eric.cousineau): Resolve this.
+    bool try_implicit_casts(handle, bool) { return false; }
+
+    static bool try_direct_conversions(handle) { return false; }
+
+private:
+    holder_type holder;
 };
 
 template <typename type, typename deleter>
@@ -1630,6 +1685,26 @@ object cast(const T &value, return_value_policy policy = return_value_policy::au
 template <typename T> T handle::cast() const { return pybind11::cast<T>(*this); }
 template <> inline void handle::cast() const { return; }
 
+template <typename T>
+detail::enable_if_t<
+        // TODO(eric.cousineau): Figure out how to prevent perfect-forwarding more elegantly.
+        std::is_rvalue_reference<T&&>::value && !detail::is_pyobject<detail::intrinsic_t<T>>::value, object>
+    move(T&& value) {
+    // TODO(eric.cousineau): Add policies, parent, etc.
+    // It'd be nice to supply a parent, but for now, just leave it as-is.
+    handle no_parent;
+    return reinterpret_steal<object>(
+        detail::make_caster<T>::cast(std::move(value), return_value_policy::take_ownership, no_parent));
+}
+
+template <typename T>
+detail::enable_if_t<
+        std::is_rvalue_reference<T&&>::value && !detail::is_pyobject<detail::intrinsic_t<T>>::value, object>
+    cast(T&& value) {
+    // Have to use `pybind11::move` because some compilers might try to bind `move` to `std::move`...
+    return pybind11::move<T>(std::move(value));
+}
+
 template <typename T>
 detail::enable_if_t<!detail::move_never<T>::value, T> move(object &&obj) {
     if (obj.ref_count() > 1)
@@ -1642,7 +1717,7 @@ detail::enable_if_t<!detail::move_never<T>::value, T> move(object &&obj) {
 #endif
 
     // Move into a temporary and return that, because the reference may be a local value of `conv`
-    T ret = std::move(detail::load_type<T>(obj).operator T&());
+    T ret = std::move(detail::cast_op<T>(detail::load_type<T>(obj)));
     return ret;
 }
 

include/pybind11/detail/internals.h

@@ -108,6 +108,15 @@ struct type_info {
     bool default_holder : 1;
     /* true if this is a type registered with py::module_local */
     bool module_local : 1;
+    /* Holder information. (For now, just release. Eventually, type and reclaim.) */
+    struct holder_info_t {
+        typedef void (*transfer_t)(detail::value_and_holder& v_h, void* existing_holder_raw);
+        // Release an instance to C++.
+        transfer_t release = nullptr;
+        // Reclaim an instance from C++.
+        transfer_t reclaim = nullptr;
+    };
+    holder_info_t holder_info;
 };
 
 /// Tracks the `internals` and `type_info` ABI version independent of the main library version

include/pybind11/pybind11.h

@@ -899,6 +899,7 @@ class generic_type : public object {
         tinfo->simple_ancestors = true;
         tinfo->default_holder = rec.default_holder;
         tinfo->module_local = rec.module_local;
+        tinfo->holder_info = rec.holder_info;
 
         auto &internals = get_internals();
         auto tindex = std::type_index(*rec.type);
@@ -1053,7 +1054,8 @@ class class_ : public detail::generic_type {
         record.init_instance = init_instance;
         record.dealloc = dealloc;
         record.default_holder = std::is_same<holder_type, std::unique_ptr<type>>::value;
-
+        record.holder_info.release = holder_release;
+        record.holder_info.reclaim = holder_reclaim;
         set_operator_new<type>(&record);
 
         /* Register base classes specified via template arguments to class_, if any */
@@ -1070,6 +1072,28 @@ class class_ : public detail::generic_type {
         }
     }
 
+    static void holder_release(detail::value_and_holder& v_h, void* external_holder_raw) {
+        // Release from `v_h.holder<...>()` into `external_holder`.
+        assert(v_h.inst->owned && v_h.holder_constructed() && "Internal error: Object must be owned");
+        assert(external_holder_raw && "Internal error: External holder must not be null");
+        holder_type& holder = v_h.holder<holder_type>();
+        holder_type& external_holder = *reinterpret_cast<holder_type*>(external_holder_raw);
+        external_holder = std::move(holder);
+        holder.~holder_type();
+        v_h.set_holder_constructed(false);
+        v_h.inst->owned = false;
+    }
+
+    static void holder_reclaim(detail::value_and_holder& v_h, void* external_holder_raw) {
+        // Reclaim from `external_holder` into `v_h.holder<...>()`.
+        assert(!v_h.inst->owned && !v_h.holder_constructed() && "Internal error: Object must not be owned");
+        assert(external_holder_raw && "Internal error: External holder must not be null");
+        holder_type& external_holder = *reinterpret_cast<holder_type*>(external_holder_raw);
+        new (&v_h.holder<holder_type>()) holder_type(std::move(external_holder));
+        v_h.set_holder_constructed();
+        v_h.inst->owned = true;
+    }
+
     template <typename Base, detail::enable_if_t<is_base<Base>::value, int> = 0>
     static void add_base(detail::type_record &rec) {
         rec.add_base(typeid(Base), [](void *src) -> void * {

tests/test_smart_ptr.cpp

@@ -272,8 +272,23 @@ TEST_SUBMODULE(smart_ptr, m) {
             return list;
         });
 
-    // At present, only used for trait checks below. In the future, will be exposed to pybind.
-    struct UniquePtrHeld {};
+    class UniquePtrHeld {
+    public:
+        UniquePtrHeld() = delete;
+        UniquePtrHeld(const UniquePtrHeld&) = delete;
+        UniquePtrHeld(UniquePtrHeld&&) = delete;
+
+        UniquePtrHeld(int value)
+            : value_(value) {
+            print_created(this, value);
+        }
+        ~UniquePtrHeld() {
+            print_destroyed(this);
+        }
+        int value() const { return value_; }
+    private:
+        int value_{};
+    };
 
     // Check traits in a concise manner.
     static_assert(
@@ -285,4 +300,40 @@ TEST_SUBMODULE(smart_ptr, m) {
     static_assert(
         !py::detail::move_if_unreferenced<std::unique_ptr<UniquePtrHeld>>::value,
         "This trait must be false.");
+
+    py::class_<UniquePtrHeld>(m, "UniquePtrHeld")
+        .def(py::init<int>())
+        .def("value", &UniquePtrHeld::value);
+
+    m.def("unique_ptr_pass_through",
+        [](std::unique_ptr<UniquePtrHeld> obj) {
+            return obj;
+        });
+    m.def("unique_ptr_terminal",
+        [](std::unique_ptr<UniquePtrHeld> obj) {
+            obj.reset();
+            return nullptr;
+        });
+
+    // Guarantee API works as expected.
+    m.def("unique_ptr_pass_through_cast_from_py",
+        [](py::object obj_py) {
+            auto obj =
+                py::cast<std::unique_ptr<UniquePtrHeld>>(std::move(obj_py));
+            return obj;
+        });
+    m.def("unique_ptr_pass_through_move_from_py",
+        [](py::object obj_py) {
+            return py::move<std::unique_ptr<UniquePtrHeld>>(std::move(obj_py));
+        });
+
+    m.def("unique_ptr_pass_through_move_to_py",
+        [](std::unique_ptr<UniquePtrHeld> obj) {
+            return py::move(std::move(obj));
+        });
+
+    m.def("unique_ptr_pass_through_cast_to_py",
+        [](std::unique_ptr<UniquePtrHeld> obj) {
+            return py::cast(std::move(obj));
+        });
 }

tests/test_smart_ptr.py

@@ -218,3 +218,35 @@ def test_shared_ptr_gc():
     pytest.gc_collect()
     for i, v in enumerate(el.get()):
         assert i == v.value()
+
+
+def test_unique_ptr_arg():
+    stats = ConstructorStats.get(m.UniquePtrHeld)
+
+    pass_through_list = [
+        m.unique_ptr_pass_through,
+        m.unique_ptr_pass_through_cast_from_py,
+        m.unique_ptr_pass_through_move_from_py,
+        m.unique_ptr_pass_through_move_to_py,
+        m.unique_ptr_pass_through_cast_to_py,
+    ]
+    for pass_through in pass_through_list:
+        obj = m.UniquePtrHeld(1)
+        obj_ref = m.unique_ptr_pass_through(obj)
+        assert stats.alive() == 1
+        assert obj.value() == 1
+        assert obj == obj_ref
+        del obj
+        del obj_ref
+        pytest.gc_collect()
+        assert stats.alive() == 0
+
+    obj = m.UniquePtrHeld(1)
+    m.unique_ptr_terminal(obj)
+    assert stats.alive() == 0
+
+    m.unique_ptr_terminal(m.UniquePtrHeld(2))
+    assert stats.alive() == 0
+
+    assert m.unique_ptr_pass_through(None) is None
+    m.unique_ptr_terminal(None)