Cleanup cast() functions of smart_holder_type_caster<std::unique_ptr<T, D>>

- forbid to pass a non-const unique_ptr reference
- forbid return_value_policy::reference_internal for unique_ptr&&:
  It's always moved.

Author: rhaschke

include/pybind11/detail/smart_holder_type_casters.h

@@ -677,12 +677,10 @@ struct smart_holder_type_caster<std::unique_ptr<T, D>> : smart_holder_type_caste
                                                          smart_holder_type_caster_class_hooks {
     static constexpr auto name = _<std::unique_ptr<T, D>>();
 
-    static handle cast(std::unique_ptr<T, D> &&src, return_value_policy policy, handle parent) {
-        if (policy != return_value_policy::automatic
-            && policy != return_value_policy::reference_internal
-            && policy != return_value_policy::move) {
+    static handle cast(std::unique_ptr<T, D> &&src, return_value_policy policy, handle) {
+        if (policy != return_value_policy::automatic && policy != return_value_policy::move) {
             // SMART_HOLDER_WIP: IMPROVABLE: Error message.
-            throw cast_error("Invalid return_value_policy for unique_ptr.");
+            throw cast_error("Invalid return_value_policy: unique_ptr&& can only move");
         }
 
         auto src_raw_ptr = src.get();
@@ -704,19 +702,23 @@ struct smart_holder_type_caster<std::unique_ptr<T, D>> : smart_holder_type_caste
         auto smhldr = pybindit::memory::smart_holder::from_unique_ptr(std::move(src));
         tinfo->init_instance(inst_raw_ptr, static_cast<const void *>(&smhldr));
 
-        if (policy == return_value_policy::reference_internal)
-            keep_alive_impl(inst, parent);
-
         return inst.release();
     }
+    static handle cast(std::unique_ptr<T, D> &, return_value_policy, handle) {
+        throw cast_error("Passing non-const unique_ptr& is not supported.\n"
+                         "If you want to transfer ownership, use unique_ptr&&.\n"
+                         "If you want to return a reference, use unique_ptr const&.");
+    }
+
     static handle
     cast(const std::unique_ptr<T, D> &src, return_value_policy policy, handle parent) {
         if (!src)
             return none().release();
         if (policy == return_value_policy::automatic)
             policy = return_value_policy::reference_internal;
         if (policy != return_value_policy::reference_internal)
-            throw cast_error("Invalid return_value_policy for unique_ptr&");
+            throw cast_error(
+                "Invalid return_value_policy: unique_ptr const& expects reference_internal");
         return smart_holder_type_caster<T>::cast(src.get(), policy, parent);
     }
 

tests/test_class_sh_basic.py

@@ -166,8 +166,11 @@ def test_unique_ptr_consumer_roundtrip(rtrn_f, moved_in):
     c.pass_uq_valu(orig)  # pass object to C++ consumer c
     try:
         recycled = rtrn_f(c)  # retrieve object back from C++
-    except RuntimeError:  # expect failure for rtrn_uq_lref
-        assert moved_in is None
+    except RuntimeError as excinfo:  # expect failure for rtrn_uq_lref
+        assert (
+            moved_in is None
+            and "Passing non-const unique_ptr& is not supported" in str(excinfo)
+        )
         return
 
     assert m.get_ptr(recycled) == ptr_orig  # do we yield the same object?