From f14846dab384f23d63b106259952e6acd94916fa Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 18 Sep 2023 19:29:16 -0700 Subject: [PATCH 01/15] Store `std::function` del_fun; in `guarded_delete` --- include/pybind11/detail/smart_holder_poc.h | 24 +++++++++++----------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/include/pybind11/detail/smart_holder_poc.h b/include/pybind11/detail/smart_holder_poc.h index e169f2f26b..d0140d6dc3 100644 --- a/include/pybind11/detail/smart_holder_poc.h +++ b/include/pybind11/detail/smart_holder_poc.h @@ -48,6 +48,7 @@ High-level aspects: #pragma once +#include #include #include #include @@ -67,15 +68,17 @@ static constexpr bool type_has_shared_from_this(const std::enable_shared_from_th return true; } +struct smart_holder; + struct guarded_delete { std::weak_ptr released_ptr; // Trick to keep the smart_holder memory footprint small. - void (*del_ptr)(void *); + std::function del_fun; bool armed_flag; - guarded_delete(void (*del_ptr)(void *), bool armed_flag) - : del_ptr{del_ptr}, armed_flag{armed_flag} {} + guarded_delete(std::function &&del_fun, bool armed_flag) + : del_fun{std::move(del_fun)}, armed_flag{armed_flag} {} void operator()(void *raw_ptr) const { if (armed_flag) { - (*del_ptr)(raw_ptr); + del_fun(raw_ptr); } } }; @@ -99,13 +102,10 @@ guarded_delete make_guarded_builtin_delete(bool armed_flag) { } template -inline void custom_delete(void *raw_ptr) { - D()(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( + [&uqp_del](void *raw_ptr) { uqp_del(static_cast(raw_ptr)); }), + armed_flag); } template @@ -309,7 +309,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)); From 24bf40bb3eca2ec74871156ffc1bdcc61de80b26 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 18 Sep 2023 20:14:35 -0700 Subject: [PATCH 02/15] Specialize the simple common case. Using a `union` is complicated: https://en.cppreference.com/w/cpp/language/union > If members of a union are classes with user-defined constructors and destructors, to switch the active member, explicit destructor and placement new are generally needed: Using `std::variant` increases compile-time overhead. It is currently unclear how much these effects matter in practice: optimization left for later. --- include/pybind11/detail/smart_holder_poc.h | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/include/pybind11/detail/smart_holder_poc.h b/include/pybind11/detail/smart_holder_poc.h index d0140d6dc3..2b333b0eef 100644 --- a/include/pybind11/detail/smart_holder_poc.h +++ b/include/pybind11/detail/smart_holder_poc.h @@ -71,14 +71,23 @@ static constexpr bool type_has_shared_from_this(const std::enable_shared_from_th struct smart_holder; struct guarded_delete { - std::weak_ptr released_ptr; // Trick to keep the smart_holder memory footprint small. - std::function del_fun; + 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)}, armed_flag{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}, use_del_fun{false}, armed_flag{armed_flag} {} void operator()(void *raw_ptr) const { if (armed_flag) { - del_fun(raw_ptr); + if (use_del_fun) { + del_fun(raw_ptr); + } else { + del_ptr(raw_ptr); + } } } }; From 884fe1cd52d0cfecefa95cb4523865bc82b2fede Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 18 Sep 2023 20:29:37 -0700 Subject: [PATCH 03/15] Add one test case (more later). --- tests/pure_cpp/smart_holder_poc_test.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/pure_cpp/smart_holder_poc_test.cpp b/tests/pure_cpp/smart_holder_poc_test.cpp index b0823552f8..1bb8461bb8 100644 --- a/tests/pure_cpp/smart_holder_poc_test.cpp +++ b/tests/pure_cpp/smart_holder_poc_test.cpp @@ -259,6 +259,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), [](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)); From 435d3864a17b39347fed9343863386fdb47573f6 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 18 Sep 2023 22:30:18 -0700 Subject: [PATCH 04/15] Add `const` to resolve clang-tidy error. ``` -- The CXX compiler identification is Clang 15.0.7 /usr/bin/cmake -E __run_co_compile --tidy="/usr/bin/clang-tidy;--use-color;--warnings-as-errors=*;--extra-arg-before=--driver-mode=g++" --source=/__w/pybind11/pybind11/tests/test_class_sh_inheritance.cpp -- /usr/bin/c++ -DPYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD_IF_AVAILABLE -DPYBIND11_TEST_EIGEN -Dpybind11_tests_EXPORTS -I/__w/pybind11/pybind11/include -isystem /usr/include/python3.9 -isystem /__w/pybind11/pybind11/build/_deps/eigen-src -Os -DNDEBUG -fPIC -fvisibility=hidden -Wall -Wextra -Wconversion -Wcast-qual -Wdeprecated -Wundef -Wnon-virtual-dtor -flto=thin -std=c++17 -o CMakeFiles/pybind11_tests.dir/test_class_sh_inheritance.cpp.o -c /__w/pybind11/pybind11/tests/test_class_sh_inheritance.cpp /__w/pybind11/pybind11/tests/pure_cpp/smart_holder_poc_test.cpp:264:30: error: pointer parameter 'raw_ptr' can be pointer to const [readability-non-const-parameter,-warnings-as-errors] new int(19), [](int *raw_ptr) { delete raw_ptr; }); ^ const ``` --- tests/pure_cpp/smart_holder_poc_test.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/pure_cpp/smart_holder_poc_test.cpp b/tests/pure_cpp/smart_holder_poc_test.cpp index 1bb8461bb8..d855f4878d 100644 --- a/tests/pure_cpp/smart_holder_poc_test.cpp +++ b/tests/pure_cpp/smart_holder_poc_test.cpp @@ -260,8 +260,8 @@ TEST_CASE("from_unique_ptr_with_deleter+as_lvalue_ref", "[S]") { } TEST_CASE("from_unique_ptr_with_std_function_deleter+as_lvalue_ref", "[S]") { - std::unique_ptr> orig_owner( - new int(19), [](int *raw_ptr) { delete raw_ptr; }); + 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); From 17098eb760c5d5ecf5be56dad6e8c9906805d9d7 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Thu, 21 Sep 2023 21:15:45 -0700 Subject: [PATCH 05/15] Introduce `struct custom_deleter` to ensure the deleter is moved as desired (the lambda function only captures a reference, which can become dangling). --- include/pybind11/detail/smart_holder_poc.h | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/include/pybind11/detail/smart_holder_poc.h b/include/pybind11/detail/smart_holder_poc.h index 2b333b0eef..ed6f322b45 100644 --- a/include/pybind11/detail/smart_holder_poc.h +++ b/include/pybind11/detail/smart_holder_poc.h @@ -68,8 +68,6 @@ static constexpr bool type_has_shared_from_this(const std::enable_shared_from_th return true; } -struct smart_holder; - struct guarded_delete { std::weak_ptr released_ptr; // Trick to keep the smart_holder memory footprint small. std::function del_fun; // Rare case. @@ -110,10 +108,16 @@ guarded_delete make_guarded_builtin_delete(bool armed_flag) { return guarded_delete(builtin_delete_if_destructible, armed_flag); } +template +struct custom_deleter { + D deleter; + custom_deleter(D &&deleter) : deleter{std::move(deleter)} {} + void operator()(void *raw_ptr) { deleter(static_cast(raw_ptr)); } +}; + template guarded_delete make_guarded_custom_deleter(D &&uqp_del, bool armed_flag) { - return guarded_delete(std::function( - [&uqp_del](void *raw_ptr) { uqp_del(static_cast(raw_ptr)); }), + return guarded_delete(std::function(custom_deleter(std::move(uqp_del))), armed_flag); } From 6083f8adbb38b3df9d61965df1795cb3b3634b62 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Thu, 21 Sep 2023 22:04:59 -0700 Subject: [PATCH 06/15] Resolve helpful clang-tidy errors. ``` /usr/bin/cmake -E __run_co_compile --tidy="/usr/bin/clang-tidy;--use-color;--warnings-as-errors=*;--extra-arg-before=--driver-mode=g++" --source=/__w/pybind11/pybind11/tests/test_class.cpp -- /usr/bin/c++ -DPYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD_IF_AVAILABLE -DPYBIND11_TEST_EIGEN -Dpybind11_tests_EXPORTS -I/__w/pybind11/pybind11/include -isystem /usr/include/python3.9 -isystem /__w/pybind11/pybind11/build/_deps/eigen-src -Os -DNDEBUG -fPIC -fvisibility=hidden -Wall -Wextra -Wconversion -Wcast-qual -Wdeprecated -Wundef -Wnon-virtual-dtor -flto=thin -std=c++17 -o CMakeFiles/pybind11_tests.dir/test_class.cpp.o -c /__w/pybind11/pybind11/tests/test_class.cpp /__w/pybind11/pybind11/include/pybind11/detail/smart_holder_poc.h:114:5: error: single-argument constructors must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor,-warnings-as-errors] custom_deleter(D &&deleter) : deleter{std::move(deleter)} {} ^ explicit /__w/pybind11/pybind11/include/pybind11/detail/smart_holder_poc.h:120:76: error: forwarding reference passed to std::move(), which may unexpectedly cause lvalues to be moved; use std::forward() instead [bugprone-move-forwarding-reference,-warnings-as-errors] return guarded_delete(std::function(custom_deleter(std::move(uqp_del))), ^~~~~~~~~ std::forward ``` --- include/pybind11/detail/smart_holder_poc.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/pybind11/detail/smart_holder_poc.h b/include/pybind11/detail/smart_holder_poc.h index ed6f322b45..65b0581617 100644 --- a/include/pybind11/detail/smart_holder_poc.h +++ b/include/pybind11/detail/smart_holder_poc.h @@ -111,14 +111,14 @@ guarded_delete make_guarded_builtin_delete(bool armed_flag) { template struct custom_deleter { D deleter; - custom_deleter(D &&deleter) : deleter{std::move(deleter)} {} + explicit custom_deleter(D &&deleter) : deleter{std::move(deleter)} {} void operator()(void *raw_ptr) { deleter(static_cast(raw_ptr)); } }; template guarded_delete make_guarded_custom_deleter(D &&uqp_del, bool armed_flag) { - return guarded_delete(std::function(custom_deleter(std::move(uqp_del))), - armed_flag); + return guarded_delete( + std::function(custom_deleter(std::forward(uqp_del))), armed_flag); } template From d815d7d321f78136b3df8b3a1c888871c0ffb3e1 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 22 Sep 2023 03:35:23 -0700 Subject: [PATCH 07/15] Workaround for gcc 4.8.5, clang 3.6 --- tests/pure_cpp/smart_holder_poc_test.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/pure_cpp/smart_holder_poc_test.cpp b/tests/pure_cpp/smart_holder_poc_test.cpp index d855f4878d..a10e0ad362 100644 --- a/tests/pure_cpp/smart_holder_poc_test.cpp +++ b/tests/pure_cpp/smart_holder_poc_test.cpp @@ -21,6 +21,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 From c684f6cec00d5a8536f070883b6bb326fefc9bae Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Tue, 31 Oct 2023 23:54:06 -0700 Subject: [PATCH 08/15] Transfer reduced test here. Reduced from a PyCLIF use case in the wild by @wangxf123456 (internal change cl/565476030). --- tests/CMakeLists.txt | 1 + ...est_class_sh_unique_ptr_custom_deleter.cpp | 40 +++++++++++++++++++ ...test_class_sh_unique_ptr_custom_deleter.py | 6 +++ 3 files changed, 47 insertions(+) create mode 100644 tests/test_class_sh_unique_ptr_custom_deleter.cpp create mode 100644 tests/test_class_sh_unique_ptr_custom_deleter.py 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/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" From a685d57e45897db746d4066b9d4be9004bd1c79d Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Wed, 1 Nov 2023 00:33:12 -0700 Subject: [PATCH 09/15] Add missing include (clangd Include Cleaner) --- include/pybind11/detail/smart_holder_poc.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/pybind11/detail/smart_holder_poc.h b/include/pybind11/detail/smart_holder_poc.h index 65b0581617..91873ab346 100644 --- a/include/pybind11/detail/smart_holder_poc.h +++ b/include/pybind11/detail/smart_holder_poc.h @@ -54,6 +54,7 @@ High-level aspects: #include #include #include +#include // pybindit = Python Bindings Innovation Track. // Currently not in pybind11 namespace to signal that this POC does not depend From 8a973d484f8ce73efb321b2a097f40439cb6af7c Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Wed, 1 Nov 2023 07:08:55 -0700 Subject: [PATCH 10/15] Change `std::move` to `std::forward` as suggested by @iwanders. --- include/pybind11/detail/smart_holder_poc.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/detail/smart_holder_poc.h b/include/pybind11/detail/smart_holder_poc.h index 91873ab346..658657dc57 100644 --- a/include/pybind11/detail/smart_holder_poc.h +++ b/include/pybind11/detail/smart_holder_poc.h @@ -112,7 +112,7 @@ guarded_delete make_guarded_builtin_delete(bool armed_flag) { template struct custom_deleter { D deleter; - explicit custom_deleter(D &&deleter) : deleter{std::move(deleter)} {} + explicit custom_deleter(D &&deleter) : deleter{std::forward(deleter)} {} void operator()(void *raw_ptr) { deleter(static_cast(raw_ptr)); } }; From 4fb7d12813a4e8bb62e5cc8589eb43b0d7f47ad4 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Wed, 1 Nov 2023 07:32:21 -0700 Subject: [PATCH 11/15] Add missing includes (clangd Include Cleaner) --- tests/pure_cpp/smart_holder_poc_test.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/pure_cpp/smart_holder_poc_test.cpp b/tests/pure_cpp/smart_holder_poc_test.cpp index a10e0ad362..1eaef90094 100644 --- a/tests/pure_cpp/smart_holder_poc_test.cpp +++ b/tests/pure_cpp/smart_holder_poc_test.cpp @@ -1,5 +1,10 @@ #include "pybind11/detail/smart_holder_poc.h" +#include +#include +#include +#include + // Catch uses _ internally, which breaks gettext style defines #ifdef _ # undef _ From 4d9d722215cce2a5aabc8a894208dc5c1fdf2182 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Wed, 1 Nov 2023 08:12:28 -0700 Subject: [PATCH 12/15] Use new `PYBIND11_TESTS_PURE_CPP_SMART_HOLDER_POC_TEST_CPP` to exclude `smart_holder::as_unique_ptr` method from production code. --- include/pybind11/detail/smart_holder_poc.h | 2 ++ tests/pure_cpp/smart_holder_poc_test.cpp | 2 ++ 2 files changed, 4 insertions(+) diff --git a/include/pybind11/detail/smart_holder_poc.h b/include/pybind11/detail/smart_holder_poc.h index 658657dc57..d9eda2f770 100644 --- a/include/pybind11/detail/smart_holder_poc.h +++ b/include/pybind11/detail/smart_holder_poc.h @@ -335,6 +335,7 @@ struct smart_holder { return hld; } +#ifdef PYBIND11_TESTS_PURE_CPP_SMART_HOLDER_POC_TEST_CPP template > std::unique_ptr as_unique_ptr() { static const char *context = "as_unique_ptr"; @@ -344,6 +345,7 @@ struct smart_holder { release_ownership(); return std::unique_ptr(raw_ptr); } +#endif template static smart_holder from_shared_ptr(std::shared_ptr shd_ptr) { diff --git a/tests/pure_cpp/smart_holder_poc_test.cpp b/tests/pure_cpp/smart_holder_poc_test.cpp index 1eaef90094..f820b9bf68 100644 --- a/tests/pure_cpp/smart_holder_poc_test.cpp +++ b/tests/pure_cpp/smart_holder_poc_test.cpp @@ -1,3 +1,5 @@ +#define PYBIND11_TESTS_PURE_CPP_SMART_HOLDER_POC_TEST_CPP + #include "pybind11/detail/smart_holder_poc.h" #include From 7669a5bea87064d52128550a81bfbd64da377da5 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Wed, 1 Nov 2023 14:40:14 -0700 Subject: [PATCH 13/15] Systematically add `PYBIND11_TESTS_PURE_CPP_SMART_HOLDER_POC_TEST_CPP` to mark code that is not used from production code. Add comment to explain. --- include/pybind11/detail/smart_holder_poc.h | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/include/pybind11/detail/smart_holder_poc.h b/include/pybind11/detail/smart_holder_poc.h index d9eda2f770..0f71586f7e 100644 --- a/include/pybind11/detail/smart_holder_poc.h +++ b/include/pybind11/detail/smart_holder_poc.h @@ -44,6 +44,16 @@ 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 @@ -246,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"; @@ -253,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"; @@ -261,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) { @@ -305,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); @@ -312,6 +327,7 @@ struct smart_holder { release_ownership(); return raw_ptr; } +#endif template static smart_holder from_unique_ptr(std::unique_ptr &&unq_ptr, @@ -335,7 +351,7 @@ struct smart_holder { return hld; } -#ifdef PYBIND11_TESTS_PURE_CPP_SMART_HOLDER_POC_TEST_CPP +#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"; @@ -343,6 +359,7 @@ 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 From fe59369f408d354edef16e3d40e2f90d9c2a9ba8 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Wed, 1 Nov 2023 15:35:04 -0700 Subject: [PATCH 14/15] Very simple experiment related to https://github.com/pybind/pybind11/pull/4850#issuecomment-1789780676 Does the `PYBIND11_TESTS_PURE_CPP_SMART_HOLDER_POC_TEST_CPP` define have anything to do with it? --- include/pybind11/detail/smart_holder_poc.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/include/pybind11/detail/smart_holder_poc.h b/include/pybind11/detail/smart_holder_poc.h index 0f71586f7e..b86c5f3b18 100644 --- a/include/pybind11/detail/smart_holder_poc.h +++ b/include/pybind11/detail/smart_holder_poc.h @@ -58,6 +58,12 @@ High-level aspects: #pragma once +#if defined(_MSC_VER) && !defined(PYBIND11_TESTS_PURE_CPP_SMART_HOLDER_POC_TEST_CPP) +// Experiment related to +// https://github.com/pybind/pybind11/pull/4850#issuecomment-1789780676 +# define PYBIND11_TESTS_PURE_CPP_SMART_HOLDER_POC_TEST_CPP +#endif + #include #include #include From 821e13a5a45f7d3f459a304392653faf79d59cec Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Wed, 1 Nov 2023 16:59:25 -0700 Subject: [PATCH 15/15] Revert "Very simple experiment related to https://github.com/pybind/pybind11/pull/4850#issuecomment-1789780676" This reverts commit fe59369f408d354edef16e3d40e2f90d9c2a9ba8. --- include/pybind11/detail/smart_holder_poc.h | 6 ------ 1 file changed, 6 deletions(-) diff --git a/include/pybind11/detail/smart_holder_poc.h b/include/pybind11/detail/smart_holder_poc.h index b86c5f3b18..0f71586f7e 100644 --- a/include/pybind11/detail/smart_holder_poc.h +++ b/include/pybind11/detail/smart_holder_poc.h @@ -58,12 +58,6 @@ High-level aspects: #pragma once -#if defined(_MSC_VER) && !defined(PYBIND11_TESTS_PURE_CPP_SMART_HOLDER_POC_TEST_CPP) -// Experiment related to -// https://github.com/pybind/pybind11/pull/4850#issuecomment-1789780676 -# define PYBIND11_TESTS_PURE_CPP_SMART_HOLDER_POC_TEST_CPP -#endif - #include #include #include