Allow references to objects held by smart pointers

Author: dean0x7d

include/pybind11/cast.h

@@ -899,20 +899,12 @@ template <typename type, typename holder_type> class type_caster_holder : public
 
         if (typeinfo->simple_type) { /* Case 1: no multiple inheritance etc. involved */
             /* Check if we can safely perform a reinterpret-style cast */
-            if (PyType_IsSubtype(tobj, typeinfo->type)) {
-                auto inst = (instance<type, holder_type> *) src.ptr();
-                value = (void *) inst->value;
-                holder = inst->holder;
-                return true;
-            }
+            if (PyType_IsSubtype(tobj, typeinfo->type))
+                return load_value_and_holder(src);
         } else { /* Case 2: multiple inheritance */
             /* Check if we can safely perform a reinterpret-style cast */
-            if (tobj == typeinfo->type) {
-                auto inst = (instance<type, holder_type> *) src.ptr();
-                value = (void *) inst->value;
-                holder = inst->holder;
-                return true;
-            }
+            if (tobj == typeinfo->type)
+                return load_value_and_holder(src);
 
             /* If this is a python class, also check the parents recursively */
             auto const &type_dict = get_internals().registered_types_py;
@@ -941,6 +933,17 @@ template <typename type, typename holder_type> class type_caster_holder : public
         return false;
     }
 
+    bool load_value_and_holder(handle src) {
+        auto inst = (instance<type, holder_type> *) src.ptr();
+        value = (void *) inst->value;
+        if (inst->holder_constructed) {
+            holder = inst->holder;
+            return true;
+        } else {
+            return false;
+        }
+    }
+
     template <typename T = holder_type, detail::enable_if_t<!std::is_constructible<T, const T &, type*>::value, int> = 0>
     bool try_implicit_casts(handle, bool) { return false; }
 

include/pybind11/pybind11.h

@@ -1138,21 +1138,26 @@ class class_ : public detail::generic_type {
     static void init_holder_helper(instance_type *inst, const holder_type * /* unused */, const std::enable_shared_from_this<T> * /* dummy */) {
         try {
             new (&inst->holder) holder_type(std::static_pointer_cast<typename holder_type::element_type>(inst->value->shared_from_this()));
+            inst->holder_constructed = true;
         } catch (const std::bad_weak_ptr &) {
-            new (&inst->holder) holder_type(inst->value);
+            if (inst->owned) {
+                new (&inst->holder) holder_type(inst->value);
+                inst->holder_constructed = true;
+            }
         }
-        inst->holder_constructed = true;
     }
 
     /// Initialize holder object, variant 2: try to construct from existing holder object, if possible
     template <typename T = holder_type,
               detail::enable_if_t<std::is_copy_constructible<T>::value, int> = 0>
     static void init_holder_helper(instance_type *inst, const holder_type *holder_ptr, const void * /* dummy */) {
-        if (holder_ptr)
+        if (holder_ptr) {
             new (&inst->holder) holder_type(*holder_ptr);
-        else
+            inst->holder_constructed = true;
+        } else if (inst->owned) {
             new (&inst->holder) holder_type(inst->value);
-        inst->holder_constructed = true;
+            inst->holder_constructed = true;
+        }
     }
 
     /// Initialize holder object, variant 3: holder is not copy constructible (e.g. unique_ptr), always initialize from raw pointer

tests/test_smart_ptr.cpp

@@ -165,3 +165,45 @@ test_initializer smart_ptr([](py::module &m) {
     // Expose constructor stats for the ref type
     m.def("cstats_ref", &ConstructorStats::get<ref_tag>);
 });
+
+struct SmartPtrs {
+    struct Shared {
+        Shared() { print_created(this); }
+        ~Shared() { print_destroyed(this); }
+    };
+
+    struct SharedFromThis : std::enable_shared_from_this<SharedFromThis> {
+        SharedFromThis() { print_created(this); }
+        ~SharedFromThis() { print_destroyed(this); }
+    };
+
+    std::shared_ptr<Shared> shared = std::make_shared<Shared>();
+    std::shared_ptr<SharedFromThis> from_this = std::make_shared<SharedFromThis>();
+    SharedFromThis bad_wp = {};
+};
+
+test_initializer smart_ptr_and_references([](py::module &pm) {
+    auto m = pm.def_submodule("smart_ptr");
+
+    using Shared = SmartPtrs::Shared;
+    using SharedFromThis = SmartPtrs::SharedFromThis;
+    py::class_<Shared, std::shared_ptr<Shared>>(m, "Shared");
+    py::class_<SharedFromThis, std::shared_ptr<SharedFromThis>>(m, "SharedFromThis");
+
+    py::class_<SmartPtrs>(m, "SmartPtrs")
+        .def(py::init<>())
+        .def_property("shared_ref",
+                      [](const SmartPtrs &s) -> const Shared & { return *s.shared; },
+                      [](SmartPtrs &, const Shared &) { })
+        .def_property("shared_holder",
+                      [](const SmartPtrs &) { return std::make_shared<Shared>(); },
+                      [](SmartPtrs &, std::shared_ptr<Shared>) { })
+        .def_property("from_this_ref",
+                      [](const SmartPtrs &s) -> const SharedFromThis & { return *s.from_this; },
+                      [](SmartPtrs &, const SharedFromThis &) { })
+        .def_property("from_this_holder",
+                      [](const SmartPtrs &) { return std::make_shared<SharedFromThis>(); },
+                      [](SmartPtrs &, std::shared_ptr<SharedFromThis>) { })
+        .def_property_readonly("from_this_bad_wp",
+                               [](const SmartPtrs &s) -> const SharedFromThis & { return s.bad_wp; });
+});

tests/test_smart_ptr.py

@@ -1,3 +1,4 @@
+import pytest
 from pybind11_tests import ConstructorStats
 
 
@@ -124,3 +125,49 @@ def test_unique_nodelete():
     del o
     cstats = ConstructorStats.get(MyObject4)
     assert cstats.alive() == 1  # Leak, but that's intentional
+
+
+def test_smart_ptr_and_references():
+    from pybind11_tests.smart_ptr import SmartPtrs, Shared, SharedFromThis
+
+    s = SmartPtrs()
+    shared_stats = ConstructorStats.get(Shared)
+    from_this_stats = ConstructorStats.get(SharedFromThis)
+    assert shared_stats.alive() == 1
+    assert from_this_stats.alive() == 2
+
+    # std::shared_ptr
+    ref = s.shared_ref
+    s.shared_ref = ref
+    with pytest.raises(TypeError):
+        s.shared_holder = ref
+
+    holder = s.shared_holder
+    s.shared_ref = holder
+    s.shared_holder = holder
+
+    assert shared_stats.alive() == 2
+    del ref, holder  # reference is not owned
+    assert shared_stats.alive() == 1
+
+    # std::enable_shared_form_this
+    ref = s.from_this_ref
+    s.from_this_ref = ref
+    s.from_this_holder = ref
+
+    holder = s.from_this_holder
+    s.from_this_ref = holder
+    s.from_this_holder = holder
+
+    bad_wp = s.from_this_bad_wp
+    s.from_this_ref = bad_wp
+    with pytest.raises(TypeError):
+        s.from_this_holder = bad_wp
+
+    assert from_this_stats.alive() == 3
+    del ref, holder, bad_wp  # reference and bad_weak_ptr are not owned
+    assert from_this_stats.alive() == 2
+
+    del s
+    assert shared_stats.alive() == 0
+    assert from_this_stats.alive() == 0