diff --git a/include/pybind11/detail/smart_holder_poc.h b/include/pybind11/detail/smart_holder_poc.h index e169f2f26b..0f71586f7e 100644 --- a/include/pybind11/detail/smart_holder_poc.h +++ b/include/pybind11/detail/smart_holder_poc.h @@ -44,15 +44,27 @@ High-level aspects: * 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. + +* Regarding `PYBIND11_TESTS_PURE_CPP_SMART_HOLDER_POC_TEST_CPP` below: + This define serves as a marker for code that is NOT used + from smart_holder_type_casters.h, but is exercised only from + tests/pure_cpp/smart_holder_poc_test.cpp. The marked code was useful + mainly for bootstrapping the smart_holder work. At this stage, with + smart_holder_type_casters.h in production use (at Google) since around + February 2021, it could be moved from here to tests/pure_cpp/ (help welcome). + It will probably be best in most cases to add tests for new functionality + under test/test_class_sh_*. */ #pragma once +#include #include #include #include #include #include +#include // pybindit = Python Bindings Innovation Track. // Currently not in pybind11 namespace to signal that this POC does not depend @@ -68,14 +80,23 @@ static constexpr bool type_has_shared_from_this(const std::enable_shared_from_th } struct guarded_delete { - std::weak_ptr released_ptr; // Trick to keep the smart_holder memory footprint small. - void (*del_ptr)(void *); + std::weak_ptr released_ptr; // Trick to keep the smart_holder memory footprint small. + std::function del_fun; // Rare case. + void (*del_ptr)(void *); // Common case. + bool use_del_fun; bool armed_flag; + guarded_delete(std::function &&del_fun, bool armed_flag) + : del_fun{std::move(del_fun)}, del_ptr{nullptr}, use_del_fun{true}, + armed_flag{armed_flag} {} guarded_delete(void (*del_ptr)(void *), bool armed_flag) - : del_ptr{del_ptr}, armed_flag{armed_flag} {} + : del_ptr{del_ptr}, use_del_fun{false}, armed_flag{armed_flag} {} void operator()(void *raw_ptr) const { if (armed_flag) { - (*del_ptr)(raw_ptr); + if (use_del_fun) { + del_fun(raw_ptr); + } else { + del_ptr(raw_ptr); + } } } }; @@ -99,13 +120,16 @@ guarded_delete make_guarded_builtin_delete(bool armed_flag) { } template -inline void custom_delete(void *raw_ptr) { - D()(static_cast(raw_ptr)); -} +struct custom_deleter { + D deleter; + explicit custom_deleter(D &&deleter) : deleter{std::forward(deleter)} {} + void operator()(void *raw_ptr) { deleter(static_cast(raw_ptr)); } +}; template -guarded_delete make_guarded_custom_deleter(bool armed_flag) { - return guarded_delete(custom_delete, armed_flag); +guarded_delete make_guarded_custom_deleter(D &&uqp_del, bool armed_flag) { + return guarded_delete( + std::function(custom_deleter(std::forward(uqp_del))), armed_flag); } template @@ -232,6 +256,7 @@ struct smart_holder { return static_cast(vptr.get()); } +#ifdef PYBIND11_TESTS_PURE_CPP_SMART_HOLDER_POC_TEST_CPP // See comment near top. template T &as_lvalue_ref() const { static const char *context = "as_lvalue_ref"; @@ -239,7 +264,9 @@ struct smart_holder { ensure_has_pointee(context); return *as_raw_ptr_unowned(); } +#endif +#ifdef PYBIND11_TESTS_PURE_CPP_SMART_HOLDER_POC_TEST_CPP // See comment near top. template T &&as_rvalue_ref() const { static const char *context = "as_rvalue_ref"; @@ -247,6 +274,7 @@ struct smart_holder { ensure_has_pointee(context); return std::move(*as_raw_ptr_unowned()); } +#endif template static smart_holder from_raw_ptr_take_ownership(T *raw_ptr, bool void_cast_raw_ptr = false) { @@ -291,6 +319,7 @@ struct smart_holder { release_disowned(); } +#ifdef PYBIND11_TESTS_PURE_CPP_SMART_HOLDER_POC_TEST_CPP // See comment near top. template T *as_raw_ptr_release_ownership(const char *context = "as_raw_ptr_release_ownership") { ensure_can_release_ownership(context); @@ -298,6 +327,7 @@ struct smart_holder { release_ownership(); return raw_ptr; } +#endif template static smart_holder from_unique_ptr(std::unique_ptr &&unq_ptr, @@ -309,7 +339,7 @@ struct smart_holder { if (hld.vptr_is_using_builtin_delete) { gd = make_guarded_builtin_delete(true); } else { - gd = make_guarded_custom_deleter(true); + gd = make_guarded_custom_deleter(std::move(unq_ptr.get_deleter()), true); } if (void_ptr != nullptr) { hld.vptr.reset(void_ptr, std::move(gd)); @@ -321,6 +351,7 @@ struct smart_holder { return hld; } +#ifdef PYBIND11_TESTS_PURE_CPP_SMART_HOLDER_POC_TEST_CPP // See comment near top. template > std::unique_ptr as_unique_ptr() { static const char *context = "as_unique_ptr"; @@ -328,8 +359,10 @@ struct smart_holder { ensure_use_count_1(context); T *raw_ptr = as_raw_ptr_unowned(); release_ownership(); + // KNOWN DEFECT (see PR #4850): Does not copy the deleter. return std::unique_ptr(raw_ptr); } +#endif template static smart_holder from_shared_ptr(std::shared_ptr shd_ptr) { diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 4bdd669ee1..e8cff2ead5 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -134,6 +134,7 @@ set(PYBIND11_TEST_FILES test_class_sh_trampoline_shared_from_this test_class_sh_trampoline_shared_ptr_cpp_arg test_class_sh_trampoline_unique_ptr + test_class_sh_unique_ptr_custom_deleter test_class_sh_unique_ptr_member test_class_sh_virtual_py_cpp_mix test_class_sh_void_ptr_capsule diff --git a/tests/pure_cpp/smart_holder_poc_test.cpp b/tests/pure_cpp/smart_holder_poc_test.cpp index b0823552f8..f820b9bf68 100644 --- a/tests/pure_cpp/smart_holder_poc_test.cpp +++ b/tests/pure_cpp/smart_holder_poc_test.cpp @@ -1,5 +1,12 @@ +#define PYBIND11_TESTS_PURE_CPP_SMART_HOLDER_POC_TEST_CPP + #include "pybind11/detail/smart_holder_poc.h" +#include +#include +#include +#include + // Catch uses _ internally, which breaks gettext style defines #ifdef _ # undef _ @@ -21,6 +28,15 @@ struct movable_int { template struct functor_builtin_delete { void operator()(T *ptr) { delete ptr; } +#if (defined(__GNUC__) && __GNUC__ == 4 && __GNUC_MINOR__ == 8) \ + || (defined(__clang_major__) && __clang_major__ == 3 && __clang_minor__ == 6) + // Workaround for these errors: + // gcc 4.8.5: too many initializers for 'helpers::functor_builtin_delete' + // clang 3.6: excess elements in struct initializer + functor_builtin_delete() = default; + functor_builtin_delete(const functor_builtin_delete &) {} + functor_builtin_delete(functor_builtin_delete &&) {} +#endif }; template @@ -259,6 +275,14 @@ TEST_CASE("from_unique_ptr_with_deleter+as_lvalue_ref", "[S]") { REQUIRE(hld.as_lvalue_ref() == 19); } +TEST_CASE("from_unique_ptr_with_std_function_deleter+as_lvalue_ref", "[S]") { + std::unique_ptr> orig_owner( + new int(19), [](const int *raw_ptr) { delete raw_ptr; }); + auto hld = smart_holder::from_unique_ptr(std::move(orig_owner)); + REQUIRE(orig_owner.get() == nullptr); + REQUIRE(hld.as_lvalue_ref() == 19); +} + TEST_CASE("from_unique_ptr_with_deleter+as_raw_ptr_release_ownership", "[E]") { std::unique_ptr> orig_owner(new int(19)); auto hld = smart_holder::from_unique_ptr(std::move(orig_owner)); diff --git a/tests/test_class_sh_unique_ptr_custom_deleter.cpp b/tests/test_class_sh_unique_ptr_custom_deleter.cpp new file mode 100644 index 0000000000..070a10e0fb --- /dev/null +++ b/tests/test_class_sh_unique_ptr_custom_deleter.cpp @@ -0,0 +1,40 @@ +#include + +#include "pybind11_tests.h" + +#include + +namespace pybind11_tests { +namespace class_sh_unique_ptr_custom_deleter { + +// Reduced from a PyCLIF use case in the wild by @wangxf123456. +class Pet { +public: + using Ptr = std::unique_ptr>; + + std::string name; + + static Ptr New(const std::string &name) { + return Ptr(new Pet(name), std::default_delete()); + } + +private: + explicit Pet(const std::string &name) : name(name) {} +}; + +} // namespace class_sh_unique_ptr_custom_deleter +} // namespace pybind11_tests + +PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_unique_ptr_custom_deleter::Pet) + +namespace pybind11_tests { +namespace class_sh_unique_ptr_custom_deleter { + +TEST_SUBMODULE(class_sh_unique_ptr_custom_deleter, m) { + py::classh(m, "Pet").def_readwrite("name", &Pet::name); + + m.def("create", &Pet::New); +} + +} // namespace class_sh_unique_ptr_custom_deleter +} // namespace pybind11_tests diff --git a/tests/test_class_sh_unique_ptr_custom_deleter.py b/tests/test_class_sh_unique_ptr_custom_deleter.py new file mode 100644 index 0000000000..d9cf1d4508 --- /dev/null +++ b/tests/test_class_sh_unique_ptr_custom_deleter.py @@ -0,0 +1,6 @@ +from pybind11_tests import class_sh_unique_ptr_custom_deleter as m + + +def test_create(): + pet = m.create("abc") + assert pet.name == "abc"