diff --git a/CMakeLists.txt b/CMakeLists.txt index b698b016da..de5ed3b7e1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -131,7 +131,7 @@ set(PYBIND11_HEADERS include/pybind11/smart_holder.h include/pybind11/stl.h include/pybind11/stl_bind.h - include/pybind11/virtual_overrider_self_life_support.h) + include/pybind11/trampoline_self_life_support.h) # Compare with grep and warn if mismatched if(PYBIND11_MASTER_PROJECT AND NOT CMAKE_VERSION VERSION_LESS 3.12) diff --git a/README_smart_holder.rst b/README_smart_holder.rst index 3054dfbf67..baf7b8ff07 100644 --- a/README_smart_holder.rst +++ b/README_smart_holder.rst @@ -164,6 +164,26 @@ of interest have made the switch, because then the code will continue to work in either mode. +Trampolines and std::unique_ptr +------------------------------- + +A pybind11 `"trampoline" +`_ +is a C++ helper class with virtual function overrides that transparently +call back from C++ into Python. To enable safely passing a ``std::unique_ptr`` +to a trampoline object between Python and C++, the trampoline class must +inherit from ``py::trampoline_self_life_support``, for example: + +.. code-block:: cpp + + class PyAnimal : public Animal, public py::trampoline_self_life_support { + ... + }; + +This is the only difference compared to traditional pybind11. A fairly +minimal but complete example is tests/test_class_sh_trampoline_unique_ptr.cpp. + + Ideas for the long-term ----------------------- diff --git a/include/pybind11/detail/smart_holder_poc.h b/include/pybind11/detail/smart_holder_poc.h index cdae24845f..2e03bc5e1b 100644 --- a/include/pybind11/detail/smart_holder_poc.h +++ b/include/pybind11/detail/smart_holder_poc.h @@ -102,7 +102,7 @@ struct smart_holder { bool vptr_is_using_builtin_delete : 1; bool vptr_is_external_shared_ptr : 1; bool is_populated : 1; - bool was_disowned : 1; + bool is_disowned : 1; bool pointee_depends_on_holder_owner : 1; // SMART_HOLDER_WIP: See PR #2839. // Design choice: smart_holder is movable but not copyable. @@ -113,13 +113,13 @@ struct smart_holder { smart_holder() : vptr_is_using_noop_deleter{false}, vptr_is_using_builtin_delete{false}, - vptr_is_external_shared_ptr{false}, is_populated{false}, was_disowned{false}, + vptr_is_external_shared_ptr{false}, is_populated{false}, is_disowned{false}, pointee_depends_on_holder_owner{false} {} explicit smart_holder(bool vptr_deleter_armed_flag) : vptr_deleter_armed_flag_ptr{new bool{vptr_deleter_armed_flag}}, vptr_is_using_noop_deleter{false}, vptr_is_using_builtin_delete{false}, - vptr_is_external_shared_ptr{false}, is_populated{false}, was_disowned{false}, + vptr_is_external_shared_ptr{false}, is_populated{false}, is_disowned{false}, pointee_depends_on_holder_owner{false} {} bool has_pointee() const { return vptr.get() != nullptr; } @@ -136,8 +136,8 @@ struct smart_holder { throw std::runtime_error(std::string("Unpopulated holder (") + context + ")."); } } - void ensure_was_not_disowned(const char *context) const { - if (was_disowned) { + void ensure_is_not_disowned(const char *context) const { + if (is_disowned) { throw std::runtime_error(std::string("Holder was disowned already (") + context + ")."); } @@ -237,7 +237,13 @@ struct smart_holder { // Caller is responsible for ensuring preconditions (SMART_HOLDER_WIP: details). void disown() { *vptr_deleter_armed_flag_ptr = false; - was_disowned = true; + is_disowned = true; + } + + // Caller is responsible for ensuring preconditions (SMART_HOLDER_WIP: details). + void reclaim_disowned() { + *vptr_deleter_armed_flag_ptr = true; + is_disowned = false; } // Caller is responsible for ensuring preconditions (SMART_HOLDER_WIP: details). @@ -248,7 +254,7 @@ struct smart_holder { // SMART_HOLDER_WIP: review this function. void ensure_can_release_ownership(const char *context = "ensure_can_release_ownership") { - ensure_was_not_disowned(context); + ensure_is_not_disowned(context); ensure_vptr_is_using_builtin_delete(context); ensure_use_count_1(context); } diff --git a/include/pybind11/detail/smart_holder_type_casters.h b/include/pybind11/detail/smart_holder_type_casters.h index 14b3ea9979..97791ceb12 100644 --- a/include/pybind11/detail/smart_holder_type_casters.h +++ b/include/pybind11/detail/smart_holder_type_casters.h @@ -6,7 +6,7 @@ #include "../gil.h" #include "../pytypes.h" -#include "../virtual_overrider_self_life_support.h" +#include "../trampoline_self_life_support.h" #include "common.h" #include "descr.h" #include "dynamic_raw_ptr_cast_if_possible.h" @@ -353,7 +353,7 @@ struct smart_holder_type_caster_load { if (!have_holder()) return nullptr; throw_if_uninitialized_or_disowned_holder(); - holder().ensure_was_not_disowned("loaded_as_shared_ptr"); + holder().ensure_is_not_disowned("loaded_as_shared_ptr"); auto void_raw_ptr = holder().template as_raw_ptr_unowned(); auto type_raw_ptr = convert_type(void_raw_ptr); if (holder().pointee_depends_on_holder_owner) { @@ -375,7 +375,7 @@ struct smart_holder_type_caster_load { if (!have_holder()) return nullptr; throw_if_uninitialized_or_disowned_holder(); - holder().ensure_was_not_disowned(context); + holder().ensure_is_not_disowned(context); holder().template ensure_compatible_rtti_uqp_del(context); holder().ensure_use_count_1(context); auto raw_void_ptr = holder().template as_raw_ptr_unowned(); @@ -391,11 +391,11 @@ struct smart_holder_type_caster_load { T *raw_type_ptr = convert_type(raw_void_ptr); auto *self_life_support - = dynamic_raw_ptr_cast_if_possible(raw_type_ptr); + = dynamic_raw_ptr_cast_if_possible(raw_type_ptr); if (self_life_support == nullptr && holder().pointee_depends_on_holder_owner) { throw value_error("Alias class (also known as trampoline) does not inherit from " - "py::virtual_overrider_self_life_support, therefore the " - "ownership of this instance cannot safely be transferred to C++."); + "py::trampoline_self_life_support, therefore the ownership of this " + "instance cannot safely be transferred to C++."); } // Critical transfer-of-ownership section. This must stay together. @@ -406,8 +406,7 @@ struct smart_holder_type_caster_load { } auto result = std::unique_ptr(raw_type_ptr); if (self_life_support != nullptr) { - Py_INCREF((PyObject *) load_impl.loaded_v_h.inst); - self_life_support->loaded_v_h = load_impl.loaded_v_h; + self_life_support->activate_life_support(load_impl.loaded_v_h); } else { load_impl.loaded_v_h.value_ptr() = nullptr; deregister_instance( @@ -700,8 +699,27 @@ struct smart_holder_type_caster> : smart_holder_type_caste 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)) + if (handle existing_inst = find_registered_python_instance(src_raw_void_ptr, tinfo)) { + auto *self_life_support + = dynamic_raw_ptr_cast_if_possible(src_raw_ptr); + if (self_life_support != nullptr) { + value_and_holder &v_h = self_life_support->v_h; + if (v_h.inst != nullptr && v_h.vh != nullptr) { + auto &holder = v_h.holder(); + if (!holder.is_disowned) { + pybind11_fail("smart_holder_type_casters: unexpected " + "smart_holder.is_disowned failure."); + } + // Critical transfer-of-ownership section. This must stay together. + self_life_support->deactivate_life_support(); + holder.reclaim_disowned(); + src.release(); + // Critical section end. + return existing_inst; + } + } 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()); diff --git a/include/pybind11/trampoline_self_life_support.h b/include/pybind11/trampoline_self_life_support.h new file mode 100644 index 0000000000..fe2e4899f9 --- /dev/null +++ b/include/pybind11/trampoline_self_life_support.h @@ -0,0 +1,57 @@ +// Copyright (c) 2021 The Pybind Development Team. +// All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +#pragma once + +#include "detail/common.h" +#include "detail/smart_holder_poc.h" +#include "detail/type_caster_base.h" + +PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) + +PYBIND11_NAMESPACE_BEGIN(detail) +// SMART_HOLDER_WIP: Needs refactoring of existing pybind11 code. +inline bool deregister_instance(instance *self, void *valptr, const type_info *tinfo); +PYBIND11_NAMESPACE_END(detail) + +// The original core idea for this struct goes back to PyCLIF: +// https://github.com/google/clif/blob/07f95d7e69dca2fcf7022978a55ef3acff506c19/clif/python/runtime.cc#L37 +// URL provided here mainly to give proper credit. To fully explain the `HoldPyObj` feature, more +// context is needed (SMART_HOLDER_WIP). +struct trampoline_self_life_support { + detail::value_and_holder v_h; + + void activate_life_support(const detail::value_and_holder &v_h_) { + Py_INCREF((PyObject *) v_h_.inst); + v_h = v_h_; + } + + void deactivate_life_support() { + Py_DECREF((PyObject *) v_h.inst); + v_h = detail::value_and_holder(); + } + + ~trampoline_self_life_support() { + if (v_h.inst != nullptr && v_h.vh != nullptr) { + void *value_void_ptr = v_h.value_ptr(); + if (value_void_ptr != nullptr) { + PyGILState_STATE threadstate = PyGILState_Ensure(); + v_h.value_ptr() = nullptr; + v_h.holder().release_disowned(); + detail::deregister_instance(v_h.inst, value_void_ptr, v_h.type); + Py_DECREF((PyObject *) v_h.inst); // Must be after deregister. + PyGILState_Release(threadstate); + } + } + } + + // Some compilers complain about implicitly defined versions of some of the following: + trampoline_self_life_support() = default; + trampoline_self_life_support(const trampoline_self_life_support &) = default; + trampoline_self_life_support(trampoline_self_life_support &&) = default; + trampoline_self_life_support &operator=(const trampoline_self_life_support &) = default; + trampoline_self_life_support &operator=(trampoline_self_life_support &&) = default; +}; + +PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) diff --git a/include/pybind11/virtual_overrider_self_life_support.h b/include/pybind11/virtual_overrider_self_life_support.h deleted file mode 100644 index bf82f69a67..0000000000 --- a/include/pybind11/virtual_overrider_self_life_support.h +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright (c) 2021 The Pybind Development Team. -// All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. - -#pragma once - -#include "detail/common.h" -#include "detail/smart_holder_poc.h" -#include "detail/type_caster_base.h" - -PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) - -PYBIND11_NAMESPACE_BEGIN(detail) -// SMART_HOLDER_WIP: Needs refactoring of existing pybind11 code. -inline bool deregister_instance(instance *self, void *valptr, const type_info *tinfo); -PYBIND11_NAMESPACE_END(detail) - -// The original core idea for this struct goes back to PyCLIF: -// https://github.com/google/clif/blob/07f95d7e69dca2fcf7022978a55ef3acff506c19/clif/python/runtime.cc#L37 -// URL provided here mainly to give proper credit. To fully explain the `HoldPyObj` feature, more -// context is needed (SMART_HOLDER_WIP). -struct virtual_overrider_self_life_support { - detail::value_and_holder loaded_v_h; - ~virtual_overrider_self_life_support() { - if (loaded_v_h.inst != nullptr && loaded_v_h.vh != nullptr) { - void *value_void_ptr = loaded_v_h.value_ptr(); - if (value_void_ptr != nullptr) { - PyGILState_STATE threadstate = PyGILState_Ensure(); - loaded_v_h.value_ptr() = nullptr; - loaded_v_h.holder().release_disowned(); - detail::deregister_instance(loaded_v_h.inst, value_void_ptr, loaded_v_h.type); - Py_DECREF((PyObject *) loaded_v_h.inst); // Must be after deregister. - PyGILState_Release(threadstate); - } - } - } - - // Some compilers complain about implicitly defined versions of some of the following: - virtual_overrider_self_life_support() = default; - virtual_overrider_self_life_support(const virtual_overrider_self_life_support &) = default; - virtual_overrider_self_life_support(virtual_overrider_self_life_support &&) = default; - virtual_overrider_self_life_support &operator=(const virtual_overrider_self_life_support &) - = default; - virtual_overrider_self_life_support &operator=(virtual_overrider_self_life_support &&) - = default; -}; - -PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index bf77050510..887503aef9 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -106,11 +106,11 @@ set(PYBIND11_TEST_FILES test_class_sh_disowning_mi.cpp test_class_sh_factory_constructors.cpp test_class_sh_inheritance.cpp + test_class_sh_trampoline_basic.cpp test_class_sh_trampoline_shared_ptr_cpp_arg.cpp test_class_sh_trampoline_unique_ptr.cpp test_class_sh_unique_ptr_member.cpp test_class_sh_virtual_py_cpp_mix.cpp - test_class_sh_with_alias.cpp test_constants_and_functions.cpp test_copy_move.cpp test_custom_type_casters.cpp diff --git a/tests/extra_python_package/test_files.py b/tests/extra_python_package/test_files.py index 3c79ac6382..e912a48fa9 100644 --- a/tests/extra_python_package/test_files.py +++ b/tests/extra_python_package/test_files.py @@ -35,7 +35,7 @@ "include/pybind11/smart_holder.h", "include/pybind11/stl.h", "include/pybind11/stl_bind.h", - "include/pybind11/virtual_overrider_self_life_support.h", + "include/pybind11/trampoline_self_life_support.h", } detail_headers = { diff --git a/tests/pure_cpp/smart_holder_poc_test.cpp b/tests/pure_cpp/smart_holder_poc_test.cpp index 63c22e11b5..18c57fa197 100644 --- a/tests/pure_cpp/smart_holder_poc_test.cpp +++ b/tests/pure_cpp/smart_holder_poc_test.cpp @@ -129,6 +129,20 @@ TEST_CASE("from_raw_ptr_take_ownership+as_shared_ptr", "[S]") { REQUIRE(*new_owner == 19); } +TEST_CASE("from_raw_ptr_take_ownership+disown+reclaim_disowned", "[S]") { + auto hld = smart_holder::from_raw_ptr_take_ownership(new int(19)); + std::unique_ptr new_owner(hld.as_raw_ptr_unowned()); + hld.disown(); + REQUIRE(hld.as_lvalue_ref() == 19); + REQUIRE(*new_owner == 19); + hld.reclaim_disowned(); // Manually veriified: without this, clang++ -fsanitize=address reports + // "detected memory leaks". + new_owner.release(); // Manually verified: without this, clang++ -fsanitize=address reports + // "attempting double-free". + REQUIRE(hld.as_lvalue_ref() == 19); + REQUIRE(new_owner.get() == nullptr); +} + TEST_CASE("from_raw_ptr_take_ownership+disown+release_disowned", "[S]") { auto hld = smart_holder::from_raw_ptr_take_ownership(new int(19)); std::unique_ptr new_owner(hld.as_raw_ptr_unowned()); @@ -139,13 +153,13 @@ TEST_CASE("from_raw_ptr_take_ownership+disown+release_disowned", "[S]") { REQUIRE(!hld.has_pointee()); } -TEST_CASE("from_raw_ptr_take_ownership+disown+ensure_was_not_disowned", "[E]") { +TEST_CASE("from_raw_ptr_take_ownership+disown+ensure_is_not_disowned", "[E]") { const char *context = "test_case"; auto hld = smart_holder::from_raw_ptr_take_ownership(new int(19)); - hld.ensure_was_not_disowned(context); // Does not throw. + hld.ensure_is_not_disowned(context); // Does not throw. std::unique_ptr new_owner(hld.as_raw_ptr_unowned()); hld.disown(); - REQUIRE_THROWS_WITH(hld.ensure_was_not_disowned(context), + REQUIRE_THROWS_WITH(hld.ensure_is_not_disowned(context), "Holder was disowned already (test_case)."); } diff --git a/tests/test_class_sh_disowning.py b/tests/test_class_sh_disowning.py index 0c3de77c6f..f98157138b 100644 --- a/tests/test_class_sh_disowning.py +++ b/tests/test_class_sh_disowning.py @@ -41,7 +41,7 @@ def test_mixed(): # the already disowned obj1a fails as expected. m.mixed(obj1a, obj2b) - def was_disowned(obj): + def is_disowned(obj): try: obj.get() except ValueError: @@ -50,13 +50,13 @@ def was_disowned(obj): # Either obj1b or obj2b was disowned in the expected failed m.mixed() calls above, but not # both. - was_disowned_results = (was_disowned(obj1b), was_disowned(obj2b)) - assert was_disowned_results.count(True) == 1 + is_disowned_results = (is_disowned(obj1b), is_disowned(obj2b)) + assert is_disowned_results.count(True) == 1 if first_pass: first_pass = False print( "\nC++ function argument %d is evaluated first." - % (was_disowned_results.index(True) + 1) + % (is_disowned_results.index(True) + 1) ) return # Comment out for manual leak checking (use `top` command). diff --git a/tests/test_class_sh_disowning_mi.py b/tests/test_class_sh_disowning_mi.py index a236588cae..61cb9c6a25 100644 --- a/tests/test_class_sh_disowning_mi.py +++ b/tests/test_class_sh_disowning_mi.py @@ -18,7 +18,7 @@ def test_diamond_inheritance(): assert d is d.c0().c1().b().c0().b() -def was_disowned(callable_method): +def is_disowned(callable_method): try: callable_method() except ValueError as e: @@ -34,7 +34,7 @@ def test_disown_b(): b = m.B() assert b.get() == 10 m.disown_b(b) - assert was_disowned(b.get) + assert is_disowned(b.get) @pytest.mark.parametrize("var_to_disown", ["c0", "b"]) @@ -43,8 +43,8 @@ def test_disown_c0(var_to_disown): assert c0.get() == 1020 b = c0.b() m.disown_b(locals()[var_to_disown]) - assert was_disowned(c0.get) - assert was_disowned(b.get) + assert is_disowned(c0.get) + assert is_disowned(b.get) @pytest.mark.parametrize("var_to_disown", ["c1", "b"]) @@ -53,8 +53,8 @@ def test_disown_c1(var_to_disown): assert c1.get() == 1021 b = c1.b() m.disown_b(locals()[var_to_disown]) - assert was_disowned(c1.get) - assert was_disowned(b.get) + assert is_disowned(c1.get) + assert is_disowned(b.get) @pytest.mark.parametrize("var_to_disown", ["d", "c1", "c0", "b"]) @@ -65,10 +65,10 @@ def test_disown_d(var_to_disown): c0 = d.c0() c1 = d.c1() m.disown_b(locals()[var_to_disown]) - assert was_disowned(d.get) - assert was_disowned(c1.get) - assert was_disowned(c0.get) - assert was_disowned(b.get) + assert is_disowned(d.get) + assert is_disowned(c1.get) + assert is_disowned(c0.get) + assert is_disowned(b.get) # Based on test_multiple_inheritance.py:test_multiple_inheritance_python. @@ -211,10 +211,10 @@ def test_disown_base1_first(cls, i, j, v): obj = cls(i, j) assert obj.foo() == i assert m.disown_base1(obj) == 2000 * i + 1 - assert was_disowned(obj.foo) + assert is_disowned(obj.foo) assert obj.bar() == j assert m.disown_base2(obj) == 2000 * j + 2 - assert was_disowned(obj.bar) + assert is_disowned(obj.bar) if v is not None: assert obj.v() == v @@ -226,10 +226,10 @@ def test_disown_base2_first(cls, i, j, v): obj = cls(i, j) assert obj.bar() == j assert m.disown_base2(obj) == 2000 * j + 2 - assert was_disowned(obj.bar) + assert is_disowned(obj.bar) assert obj.foo() == i assert m.disown_base1(obj) == 2000 * i + 1 - assert was_disowned(obj.foo) + assert is_disowned(obj.foo) if v is not None: assert obj.v() == v @@ -249,5 +249,5 @@ def test_disown_base2(cls, j, v): obj = cls(j) assert obj.bar() == j assert m.disown_base2(obj) == 2000 * j + 2 - assert was_disowned(obj.bar) + assert is_disowned(obj.bar) assert obj.v() == v diff --git a/tests/test_class_sh_with_alias.cpp b/tests/test_class_sh_trampoline_basic.cpp similarity index 85% rename from tests/test_class_sh_with_alias.cpp rename to tests/test_class_sh_trampoline_basic.cpp index 649eb61f0a..935dafd940 100644 --- a/tests/test_class_sh_with_alias.cpp +++ b/tests/test_class_sh_trampoline_basic.cpp @@ -5,7 +5,7 @@ #include namespace pybind11_tests { -namespace class_sh_with_alias { +namespace class_sh_trampoline_basic { template // Using int as a trick to easily generate a series of types. struct Abase { @@ -35,7 +35,7 @@ struct AbaseAlias : Abase { }; template <> -struct AbaseAlias<1> : Abase<1>, py::virtual_overrider_self_life_support { +struct AbaseAlias<1> : Abase<1>, py::trampoline_self_life_support { using Abase<1>::Abase; int Add(int other_val) const override { @@ -73,14 +73,14 @@ void wrap(py::module_ m, const char *py_class_name) { m.def("AddInCppUniquePtr", AddInCppUniquePtr, py::arg("obj"), py::arg("other_val")); } -} // namespace class_sh_with_alias +} // namespace class_sh_trampoline_basic } // namespace pybind11_tests -PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_with_alias::Abase<0>) -PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_with_alias::Abase<1>) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_trampoline_basic::Abase<0>) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_trampoline_basic::Abase<1>) -TEST_SUBMODULE(class_sh_with_alias, m) { - using namespace pybind11_tests::class_sh_with_alias; +TEST_SUBMODULE(class_sh_trampoline_basic, m) { + using namespace pybind11_tests::class_sh_trampoline_basic; wrap<0>(m, "Abase0"); wrap<1>(m, "Abase1"); } diff --git a/tests/test_class_sh_with_alias.py b/tests/test_class_sh_trampoline_basic.py similarity index 91% rename from tests/test_class_sh_with_alias.py rename to tests/test_class_sh_trampoline_basic.py index 4650eaa5e1..8c82decf8f 100644 --- a/tests/test_class_sh_with_alias.py +++ b/tests/test_class_sh_trampoline_basic.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- import pytest -from pybind11_tests import class_sh_with_alias as m +from pybind11_tests import class_sh_trampoline_basic as m class PyDrvd0(m.Abase0): @@ -45,7 +45,7 @@ def test_drvd0_add_in_cpp_unique_ptr(): assert ( str(exc_info.value) == "Alias class (also known as trampoline) does not inherit from" - " py::virtual_overrider_self_life_support, therefore the ownership of this" + " py::trampoline_self_life_support, therefore the ownership of this" " instance cannot safely be transferred to C++." ) return # Comment out for manual leak checking (use `top` command). diff --git a/tests/test_class_sh_trampoline_unique_ptr.cpp b/tests/test_class_sh_trampoline_unique_ptr.cpp index 375f146575..8a54da8f86 100644 --- a/tests/test_class_sh_trampoline_unique_ptr.cpp +++ b/tests/test_class_sh_trampoline_unique_ptr.cpp @@ -3,14 +3,20 @@ // BSD-style license that can be found in the LICENSE file. #include "pybind11/smart_holder.h" -#include "pybind11/virtual_overrider_self_life_support.h" +#include "pybind11/trampoline_self_life_support.h" #include "pybind11_tests.h" +#include + namespace { class Class { public: - virtual ~Class() = default; + virtual ~Class() = default; + + void setVal(std::uint64_t val) { val_ = val; } + std::uint64_t getVal() const { return val_; } + virtual std::unique_ptr clone() const = 0; virtual int foo() const = 0; @@ -19,6 +25,9 @@ class Class { // Some compilers complain about implicitly defined versions of some of the following: Class(const Class &) = default; + +private: + std::uint64_t val_ = 0; }; } // namespace @@ -27,7 +36,7 @@ PYBIND11_SMART_HOLDER_TYPE_CASTERS(Class) namespace { -class PyClass : public Class, public py::virtual_overrider_self_life_support { +class PyClass : public Class, public py::trampoline_self_life_support { public: std::unique_ptr clone() const override { PYBIND11_OVERRIDE_PURE(std::unique_ptr, Class, clone); @@ -41,8 +50,11 @@ class PyClass : public Class, public py::virtual_overrider_self_life_support { TEST_SUBMODULE(class_sh_trampoline_unique_ptr, m) { py::classh(m, "Class") .def(py::init<>()) + .def("set_val", &Class::setVal) + .def("get_val", &Class::getVal) .def("clone", &Class::clone) .def("foo", &Class::foo); + m.def("clone", [](const Class &obj) { return obj.clone(); }); m.def("clone_and_foo", [](const Class &obj) { return obj.clone()->foo(); }); } diff --git a/tests/test_class_sh_trampoline_unique_ptr.py b/tests/test_class_sh_trampoline_unique_ptr.py index d43cfeb4fd..932db659ef 100644 --- a/tests/test_class_sh_trampoline_unique_ptr.py +++ b/tests/test_class_sh_trampoline_unique_ptr.py @@ -5,13 +5,27 @@ class MyClass(m.Class): def foo(self): - return 10 + return 10 + self.get_val() def clone(self): - return MyClass() + cloned = MyClass() + cloned.set_val(self.get_val() + 3) + return cloned -def test_py_clone_and_foo(): +def test_m_clone(): obj = MyClass() - assert obj.foo() == 10 - assert m.clone_and_foo(obj) == 10 + while True: + obj.set_val(5) + obj = m.clone(obj) + assert obj.get_val() == 5 + 3 + assert obj.foo() == 10 + 5 + 3 + return # Comment out for manual leak checking (use `top` command). + + +def test_m_clone_and_foo(): + obj = MyClass() + obj.set_val(7) + while True: + assert m.clone_and_foo(obj) == 10 + 7 + 3 + return # Comment out for manual leak checking (use `top` command). diff --git a/tests/test_class_sh_virtual_py_cpp_mix.cpp b/tests/test_class_sh_virtual_py_cpp_mix.cpp index 02850fedfa..696368ad45 100644 --- a/tests/test_class_sh_virtual_py_cpp_mix.cpp +++ b/tests/test_class_sh_virtual_py_cpp_mix.cpp @@ -31,13 +31,13 @@ int get_from_cpp_plainc_ptr(const Base *b) { return b->get() + 4000; } int get_from_cpp_unique_ptr(std::unique_ptr b) { return b->get() + 5000; } -struct BaseVirtualOverrider : Base, py::virtual_overrider_self_life_support { +struct BaseVirtualOverrider : Base, py::trampoline_self_life_support { using Base::Base; int get() const override { PYBIND11_OVERRIDE(int, Base, get); } }; -struct CppDerivedVirtualOverrider : CppDerived, py::virtual_overrider_self_life_support { +struct CppDerivedVirtualOverrider : CppDerived, py::trampoline_self_life_support { using CppDerived::CppDerived; int get() const override { PYBIND11_OVERRIDE(int, CppDerived, get); }