Add instance::is_alias and migrate from using smart_holder::pointee_depends_on_holder_owner to that. (#5318)

This commit is contained in:
Ralf W. Grosse-Kunstleve 2024-08-19 03:07:29 +07:00 committed by GitHub
parent 01b6ccb7fa
commit 4672f2bd15
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 34 additions and 13 deletions

View File

@ -858,8 +858,12 @@ public:
using base::value; using base::value;
bool load(handle src, bool convert) { bool load(handle src, bool convert) {
return base::template load_impl<copyable_holder_caster<type, std::shared_ptr<type>>>( if (base::template load_impl<copyable_holder_caster<type, std::shared_ptr<type>>>(
src, convert); src, convert)) {
sh_load_helper.maybe_set_python_instance_is_alias(src);
return true;
}
return false;
} }
explicit operator std::shared_ptr<type> *() { explicit operator std::shared_ptr<type> *() {
@ -914,6 +918,7 @@ protected:
void load_value(value_and_holder &&v_h) { void load_value(value_and_holder &&v_h) {
if (typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder) { if (typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder) {
sh_load_helper.loaded_v_h = v_h; sh_load_helper.loaded_v_h = v_h;
sh_load_helper.was_populated = true;
value = sh_load_helper.get_void_ptr_or_nullptr(); value = sh_load_helper.get_void_ptr_or_nullptr();
return; return;
} }
@ -1041,14 +1046,19 @@ public:
} }
bool load(handle src, bool convert) { bool load(handle src, bool convert) {
return base::template load_impl< if (base::template load_impl<
move_only_holder_caster<type, std::unique_ptr<type, deleter>>>(src, convert); move_only_holder_caster<type, std::unique_ptr<type, deleter>>>(src, convert)) {
sh_load_helper.maybe_set_python_instance_is_alias(src);
return true;
}
return false;
} }
void load_value(value_and_holder &&v_h) { void load_value(value_and_holder &&v_h) {
if (typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder) { if (typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder) {
sh_load_helper.loaded_v_h = v_h; sh_load_helper.loaded_v_h = v_h;
sh_load_helper.loaded_v_h.type = typeinfo; sh_load_helper.loaded_v_h.type = typeinfo;
sh_load_helper.was_populated = true;
value = sh_load_helper.get_void_ptr_or_nullptr(); value = sh_load_helper.get_void_ptr_or_nullptr();
return; return;
} }

View File

@ -637,6 +637,11 @@ struct instance {
bool simple_instance_registered : 1; bool simple_instance_registered : 1;
/// If true, get_internals().patients has an entry for this object /// If true, get_internals().patients has an entry for this object
bool has_patients : 1; bool has_patients : 1;
// Cannot use PYBIND11_INTERNALS_VERSION >= 6 here without refactoring.
#if PYBIND11_VERSION_MAJOR >= 3
/// If true, this Python object needs to be kept alive for the lifetime of the C++ value.
bool is_alias : 1;
#endif
/// Initializes all of the above type/values/holders data (but not the instance values /// Initializes all of the above type/values/holders data (but not the instance values
/// themselves) /// themselves)

View File

@ -136,7 +136,6 @@ struct smart_holder {
bool vptr_is_external_shared_ptr : 1; bool vptr_is_external_shared_ptr : 1;
bool is_populated : 1; bool is_populated : 1;
bool is_disowned : 1; bool is_disowned : 1;
bool pointee_depends_on_holder_owner : 1; // SMART_HOLDER_WIP: See PR #2839.
// Design choice: smart_holder is movable but not copyable. // Design choice: smart_holder is movable but not copyable.
smart_holder(smart_holder &&) = default; smart_holder(smart_holder &&) = default;
@ -146,8 +145,7 @@ struct smart_holder {
smart_holder() smart_holder()
: vptr_is_using_noop_deleter{false}, vptr_is_using_builtin_delete{false}, : vptr_is_using_noop_deleter{false}, vptr_is_using_builtin_delete{false},
vptr_is_external_shared_ptr{false}, is_populated{false}, is_disowned{false}, vptr_is_external_shared_ptr{false}, is_populated{false}, is_disowned{false} {}
pointee_depends_on_holder_owner{false} {}
bool has_pointee() const { return vptr != nullptr; } bool has_pointee() const { return vptr != nullptr; }

View File

@ -704,6 +704,15 @@ inline std::unique_ptr<T, D> unique_with_deleter(T *raw_ptr, std::unique_ptr<D>
template <typename T> template <typename T>
struct load_helper : value_and_holder_helper { struct load_helper : value_and_holder_helper {
bool was_populated = false;
bool python_instance_is_alias = false;
void maybe_set_python_instance_is_alias(handle src) {
if (was_populated) {
python_instance_is_alias = reinterpret_cast<instance *>(src.ptr())->is_alias;
}
}
static std::shared_ptr<T> make_shared_ptr_with_responsible_parent(T *raw_ptr, handle parent) { static std::shared_ptr<T> make_shared_ptr_with_responsible_parent(T *raw_ptr, handle parent) {
return std::shared_ptr<T>(raw_ptr, shared_ptr_parent_life_support(parent.ptr())); return std::shared_ptr<T>(raw_ptr, shared_ptr_parent_life_support(parent.ptr()));
} }
@ -724,7 +733,7 @@ struct load_helper : value_and_holder_helper {
throw std::runtime_error("Non-owning holder (load_as_shared_ptr)."); throw std::runtime_error("Non-owning holder (load_as_shared_ptr).");
} }
auto *type_raw_ptr = static_cast<T *>(void_raw_ptr); auto *type_raw_ptr = static_cast<T *>(void_raw_ptr);
if (hld.pointee_depends_on_holder_owner) { if (python_instance_is_alias) {
auto *vptr_gd_ptr = std::get_deleter<pybindit::memory::guarded_delete>(hld.vptr); auto *vptr_gd_ptr = std::get_deleter<pybindit::memory::guarded_delete>(hld.vptr);
if (vptr_gd_ptr != nullptr) { if (vptr_gd_ptr != nullptr) {
std::shared_ptr<void> released_ptr = vptr_gd_ptr->released_ptr.lock(); std::shared_ptr<void> released_ptr = vptr_gd_ptr->released_ptr.lock();
@ -778,7 +787,7 @@ struct load_helper : value_and_holder_helper {
auto *self_life_support auto *self_life_support
= dynamic_raw_ptr_cast_if_possible<trampoline_self_life_support>(raw_type_ptr); = dynamic_raw_ptr_cast_if_possible<trampoline_self_life_support>(raw_type_ptr);
if (self_life_support == nullptr && holder().pointee_depends_on_holder_owner) { if (self_life_support == nullptr && python_instance_is_alias) {
throw value_error("Alias class (also known as trampoline) does not inherit from " throw value_error("Alias class (also known as trampoline) does not inherit from "
"py::trampoline_self_life_support, therefore the ownership of this " "py::trampoline_self_life_support, therefore the ownership of this "
"instance cannot safely be transferred to C++."); "instance cannot safely be transferred to C++.");

View File

@ -2267,7 +2267,8 @@ private:
} }
auto *uninitialized_location = std::addressof(v_h.holder<holder_type>()); auto *uninitialized_location = std::addressof(v_h.holder<holder_type>());
auto *value_ptr_w_t = v_h.value_ptr<type>(); auto *value_ptr_w_t = v_h.value_ptr<type>();
bool pointee_depends_on_holder_owner // Try downcast from `type` to `type_alias`:
inst->is_alias
= detail::dynamic_raw_ptr_cast_if_possible<type_alias>(value_ptr_w_t) != nullptr; = detail::dynamic_raw_ptr_cast_if_possible<type_alias>(value_ptr_w_t) != nullptr;
if (holder_void_ptr) { if (holder_void_ptr) {
// Note: inst->owned ignored. // Note: inst->owned ignored.
@ -2277,14 +2278,12 @@ private:
uninitialized_location, value_ptr_w_t, value_ptr_w_t)) { uninitialized_location, value_ptr_w_t, value_ptr_w_t)) {
if (inst->owned) { if (inst->owned) {
new (uninitialized_location) holder_type(holder_type::from_raw_ptr_take_ownership( new (uninitialized_location) holder_type(holder_type::from_raw_ptr_take_ownership(
value_ptr_w_t, /*void_cast_raw_ptr*/ pointee_depends_on_holder_owner)); value_ptr_w_t, /*void_cast_raw_ptr*/ inst->is_alias));
} else { } else {
new (uninitialized_location) new (uninitialized_location)
holder_type(holder_type::from_raw_ptr_unowned(value_ptr_w_t)); holder_type(holder_type::from_raw_ptr_unowned(value_ptr_w_t));
} }
} }
v_h.holder<holder_type>().pointee_depends_on_holder_owner
= pointee_depends_on_holder_owner;
v_h.set_holder_constructed(); v_h.set_holder_constructed();
} }