From 1220156961ce2d0c96a525f3c27b88e824b997ce Mon Sep 17 00:00:00 2001
From: Wenzel Jakob <wenzel.jakob@epfl.ch>
Date: Fri, 29 Sep 2023 10:50:06 +0200
Subject: [PATCH] Be more careful with move semantics in type casters

This commit addresses a serious issue involving combinations of bound
types (e.g., ``T``) and type casters (e.g., ``std::vector<T>``), where
nanobind was too aggressive in its use of move semantics.

Calling a bound function from Python taking such a list (e.g., ``f([t1,
t2, ..])``) would lead to the destruction of the Python objects ``t1,
t2, ..`` if the type ``T`` exposed a move constructor, which is highly
non-intuitive.

This commit fixes, simplifies, and documents the rules of this process.
In particular, ``type_caster::Cast`` continues to serve its role to
infer what types a type caster can and wants to produce. It aggressively
tries to move, but now only does so when this is legal (i.e., when the
type caster created a temporary copy that is safe to move without
affecting program state elsewhere).

This involves two new default casting rules: ``movable_cast_t`` (adapted
to be more aggressive with moves) and ``precise_cast_t`` for bound types
that does not involve a temporary and consequently should only move when
this was explicitly requested by the user.

The fix also revealed inefficiencies in the previous implementation
where moves were actually possible but not done (e.g. for functions
taking an STL list by value). Some binding projects may see speedups as
a consequence of this change.

Fixes issue #307.
---
 include/nanobind/eigen/dense.h         |  3 --
 include/nanobind/nb_cast.h             | 71 +++++++++++++++++---------
 include/nanobind/nb_class.h            | 40 ++++++++-------
 include/nanobind/nb_func.h             |  4 +-
 include/nanobind/nb_traits.h           | 15 ++++++
 include/nanobind/nb_types.h            |  2 +-
 include/nanobind/stl/detail/nb_array.h |  2 +-
 include/nanobind/stl/detail/nb_dict.h  |  4 +-
 include/nanobind/stl/detail/nb_list.h  |  2 +-
 include/nanobind/stl/detail/nb_set.h   |  2 +-
 include/nanobind/stl/detail/traits.h   |  2 +-
 include/nanobind/stl/optional.h        | 46 ++++++-----------
 include/nanobind/stl/pair.h            | 29 ++---------
 include/nanobind/stl/shared_ptr.h      | 30 +++--------
 include/nanobind/stl/tuple.h           | 25 +++------
 include/nanobind/stl/unique_ptr.h      | 16 +++---
 include/nanobind/stl/variant.h         | 69 +++++++++----------------
 tests/object_py.h                      | 12 +----
 tests/test_issue.cpp                   | 16 ++++++
 tests/test_issue.py                    |  8 +++
 tests/test_stl.cpp                     |  8 +--
 tests/test_stl.py                      | 27 +++++-----
 22 files changed, 205 insertions(+), 228 deletions(-)

diff --git a/include/nanobind/eigen/dense.h b/include/nanobind/eigen/dense.h
index 1aafae93..07901da6 100644
--- a/include/nanobind/eigen/dense.h
+++ b/include/nanobind/eigen/dense.h
@@ -218,7 +218,6 @@ struct type_caster<T, enable_if_t<is_eigen_xpr_v<T> &&
     using Array = Eigen::Array<typename T::Scalar, T::RowsAtCompileTime,
                                T::ColsAtCompileTime>;
     using Caster = make_caster<Array>;
-    static constexpr bool IsClass = false;
     static constexpr auto Name = Caster::Name;
     template <typename T_> using Cast = T;
 
@@ -249,7 +248,6 @@ struct type_caster<Eigen::Map<T, Options, StrideType>,
                                                   const typename Map::Scalar,
                                                   typename Map::Scalar>>;
     using NDArrayCaster = type_caster<NDArray>;
-    static constexpr bool IsClass = false;
     static constexpr auto Name = NDArrayCaster::Name;
     template <typename T_> using Cast = Map;
 
@@ -395,7 +393,6 @@ struct type_caster<Eigen::Ref<T, Options, StrideType>,
     static constexpr bool DMapConstructorOwnsData =
         !Eigen::internal::traits<Ref>::template match<DMap>::type::value;
 
-    static constexpr bool IsClass = false;
     static constexpr auto Name =
         const_name<MaybeConvert>(DMapCaster::Name, MapCaster::Name);
 
diff --git a/include/nanobind/nb_cast.h b/include/nanobind/nb_cast.h
index 54440fbf..1aa50ba7 100644
--- a/include/nanobind/nb_cast.h
+++ b/include/nanobind/nb_cast.h
@@ -9,7 +9,6 @@
 
 #define NB_TYPE_CASTER(Value_, descr)                                          \
     using Value = Value_;                                                      \
-    static constexpr bool IsClass = false;                                     \
     static constexpr auto Name = descr;                                        \
     template <typename T_> using Cast = movable_cast_t<T_>;                    \
     static handle from_cpp(Value *p, rv_policy policy, cleanup_list *list) {   \
@@ -17,9 +16,9 @@
             return none().release();                                           \
         return from_cpp(*p, policy, list);                                     \
     }                                                                          \
-    explicit operator Value *() { return &value; }                             \
-    explicit operator Value &() { return value; }                              \
-    explicit operator Value &&() && { return (Value &&) value; }               \
+    explicit operator Value*() { return &value; }                              \
+    explicit operator Value&() { return (Value &) value; }                     \
+    explicit operator Value&&() { return (Value &&) value; }                   \
     Value value;
 
 #define NB_MAKE_OPAQUE(...)                                                    \
@@ -38,14 +37,45 @@ enum cast_flags : uint8_t {
     construct = (1 << 1)
 };
 
+/**
+ * Type casters expose a member 'Cast<T>' which users of a type caster must
+ * query to determine what the caster actually can (and prefers) to produce.
+ * The convenience alias ``cast_t<T>`` defined below performs this query for a
+ * given type ``T``.
+ *
+ * Often ``cast_t<T>`` is simply equal to ``T`` or ``T&``. More significant
+ * deviations are also possible, which could be due to one of the following
+ * two reasons:
+ *
+ * 1. Efficiency: most STL type casters create a local copy (``value`` member)
+ *    of the value being cast. The caller should move this value to its
+ *    intended destination instead of making further copies along the way.
+ *    Consequently, ``cast_t<std::vector<T>>`` yields ``cast_t<std::vector<T>>
+ *    &&`` to enable such behavior.
+ *
+ * 2. STL pairs may contain references, and such pairs aren't
+ *    default-constructible. The STL pair caster therefore cannot create a local
+ *    copy and must construct the pair on the fly, which in turns means that it
+ *    cannot return references. Therefore, ``cast_t<const std::pair<T1, T2>&>``
+ *    yields ``std::pair<T1, T2>``.
+ */
+
+/// Ask a type caster what flavors of a type it can actually produce -- may be different from 'T'
 template <typename T> using cast_t = typename make_caster<T>::template Cast<T>;
 
+/// This is a default choice for the 'Cast' type alias described above. It
+/// prefers to return rvalue references to allow the caller to move the object.
 template <typename T>
-using simple_cast_t =
-    std::conditional_t<is_pointer_v<T>, intrinsic_t<T> *, intrinsic_t<T> &>;
+using movable_cast_t =
+    std::conditional_t<is_pointer_v<T>, intrinsic_t<T> *,
+                       std::conditional_t<std::is_lvalue_reference_v<T>,
+                                          intrinsic_t<T> &, intrinsic_t<T> &&>>;
 
+/// This version is more careful about what the caller actually requested and
+/// only moves when this was explicitly requested. It is the default for the
+/// base type caster (i.e., types bound via ``nanobind::class_<..>``)
 template <typename T>
-using movable_cast_t =
+using precise_cast_t =
     std::conditional_t<is_pointer_v<T>, intrinsic_t<T> *,
                        std::conditional_t<std::is_rvalue_reference_v<T>,
                                           intrinsic_t<T> &&, intrinsic_t<T> &>>;
@@ -105,13 +135,11 @@ struct type_caster<T, enable_if_t<std::is_arithmetic_v<T> && !is_std_char_v<T>>>
 
 template <> struct type_caster<void_type> {
     static constexpr auto Name = const_name("None");
-    static constexpr bool IsClass = false;
 };
 
 template <> struct type_caster<void> {
     template <typename T_> using Cast = void *;
     using Value = void*;
-    static constexpr bool IsClass = false;
     static constexpr auto Name = const_name("capsule");
     explicit operator void *() { return value; }
     Value value;
@@ -175,7 +203,6 @@ template <> struct type_caster<bool> {
 template <> struct type_caster<char> {
     using Value = const char *;
     Value value;
-    static constexpr bool IsClass = false;
     static constexpr auto Name = const_name("str");
     template <typename T_>
     using Cast = std::conditional_t<is_pointer_v<T_>, const char *, char>;
@@ -289,12 +316,10 @@ template <typename T> NB_INLINE rv_policy infer_policy(rv_policy policy) {
 
 template <typename T, typename SFINAE = int> struct type_hook : std::false_type { };
 
-template <typename Type_> struct type_caster_base {
+template <typename Type_> struct type_caster_base : type_caster_base_tag {
     using Type = Type_;
     static constexpr auto Name = const_name<Type>();
-    static constexpr bool IsClass = true;
-
-    template <typename T> using Cast = movable_cast_t<T>;
+    template <typename T> using Cast = precise_cast_t<T>;
 
     NB_INLINE bool from_python(handle src, uint8_t flags,
                                cleanup_list *cleanup) noexcept {
@@ -335,7 +360,7 @@ template <typename Type_> struct type_caster_base {
         return *value;
     }
 
-    operator Type&&() && {
+    operator Type&&() {
         raise_next_overload_if_null(value);
         return (Type &&) *value;
     }
@@ -352,7 +377,6 @@ NAMESPACE_END(detail)
 template <typename T, typename Derived>
 bool try_cast(const detail::api<Derived> &value, T &out, bool convert = true) noexcept {
     using Caster = detail::make_caster<T>;
-    using Output = typename Caster::template Cast<T>;
 
     static_assert(!std::is_same_v<const char *, T>,
                   "nanobind::try_cast(): cannot return a reference to a temporary.");
@@ -361,11 +385,7 @@ bool try_cast(const detail::api<Derived> &value, T &out, bool convert = true) no
     if (caster.from_python(value.derived().ptr(),
                            convert ? (uint8_t) detail::cast_flags::convert
                                    : (uint8_t) 0, nullptr)) {
-        if constexpr (Caster::IsClass)
-            out = caster.operator Output();
-        else
-            out = std::move(caster.operator Output&&());
-
+        out = caster.operator detail::cast_t<T>();
         return true;
     }
 
@@ -378,11 +398,11 @@ T cast(const detail::api<Derived> &value, bool convert = true) {
         return;
     } else {
         using Caster = detail::make_caster<T>;
-        using Output = typename Caster::template Cast<T>;
 
         static_assert(
-            !(std::is_reference_v<T> || std::is_pointer_v<T>) || Caster::IsClass ||
-            std::is_same_v<const char *, T>,
+            !(std::is_reference_v<T> || std::is_pointer_v<T>) ||
+                detail::is_base_caster_v<Caster> ||
+                std::is_same_v<const char *, T>,
             "nanobind::cast(): cannot return a reference to a temporary.");
 
         Caster caster;
@@ -397,7 +417,7 @@ T cast(const detail::api<Derived> &value, bool convert = true) {
           #pragma GCC diagnostic ignored "-Wmaybe-uninitialized"
         #endif
 
-        return caster.operator Output();
+        return caster.operator detail::cast_t<T>();
 
         #if defined(__GNUC__) && !defined(__clang__)
           #pragma GCC diagnostic pop
@@ -411,6 +431,7 @@ object cast(T &&value, rv_policy policy = rv_policy::automatic_reference) {
                                                 policy, nullptr);
     if (!h.is_valid())
         detail::raise_cast_error();
+
     return steal(h);
 }
 
diff --git a/include/nanobind/nb_class.h b/include/nanobind/nb_class.h
index 1ab0e74b..e8cf1203 100644
--- a/include/nanobind/nb_class.h
+++ b/include/nanobind/nb_class.h
@@ -293,12 +293,6 @@ template <typename... Args> struct init {
     NB_INLINE static void execute(Class &cl, const Extra&... extra) {
         using Type = typename Class::Type;
         using Alias = typename Class::Alias;
-        static_assert(
-            detail::make_caster<Type>::IsClass,
-            "Attempted to create a constructor for a type that won't be "
-            "handled by the nanobind's class type caster. Is it possible that "
-            "you forgot to add NB_MAKE_OPAQUE() somewhere?");
-
         cl.def(
             "__init__",
             [](pointer_and_handle<Type> v, Args... args) {
@@ -324,12 +318,6 @@ template <typename Arg> struct init_implicit {
     NB_INLINE static void execute(Class &cl, const Extra&... extra) {
         using Type = typename Class::Type;
         using Alias = typename Class::Alias;
-        using Caster = detail::make_caster<Arg>;
-        static_assert(
-            detail::make_caster<Type>::IsClass,
-            "Attempted to create a constructor for a type that won't be "
-            "handled by the nanobind's class type caster. Is it possible that "
-            "you forgot to add NB_MAKE_OPAQUE() somewhere?");
 
         cl.def(
             "__init__",
@@ -344,7 +332,9 @@ template <typename Arg> struct init_implicit {
                 new ((Alias *) v.p) Alias{ (detail::forward_t<Arg>) arg };
             }, is_implicit(), extra...);
 
-        if constexpr (!Caster::IsClass) {
+        using Caster = detail::make_caster<Arg>;
+
+        if constexpr (!detail::is_class_caster_v<Caster>) {
             detail::implicitly_convertible(
                 [](PyTypeObject *, PyObject *src,
                    detail::cleanup_list *cleanup) noexcept -> bool {
@@ -364,12 +354,22 @@ class class_ : public object {
     using Base  = typename detail::extract<T, detail::is_base,  Ts...>::type;
     using Alias = typename detail::extract<T, detail::is_alias, Ts...>::type;
 
-    static_assert(sizeof(Alias) < (1 << 24), "instance size is too big!");
-    static_assert(alignof(Alias) < (1 << 8), "instance alignment is too big!");
+    static_assert(sizeof(Alias) < (1 << 24), "Instance size is too big!");
+    static_assert(alignof(Alias) < (1 << 8), "Instance alignment is too big!");
     static_assert(
         sizeof...(Ts) == !std::is_same_v<Base, T> + !std::is_same_v<Alias, T>,
         "nanobind::class_<> was invoked with extra arguments that could not be handled");
 
+    static_assert(
+        detail::is_base_caster_v<detail::make_caster<Type>>,
+        "You attempted to bind a type that is already intercepted by a type "
+        "caster. Having both at the same time is not allowed. Are you perhaps "
+        "binding an STL type, while at the same time including a matching "
+        "type caster from <nanobind/stl/*>? Or did you perhaps forget to "
+        "declare NB_MAKE_OPAQUE(..) to specifically disable the type caster "
+        "catch-all for a specific type? Please review the documentation "
+        "to learn about the difference between bindings and type casters.");
+
     template <typename... Extra>
     NB_INLINE class_(handle scope, const char *name, const Extra &... extra) {
         detail::type_init_data d;
@@ -521,7 +521,9 @@ class class_ : public object {
         static_assert(std::is_base_of_v<C, T>,
                       "def_rw() requires a (base) class member!");
 
-        using Q = std::conditional_t<detail::make_caster<D>::IsClass, const D &, D &&>;
+        using Q =
+            std::conditional_t<detail::is_base_caster_v<detail::make_caster<D>>,
+                               const D &, D &&>;
 
         def_prop_rw(name,
             [p](const T &c) -> const D & { return c.*p; },
@@ -534,7 +536,9 @@ class class_ : public object {
     template <typename D, typename... Extra>
     NB_INLINE class_ &def_rw_static(const char *name, D *p,
                                     const Extra &...extra) {
-        using Q = std::conditional_t<detail::make_caster<D>::IsClass, const D &, D &&>;
+        using Q =
+            std::conditional_t<detail::is_base_caster_v<detail::make_caster<D>>,
+                               const D &, D &&>;
 
         def_prop_rw_static(name,
             [p](handle) -> const D & { return *p; },
@@ -624,7 +628,7 @@ template <typename T> class enum_ : public class_<T> {
 template <typename Source, typename Target> void implicitly_convertible() {
     using Caster = detail::make_caster<Source>;
 
-    if constexpr (Caster::IsClass) {
+    if constexpr (detail::is_base_caster_v<Caster>) {
         detail::implicitly_convertible(&typeid(Source), &typeid(Target));
     } else {
         detail::implicitly_convertible(
diff --git a/include/nanobind/nb_func.h b/include/nanobind/nb_func.h
index d6204f17..013b3dcb 100644
--- a/include/nanobind/nb_func.h
+++ b/include/nanobind/nb_func.h
@@ -121,12 +121,12 @@ NB_INLINE PyObject *func_create(Func &&func, Return (*)(Args...),
 
         PyObject *result;
         if constexpr (std::is_void_v<Return>) {
-            cap->func(((make_caster<Args>&&) in.template get<Is>()).operator cast_t<Args>()...);
+            cap->func(in.template get<Is>().operator cast_t<Args>()...);
             result = Py_None;
             Py_INCREF(result);
         } else {
             result = cast_out::from_cpp(
-                       cap->func(((make_caster<Args> &&) in.template get<Is>())
+                       cap->func((in.template get<Is>())
                                      .operator cast_t<Args>()...),
                        policy, cleanup).ptr();
         }
diff --git a/include/nanobind/nb_traits.h b/include/nanobind/nb_traits.h
index e950f1c9..48f8158b 100644
--- a/include/nanobind/nb_traits.h
+++ b/include/nanobind/nb_traits.h
@@ -142,6 +142,21 @@ template <typename T>
 constexpr bool has_shared_from_this_v =
     decltype(has_shared_from_this_impl((T *) nullptr))::value;
 
+/// Base of all type casters for traditional bindings created via nanobind::class_<>
+struct type_caster_base_tag {
+    static constexpr bool IsClass = true;
+};
+
+/// Check if a type caster represents traditional bindings created via nanobind::class_<>
+template <typename Caster>
+constexpr bool is_base_caster_v = std::is_base_of_v<type_caster_base_tag, Caster>;
+
+template <typename T> using is_class_caster_test = std::enable_if_t<T::IsClass>;
+
+/// Generalized version of the is_base_caster_v test that also accepts unique_ptr/shared_ptr
+template <typename Caster>
+constexpr bool is_class_caster_v = detail::detector<void, is_class_caster_test, Caster>::value;
+
 NAMESPACE_END(detail)
 
 template <typename... Args>
diff --git a/include/nanobind/nb_types.h b/include/nanobind/nb_types.h
index 7369b913..5a271c1d 100644
--- a/include/nanobind/nb_types.h
+++ b/include/nanobind/nb_types.h
@@ -551,7 +551,7 @@ template <typename T>
 NB_INLINE bool isinstance(handle h) noexcept {
     if constexpr (std::is_base_of_v<handle, T>)
         return T::check_(h);
-    else if constexpr (detail::make_caster<T>::IsClass)
+    else if constexpr (detail::is_base_caster_v<detail::make_caster<T>>)
         return detail::nb_type_isinstance(h.ptr(), &typeid(detail::intrinsic_t<T>));
     else
         return detail::make_caster<T>().from_python(h, 0, nullptr);
diff --git a/include/nanobind/stl/detail/nb_array.h b/include/nanobind/stl/detail/nb_array.h
index 27a64cca..17202c5b 100644
--- a/include/nanobind/stl/detail/nb_array.h
+++ b/include/nanobind/stl/detail/nb_array.h
@@ -27,7 +27,7 @@ template <typename Value_, typename Entry, size_t Size> struct array_caster {
                     break;
                 }
 
-                value[i] = ((Caster &&) caster).operator cast_t<Entry &&>();
+                value[i] = caster.operator cast_t<Entry>();
             }
 
             Py_XDECREF(temp);
diff --git a/include/nanobind/stl/detail/nb_dict.h b/include/nanobind/stl/detail/nb_dict.h
index 849d03c4..e1e9971e 100644
--- a/include/nanobind/stl/detail/nb_dict.h
+++ b/include/nanobind/stl/detail/nb_dict.h
@@ -51,8 +51,8 @@ template <typename Value_, typename Key, typename Element> struct dict_caster {
                 break;
             }
 
-            value.emplace(((KeyCaster &&) key_caster).operator cast_t<Key &&>(),
-                          ((ElementCaster &&) element_caster).operator cast_t<Element &&>());
+            value.emplace(key_caster.operator cast_t<Key>(),
+                          element_caster.operator cast_t<Element>());
         }
 
         Py_DECREF(items);
diff --git a/include/nanobind/stl/detail/nb_list.h b/include/nanobind/stl/detail/nb_list.h
index a3ebecd5..a1c82b10 100644
--- a/include/nanobind/stl/detail/nb_list.h
+++ b/include/nanobind/stl/detail/nb_list.h
@@ -44,7 +44,7 @@ template <typename Value_, typename Entry> struct list_caster {
                 break;
             }
 
-            value.push_back(((Caster &&) caster).operator cast_t<Entry &&>());
+            value.push_back(caster.operator cast_t<Entry>());
         }
 
         Py_XDECREF(temp);
diff --git a/include/nanobind/stl/detail/nb_set.h b/include/nanobind/stl/detail/nb_set.h
index b7a6650b..21fe58ce 100644
--- a/include/nanobind/stl/detail/nb_set.h
+++ b/include/nanobind/stl/detail/nb_set.h
@@ -39,7 +39,7 @@ template <typename Value_, typename Key> struct set_caster {
             if (!success)
                 break;
 
-            value.emplace(((KeyCaster &&) key_caster).operator cast_t<Key&&>());
+            value.emplace(key_caster.operator cast_t<Key>());
         }
 
         if (PyErr_Occurred()) {
diff --git a/include/nanobind/stl/detail/traits.h b/include/nanobind/stl/detail/traits.h
index 5abfbb4f..24917c3d 100644
--- a/include/nanobind/stl/detail/traits.h
+++ b/include/nanobind/stl/detail/traits.h
@@ -62,7 +62,7 @@ template <typename T>
 constexpr bool is_copy_assignable_v = is_copy_assignable<T>::value;
 
 // Analogous template for checking comparability
-template<typename T> using comparable_test = decltype(std::declval<T>() == std::declval<T>());
+template <typename T> using comparable_test = decltype(std::declval<T>() == std::declval<T>());
 
 template <typename T, typename SFINAE = int>
 struct is_equality_comparable {
diff --git a/include/nanobind/stl/optional.h b/include/nanobind/stl/optional.h
index 8f6734ef..8a10296f 100644
--- a/include/nanobind/stl/optional.h
+++ b/include/nanobind/stl/optional.h
@@ -15,21 +15,18 @@
 NAMESPACE_BEGIN(NB_NAMESPACE)
 NAMESPACE_BEGIN(detail)
 
-template <typename T> struct remove_opt_mono<std::optional<T>> : remove_opt_mono<T> {};
+template <typename T> struct remove_opt_mono<std::optional<T>>
+    : remove_opt_mono<T> { };
 
 template <typename T>
 struct type_caster<std::optional<T>> {
-    using Value = std::optional<T>;
-    using Ti = detail::intrinsic_t<T>;
-    using Caster = make_caster<Ti>;
+    using Caster = make_caster<T>;
 
-    static constexpr auto Name = const_name("Optional[") + concat(Caster::Name) + const_name("]");
-    static constexpr bool IsClass = false;
+    NB_TYPE_CASTER(std::optional<T>, const_name("Optional[") +
+                                         concat(Caster::Name) +
+                                         const_name("]"));
 
-    template <typename T_>
-    using Cast = movable_cast_t<T_>;
-
-    Value value = std::nullopt;
+    type_caster() : value(std::nullopt) { }
 
     bool from_python(handle src, uint8_t flags, cleanup_list* cleanup) noexcept {
         if (src.is_none())
@@ -39,36 +36,25 @@ struct type_caster<std::optional<T>> {
         if (!caster.from_python(src, flags, cleanup))
             return false;
 
-        if constexpr (is_pointer_v<T>) {
-            static_assert(Caster::IsClass,
-                            "Binding 'optional<T*>' requires that 'T' can also be bound by nanobind.");
-            value = caster.operator cast_t<T>();
-        } else if constexpr (Caster::IsClass) {
-            value = caster.operator cast_t<T&>();
-        } else {
-            value = std::move(caster).operator cast_t<T&&>();
-        }
+        static_assert(
+            !std::is_pointer_v<T> || is_base_caster_v<Caster>,
+            "Binding ``optional<T*>`` requires that ``T`` is handled "
+            "by nanobind's regular class binding mechanism. However, a "
+            "type caster was registered to intercept this particular "
+            "type, which is not allowed.");
 
-        return true;
-    }
+        value = caster.operator cast_t<T>();
 
-    template <typename T_>
-    static handle from_cpp(T_ *value, rv_policy policy, cleanup_list *cleanup) noexcept {
-        if (!value)
-            return none().release();
-        return from_cpp(*value, policy, cleanup);
+        return true;
     }
 
     template <typename T_>
     static handle from_cpp(T_ &&value, rv_policy policy, cleanup_list *cleanup) noexcept {
         if (!value)
             return none().release();
+
         return Caster::from_cpp(forward_like<T_>(*value), policy, cleanup);
     }
-
-    explicit operator Value *() { return &value; }
-    explicit operator Value &() { return value; }
-    explicit operator Value &&() && { return (Value &&) value; }
 };
 
 NAMESPACE_END(detail)
diff --git a/include/nanobind/stl/pair.h b/include/nanobind/stl/pair.h
index 08abc2c7..2ab49a9b 100644
--- a/include/nanobind/stl/pair.h
+++ b/include/nanobind/stl/pair.h
@@ -22,28 +22,15 @@ template <typename T1, typename T2> struct type_caster<std::pair<T1, T2>> {
     using Caster1 = make_caster<T1>;
     using Caster2 = make_caster<T2>;
 
-    /**
-     * \brief Target type for extracting the result of a `from_python` cast
-     * using the C++ cast operator (see operator Value() below)
-     *
-     * The std::pair type caster is slightly restricted: to support pairs
-     * containing references, the pair is constructed on the fly in the cast
-     * operator. Other classes will typically specify the more general
-     *
-     * ```
-     * template <typename T> using Cast = default_cast_t<T_>;
-     * // or, even better:
-     * template <typename T> using Cast = movable_cast_t<T_>;
-     * ```
-     *
-     * which can also cast to pointers, or lvalue/rvalue references.
-     */
+    /// This caster constructs instances on the fly (otherwise it would not be
+    /// able to handle pairs containing references). Because of this, only the
+    /// `operator Value()` cast operator is implemented below, and the type
+    /// alias below informs users of this class of this fact.
     template <typename T> using Cast = Value;
 
     // Value name for docstring generation
     static constexpr auto Name =
         const_name(NB_TYPING_TUPLE "[") + concat(Caster1::Name, Caster2::Name) + const_name("]");
-    static constexpr bool IsClass = false;
 
     /// Python -> C++ caster, populates `caster1` and `caster2` upon success
     bool from_python(handle src, uint8_t flags,
@@ -87,17 +74,11 @@ template <typename T1, typename T2> struct type_caster<std::pair<T1, T2>> {
     }
 
     /// Return the constructed tuple by copying from the sub-casters
-    explicit operator Value() & {
+    explicit operator Value() {
         return Value(caster1.operator cast_t<T1>(),
                      caster2.operator cast_t<T2>());
     }
 
-    // Return the constructed type by moving from the sub-casters
-    explicit operator Value() && {
-        return Value(((Caster1 &&) caster1).operator cast_t<T1>(),
-                     ((Caster2 &&) caster2).operator cast_t<T2>());
-    }
-
     Caster1 caster1;
     Caster2 caster2;
 };
diff --git a/include/nanobind/stl/shared_ptr.h b/include/nanobind/stl/shared_ptr.h
index c37044f4..4b73ed93 100644
--- a/include/nanobind/stl/shared_ptr.h
+++ b/include/nanobind/stl/shared_ptr.h
@@ -58,20 +58,17 @@ inline NB_NOINLINE void shared_from_cpp(std::shared_ptr<void> &&ptr,
 }
 
 template <typename T> struct type_caster<std::shared_ptr<T>> {
-    using Td = std::decay_t<T>;
-    using Value = std::shared_ptr<T>;
-    using Caster = make_caster<Td>;
-    static_assert(Caster::IsClass,
-                  "Binding 'shared_ptr<T>' requires that 'T' can also be bound "
-                  "by nanobind. It appears that you specified a type which "
-                  "would undergo conversion/copying, which is not allowed.");
-
-    static constexpr auto Name = Caster::Name;
     static constexpr bool IsClass = true;
+    using Caster = make_caster<T>;
+    using Td = std::decay_t<T>;
 
-    template <typename T_> using Cast = movable_cast_t<T_>;
+    NB_TYPE_CASTER(std::shared_ptr<T>, Caster::Name)
 
-    Value value;
+    static_assert(is_base_caster_v<Caster>,
+                  "Conversion of ``shared_ptr<T>`` requires that ``T`` is "
+                  "handled by nanobind's regular class binding mechanism. "
+                  "However, a type caster was registered to intercept this "
+                  "particular type, which is not allowed.");
 
     bool from_python(handle src, uint8_t flags,
                      cleanup_list *cleanup) noexcept {
@@ -99,13 +96,6 @@ template <typename T> struct type_caster<std::shared_ptr<T>> {
         return true;
     }
 
-    static handle from_cpp(const Value *value, rv_policy policy,
-                           cleanup_list *cleanup) noexcept {
-        if (!value)
-            return (PyObject *) nullptr;
-        return from_cpp(*value, policy, cleanup);
-    }
-
     static handle from_cpp(const Value &value, rv_policy,
                            cleanup_list *cleanup) noexcept {
         bool is_new = false;
@@ -141,10 +131,6 @@ template <typename T> struct type_caster<std::shared_ptr<T>> {
 
         return result;
     }
-
-    explicit operator Value *() { return &value; }
-    explicit operator Value &() { return value; }
-    explicit operator Value &&() && { return (Value &&) value; }
 };
 
 NAMESPACE_END(detail)
diff --git a/include/nanobind/stl/tuple.h b/include/nanobind/stl/tuple.h
index 6cf03dff..4f440f5d 100644
--- a/include/nanobind/stl/tuple.h
+++ b/include/nanobind/stl/tuple.h
@@ -22,11 +22,14 @@ template <typename... Ts> struct type_caster<std::tuple<Ts...>> {
     using Value = std::tuple<Ts...>;
     using Indices = std::make_index_sequence<N>;
 
-    static constexpr bool IsClass = false;
     static constexpr auto Name = const_name(NB_TYPING_TUPLE "[") +
                                  concat(make_caster<Ts>::Name...) +
                                  const_name("]");
 
+    /// This caster constructs instances on the fly (otherwise it would not be
+    /// able to handle tuples containing references_). Because of this, only the
+    /// `operator Value()` cast operator is implemented below, and the type
+    /// alias below informs users of this class of this fact.
     template <typename T> using Cast = Value;
 
     bool from_python(handle src, uint8_t flags,
@@ -35,7 +38,7 @@ template <typename... Ts> struct type_caster<std::tuple<Ts...>> {
     }
 
     template <size_t... Is>
-    NB_INLINE bool from_python_impl(handle src, uint8_t flags,
+    bool from_python_impl(handle src, uint8_t flags,
                                     cleanup_list *cleanup,
                                     std::index_sequence<Is...>) noexcept {
         (void) src; (void) flags; (void) cleanup;
@@ -66,7 +69,7 @@ template <typename... Ts> struct type_caster<std::tuple<Ts...>> {
     }
 
     template <typename T, size_t... Is>
-    static NB_INLINE handle from_cpp_impl(T &&value, rv_policy policy,
+    static handle from_cpp_impl(T &&value, rv_policy policy,
                                           cleanup_list *cleanup,
                                           std::index_sequence<Is...>) noexcept {
         (void) value; (void) policy; (void) cleanup;
@@ -86,24 +89,12 @@ template <typename... Ts> struct type_caster<std::tuple<Ts...>> {
         return r;
     }
 
-    explicit operator Value() & {
-        return cast_impl(Indices{});
-    }
-
-    explicit operator Value() && {
-        return ((type_caster &&) *this).cast_impl(Indices{});
-    }
+    explicit operator Value() { return cast_impl(Indices{}); }
 
-    template <size_t... Is>
-    NB_INLINE Value cast_impl(std::index_sequence<Is...>) & {
+    template <size_t... Is> Value cast_impl(std::index_sequence<Is...>) {
         return Value(std::get<Is>(casters).operator cast_t<Ts>()...);
     }
 
-    template <size_t... Is>
-    NB_INLINE Value cast_impl(std::index_sequence<Is...>) && {
-        return Value(((make_caster<Ts> &&) std::get<Is>(casters)).operator cast_t<Ts>()...);
-    }
-
     std::tuple<make_caster<Ts>...> casters;
 };
 
diff --git a/include/nanobind/stl/unique_ptr.h b/include/nanobind/stl/unique_ptr.h
index de8fb51c..ec1433ff 100644
--- a/include/nanobind/stl/unique_ptr.h
+++ b/include/nanobind/stl/unique_ptr.h
@@ -45,6 +45,7 @@ NAMESPACE_BEGIN(detail)
 
 template <typename T, typename Deleter>
 struct type_caster<std::unique_ptr<T, Deleter>> {
+    static constexpr bool IsClass = true;
     using Value = std::unique_ptr<T, Deleter>;
     using Caster = make_caster<T>;
 
@@ -53,17 +54,18 @@ struct type_caster<std::unique_ptr<T, Deleter>> {
     static constexpr bool IsNanobindDeleter =
         std::is_same_v<Deleter, deleter<T>>;
 
-    static_assert(Caster::IsClass,
-                  "Binding 'std::unique_ptr<T>' requires that 'T' can also be "
-                  "bound by nanobind. It appears that you specified a type which "
-                  "would undergo conversion/copying, which is not allowed.");
+    static_assert(is_base_caster_v<Caster>,
+                  "Conversion of ``unique_ptr<T>`` requires that ``T`` is "
+                  "handled by nanobind's regular class binding mechanism. "
+                  "However, a type caster was registered to intercept this "
+                  "particular type, which is not allowed.");
 
     static_assert(IsDefaultDeleter || IsNanobindDeleter,
-                  "Binding std::unique_ptr<T, Deleter> requires that 'Deleter' is either "
-                  "'std::default_delete<T>' or 'nanobind::deleter<T>'");
+                  "Binding std::unique_ptr<T, Deleter> requires that "
+                  "'Deleter' is either 'std::default_delete<T>' or "
+                  "'nanobind::deleter<T>'");
 
     static constexpr auto Name = Caster::Name;
-    static constexpr bool IsClass = true;
     template <typename T_> using Cast = Value;
 
     Caster caster;
diff --git a/include/nanobind/stl/variant.h b/include/nanobind/stl/variant.h
index 3cae03b9..c54e367f 100644
--- a/include/nanobind/stl/variant.h
+++ b/include/nanobind/stl/variant.h
@@ -24,8 +24,7 @@ struct concat_variant<std::variant<Ts1...>, std::variant<Ts2...>, Ts3...>
 template <typename... Ts> struct remove_opt_mono<std::variant<Ts...>>
     : concat_variant<std::conditional_t<std::is_same_v<std::monostate, Ts>, std::variant<>, std::variant<Ts>>...> {};
 
-template <>
-struct type_caster<std::monostate> {
+template <> struct type_caster<std::monostate> {
     NB_TYPE_CASTER(std::monostate, const_name("None"));
 
     bool from_python(handle src, uint8_t, cleanup_list *) noexcept {
@@ -40,62 +39,44 @@ struct type_caster<std::monostate> {
     }
 };
 
-template <typename... Ts>
-class type_caster<std::variant<Ts...>> {
-    template <typename T> using Caster = make_caster<detail::intrinsic_t<T>>;
+template <typename... Ts> struct type_caster<std::variant<Ts...>> {
+    NB_TYPE_CASTER(std::variant<Ts...>,
+        const_name("Union[") + concat(make_caster<Ts>::Name...) + const_name("]"));
 
     template <typename T>
-    bool variadic_caster(const handle &src, uint8_t flags, cleanup_list *cleanup) {
-        Caster<T> caster;
-        if (!caster.from_python(src, flags, cleanup))
-            return false;
-
-        if constexpr (is_pointer_v<T>) {
-            static_assert(Caster<T>::IsClass,
-                          "Binding 'variant<T*,...>' requires that 'T' can also be bound by nanobind.");
-            value = caster.operator cast_t<T>();
-        } else if constexpr (Caster<T>::IsClass) {
-            // Non-pointer classes do not accept a null pointer
-            if (src.is_none())
-                return false;
-            value = caster.operator cast_t<T &>();
-        } else {
-            value = std::move(caster).operator cast_t<T &&>();
-        }
-        return true;
-    }
+    bool try_variant(const handle &src, uint8_t flags, cleanup_list *cleanup) {
+        using CasterT = make_caster<T>;
 
-public:
-    using Value = std::variant<Ts...>;
+        static_assert(
+            !std::is_pointer_v<T> || is_base_caster_v<CasterT>,
+            "Binding ``variant<T*, ...>`` requires that ``T`` is handled "
+            "by nanobind's regular class binding mechanism. However, a "
+            "type caster was registered to intercept this particular "
+            "type, which is not allowed.");
 
-    static constexpr auto Name = const_name("Union[") + concat(Caster<Ts>::Name...) + const_name("]");
-    static constexpr bool IsClass = false;
+        CasterT caster;
 
-    template <typename T> using Cast = movable_cast_t<T>;
+        if (!caster.from_python(src, flags, cleanup))
+            return false;
 
-    Value value;
+        value = caster.operator cast_t<T>();
 
-    bool from_python(handle src, uint8_t flags, cleanup_list *cleanup) noexcept {
-        return (variadic_caster<Ts>(src, flags, cleanup) || ...);
+        return true;
     }
 
-    template <typename T>
-    static handle from_cpp(T *value, rv_policy policy, cleanup_list *cleanup) noexcept {
-        if (!value)
-            return none().release();
-        return from_cpp(*value, policy, cleanup);
+    bool from_python(handle src, uint8_t flags, cleanup_list *cleanup) noexcept {
+        return (try_variant<Ts>(src, flags, cleanup) || ...);
     }
 
     template <typename T>
     static handle from_cpp(T &&value, rv_policy policy, cleanup_list *cleanup) noexcept {
-        return std::visit([&](auto &&v) {
-                return Caster<decltype(v)>::from_cpp(std::forward<decltype(v)>(v), policy, cleanup);
-            }, std::forward<T>(value));
+        return std::visit(
+            [&](auto &&v) {
+                return make_caster<decltype(v)>::from_cpp(
+                    std::forward<decltype(v)>(v), policy, cleanup);
+            },
+            std::forward<T>(value));
     }
-
-    explicit operator Value *() { return &value; }
-    explicit operator Value &() { return value; }
-    explicit operator Value &&() && { return (Value &&) value; }
 };
 
 NAMESPACE_END(detail)
diff --git a/tests/object_py.h b/tests/object_py.h
index e1ba3715..646e2662 100644
--- a/tests/object_py.h
+++ b/tests/object_py.h
@@ -4,14 +4,9 @@ NAMESPACE_BEGIN(nanobind)
 NAMESPACE_BEGIN(detail)
 
 template <typename T> struct type_caster<ref<T>> {
-    using Value = ref<T>;
     using Caster = make_caster<T>;
-    static constexpr auto Name = Caster::Name;
     static constexpr bool IsClass = true;
-
-    template <typename T_> using Cast = movable_cast_t<T_>;
-
-    Value value;
+    NB_TYPE_CASTER(ref<T>, Caster::Name);
 
     bool from_python(handle src, uint8_t flags,
                      cleanup_list *cleanup) noexcept {
@@ -20,7 +15,6 @@ template <typename T> struct type_caster<ref<T>> {
             return false;
 
         value = Value(caster.operator T *());
-
         return true;
     }
 
@@ -28,10 +22,6 @@ template <typename T> struct type_caster<ref<T>> {
                            cleanup_list *cleanup) noexcept {
         return Caster::from_cpp(value.get(), policy, cleanup);
     }
-
-    explicit operator Value *() { return &value; }
-    explicit operator Value &() { return value; }
-    explicit operator Value &&() && { return (Value &&) value; }
 };
 
 NAMESPACE_END(detail)
diff --git a/tests/test_issue.cpp b/tests/test_issue.cpp
index 3d824972..e300de8d 100644
--- a/tests/test_issue.cpp
+++ b/tests/test_issue.cpp
@@ -1,5 +1,6 @@
 #include <nanobind/stl/shared_ptr.h>
 #include <nanobind/stl/string.h>
+#include <nanobind/stl/vector.h>
 #include <unordered_map>
 
 namespace nb = nanobind;
@@ -42,4 +43,19 @@ NB_MODULE(test_issue_ext, m) {
         .def("_get_param", &Model::get_param, "name"_a)
         .def("_add_param", &Model::add_param, "name"_a, "p"_a);
     nb::class_<ModelA, Model>(m, "ModelA").def(nb::init<>{});
+
+    /// Issue #307: move constructor unexpectedly called
+    struct Example { std::string text; };
+
+    nb::class_<Example>(m, "Example")
+        .def(nb::init<const std::string&>())
+        .def("__repr__",
+             [](const Example& e) {
+                 return std::string("Example(\"") + e.text + "\")";
+             });
+
+    m.def("process",
+          [](const std::vector<Example>& v) {
+              return v.size();
+          }, nb::arg("v"));
 }
diff --git a/tests/test_issue.py b/tests/test_issue.py
index 3f67a6f0..d2d81b40 100644
--- a/tests/test_issue.py
+++ b/tests/test_issue.py
@@ -27,3 +27,11 @@ def __init__(self):
     top = Top()
     str(top.model_a)
     str(top.model_a.a)
+
+
+# Issue #307: move constructor unexpectedly called
+def test02_issue_307():
+    l = [m.Example("A"), m.Example("B")]
+    assert str(l) == '[Example("A"), Example("B")]'
+    m.process(l)
+    assert str(l) == '[Example("A"), Example("B")]'
diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp
index 9115654b..6e9ca652 100644
--- a/tests/test_stl.cpp
+++ b/tests/test_stl.cpp
@@ -160,7 +160,7 @@ NB_MODULE(test_stl_ext, m) {
         return x;
     });
 
-    m.def("vec_moveable_in_value", [](std::vector<Movable> x) {
+    m.def("vec_movable_in_value", [](std::vector<Movable> x) {
         if (x.size() != 10)
             fail();
         for (int i = 0; i< 10; ++i)
@@ -178,7 +178,7 @@ NB_MODULE(test_stl_ext, m) {
     });
 
 
-    m.def("vec_moveable_in_lvalue_ref", [](std::vector<Movable> &x) {
+    m.def("vec_movable_in_lvalue_ref", [](std::vector<Movable> &x) {
         if (x.size() != 10)
             fail();
         for (int i = 0; i< 10; ++i)
@@ -187,7 +187,7 @@ NB_MODULE(test_stl_ext, m) {
     });
 
 
-    m.def("vec_moveable_in_rvalue_ref", [](std::vector<Movable> &&x) {
+    m.def("vec_movable_in_rvalue_ref", [](std::vector<Movable> &&x) {
         if (x.size() != 10)
             fail();
         for (int i = 0; i< 10; ++i)
@@ -195,7 +195,7 @@ NB_MODULE(test_stl_ext, m) {
                 fail();
     });
 
-    m.def("vec_moveable_in_ptr_2", [](std::vector<Movable *> x) {
+    m.def("vec_movable_in_ptr_2", [](std::vector<Movable *> x) {
         if (x.size() != 10)
             fail();
         for (int i = 0; i< 10; ++i)
diff --git a/tests/test_stl.py b/tests/test_stl.py
index 8da3d8e8..19e42c12 100644
--- a/tests/test_stl.py
+++ b/tests/test_stl.py
@@ -255,12 +255,11 @@ def test23_vec_return_copyable(clean):
 
 
 def test24_vec_movable_in_value(clean):
-    t.vec_moveable_in_value([t.Movable(i) for i in range(10)])
+    t.vec_movable_in_value([t.Movable(i) for i in range(10)])
     assert_stats(
         value_constructed=10,
         copy_constructed=10,
-        move_constructed=10,
-        destructed=30
+        destructed=20
     )
 
 
@@ -268,22 +267,22 @@ def test25_vec_movable_in_value(clean):
     t.vec_copyable_in_value([t.Copyable(i) for i in range(10)])
     assert_stats(
         value_constructed=10,
-        copy_constructed=20,
-        destructed=30
+        copy_constructed=10,
+        destructed=20
     )
 
 
 def test26_vec_movable_in_lvalue_ref(clean):
-    t.vec_moveable_in_lvalue_ref([t.Movable(i) for i in range(10)])
+    t.vec_movable_in_lvalue_ref([t.Movable(i) for i in range(10)])
     assert_stats(
         value_constructed=10,
-        move_constructed=10,
+        copy_constructed=10,
         destructed=20
     )
 
 
 def test27_vec_movable_in_ptr_2(clean):
-    t.vec_moveable_in_ptr_2([t.Movable(i) for i in range(10)])
+    t.vec_movable_in_ptr_2([t.Movable(i) for i in range(10)])
     assert_stats(
         value_constructed=10,
         destructed=10
@@ -291,10 +290,10 @@ def test27_vec_movable_in_ptr_2(clean):
 
 
 def test28_vec_movable_in_rvalue_ref(clean):
-    t.vec_moveable_in_rvalue_ref([t.Movable(i) for i in range(10)])
+    t.vec_movable_in_rvalue_ref([t.Movable(i) for i in range(10)])
     assert_stats(
         value_constructed=10,
-        move_constructed=10,
+        copy_constructed=10,
         destructed=20
     )
 
@@ -365,7 +364,7 @@ def f():
 
 def test33_vec_type_check():
     with pytest.raises(TypeError) as excinfo:
-        t.vec_moveable_in_value(0)
+        t.vec_movable_in_value(0)
 
 
 def test34_list():
@@ -708,7 +707,7 @@ def test65_class_with_movable_field(clean):
 
     assert_stats(
         value_constructed=2,
-        move_constructed=2
+        copy_constructed=2
     )
 
     del m1, m2
@@ -716,7 +715,7 @@ def test65_class_with_movable_field(clean):
 
     assert_stats(
         value_constructed=2,
-        move_constructed=2,
+        copy_constructed=2,
         destructed=2
     )
 
@@ -725,7 +724,7 @@ def test65_class_with_movable_field(clean):
 
     assert_stats(
         value_constructed=2,
-        move_constructed=2,
+        copy_constructed=2,
         destructed=4
     )