From a9bfcbdcc381613f53f53acdbb3a01b00e0151e9 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 1 Feb 2021 14:06:58 -0800 Subject: [PATCH] Adding smart_holder_type_casters for unique_ptr with custom deleter. SEVERE CODE DUPLICATION. This commit is to establish a baseline for consolidating the unique_ptr code. --- .../detail/smart_holder_type_casters.h | 95 +++++++++++++++++++ tests/test_class_sh_basic.cpp | 15 +++ tests/test_class_sh_basic.py | 10 ++ 3 files changed, 120 insertions(+) diff --git a/include/pybind11/detail/smart_holder_type_casters.h b/include/pybind11/detail/smart_holder_type_casters.h index 74624c5b7..602a9301d 100644 --- a/include/pybind11/detail/smart_holder_type_casters.h +++ b/include/pybind11/detail/smart_holder_type_casters.h @@ -318,6 +318,29 @@ struct smart_holder_type_caster_load { return result; } + template + std::unique_ptr loaded_as_unique_ptr_with_deleter() { + holder().template ensure_compatible_rtti_uqp_del("loaded_as_unique_ptr_with_deleter"); + holder().ensure_use_count_1("loaded_as_unique_ptr_with_deleter"); + auto raw_void_ptr = holder().template as_raw_ptr_unowned(); + // MISSING: Safety checks for type conversions + // (T must be polymorphic or meet certain other conditions). + T *raw_type_ptr = convert_type(raw_void_ptr); + + // Critical transfer-of-ownership section. This must stay together. + holder().release_ownership(); + auto result = std::unique_ptr(raw_type_ptr); + + void *value_void_ptr + = load_impl.loaded_v_h.value_ptr(); // Expected to be identical to raw_void_ptr. + load_impl.loaded_v_h.holder().~holder_type(); + load_impl.loaded_v_h.set_holder_constructed(false); + 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); + + return result; + } + private: modified_type_caster_generic_load_impl load_impl; @@ -622,6 +645,72 @@ struct smart_holder_type_caster> : smart_holder_type_ca operator std::unique_ptr() { return this->loaded_as_unique_ptr(); } }; +template +struct smart_holder_type_caster> : smart_holder_type_caster_load, + smart_holder_type_caster_class_hooks { + static constexpr auto name = _>(); + + static handle cast(std::unique_ptr &&src, return_value_policy policy, handle parent) { + if (policy != return_value_policy::automatic + && policy != return_value_policy::reference_internal) { + // IMPROVABLE: Error message. + throw cast_error("Invalid return_value_policy for unique_ptr."); + } + + auto src_raw_ptr = src.get(); + auto st = type_caster_base::src_and_type(src_raw_ptr); + if (st.first == nullptr) + return none().release(); // PyErr was set already. + + void *src_raw_void_ptr = static_cast(src_raw_ptr); + const detail::type_info *tinfo = st.second; + if (find_registered_python_instance(src_raw_void_ptr, tinfo)) + throw cast_error("Invalid unique_ptr: another instance owns this pointer already."); + + auto inst = reinterpret_steal(make_new_instance(tinfo->type)); + auto *inst_raw_ptr = reinterpret_cast(inst.ptr()); + inst_raw_ptr->owned = true; + void *&valueptr = values_and_holders(inst_raw_ptr).begin()->value_ptr(); + valueptr = src_raw_void_ptr; + + auto smhldr = pybindit::memory::smart_holder::from_unique_ptr_with_deleter(std::move(src)); + tinfo->init_instance(inst_raw_ptr, static_cast(&smhldr)); + + if (policy == return_value_policy::reference_internal) + keep_alive_impl(inst, parent); + + return inst.release(); + } + + template + using cast_op_type = std::unique_ptr; + + operator std::unique_ptr() { + return this->template loaded_as_unique_ptr_with_deleter(); + } +}; + +template +struct smart_holder_type_caster> + : smart_holder_type_caster_load, smart_holder_type_caster_class_hooks { + static constexpr auto name = _>(); + + static handle + cast(std::unique_ptr &&src, return_value_policy policy, handle parent) { + return smart_holder_type_caster>::cast( + std::unique_ptr(const_cast(src.release())), // Const2Mutbl + policy, + parent); + } + + template + using cast_op_type = std::unique_ptr; + + operator std::unique_ptr() { + return this->template loaded_as_unique_ptr_with_deleter(); + } +}; + #define PYBIND11_SMART_HOLDER_TYPE_CASTERS(T) \ namespace pybind11 { \ namespace detail { \ @@ -639,6 +728,12 @@ struct smart_holder_type_caster> : smart_holder_type_ca template <> \ class type_caster> \ : public smart_holder_type_caster> {}; \ + template \ + class type_caster> \ + : public smart_holder_type_caster> {}; \ + template \ + class type_caster> \ + : public smart_holder_type_caster> {}; \ } \ } diff --git a/tests/test_class_sh_basic.cpp b/tests/test_class_sh_basic.cpp index b638fd854..ef08f7e0e 100644 --- a/tests/test_class_sh_basic.cpp +++ b/tests/test_class_sh_basic.cpp @@ -40,6 +40,15 @@ std::unique_ptr rtrn_uqcp_atyp() { return std::unique_ptr obj) { return "pass_uqmp:" + obj->mtxt; } std::string pass_uqcp_atyp(std::unique_ptr obj) { return "pass_uqcp:" + obj->mtxt; } +struct uqmd : std::default_delete {}; +struct uqcd : std::default_delete {}; + +std::unique_ptr rtrn_uqmp_del_atyp() { return std::unique_ptr(new atyp{"rtrn_uqmp_del"}); } +std::unique_ptr rtrn_uqcp_del_atyp() { return std::unique_ptr(new atyp{"rtrn_uqcp_del"}); } + +std::string pass_uqmp_del_atyp(std::unique_ptr obj) { return "pass_uqmp_del:" + obj->mtxt; } +std::string pass_uqcp_del_atyp(std::unique_ptr obj) { return "pass_uqcp_del:" + obj->mtxt; } + // clang-format on // Helpers for testing. @@ -89,6 +98,12 @@ TEST_SUBMODULE(class_sh_basic, m) { m.def("pass_uqmp_atyp", pass_uqmp_atyp); m.def("pass_uqcp_atyp", pass_uqcp_atyp); + m.def("rtrn_uqmp_del_atyp", rtrn_uqmp_del_atyp); + m.def("rtrn_uqcp_del_atyp", rtrn_uqcp_del_atyp); + + m.def("pass_uqmp_del_atyp", pass_uqmp_del_atyp); + m.def("pass_uqcp_del_atyp", pass_uqcp_del_atyp); + // Helpers for testing. // These require selected functions above to work first, as indicated: m.def("get_mtxt", get_mtxt); // pass_cref_atyp diff --git a/tests/test_class_sh_basic.py b/tests/test_class_sh_basic.py index 62d365c98..6383a408c 100644 --- a/tests/test_class_sh_basic.py +++ b/tests/test_class_sh_basic.py @@ -51,6 +51,16 @@ def test_load_unique_ptr(): assert m.pass_uqcp_atyp(m.atyp("Uqcp")) == "pass_uqcp:Uqcp" +def test_cast_unique_ptr_with_deleter(): + assert m.get_mtxt(m.rtrn_uqmp_del_atyp()) == "rtrn_uqmp_del" + assert m.get_mtxt(m.rtrn_uqcp_del_atyp()) == "rtrn_uqcp_del" + + +def test_load_unique_ptr_with_deleter(): + assert m.pass_uqmp_del_atyp(m.rtrn_uqmp_del_atyp()) == "pass_uqmp_del:rtrn_uqmp_del" + assert m.pass_uqcp_del_atyp(m.rtrn_uqcp_del_atyp()) == "pass_uqcp_del:rtrn_uqcp_del" + + @pytest.mark.parametrize( "pass_atyp, argm, rtrn", [