Differentiate between copying and moving of custom types based on the function signature

So far, using the movable_cast_op_type, always enabled move semantics,
even when copy semantics would be expected from the function signature.

While C++ decides about copy vs move semantics based on the type of the passed argument,
Python doesn't know about lvalue and rvalue references and thus cannot decide based on the
passed argument. Instead, the wrapper needs to explicitly define the desired semantics of
argument passing as follows:
- f(T&)  passes an lvalue reference
- f(T&&) passes an rvalue reference
- f(T)   passes an lvalue reference by default (triggering copy construction)
         but falls back to an rvalue reference if copying is not supported (thus triggering move construction)

This commit essentially makes call_impl() pass the argument casters to cast_op() by lvalue references, thus
enabling the distinction between copy and move semantics based on the argument's type.

Author: rhaschke

include/pybind11/cast.h

@@ -50,14 +50,15 @@ PYBIND11_NAMESPACE_BEGIN(detail)
 template <typename type, typename SFINAE = void> class type_caster : public type_caster_base<type> { };
 template <typename type> using make_caster = type_caster<intrinsic_t<type>>;
 
-// Shortcut for calling a caster's `cast_op_type` cast operator for casting a type_caster to a T
+// Shortcuts for calling a caster's `cast_op_type` cast operator for casting a type_caster to a T
+// cast_op operating on an lvalue-reference caster, enables moving only if required by the actual function argument
 template <typename T> typename make_caster<T>::template cast_op_type<T> cast_op(make_caster<T> &caster) {
     return caster.operator typename make_caster<T>::template cast_op_type<T>();
 }
+// cast_op operating on an rvalue-referenced caster enforces an rvalue-reference for the cast_op type as well
 template <typename T> typename make_caster<T>::template cast_op_type<typename std::add_rvalue_reference<T>::type>
 cast_op(make_caster<T> &&caster) {
-    return std::move(caster).operator
-        typename make_caster<T>::template cast_op_type<typename std::add_rvalue_reference<T>::type>();
+    return caster.operator typename make_caster<T>::template cast_op_type<typename std::add_rvalue_reference<T>::type>();
 }
 
 template <typename type> class type_caster<std::reference_wrapper<type>> {
@@ -101,7 +102,7 @@ template <typename type> class type_caster<std::reference_wrapper<type>> {
         } \
         operator type*() { return &value; } \
         operator type&() { return value; } \
-        operator type&&() && { return std::move(value); } \
+        operator type&&() { return std::move(value); } \
         template <typename T_> using cast_op_type = pybind11::detail::movable_cast_op_type<T_>
 
 
@@ -795,8 +796,8 @@ class type_caster<T, enable_if_t<is_pyobject<T>::value>> : public pyobject_caste
 // - type_caster<T>::operator T&() must exist
 // - the type must be move constructible (obviously)
 // At run-time:
-// - if the type is non-copy-constructible, the object must be the sole owner of the type (i.e. it
-//   must have ref_count() == 1)h
+// - if the type is non-copy-constructible, the object must be the sole owner of the type
+//   (i.e. it must have ref_count() == 1)
 // If any of the above are not satisfied, we fall back to copying.
 template <typename T> using move_is_plain_type = satisfies_none_of<T,
     std::is_void, std::is_pointer, std::is_reference, std::is_const
@@ -1163,7 +1164,7 @@ class argument_loader {
 
     template <typename Return, typename Func, size_t... Is, typename Guard>
     Return call_impl(Func &&f, index_sequence<Is...>, Guard &&) && {
-        return std::forward<Func>(f)(cast_op<Args>(std::move(std::get<Is>(argcasters)))...);
+        return std::forward<Func>(f)(cast_op<Args>(std::get<Is>(argcasters))...);
     }
 
     std::tuple<make_caster<Args>...> argcasters;

include/pybind11/detail/type_caster_base.h

@@ -760,29 +760,44 @@ class type_caster_generic {
  * Determine suitable casting operator for pointer-or-lvalue-casting type casters.  The type caster
  * needs to provide `operator T*()` and `operator T&()` operators.
  *
- * If the type supports moving the value away via an `operator T&&() &&` method, it should use
- * `movable_cast_op_type` instead.
+ * If the type supports moving, it should use `movable_cast_op_type` instead.
  */
 template <typename T>
 using cast_op_type =
     conditional_t<std::is_pointer<remove_reference_t<T>>::value,
         typename std::add_pointer<intrinsic_t<T>>::type,
         typename std::add_lvalue_reference<intrinsic_t<T>>::type>;
 
+// A by-value argument will use lvalue references by default (triggering copy construction)
+// except the type is move-only and thus should use move semantics.
+template <typename T> using cast_op_type_for_byvalue =
+    conditional_t<std::is_copy_constructible<intrinsic_t<T>>::value,
+        typename std::add_lvalue_reference<intrinsic_t<T>>::type,
+        typename std::add_rvalue_reference<intrinsic_t<T>>::type >;
+
 /**
- * Determine suitable casting operator for a type caster with a movable value.  Such a type caster
- * needs to provide `operator T*()`, `operator T&()`, and `operator T&&() &&`.  The latter will be
- * called in appropriate contexts where the value can be moved rather than copied.
+ * Determine suitable casting operator for movable types.
+ * While C++ decides about copy vs. move semantics based on the type of the passed argument,
+ * Python doesn't know about lvalue and rvalue references and thus cannot decide based on the
+ * passed argument. Instead, the wrapper needs to define the desired semantics of argument passing
+ * explicitly:
+ * - By specifying an rvalue reference (T&&), move semantics is enforced.
+ * - By default, lvalue references (T&) are passed, triggering copy semantics if necessary.
+ *   An exception are move-only types: These are passed via move semantics.
  *
- * These operator are automatically provided when using the PYBIND11_TYPE_CASTER macro.
+ * The corresponding caster needs to provide the following operators:
+ * `operator T*()`, `operator T&()`, and `operator T&&()`.
+ * These operators are automatically provided when using the PYBIND11_TYPE_CASTER macro.
  */
 template <typename T>
 using movable_cast_op_type =
     conditional_t<std::is_pointer<typename std::remove_reference<T>::type>::value,
         typename std::add_pointer<intrinsic_t<T>>::type,
-    conditional_t<std::is_rvalue_reference<T>::value,
-        typename std::add_rvalue_reference<intrinsic_t<T>>::type,
-        typename std::add_lvalue_reference<intrinsic_t<T>>::type>>;
+        conditional_t<std::is_rvalue_reference<T>::value,
+            typename std::add_rvalue_reference<intrinsic_t<T>>::type,
+            conditional_t<std::is_lvalue_reference<T>::value,
+                typename std::add_lvalue_reference<intrinsic_t<T>>::type,
+                cast_op_type_for_byvalue<T> > > >;
 
 // std::is_copy_constructible isn't quite enough: it lets std::vector<T> (and similar) through when
 // T is non-copyable, but code containing such a copy constructor fails to actually compile.
@@ -917,7 +932,7 @@ template <typename type> class type_caster_base : public type_caster_generic {
 
     operator itype*() { return (type *) value; }
     operator itype&() { if (!value) throw reference_cast_error(); return *((itype *) value); }
-    operator itype&&() && { if (!value) throw reference_cast_error(); return std::move(*((itype *) value)); }
+    operator itype&&() { if (!value) throw reference_cast_error(); return std::move(*((itype *) value)); }
 
 protected:
     using Constructor = void *(*)(const void *);

include/pybind11/eigen.h

@@ -346,7 +346,7 @@ struct type_caster<Type, enable_if_t<is_eigen_dense_plain<Type>::value>> {
 
     operator Type*() { return &value; }
     operator Type&() { return value; }
-    operator Type&&() && { return std::move(value); }
+    operator Type&&() { return std::move(value); }
     template <typename T> using cast_op_type = movable_cast_op_type<T>;
 
 private: