Better? implementation for previous commit

This avoids the introduction of a new return_value_policy.
And it enforces copying of const pointers/references to ensure
constness.

Author: rhaschke

include/pybind11/detail/common.h

@@ -434,14 +434,7 @@ enum class return_value_policy : uint8_t {
         collected while Python is still using the child. More advanced
         variations of this scheme are also possible using combinations of
         return_value_policy::reference and the keep_alive call policy */
-    reference_internal,
-
-    /** This internally used policy only applies to C++ arguments passed
-        to virtual methods overridden in Python. This effectively will result
-        in the return_value_policy::reference policy usually.
-        It is needed to suppress copying of const unique_ptr, which is
-        otherwise applied to const refererences to ensure constness. */
-    reference_override
+    reference_internal
 };
 
 PYBIND11_NAMESPACE_BEGIN(detail)

include/pybind11/detail/smart_holder_type_casters.h

@@ -473,13 +473,7 @@ struct smart_holder_type_caster : smart_holder_type_caster_load<T>,
 
     static handle cast(T const &src, return_value_policy policy, handle parent) {
         // type_caster_base BEGIN
-        // clang-format off
-        if (policy == return_value_policy::automatic ||
-            policy == return_value_policy::automatic_reference ||
-            policy == return_value_policy::reference_override)
-            policy = return_value_policy::copy; // copy ensures constness
-        // clang-format on
-        return cast(&src, policy, parent);
+        return cast(&src, policy, parent); // copy ensures constness
         // type_caster_base END
     }
 
@@ -494,6 +488,14 @@ struct smart_holder_type_caster : smart_holder_type_caster_load<T>,
     }
 
     static handle cast(T const *src, return_value_policy policy, handle parent) {
+        // enforce constness by using policy return_value_policy::copy
+        // The only exception is reference_internal, which can be used to ignore constness
+        if (policy != return_value_policy::reference_internal)
+            policy = return_value_policy::copy;
+        return cast(const_cast<T *>(src), policy, parent);
+    }
+
+    static handle cast(T *src, return_value_policy policy, handle parent) {
         auto st = type_caster_base<T>::src_and_type(src);
         return cast_const_raw_ptr( // Originally type_caster_generic::cast.
             st.first,
@@ -504,10 +506,6 @@ struct smart_holder_type_caster : smart_holder_type_caster_load<T>,
             make_constructor::make_move_constructor(src));
     }
 
-    static handle cast(T *src, return_value_policy policy, handle parent) {
-        return cast(const_cast<T const *>(src), policy, parent); // Mutbl2Const
-    }
-
 #if defined(_MSC_VER) && _MSC_VER < 1910
     // Working around MSVC 2015 bug. const-correctness is lost.
     // SMART_HOLDER_WIP: IMPROVABLE: make common code work with MSVC 2015.
@@ -567,7 +565,6 @@ struct smart_holder_type_caster : smart_holder_type_caster_load<T>,
 
             case return_value_policy::automatic_reference:
             case return_value_policy::reference:
-            case return_value_policy::reference_override:
                 valueptr       = src;
                 wrapper->owned = false;
                 break;
@@ -732,8 +729,8 @@ struct smart_holder_type_caster<std::unique_ptr<T, D>> : smart_holder_type_caste
             return none().release();
         if (policy == return_value_policy::automatic)
             policy = return_value_policy::reference_internal;
-        else if (policy == return_value_policy::reference_override)
-            policy = return_value_policy::reference;
+        // allow reference only for overridden method calls (parent is None)
+        else if (policy == return_value_policy::reference && !parent);
         else if (policy != return_value_policy::reference_internal)
             throw cast_error("Invalid return_value_policy for unique_ptr&");
         return smart_holder_type_caster<T>::cast(src.get(), policy, parent);

include/pybind11/detail/type_caster_base.h

@@ -527,7 +527,6 @@ class type_caster_generic {
 
             case return_value_policy::automatic_reference:
             case return_value_policy::reference:
-            case return_value_policy::reference_override:
                 valueptr = src;
                 wrapper->owned = false;
                 break;

include/pybind11/eigen.h

@@ -303,7 +303,6 @@ struct type_caster<Type, enable_if_t<is_eigen_dense_plain<Type>::value>> {
                 return eigen_array_cast<props>(*src);
             case return_value_policy::reference:
             case return_value_policy::automatic_reference:
-            case return_value_policy::reference_override:
                 return eigen_ref_array<props>(*src);
             case return_value_policy::reference_internal:
                 return eigen_ref_array<props>(*src, parent);

include/pybind11/pytypes.h

@@ -105,9 +105,9 @@ class object_api : public pyobject_tag {
         function will throw a `cast_error` exception. When the Python function
         call fails, a `error_already_set` exception is thrown.
     \endrst */
-    template <return_value_policy policy = return_value_policy::reference_override, typename... Args>
+    template <return_value_policy policy = return_value_policy::reference, typename... Args>
     object operator()(Args &&...args) const;
-    template <return_value_policy policy = return_value_policy::reference_override, typename... Args>
+    template <return_value_policy policy = return_value_policy::reference, typename... Args>
     PYBIND11_DEPRECATED("call(...) was deprecated in favor of operator()(...)")
         object call(Args&&... args) const;
 

tests/test_class_sh_basic.py

@@ -29,7 +29,7 @@ def check_regex(expected, actual):
         (m.rtrn_rref, "rtrn_rref(_MvCtor){1}"),
         (m.rtrn_cref, "rtrn_cref_CpCtor"),
         (m.rtrn_mref, "rtrn_mref"),
-        (m.rtrn_cptr, "rtrn_cptr"),
+        (m.rtrn_cptr, "rtrn_cptr_CpCtor"),
         (m.rtrn_mptr, "rtrn_mptr"),
         (m.rtrn_shmp, "rtrn_shmp"),
         (m.rtrn_shcp, "rtrn_shcp"),