Skip to content

[smart_holder] Enable properties for non-owning holders #4586

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 38 additions & 1 deletion include/pybind11/detail/smart_holder_type_casters.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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.");
}
Expand All @@ -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>();
Expand Down Expand Up @@ -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;

Expand Down
15 changes: 10 additions & 5 deletions include/pybind11/pybind11.h
Original file line number Diff line number Diff line change
Expand Up @@ -1586,7 +1586,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);
},
Expand Down Expand Up @@ -1622,8 +1623,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));
Expand All @@ -1632,7 +1633,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));
Expand Down Expand Up @@ -1669,7 +1671,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));
}

Expand Down
68 changes: 68 additions & 0 deletions tests/test_class_sh_property_non_owning.cpp
Original file line number Diff line number Diff line change
@@ -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:
DataFieldsHolder(std::size_t vec_size) {
for (std::size_t i = 0; i < vec_size; i++) {
int i11 = static_cast<int>(i) * 11;
vec.push_back(DataField(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);
}
28 changes: 28 additions & 0 deletions tests/test_class_sh_property_non_owning.py
Original file line number Diff line number Diff line change
@@ -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