Merge branch 'smart_holder' into pywrapcc_merge_sh

Author: rwgk

include/pybind11/detail/descr.h

@@ -238,12 +238,26 @@ constexpr descr<N, Ts...> concat(const descr<N, Ts...> &descr) {
     return descr;
 }
 
+#if defined(PYBIND11_CPP17)
+template <size_t N1, size_t N2, typename... Ts1, typename... Ts2>
+constexpr descr<N1 + N2 + 2, Ts1..., Ts2...> operator,(const descr<N1, Ts1...> &a,
+                                                       const descr<N2, Ts2...> &b) {
+    // Ensure that src_loc of existing descr is used.
+    return a + const_name(", ", src_loc{nullptr, 0}) + b;
+}
+
+template <size_t N, typename... Ts, typename... Args>
+constexpr auto concat(const descr<N, Ts...> &d, const Args &...args) {
+    return (d, ..., args);
+}
+#else
 template <size_t N, typename... Ts, typename... Args>
 constexpr auto concat(const descr<N, Ts...> &d, const Args &...args)
     -> decltype(std::declval<descr<N + 2, Ts...>>() + concat(args...)) {
     // Ensure that src_loc of existing descr is used.
     return d + const_name(", ", src_loc{nullptr, 0}) + concat(args...);
 }
+#endif
 
 template <size_t N, typename... Ts>
 constexpr descr<N + 2, Ts...> type_descr(const descr<N, Ts...> &descr) {

include/pybind11/detail/smart_holder_type_casters.h

@@ -408,6 +408,18 @@ struct smart_holder_type_caster_class_hooks : smart_holder_type_caster_base_tag
     }
 };
 
+struct shared_ptr_parent_life_support {
+    PyObject *parent;
+    explicit shared_ptr_parent_life_support(PyObject *parent) : parent{parent} {
+        Py_INCREF(parent);
+    }
+    // NOLINTNEXTLINE(readability-make-member-function-const)
+    void operator()(void *) {
+        gil_scoped_acquire gil;
+        Py_DECREF(parent);
+    }
+};
+
 struct shared_ptr_trampoline_self_life_support {
     PyObject *self;
     explicit shared_ptr_trampoline_self_life_support(instance *inst)
@@ -462,12 +474,23 @@ struct smart_holder_type_caster_load {
         return *raw_ptr;
     }
 
-    std::shared_ptr<T> loaded_as_shared_ptr() const {
+    std::shared_ptr<T> make_shared_ptr_with_responsible_parent(handle parent) const {
+        return std::shared_ptr<T>(loaded_as_raw_ptr_unowned(),
+                                  shared_ptr_parent_life_support(parent.ptr()));
+    }
+
+    std::shared_ptr<T> loaded_as_shared_ptr(handle responsible_parent = nullptr) const {
         if (load_impl.unowned_void_ptr_from_void_ptr_capsule) {
+            if (responsible_parent) {
+                return make_shared_ptr_with_responsible_parent(responsible_parent);
+            }
             throw cast_error("Unowned pointer from a void pointer capsule cannot be converted to a"
                              " std::shared_ptr.");
         }
         if (load_impl.unowned_void_ptr_from_direct_conversion != nullptr) {
+            if (responsible_parent) {
+                return make_shared_ptr_with_responsible_parent(responsible_parent);
+            }
             throw cast_error("Unowned pointer from direct conversion cannot be converted to a"
                              " std::shared_ptr.");
         }
@@ -478,6 +501,9 @@ struct smart_holder_type_caster_load {
         holder_type &hld = holder();
         hld.ensure_is_not_disowned("loaded_as_shared_ptr");
         if (hld.vptr_is_using_noop_deleter) {
+            if (responsible_parent) {
+                return make_shared_ptr_with_responsible_parent(responsible_parent);
+            }
             throw std::runtime_error("Non-owning holder (loaded_as_shared_ptr).");
         }
         auto *void_raw_ptr = hld.template as_raw_ptr_unowned<void>();
@@ -579,6 +605,17 @@ struct smart_holder_type_caster_load {
         return result;
     }
 
+    // This function will succeed even if the `responsible_parent` does not own the
+    // wrapped C++ object directly.
+    // It is the responsibility of the caller to ensure that the `responsible_parent`
+    // has a `keep_alive` relationship with the owner of the wrapped C++ object, or
+    // that the wrapped C++ object lives for the duration of the process.
+    static std::shared_ptr<T> shared_ptr_from_python(handle responsible_parent) {
+        smart_holder_type_caster_load<T> loader;
+        loader.load(responsible_parent, false);
+        return loader.loaded_as_shared_ptr(responsible_parent);
+    }
+
 private:
     modified_type_caster_generic_load_impl load_impl;
 

include/pybind11/pybind11.h

@@ -1594,7 +1594,8 @@ struct property_cpp_function<
     template <typename PM, must_be_member_function_pointer<PM> = 0>
     static cpp_function readonly(PM pm, const handle &hdl) {
         return cpp_function(
-            [pm](const std::shared_ptr<T> &c_sp) -> std::shared_ptr<drp> {
+            [pm](handle c_hdl) -> std::shared_ptr<drp> {
+                std::shared_ptr<T> c_sp = detail::type_caster<T>::shared_ptr_from_python(c_hdl);
                 D ptr = (*c_sp).*pm;
                 return std::shared_ptr<drp>(c_sp, ptr);
             },
@@ -1630,8 +1631,8 @@ struct property_cpp_function<
     template <typename PM, must_be_member_function_pointer<PM> = 0>
     static cpp_function readonly(PM pm, const handle &hdl) {
         return cpp_function(
-            [pm](const std::shared_ptr<T> &c_sp)
-                -> std::shared_ptr<typename std::add_const<D>::type> {
+            [pm](handle c_hdl) -> std::shared_ptr<typename std::add_const<D>::type> {
+                std::shared_ptr<T> c_sp = detail::type_caster<T>::shared_ptr_from_python(c_hdl);
                 return std::shared_ptr<typename std::add_const<D>::type>(c_sp, &(c_sp.get()->*pm));
             },
             is_method(hdl));
@@ -1640,7 +1641,8 @@ struct property_cpp_function<
     template <typename PM, must_be_member_function_pointer<PM> = 0>
     static cpp_function read(PM pm, const handle &hdl) {
         return cpp_function(
-            [pm](const std::shared_ptr<T> &c_sp) -> std::shared_ptr<D> {
+            [pm](handle c_hdl) -> std::shared_ptr<D> {
+                std::shared_ptr<T> c_sp = detail::type_caster<T>::shared_ptr_from_python(c_hdl);
                 return std::shared_ptr<D>(c_sp, &(c_sp.get()->*pm));
             },
             is_method(hdl));
@@ -1677,7 +1679,10 @@ struct property_cpp_function<
     template <typename PM, must_be_member_function_pointer<PM> = 0>
     static cpp_function read(PM pm, const handle &hdl) {
         return cpp_function(
-            [pm](const std::shared_ptr<T> &c_sp) -> D { return D{std::move(c_sp.get()->*pm)}; },
+            [pm](handle c_hdl) -> D {
+                std::shared_ptr<T> c_sp = detail::type_caster<T>::shared_ptr_from_python(c_hdl);
+                return D{std::move(c_sp.get()->*pm)};
+            },
             is_method(hdl));
     }
 

tests/CMakeLists.txt

@@ -130,6 +130,7 @@ set(PYBIND11_TEST_FILES
     test_class_sh_mi_thunks
     test_class_sh_module_local.py
     test_class_sh_property
+    test_class_sh_property_non_owning
     test_class_sh_property_stl
     test_class_sh_shared_ptr_copy_move
     test_class_sh_trampoline_basic

tests/test_class_sh_property_non_owning.cpp

@@ -0,0 +1,68 @@
+#include "pybind11/smart_holder.h"
+#include "pybind11_tests.h"
+
+#include <cstddef>
+#include <vector>
+
+namespace test_class_sh_property_non_owning {
+
+struct CoreField {
+    explicit CoreField(int int_value = -99) : int_value{int_value} {}
+    int int_value;
+};
+
+struct DataField {
+    DataField(int i_value, int i_shared, int i_unique)
+        : core_fld_value{i_value}, core_fld_shared_ptr{new CoreField{i_shared}},
+          core_fld_raw_ptr{core_fld_shared_ptr.get()}, core_fld_unique_ptr{
+                                                           new CoreField{i_unique}} {}
+    CoreField core_fld_value;
+    std::shared_ptr<CoreField> core_fld_shared_ptr;
+    CoreField *core_fld_raw_ptr;
+    std::unique_ptr<CoreField> core_fld_unique_ptr;
+};
+
+struct DataFieldsHolder {
+private:
+    std::vector<DataField> vec;
+
+public:
+    explicit DataFieldsHolder(std::size_t vec_size) {
+        for (std::size_t i = 0; i < vec_size; i++) {
+            int i11 = static_cast<int>(i) * 11;
+            vec.emplace_back(13 + i11, 14 + i11, 15 + i11);
+        }
+    }
+
+    DataField *vec_at(std::size_t index) {
+        if (index >= vec.size()) {
+            return nullptr;
+        }
+        return &vec[index];
+    }
+};
+
+} // namespace test_class_sh_property_non_owning
+
+using namespace test_class_sh_property_non_owning;
+
+PYBIND11_SMART_HOLDER_TYPE_CASTERS(CoreField)
+PYBIND11_SMART_HOLDER_TYPE_CASTERS(DataField)
+PYBIND11_SMART_HOLDER_TYPE_CASTERS(DataFieldsHolder)
+
+TEST_SUBMODULE(class_sh_property_non_owning, m) {
+    py::classh<CoreField>(m, "CoreField").def_readwrite("int_value", &CoreField::int_value);
+
+    py::classh<DataField>(m, "DataField")
+        .def_readonly("core_fld_value_ro", &DataField::core_fld_value)
+        .def_readwrite("core_fld_value_rw", &DataField::core_fld_value)
+        .def_readonly("core_fld_shared_ptr_ro", &DataField::core_fld_shared_ptr)
+        .def_readwrite("core_fld_shared_ptr_rw", &DataField::core_fld_shared_ptr)
+        .def_readonly("core_fld_raw_ptr_ro", &DataField::core_fld_raw_ptr)
+        .def_readwrite("core_fld_raw_ptr_rw", &DataField::core_fld_raw_ptr)
+        .def_readwrite("core_fld_unique_ptr_rw", &DataField::core_fld_unique_ptr);
+
+    py::classh<DataFieldsHolder>(m, "DataFieldsHolder")
+        .def(py::init<std::size_t>())
+        .def("vec_at", &DataFieldsHolder::vec_at, py::return_value_policy::reference_internal);
+}

tests/test_class_sh_property_non_owning.py

@@ -0,0 +1,28 @@
+import pytest
+
+from pybind11_tests import class_sh_property_non_owning as m
+
+
+@pytest.mark.parametrize("persistent_holder", [True, False])
+@pytest.mark.parametrize(
+    ("core_fld", "expected"),
+    [
+        ("core_fld_value_ro", (13, 24)),
+        ("core_fld_value_rw", (13, 24)),
+        ("core_fld_shared_ptr_ro", (14, 25)),
+        ("core_fld_shared_ptr_rw", (14, 25)),
+        ("core_fld_raw_ptr_ro", (14, 25)),
+        ("core_fld_raw_ptr_rw", (14, 25)),
+        ("core_fld_unique_ptr_rw", (15, 26)),
+    ],
+)
+def test_core_fld_common(core_fld, expected, persistent_holder):
+    if persistent_holder:
+        h = m.DataFieldsHolder(2)
+        for i, exp in enumerate(expected):
+            c = getattr(h.vec_at(i), core_fld)
+            assert c.int_value == exp
+    else:
+        for i, exp in enumerate(expected):
+            c = getattr(m.DataFieldsHolder(2).vec_at(i), core_fld)
+            assert c.int_value == exp