git merge smart_holder (pybind/pybind11#4917)

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 <functional>
 #include <memory>
 #include <stdexcept>
 #include <string>
 #include <type_traits>
 #include <typeinfo>
+#include <utility>
 
 // 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<void> released_ptr; // Trick to keep the smart_holder memory footprint small.
-    void (*del_ptr)(void *);
+    std::weak_ptr<void> released_ptr;    // Trick to keep the smart_holder memory footprint small.
+    std::function<void(void *)> del_fun; // Rare case.
+    void (*del_ptr)(void *);             // Common case.
+    bool use_del_fun;
     bool armed_flag;
+    guarded_delete(std::function<void(void *)> &&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 <typename T, typename D>
-inline void custom_delete(void *raw_ptr) {
-    D()(static_cast<T *>(raw_ptr));
-}
+struct custom_deleter {
+    D deleter;
+    explicit custom_deleter(D &&deleter) : deleter{std::forward<D>(deleter)} {}
+    void operator()(void *raw_ptr) { deleter(static_cast<T *>(raw_ptr)); }
+};
 
 template <typename T, typename D>
-guarded_delete make_guarded_custom_deleter(bool armed_flag) {
-    return guarded_delete(custom_delete<T, D>, armed_flag);
+guarded_delete make_guarded_custom_deleter(D &&uqp_del, bool armed_flag) {
+    return guarded_delete(
+        std::function<void(void *)>(custom_deleter<T, D>(std::forward<D>(uqp_del))), armed_flag);
 }
 
 template <typename T>
@@ -232,21 +256,25 @@ struct smart_holder {
         return static_cast<T *>(vptr.get());
     }
 
+#ifdef PYBIND11_TESTS_PURE_CPP_SMART_HOLDER_POC_TEST_CPP // See comment near top.
     template <typename T>
     T &as_lvalue_ref() const {
         static const char *context = "as_lvalue_ref";
         ensure_is_populated(context);
         ensure_has_pointee(context);
         return *as_raw_ptr_unowned<T>();
     }
+#endif
 
+#ifdef PYBIND11_TESTS_PURE_CPP_SMART_HOLDER_POC_TEST_CPP // See comment near top.
     template <typename T>
     T &&as_rvalue_ref() const {
         static const char *context = "as_rvalue_ref";
         ensure_is_populated(context);
         ensure_has_pointee(context);
         return std::move(*as_raw_ptr_unowned<T>());
     }
+#endif
 
     template <typename T>
     static smart_holder from_raw_ptr_take_ownership(T *raw_ptr, bool void_cast_raw_ptr = false) {
@@ -291,13 +319,15 @@ struct smart_holder {
         release_disowned();
     }
 
+#ifdef PYBIND11_TESTS_PURE_CPP_SMART_HOLDER_POC_TEST_CPP // See comment near top.
     template <typename T>
     T *as_raw_ptr_release_ownership(const char *context = "as_raw_ptr_release_ownership") {
         ensure_can_release_ownership(context);
         T *raw_ptr = as_raw_ptr_unowned<T>();
         release_ownership();
         return raw_ptr;
     }
+#endif
 
     template <typename T, typename D>
     static smart_holder from_unique_ptr(std::unique_ptr<T, D> &&unq_ptr,
@@ -309,7 +339,7 @@ struct smart_holder {
         if (hld.vptr_is_using_builtin_delete) {
             gd = make_guarded_builtin_delete<T>(true);
         } else {
-            gd = make_guarded_custom_deleter<T, D>(true);
+            gd = make_guarded_custom_deleter<T, D>(std::move(unq_ptr.get_deleter()), true);
         }
         if (void_ptr != nullptr) {
             hld.vptr.reset(void_ptr, std::move(gd));
@@ -321,15 +351,18 @@ struct smart_holder {
         return hld;
     }
 
+#ifdef PYBIND11_TESTS_PURE_CPP_SMART_HOLDER_POC_TEST_CPP // See comment near top.
     template <typename T, typename D = std::default_delete<T>>
     std::unique_ptr<T, D> as_unique_ptr() {
         static const char *context = "as_unique_ptr";
         ensure_compatible_rtti_uqp_del<T, D>(context);
         ensure_use_count_1(context);
         T *raw_ptr = as_raw_ptr_unowned<T>();
         release_ownership();
+        // KNOWN DEFECT (see PR #4850): Does not copy the deleter.
         return std::unique_ptr<T, D>(raw_ptr);
     }
+#endif
 
     template <typename T>
     static smart_holder from_shared_ptr(std::shared_ptr<T> shd_ptr) {

tests/CMakeLists.txt

@@ -135,6 +135,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

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 <functional>
+#include <memory>
+#include <type_traits>
+#include <utility>
+
 // Catch uses _ internally, which breaks gettext style defines
 #ifdef _
 #    undef _
@@ -21,6 +28,15 @@ struct movable_int {
 template <typename T>
 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<int>'
+    // 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 <typename T>
@@ -259,6 +275,14 @@ TEST_CASE("from_unique_ptr_with_deleter+as_lvalue_ref", "[S]") {
     REQUIRE(hld.as_lvalue_ref<int>() == 19);
 }
 
+TEST_CASE("from_unique_ptr_with_std_function_deleter+as_lvalue_ref", "[S]") {
+    std::unique_ptr<int, std::function<void(const int *)>> 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<int>() == 19);
+}
+
 TEST_CASE("from_unique_ptr_with_deleter+as_raw_ptr_release_ownership", "[E]") {
     std::unique_ptr<int, helpers::functor_builtin_delete<int>> orig_owner(new int(19));
     auto hld = smart_holder::from_unique_ptr(std::move(orig_owner));

tests/test_class_sh_unique_ptr_custom_deleter.cpp

@@ -0,0 +1,40 @@
+#include <pybind11/smart_holder.h>
+
+#include "pybind11_tests.h"
+
+#include <memory>
+
+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<Pet, std::function<void(Pet *)>>;
+
+    std::string name;
+
+    static Ptr New(const std::string &name) {
+        return Ptr(new Pet(name), std::default_delete<Pet>());
+    }
+
+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<Pet>(m, "Pet").def_readwrite("name", &Pet::name);
+
+    m.def("create", &Pet::New);
+}
+
+} // namespace class_sh_unique_ptr_custom_deleter
+} // namespace pybind11_tests

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"

tools/pybind11NewTools.cmake

@@ -32,12 +32,22 @@ if(NOT Python_FOUND AND NOT Python3_FOUND)
     set(Python_ROOT_DIR "$ENV{pythonLocation}")
   endif()
 
-  find_package(Python 3.6 REQUIRED COMPONENTS Interpreter Development ${_pybind11_quiet})
+  # Development.Module support (required for manylinux) started in 3.18
+  if(CMAKE_VERSION VERSION_LESS 3.18)
+    set(_pybind11_dev_component Development)
+  else()
+    set(_pybind11_dev_component Development.Module OPTIONAL_COMPONENTS Development.Embed)
+  endif()
+
+  find_package(Python 3.6 REQUIRED COMPONENTS Interpreter ${_pybind11_dev_component}
+                                              ${_pybind11_quiet})
 
   # If we are in submodule mode, export the Python targets to global targets.
   # If this behavior is not desired, FindPython _before_ pybind11.
   if(NOT is_config)
-    set_property(TARGET Python::Python PROPERTY IMPORTED_GLOBAL TRUE)
+    if(TARGET Python::Python)
+      set_property(TARGET Python::Python PROPERTY IMPORTED_GLOBAL TRUE)
+    endif()
     set_property(TARGET Python::Interpreter PROPERTY IMPORTED_GLOBAL TRUE)
     if(TARGET Python::Module)
       set_property(TARGET Python::Module PROPERTY IMPORTED_GLOBAL TRUE)

tools/pybind11Tools.cmake

@@ -43,7 +43,7 @@ endif()
 
 # A user can set versions manually too
 set(Python_ADDITIONAL_VERSIONS
-    "3.11;3.10;3.9;3.8;3.7;3.6"
+    "3.12;3.11;3.10;3.9;3.8;3.7;3.6"
     CACHE INTERNAL "")
 
 list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}")