Adding "Lazy allocation for unallocated values" (for old-style __init__) into load_value_and_holder. Deferring destruction of disowned holder until clear_instance, to remain inspectable for "uninitialized" or "disowned" detection. New stats for all tests combined: 5 failed, 465 passed.

This commit is contained in:
Ralf W. Grosse-Kunstleve 2021-02-09 01:00:13 -08:00
parent 233eb9e6a8
commit c6620cef90
5 changed files with 67 additions and 38 deletions

View File

@ -980,16 +980,29 @@ public:
// Base methods for generic caster; there are overridden in copyable_holder_caster // Base methods for generic caster; there are overridden in copyable_holder_caster
void load_value_and_holder(value_and_holder &&v_h) { void load_value_and_holder(value_and_holder &&v_h) {
if (!v_h.holder_constructed()) {
// This is needed for old-style __init__.
// type_caster_generic::load_value BEGIN
auto *&vptr = v_h.value_ptr();
// Lazy allocation for unallocated values:
if (vptr == nullptr) {
// Lazy allocation for unallocated values:
auto *type = v_h.type ? v_h.type : typeinfo;
if (type->operator_new) {
vptr = type->operator_new(type->type_size);
} else {
#if defined(__cpp_aligned_new) && (!defined(_MSC_VER) || _MSC_VER >= 1912)
if (type->type_align > __STDCPP_DEFAULT_NEW_ALIGNMENT__)
vptr = ::operator new(type->type_size,
std::align_val_t(type->type_align));
else
#endif
vptr = ::operator new(type->type_size);
}
}
// type_caster_generic::load_value END
}
loaded_v_h = std::move(v_h); loaded_v_h = std::move(v_h);
if (!loaded_v_h.holder_constructed()) {
// IMPROVABLE: Error message. A change to the existing internals is
// needed to cleanly distinguish between uninitialized or disowned.
throw std::runtime_error("Missing value for wrapped C++ type:"
" Python instance is uninitialized or was disowned.");
}
if (v_h.value_ptr() == nullptr) {
pybind11_fail("smart_holder_type_casters: Unexpected v_h.value_ptr() nullptr.");
}
loaded_v_h.type = typeinfo; loaded_v_h.type = typeinfo;
} }
@ -1222,40 +1235,37 @@ struct smart_holder_type_caster_load {
} }
T *loaded_as_raw_ptr_unowned() const { T *loaded_as_raw_ptr_unowned() const {
if (load_impl.unowned_void_ptr_from_direct_conversion != nullptr) void *void_ptr = load_impl.unowned_void_ptr_from_direct_conversion; // UNTESTED.
// UNTESTED. if (void_ptr == nullptr) {
return convert_type(load_impl.unowned_void_ptr_from_direct_conversion); if (have_holder()) {
if (!have_holder()) return nullptr; throw_if_uninitialized_or_disowned_holder();
return convert_type(holder().template as_raw_ptr_unowned<void>()); void_ptr = holder().template as_raw_ptr_unowned<void>();
} else if (load_impl.loaded_v_h.vh != nullptr)
void_ptr = load_impl.loaded_v_h.value_ptr();
if (void_ptr == nullptr)
return nullptr;
}
return convert_type(void_ptr);
} }
T &loaded_as_lvalue_ref() const { T &loaded_as_lvalue_ref() const {
if (load_impl.unowned_void_ptr_from_direct_conversion != nullptr) T *raw_ptr = loaded_as_raw_ptr_unowned();
// UNTESTED. if (raw_ptr == nullptr) throw reference_cast_error();
return *convert_type(load_impl.unowned_void_ptr_from_direct_conversion); return *raw_ptr;
if (!have_holder()) throw reference_cast_error();
static const char *context = "loaded_as_lvalue_ref";
holder().ensure_is_populated(context);
holder().ensure_has_pointee(context);
return *loaded_as_raw_ptr_unowned();
} }
T &&loaded_as_rvalue_ref() const { T &&loaded_as_rvalue_ref() const {
if (load_impl.unowned_void_ptr_from_direct_conversion != nullptr) T *raw_ptr = loaded_as_raw_ptr_unowned();
// UNTESTED. if (raw_ptr == nullptr) throw reference_cast_error();
return std::move(*convert_type(load_impl.unowned_void_ptr_from_direct_conversion)); return std::move(*raw_ptr);
if (!have_holder()) throw reference_cast_error();
static const char *context = "loaded_as_rvalue_ref";
holder().ensure_is_populated(context);
holder().ensure_has_pointee(context);
return std::move(*loaded_as_raw_ptr_unowned());
} }
std::shared_ptr<T> loaded_as_shared_ptr() { std::shared_ptr<T> loaded_as_shared_ptr() const {
if (load_impl.unowned_void_ptr_from_direct_conversion != nullptr) if (load_impl.unowned_void_ptr_from_direct_conversion != nullptr)
throw cast_error("Unowned pointer from direct conversion cannot be converted to a" throw cast_error("Unowned pointer from direct conversion cannot be converted to a"
" std::shared_ptr."); // UNTESTED. " std::shared_ptr."); // UNTESTED.
if (!have_holder()) return nullptr; if (!have_holder()) return nullptr;
throw_if_uninitialized_or_disowned_holder();
std::shared_ptr<void> void_ptr = holder().template as_shared_ptr<void>(); std::shared_ptr<void> void_ptr = holder().template as_shared_ptr<void>();
return std::shared_ptr<T>(void_ptr, convert_type(void_ptr.get())); return std::shared_ptr<T>(void_ptr, convert_type(void_ptr.get()));
} }
@ -1266,6 +1276,7 @@ struct smart_holder_type_caster_load {
throw cast_error("Unowned pointer from direct conversion cannot be converted to a" throw cast_error("Unowned pointer from direct conversion cannot be converted to a"
" std::unique_ptr."); // UNTESTED. " std::unique_ptr."); // UNTESTED.
if (!have_holder()) return nullptr; if (!have_holder()) return nullptr;
throw_if_uninitialized_or_disowned_holder();
holder().template ensure_compatible_rtti_uqp_del<T, D>(context); holder().template ensure_compatible_rtti_uqp_del<T, D>(context);
holder().ensure_use_count_1(context); holder().ensure_use_count_1(context);
auto raw_void_ptr = holder().template as_raw_ptr_unowned<void>(); auto raw_void_ptr = holder().template as_raw_ptr_unowned<void>();
@ -1277,10 +1288,11 @@ struct smart_holder_type_caster_load {
holder().release_ownership(); holder().release_ownership();
auto result = std::unique_ptr<T, D>(raw_type_ptr); auto result = std::unique_ptr<T, D>(raw_type_ptr);
void *value_void_ptr void *value_void_ptr = load_impl.loaded_v_h.value_ptr();
= load_impl.loaded_v_h.value_ptr(); // Expected to be identical to raw_void_ptr. if (value_void_ptr != raw_void_ptr) {
load_impl.loaded_v_h.holder<holder_type>().~holder_type(); pybind11_fail("smart_holder_type_casters: loaded_as_unique_ptr failure:"
load_impl.loaded_v_h.set_holder_constructed(false); " value_void_ptr != raw_void_ptr");
}
load_impl.loaded_v_h.value_ptr() = nullptr; load_impl.loaded_v_h.value_ptr() = nullptr;
deregister_instance(load_impl.loaded_v_h.inst, value_void_ptr, load_impl.loaded_v_h.type); deregister_instance(load_impl.loaded_v_h.inst, value_void_ptr, load_impl.loaded_v_h.type);
@ -1290,10 +1302,24 @@ struct smart_holder_type_caster_load {
private: private:
modified_type_caster_generic_load_impl load_impl; modified_type_caster_generic_load_impl load_impl;
bool have_holder() const { return load_impl.loaded_v_h.vh != nullptr; } bool have_holder() const {
return load_impl.loaded_v_h.vh != nullptr && load_impl.loaded_v_h.holder_constructed();
}
holder_type &holder() const { return load_impl.loaded_v_h.holder<holder_type>(); } holder_type &holder() const { return load_impl.loaded_v_h.holder<holder_type>(); }
// have_holder() must be true or this function will fail.
void throw_if_uninitialized_or_disowned_holder() const {
if (!holder().is_populated) {
pybind11_fail("Missing value for wrapped C++ type:"
" Python instance is uninitialized.");
}
if (!holder().has_pointee()) {
throw cast_error("Missing value for wrapped C++ type:"
" Python instance was disowned.");
}
}
T *convert_type(void *void_ptr) const { T *convert_type(void *void_ptr) const {
if (void_ptr != nullptr && load_impl.loaded_v_h_cpptype != nullptr if (void_ptr != nullptr && load_impl.loaded_v_h_cpptype != nullptr
&& !load_impl.reinterpret_cast_deemed_ok && load_impl.implicit_cast != nullptr) { && !load_impl.reinterpret_cast_deemed_ok && load_impl.implicit_cast != nullptr) {

View File

@ -398,6 +398,8 @@ inline void clear_instance(PyObject *self) {
if (instance->owned || v_h.holder_constructed()) if (instance->owned || v_h.holder_constructed())
v_h.type->dealloc(v_h); v_h.type->dealloc(v_h);
} else if (v_h.holder_constructed()) {
v_h.type->dealloc(v_h); // Disowned instance.
} }
} }
// Deallocate the value/holder layout internals: // Deallocate the value/holder layout internals:

View File

@ -208,6 +208,7 @@ struct smart_holder {
void release_ownership() { void release_ownership() {
*vptr_deleter_armed_flag_ptr = false; *vptr_deleter_armed_flag_ptr = false;
vptr.reset(); vptr.reset();
vptr_deleter_armed_flag_ptr.reset();
} }
template <typename T> template <typename T>

View File

@ -77,7 +77,7 @@ def test_pass_unique_ptr_disowns(rtrn_atyp, pass_atyp, rtrn):
m.pass_uqmp_atyp(obj) m.pass_uqmp_atyp(obj)
assert str(exc_info.value) == ( assert str(exc_info.value) == (
"Missing value for wrapped C++ type:" "Missing value for wrapped C++ type:"
" Python instance is uninitialized or was disowned." " Python instance was disowned."
) )

View File

@ -21,7 +21,7 @@ def test_pointee_and_ptr_owner(give_up_ownership_via):
obj.get_int() obj.get_int()
assert ( assert (
str(exc_info.value) str(exc_info.value)
== "Missing value for wrapped C++ type: Python instance is uninitialized or was disowned." == "Missing value for wrapped C++ type: Python instance was disowned."
) )
assert owner.is_owner() assert owner.is_owner()
reclaimed = getattr(owner, give_up_ownership_via)() reclaimed = getattr(owner, give_up_ownership_via)()