Resolving TODOs for from_unique_ptr void_cast_raw_ptr. Adding test to exercise the path through cast. The path through init.h is missing a test that would fail if the flag is incorrect. The plan is to systematically cover all situations (there are many that are not exercised for shared_from_this).

This commit is contained in:
Ralf W. Grosse-Kunstleve 2021-06-24 06:04:34 -07:00 committed by Ralf W. Grosse-Kunstleve
parent a0e19bdc46
commit 146a925e4e
5 changed files with 27 additions and 12 deletions

View File

@ -182,8 +182,8 @@ void construct(value_and_holder &v_h, std::unique_ptr<Cpp<Class>, D> &&unq_ptr,
if (Class::has_alias && need_alias && !is_alias<Class>(ptr)) if (Class::has_alias && need_alias && !is_alias<Class>(ptr))
throw type_error("pybind11::init(): construction failed: returned std::unique_ptr pointee " throw type_error("pybind11::init(): construction failed: returned std::unique_ptr pointee "
"is not an alias instance"); "is not an alias instance");
auto smhldr auto smhldr = type_caster<Cpp<Class>>::template smart_holder_from_unique_ptr(
= type_caster<Cpp<Class>>::template smart_holder_from_unique_ptr(std::move(unq_ptr)); std::move(unq_ptr), Class::has_alias && is_alias<Class>(ptr));
v_h.value_ptr() = ptr; v_h.value_ptr() = ptr;
v_h.type->init_instance(v_h.inst, &smhldr); v_h.type->init_instance(v_h.inst, &smhldr);
} }
@ -197,8 +197,8 @@ void construct(value_and_holder &v_h,
bool /*need_alias*/) { bool /*need_alias*/) {
auto *ptr = unq_ptr.get(); auto *ptr = unq_ptr.get();
no_nullptr(ptr); no_nullptr(ptr);
auto smhldr auto smhldr = type_caster<Alias<Class>>::template smart_holder_from_unique_ptr(
= type_caster<Alias<Class>>::template smart_holder_from_unique_ptr(std::move(unq_ptr)); std::move(unq_ptr), true);
v_h.value_ptr() = ptr; v_h.value_ptr() = ptr;
v_h.type->init_instance(v_h.inst, &smhldr); v_h.type->init_instance(v_h.inst, &smhldr);
} }

View File

@ -40,6 +40,10 @@ Details:
* By choice, the smart_holder is movable but not copyable, to keep the design * By choice, the smart_holder is movable but not copyable, to keep the design
simple, and to guard against accidental copying overhead. simple, and to guard against accidental copying overhead.
* The `void_cast_raw_ptr` option is needed to make the `smart_holder` `vptr`
member invisible to the `shared_from_this` mechanism, in case the lifetime
of a `PyObject` is tied to the pointee.
*/ */
#pragma once #pragma once
@ -51,7 +55,7 @@ Details:
#include <type_traits> #include <type_traits>
#include <typeinfo> #include <typeinfo>
//#include <iostream> // #include <iostream>
// inline void to_cout(const std::string &msg) { std::cout << msg << std::endl; } // inline void to_cout(const std::string &msg) { std::cout << msg << std::endl; }
inline void to_cout(const std::string &) {} inline void to_cout(const std::string &) {}

View File

@ -317,7 +317,7 @@ struct smart_holder_type_caster_class_hooks : smart_holder_type_caster_base_tag
if (inst->owned) { if (inst->owned) {
new (uninitialized_location) new (uninitialized_location)
holder_type(holder_type::from_raw_ptr_take_ownership( holder_type(holder_type::from_raw_ptr_take_ownership(
value_ptr_w_t, pointee_depends_on_holder_owner)); value_ptr_w_t, /*void_cast_raw_ptr*/ pointee_depends_on_holder_owner));
} 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));
@ -330,9 +330,10 @@ struct smart_holder_type_caster_class_hooks : smart_holder_type_caster_base_tag
} }
template <typename T, typename D> template <typename T, typename D>
static smart_holder smart_holder_from_unique_ptr(std::unique_ptr<T, D> &&unq_ptr) { static smart_holder smart_holder_from_unique_ptr(std::unique_ptr<T, D> &&unq_ptr,
return pybindit::memory::smart_holder::from_unique_ptr( bool void_cast_raw_ptr) {
std::move(unq_ptr), /*TODO pointee_depends_on_holder_owner*/ true); return pybindit::memory::smart_holder::from_unique_ptr(std::move(unq_ptr),
void_cast_raw_ptr);
} }
template <typename T> template <typename T>
@ -805,8 +806,8 @@ struct smart_holder_type_caster<std::unique_ptr<T, D>> : smart_holder_type_caste
void *&valueptr = values_and_holders(inst_raw_ptr).begin()->value_ptr(); void *&valueptr = values_and_holders(inst_raw_ptr).begin()->value_ptr();
valueptr = src_raw_void_ptr; valueptr = src_raw_void_ptr;
auto smhldr = pybindit::memory::smart_holder::from_unique_ptr( auto smhldr = pybindit::memory::smart_holder::from_unique_ptr(std::move(src),
std::move(src), /*TODO pointee_depends_on_holder_owner*/ true); /*void_cast_raw_ptr*/ false);
tinfo->init_instance(inst_raw_ptr, static_cast<const void *>(&smhldr)); tinfo->init_instance(inst_raw_ptr, static_cast<const void *>(&smhldr));
if (policy == return_value_policy::reference_internal) if (policy == return_value_policy::reference_internal)

View File

@ -89,6 +89,10 @@ void pass_unique_ptr(const std::unique_ptr<Sft> &) {}
Sft *make_pure_cpp_sft_raw_ptr(const std::string &history_seed) { return new Sft{history_seed}; } Sft *make_pure_cpp_sft_raw_ptr(const std::string &history_seed) { return new Sft{history_seed}; }
std::unique_ptr<Sft> make_pure_cpp_sft_unq_ptr(const std::string &history_seed) {
return std::unique_ptr<Sft>(new Sft{history_seed});
}
std::shared_ptr<Sft> make_pure_cpp_sft_shd_ptr(const std::string &history_seed) { std::shared_ptr<Sft> make_pure_cpp_sft_shd_ptr(const std::string &history_seed) {
return std::make_shared<Sft>(history_seed); return std::make_shared<Sft>(history_seed);
} }
@ -115,6 +119,7 @@ TEST_SUBMODULE(class_sh_trampoline_shared_from_this, m) {
m.def("pass_shared_ptr", pass_shared_ptr); m.def("pass_shared_ptr", pass_shared_ptr);
m.def("pass_unique_ptr", pass_unique_ptr); m.def("pass_unique_ptr", pass_unique_ptr);
m.def("make_pure_cpp_sft_raw_ptr", make_pure_cpp_sft_raw_ptr); m.def("make_pure_cpp_sft_raw_ptr", make_pure_cpp_sft_raw_ptr);
m.def("make_pure_cpp_sft_unq_ptr", make_pure_cpp_sft_unq_ptr);
m.def("make_pure_cpp_sft_shd_ptr", make_pure_cpp_sft_shd_ptr); m.def("make_pure_cpp_sft_shd_ptr", make_pure_cpp_sft_shd_ptr);
m.def("to_cout", to_cout); m.def("to_cout", to_cout);
} }

View File

@ -131,7 +131,12 @@ def test_pass_released_shared_ptr_as_unique_ptr():
@pytest.mark.parametrize( @pytest.mark.parametrize(
"make_f", [m.make_pure_cpp_sft_raw_ptr, m.make_pure_cpp_sft_shd_ptr] "make_f",
[
m.make_pure_cpp_sft_raw_ptr,
m.make_pure_cpp_sft_unq_ptr,
m.make_pure_cpp_sft_shd_ptr,
],
) )
def test_pure_cpp_sft_raw_ptr(make_f): def test_pure_cpp_sft_raw_ptr(make_f):
obj = make_f("PureCppSft") obj = make_f("PureCppSft")