From b41a0db1b10f01865903d0efb10284f540fafb72 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Thu, 14 Jan 2021 22:23:07 -0800 Subject: [PATCH] Calling deregister_instance after disowning via unique_ptr. --- tests/test_classh_wip.cpp | 55 +++++++++++++++++++++++++-------------- tests/test_classh_wip.py | 12 ++++++++- 2 files changed, 46 insertions(+), 21 deletions(-) diff --git a/tests/test_classh_wip.cpp b/tests/test_classh_wip.cpp index 860736c27..8876b8a3f 100644 --- a/tests/test_classh_wip.cpp +++ b/tests/test_classh_wip.cpp @@ -54,17 +54,35 @@ using namespace pybind11_tests::classh_wip; template struct smart_holder_type_caster_load { + using holder_type = pybindit::memory::smart_holder; + bool load(handle src, bool /*convert*/) { if (!isinstance(src)) return false; auto inst = reinterpret_cast(src.ptr()); - auto v_h = inst->get_value_and_holder(get_type_info(typeid(T))); - smhldr_ptr = &v_h.holder(); + loaded_v_h = inst->get_value_and_holder(get_type_info(typeid(T))); + if (!loaded_v_h.holder_constructed()) { + // IMPROVEABLE: Error message. + throw std::runtime_error("Missing value for wrapped C++ type:" + " Python instance is uninitialized or was disowned."); + } + loaded_smhldr_ptr = &loaded_v_h.holder(); return true; } + std::unique_ptr loaded_as_unique_ptr() { + void *value_void_ptr = loaded_v_h.value_ptr(); + auto unq_ptr = loaded_smhldr_ptr->as_unique_ptr(); + loaded_v_h.holder().~holder_type(); + loaded_v_h.set_holder_constructed(false); + loaded_v_h.value_ptr() = nullptr; + deregister_instance(loaded_v_h.inst, value_void_ptr, loaded_v_h.type); + return unq_ptr; + } + protected: - pybindit::memory::smart_holder *smhldr_ptr = nullptr; + value_and_holder loaded_v_h; + holder_type *loaded_smhldr_ptr = nullptr; }; template <> @@ -127,12 +145,12 @@ struct type_caster : smart_holder_type_caster_load { // clang-format off - operator mpty() { return smhldr_ptr->lvalue_ref(); } - operator mpty&&() && { return smhldr_ptr->rvalue_ref(); } - operator mpty const&() { return smhldr_ptr->lvalue_ref(); } - operator mpty&() { return smhldr_ptr->lvalue_ref(); } - operator mpty const*() { return smhldr_ptr->as_raw_ptr_unowned(); } - operator mpty*() { return smhldr_ptr->as_raw_ptr_unowned(); } + operator mpty() { return loaded_smhldr_ptr->lvalue_ref(); } + operator mpty&&() && { return loaded_smhldr_ptr->rvalue_ref(); } + operator mpty const&() { return loaded_smhldr_ptr->lvalue_ref(); } + operator mpty&() { return loaded_smhldr_ptr->lvalue_ref(); } + operator mpty const*() { return loaded_smhldr_ptr->as_raw_ptr_unowned(); } + operator mpty*() { return loaded_smhldr_ptr->as_raw_ptr_unowned(); } // clang-format on @@ -331,7 +349,7 @@ struct type_caster> : smart_holder_type_caster_load template using cast_op_type = std::shared_ptr; - operator std::shared_ptr() { return smhldr_ptr->as_shared_ptr(); } + operator std::shared_ptr() { return loaded_smhldr_ptr->as_shared_ptr(); } }; template <> @@ -349,7 +367,7 @@ struct type_caster> : smart_holder_type_caster_load< template using cast_op_type = std::shared_ptr; - operator std::shared_ptr() { return smhldr_ptr->as_shared_ptr(); } + operator std::shared_ptr() { return loaded_smhldr_ptr->as_shared_ptr(); } }; template <> @@ -398,10 +416,7 @@ struct type_caster> : smart_holder_type_caster_load template using cast_op_type = std::unique_ptr; - operator std::unique_ptr() { - // MISSING: value_and_holder value_ptr reset, deregister_instance. - return smhldr_ptr->as_unique_ptr(); - } + operator std::unique_ptr() { return loaded_as_unique_ptr(); } }; template <> @@ -419,10 +434,7 @@ struct type_caster> : smart_holder_type_caster_load< template using cast_op_type = std::unique_ptr; - operator std::unique_ptr() { - // MISSING: value_and_holder value_ptr reset, deregister_instance. - return smhldr_ptr->as_unique_ptr(); - } + operator std::unique_ptr() { return loaded_as_unique_ptr(); } }; } // namespace detail @@ -466,7 +478,10 @@ TEST_SUBMODULE(classh_wip, m) { m.def("pass_mpty_uqmp", pass_mpty_uqmp); m.def("pass_mpty_uqcp", pass_mpty_uqcp); - m.def("get_mtxt", get_mtxt); // Requires pass_mpty_cref to work properly. + // Helpers, these require selected functions above to work, as indicated: + m.def("get_mtxt", get_mtxt); // pass_mpty_cref + m.def("unique_ptr_roundtrip", + [](std::unique_ptr obj) { return obj; }); // pass_mpty_uqmp, rtrn_mpty_uqmp } } // namespace classh_wip diff --git a/tests/test_classh_wip.py b/tests/test_classh_wip.py index f816af06f..4f467b2bd 100644 --- a/tests/test_classh_wip.py +++ b/tests/test_classh_wip.py @@ -63,4 +63,14 @@ def test_pass_unique_ptr_disowns(pass_mpty, argm, rtrn): assert pass_mpty(obj) == rtrn with pytest.raises(RuntimeError) as exc_info: m.pass_mpty_uqmp(obj) - assert str(exc_info.value) == "Cannot disown nullptr (as_unique_ptr)." + assert str(exc_info.value) == ( + "Missing value for wrapped C++ type:" + " Python instance is uninitialized or was disowned." + ) + + +def test_unique_ptr_roundtrip(num_round_trips=1000): + recycled = m.mpty("passenger") + for _ in range(num_round_trips): + recycled = m.unique_ptr_roundtrip(recycled) + assert m.get_mtxt(recycled) == "passenger"