std::unique_ptr deleter unit tests.

include/pybind11/detail/smart_holder_poc.h

@@ -68,6 +68,23 @@ static constexpr bool type_has_shared_from_this(const std::enable_shared_from_th
     return true;
 }
 
+namespace detail {
+template <typename D, bool> struct delete_assigner {
+  static bool assign(D&, bool, std::function<void(void*)>&, void(*)(void*)&){ return false; }
+};
+
+template <typename D> struct delete_assigner<D, true> {
+  static bool assign(D& deleter, bool use_del_fun, std::function<void(void*)>& del_fun, void(*del_ptr)(void*)&){
+    if (use_del_fun) {
+      deleter = std::move(del_fun);
+    } else {
+      deleter = del_ptr;
+    }
+    return true;
+  }
+};
+}
+
 struct guarded_delete {
     std::weak_ptr<void> released_ptr;    // Trick to keep the smart_holder memory footprint small.
     std::function<void(void *)> del_fun; // Rare case.
@@ -88,6 +105,19 @@ struct guarded_delete {
             }
         }
     }
+
+    template <typename D>
+    bool extract_deleter(D& deleter) {
+      if (armed_flag) {
+        // Just doing a direct assignment here like; 'deleter = del_ptr' doesn't work in case
+        // the deleter is of a type non-constructible from the custom deleter like
+        // helpers::functor_other_delete<int>’, so we use a simple trait system to assign if the
+        // types allow it.
+        armed_flag = false;
+        return detail::delete_assigner<D, std::is_constructible<D, decltype(del_fun)>::value>::assign(deleter, use_del_fun, del_fun, del_ptr);
+      }
+      return false;
+    }
 };
 
 template <typename T, typename std::enable_if<std::is_destructible<T>::value, int>::type = 0>
@@ -340,7 +370,23 @@ struct smart_holder {
         ensure_compatible_rtti_uqp_del<T, D>(context);
         ensure_use_count_1(context);
         T *raw_ptr = as_raw_ptr_unowned<T>();
+
+        // Temporary storage of the deleter function if it exists in the holder, places the
+        // requirement that the deleter is default constructible...
+        D extracted_deleter;
+
+        // Extract the deleter function if it exists.
+        auto *gd = std::get_deleter<guarded_delete>(vptr);
+        const bool found_deleter = gd && gd->extract_deleter<D>(extracted_deleter);
+
         release_ownership();
+
+        // If we have a deleter, pass that onto the unique pointer.
+        if (found_deleter) {
+          return std::unique_ptr<T, D>(raw_ptr, std::move(extracted_deleter));
+        }
+
+        // No special destructor.
         return std::unique_ptr<T, D>(raw_ptr);
     }
 

tests/pure_cpp/CMakeLists.txt

@@ -9,6 +9,7 @@ else()
 endif()
 
 add_executable(smart_holder_poc_test smart_holder_poc_test.cpp)
+target_compile_options(smart_holder_poc_test PRIVATE "-g")
 pybind11_enable_warnings(smart_holder_poc_test)
 target_link_libraries(smart_holder_poc_test PRIVATE pybind11::headers Catch2::Catch2)
 

tests/pure_cpp/smart_holder_poc_test.cpp

@@ -276,6 +276,23 @@ TEST_CASE("from_unique_ptr_with_std_function_deleter+as_lvalue_ref", "[S]") {
     REQUIRE(hld.as_lvalue_ref<int>() == 19);
 }
 
+TEST_CASE("from_unique_ptr_with_std_function_deleter+as_lvalue_ref+destructor_called", "[S]") {
+    bool destructor_called = false;
+    std::unique_ptr<int, std::function<void(const int *)>> orig_owner(
+        new int(19),
+        [&destructor_called](const int *raw_ptr) { destructor_called = true; 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);
+
+      // At this point, the destructor shouldn't have been called yet, it is moved to the holder.
+      REQUIRE(destructor_called == false);
+    }
+    // Smart holder goes out of scope, the original destructor should have been called.
+    REQUIRE(destructor_called);
+}
+
 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));
@@ -284,6 +301,26 @@ TEST_CASE("from_unique_ptr_with_deleter+as_raw_ptr_release_ownership", "[E]") {
                         "Cannot disown custom deleter (as_raw_ptr_release_ownership).");
 }
 
+TEST_CASE("from_unique_ptr_with_std_function_deleter+as_raw_ptr_release_ownership", "[S]") {
+    bool destructor_called = false;
+    std::unique_ptr<int, std::function<void(const int *)>> orig_owner(
+        new int(19),
+        [&destructor_called](const int *raw_ptr) { destructor_called = true; delete raw_ptr; });
+    {
+      auto hld = smart_holder::from_unique_ptr(std::move(orig_owner));
+      REQUIRE(orig_owner.get() == nullptr);
+
+      // Destructor shouldn't be called during the moving to the smart holder.
+      REQUIRE(destructor_called == false);
+
+      REQUIRE_THROWS_WITH(hld.as_raw_ptr_release_ownership<int>(),
+                          "Cannot disown custom deleter (as_raw_ptr_release_ownership).");
+
+    }
+    // Smart holder goes out of scope, the original destructor should have been called.
+    REQUIRE(destructor_called);
+}
+
 TEST_CASE("from_unique_ptr_with_deleter+as_unique_ptr", "[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));
@@ -302,6 +339,26 @@ TEST_CASE("from_unique_ptr_with_deleter+as_unique_ptr_with_deleter1", "[S]") {
     REQUIRE(*new_owner == 19);
 }
 
+
+TEST_CASE("from_unique_ptr_std_function_with_deleter+as_unique_ptr_with_std_function_deleter1", "[S]") {
+    bool destructor_called = false;
+    using OpaqueDeleter = std::function<void(void *)>;
+
+    std::unique_ptr<int, OpaqueDeleter> orig_owner(
+        new int(19),
+        OpaqueDeleter([&destructor_called](const void *raw_ptr) { destructor_called = true; delete static_cast<const int*>(raw_ptr); }));
+    auto hld = smart_holder::from_unique_ptr(std::move(orig_owner));
+    REQUIRE(orig_owner.get() == nullptr);
+    std::unique_ptr<int, OpaqueDeleter> new_owner = hld.as_unique_ptr<int, OpaqueDeleter>();
+    REQUIRE(!hld.has_pointee());
+    REQUIRE(*new_owner == 19);
+
+    REQUIRE(destructor_called == false);
+    new_owner.reset();
+    REQUIRE(destructor_called);
+}
+
+
 TEST_CASE("from_unique_ptr_with_deleter+as_unique_ptr_with_deleter2", "[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));