Skip to content

Commit 4672f2b

Browse files
authored
Add instance::is_alias and migrate from using smart_holder::pointee_depends_on_holder_owner to that. (#5318)
1 parent 01b6ccb commit 4672f2b

File tree

5 files changed

+34
-13
lines changed

5 files changed

+34
-13
lines changed

include/pybind11/cast.h

+14-4
Original file line numberDiff line numberDiff line change
@@ -858,8 +858,12 @@ struct copyable_holder_caster<
858858
using base::value;
859859

860860
bool load(handle src, bool convert) {
861-
return base::template load_impl<copyable_holder_caster<type, std::shared_ptr<type>>>(
862-
src, convert);
861+
if (base::template load_impl<copyable_holder_caster<type, std::shared_ptr<type>>>(
862+
src, convert)) {
863+
sh_load_helper.maybe_set_python_instance_is_alias(src);
864+
return true;
865+
}
866+
return false;
863867
}
864868

865869
explicit operator std::shared_ptr<type> *() {
@@ -914,6 +918,7 @@ struct copyable_holder_caster<
914918
void load_value(value_and_holder &&v_h) {
915919
if (typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder) {
916920
sh_load_helper.loaded_v_h = v_h;
921+
sh_load_helper.was_populated = true;
917922
value = sh_load_helper.get_void_ptr_or_nullptr();
918923
return;
919924
}
@@ -1041,14 +1046,19 @@ struct move_only_holder_caster<
10411046
}
10421047

10431048
bool load(handle src, bool convert) {
1044-
return base::template load_impl<
1045-
move_only_holder_caster<type, std::unique_ptr<type, deleter>>>(src, convert);
1049+
if (base::template load_impl<
1050+
move_only_holder_caster<type, std::unique_ptr<type, deleter>>>(src, convert)) {
1051+
sh_load_helper.maybe_set_python_instance_is_alias(src);
1052+
return true;
1053+
}
1054+
return false;
10461055
}
10471056

10481057
void load_value(value_and_holder &&v_h) {
10491058
if (typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder) {
10501059
sh_load_helper.loaded_v_h = v_h;
10511060
sh_load_helper.loaded_v_h.type = typeinfo;
1061+
sh_load_helper.was_populated = true;
10521062
value = sh_load_helper.get_void_ptr_or_nullptr();
10531063
return;
10541064
}

include/pybind11/detail/common.h

+5
Original file line numberDiff line numberDiff line change
@@ -637,6 +637,11 @@ struct instance {
637637
bool simple_instance_registered : 1;
638638
/// If true, get_internals().patients has an entry for this object
639639
bool has_patients : 1;
640+
// Cannot use PYBIND11_INTERNALS_VERSION >= 6 here without refactoring.
641+
#if PYBIND11_VERSION_MAJOR >= 3
642+
/// If true, this Python object needs to be kept alive for the lifetime of the C++ value.
643+
bool is_alias : 1;
644+
#endif
640645

641646
/// Initializes all of the above type/values/holders data (but not the instance values
642647
/// themselves)

include/pybind11/detail/struct_smart_holder.h

+1-3
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,6 @@ struct smart_holder {
136136
bool vptr_is_external_shared_ptr : 1;
137137
bool is_populated : 1;
138138
bool is_disowned : 1;
139-
bool pointee_depends_on_holder_owner : 1; // SMART_HOLDER_WIP: See PR #2839.
140139

141140
// Design choice: smart_holder is movable but not copyable.
142141
smart_holder(smart_holder &&) = default;
@@ -146,8 +145,7 @@ struct smart_holder {
146145

147146
smart_holder()
148147
: vptr_is_using_noop_deleter{false}, vptr_is_using_builtin_delete{false},
149-
vptr_is_external_shared_ptr{false}, is_populated{false}, is_disowned{false},
150-
pointee_depends_on_holder_owner{false} {}
148+
vptr_is_external_shared_ptr{false}, is_populated{false}, is_disowned{false} {}
151149

152150
bool has_pointee() const { return vptr != nullptr; }
153151

include/pybind11/detail/type_caster_base.h

+11-2
Original file line numberDiff line numberDiff line change
@@ -704,6 +704,15 @@ inline std::unique_ptr<T, D> unique_with_deleter(T *raw_ptr, std::unique_ptr<D>
704704

705705
template <typename T>
706706
struct load_helper : value_and_holder_helper {
707+
bool was_populated = false;
708+
bool python_instance_is_alias = false;
709+
710+
void maybe_set_python_instance_is_alias(handle src) {
711+
if (was_populated) {
712+
python_instance_is_alias = reinterpret_cast<instance *>(src.ptr())->is_alias;
713+
}
714+
}
715+
707716
static std::shared_ptr<T> make_shared_ptr_with_responsible_parent(T *raw_ptr, handle parent) {
708717
return std::shared_ptr<T>(raw_ptr, shared_ptr_parent_life_support(parent.ptr()));
709718
}
@@ -724,7 +733,7 @@ struct load_helper : value_and_holder_helper {
724733
throw std::runtime_error("Non-owning holder (load_as_shared_ptr).");
725734
}
726735
auto *type_raw_ptr = static_cast<T *>(void_raw_ptr);
727-
if (hld.pointee_depends_on_holder_owner) {
736+
if (python_instance_is_alias) {
728737
auto *vptr_gd_ptr = std::get_deleter<pybindit::memory::guarded_delete>(hld.vptr);
729738
if (vptr_gd_ptr != nullptr) {
730739
std::shared_ptr<void> released_ptr = vptr_gd_ptr->released_ptr.lock();
@@ -778,7 +787,7 @@ struct load_helper : value_and_holder_helper {
778787

779788
auto *self_life_support
780789
= dynamic_raw_ptr_cast_if_possible<trampoline_self_life_support>(raw_type_ptr);
781-
if (self_life_support == nullptr && holder().pointee_depends_on_holder_owner) {
790+
if (self_life_support == nullptr && python_instance_is_alias) {
782791
throw value_error("Alias class (also known as trampoline) does not inherit from "
783792
"py::trampoline_self_life_support, therefore the ownership of this "
784793
"instance cannot safely be transferred to C++.");

include/pybind11/pybind11.h

+3-4
Original file line numberDiff line numberDiff line change
@@ -2267,7 +2267,8 @@ class class_ : public detail::generic_type {
22672267
}
22682268
auto *uninitialized_location = std::addressof(v_h.holder<holder_type>());
22692269
auto *value_ptr_w_t = v_h.value_ptr<type>();
2270-
bool pointee_depends_on_holder_owner
2270+
// Try downcast from `type` to `type_alias`:
2271+
inst->is_alias
22712272
= detail::dynamic_raw_ptr_cast_if_possible<type_alias>(value_ptr_w_t) != nullptr;
22722273
if (holder_void_ptr) {
22732274
// Note: inst->owned ignored.
@@ -2277,14 +2278,12 @@ class class_ : public detail::generic_type {
22772278
uninitialized_location, value_ptr_w_t, value_ptr_w_t)) {
22782279
if (inst->owned) {
22792280
new (uninitialized_location) holder_type(holder_type::from_raw_ptr_take_ownership(
2280-
value_ptr_w_t, /*void_cast_raw_ptr*/ pointee_depends_on_holder_owner));
2281+
value_ptr_w_t, /*void_cast_raw_ptr*/ inst->is_alias));
22812282
} else {
22822283
new (uninitialized_location)
22832284
holder_type(holder_type::from_raw_ptr_unowned(value_ptr_w_t));
22842285
}
22852286
}
2286-
v_h.holder<holder_type>().pointee_depends_on_holder_owner
2287-
= pointee_depends_on_holder_owner;
22882287
v_h.set_holder_constructed();
22892288
}
22902289

0 commit comments

Comments
 (0)