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 && using Array = Eigen::Array; using Caster = make_caster; - static constexpr bool IsClass = false; static constexpr auto Name = Caster::Name; template using Cast = T; @@ -249,7 +248,6 @@ struct type_caster, const typename Map::Scalar, typename Map::Scalar>>; using NDArrayCaster = type_caster; - static constexpr bool IsClass = false; static constexpr auto Name = NDArrayCaster::Name; template using Cast = Map; @@ -395,7 +393,6 @@ struct type_caster, static constexpr bool DMapConstructorOwnsData = !Eigen::internal::traits::template match::type::value; - static constexpr bool IsClass = false; static constexpr auto Name = const_name(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 using Cast = movable_cast_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' which users of a type caster must + * query to determine what the caster actually can (and prefers) to produce. + * The convenience alias ``cast_t`` defined below performs this query for a + * given type ``T``. + * + * Often ``cast_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>`` yields ``cast_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&>`` + * yields ``std::pair``. + */ + +/// Ask a type caster what flavors of a type it can actually produce -- may be different from 'T' template using cast_t = typename make_caster::template Cast; +/// 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 -using simple_cast_t = - std::conditional_t, intrinsic_t *, intrinsic_t &>; +using movable_cast_t = + std::conditional_t, intrinsic_t *, + std::conditional_t, + intrinsic_t &, intrinsic_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 -using movable_cast_t = +using precise_cast_t = std::conditional_t, intrinsic_t *, std::conditional_t, intrinsic_t &&, intrinsic_t &>>; @@ -105,13 +135,11 @@ struct type_caster && !is_std_char_v>> template <> struct type_caster { static constexpr auto Name = const_name("None"); - static constexpr bool IsClass = false; }; template <> struct type_caster { template 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 { template <> struct type_caster { using Value = const char *; Value value; - static constexpr bool IsClass = false; static constexpr auto Name = const_name("str"); template using Cast = std::conditional_t, const char *, char>; @@ -289,12 +316,10 @@ template NB_INLINE rv_policy infer_policy(rv_policy policy) { template struct type_hook : std::false_type { }; -template struct type_caster_base { +template struct type_caster_base : type_caster_base_tag { using Type = Type_; static constexpr auto Name = const_name(); - static constexpr bool IsClass = true; - - template using Cast = movable_cast_t; + template using Cast = precise_cast_t; NB_INLINE bool from_python(handle src, uint8_t flags, cleanup_list *cleanup) noexcept { @@ -335,7 +360,7 @@ template 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 bool try_cast(const detail::api &value, T &out, bool convert = true) noexcept { using Caster = detail::make_caster; - using Output = typename Caster::template Cast; static_assert(!std::is_same_v, "nanobind::try_cast(): cannot return a reference to a temporary."); @@ -361,11 +385,7 @@ bool try_cast(const detail::api &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(); return true; } @@ -378,11 +398,11 @@ T cast(const detail::api &value, bool convert = true) { return; } else { using Caster = detail::make_caster; - using Output = typename Caster::template Cast; static_assert( - !(std::is_reference_v || std::is_pointer_v) || Caster::IsClass || - std::is_same_v, + !(std::is_reference_v || std::is_pointer_v) || + detail::is_base_caster_v || + std::is_same_v, "nanobind::cast(): cannot return a reference to a temporary."); Caster caster; @@ -397,7 +417,7 @@ T cast(const detail::api &value, bool convert = true) { #pragma GCC diagnostic ignored "-Wmaybe-uninitialized" #endif - return caster.operator Output(); + return caster.operator detail::cast_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 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::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 v, Args... args) { @@ -324,12 +318,6 @@ template 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; - static_assert( - detail::make_caster::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 struct init_implicit { new ((Alias *) v.p) Alias{ (detail::forward_t) arg }; }, is_implicit(), extra...); - if constexpr (!Caster::IsClass) { + using Caster = detail::make_caster; + + if constexpr (!detail::is_class_caster_v) { 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::type; using Alias = typename detail::extract::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 + !std::is_same_v, "nanobind::class_<> was invoked with extra arguments that could not be handled"); + static_assert( + detail::is_base_caster_v>, + "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 ? 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 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, "def_rw() requires a (base) class member!"); - using Q = std::conditional_t::IsClass, const D &, D &&>; + using Q = + std::conditional_t>, + 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 NB_INLINE class_ &def_rw_static(const char *name, D *p, const Extra &...extra) { - using Q = std::conditional_t::IsClass, const D &, D &&>; + using Q = + std::conditional_t>, + const D &, D &&>; def_prop_rw_static(name, [p](handle) -> const D & { return *p; }, @@ -624,7 +628,7 @@ template class enum_ : public class_ { template void implicitly_convertible() { using Caster = detail::make_caster; - if constexpr (Caster::IsClass) { + if constexpr (detail::is_base_caster_v) { 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) { - cap->func(((make_caster&&) in.template get()).operator cast_t()...); + cap->func(in.template get().operator cast_t()...); result = Py_None; Py_INCREF(result); } else { result = cast_out::from_cpp( - cap->func(((make_caster &&) in.template get()) + cap->func((in.template get()) .operator cast_t()...), 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 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 +constexpr bool is_base_caster_v = std::is_base_of_v; + +template using is_class_caster_test = std::enable_if_t; + +/// Generalized version of the is_base_caster_v test that also accepts unique_ptr/shared_ptr +template +constexpr bool is_class_caster_v = detail::detector::value; + NAMESPACE_END(detail) template 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 NB_INLINE bool isinstance(handle h) noexcept { if constexpr (std::is_base_of_v) return T::check_(h); - else if constexpr (detail::make_caster::IsClass) + else if constexpr (detail::is_base_caster_v>) return detail::nb_type_isinstance(h.ptr(), &typeid(detail::intrinsic_t)); else return detail::make_caster().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 struct array_caster { break; } - value[i] = ((Caster &&) caster).operator cast_t(); + value[i] = caster.operator cast_t(); } 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 struct dict_caster { break; } - value.emplace(((KeyCaster &&) key_caster).operator cast_t(), - ((ElementCaster &&) element_caster).operator cast_t()); + value.emplace(key_caster.operator cast_t(), + element_caster.operator cast_t()); } 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 struct list_caster { break; } - value.push_back(((Caster &&) caster).operator cast_t()); + value.push_back(caster.operator cast_t()); } 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 struct set_caster { if (!success) break; - value.emplace(((KeyCaster &&) key_caster).operator cast_t()); + value.emplace(key_caster.operator cast_t()); } 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 constexpr bool is_copy_assignable_v = is_copy_assignable::value; // Analogous template for checking comparability -template using comparable_test = decltype(std::declval() == std::declval()); +template using comparable_test = decltype(std::declval() == std::declval()); template 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 struct remove_opt_mono> : remove_opt_mono {}; +template struct remove_opt_mono> + : remove_opt_mono { }; template struct type_caster> { - using Value = std::optional; - using Ti = detail::intrinsic_t; - using Caster = make_caster; + using Caster = make_caster; - static constexpr auto Name = const_name("Optional[") + concat(Caster::Name) + const_name("]"); - static constexpr bool IsClass = false; + NB_TYPE_CASTER(std::optional, const_name("Optional[") + + concat(Caster::Name) + + const_name("]")); - template - using Cast = movable_cast_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> { if (!caster.from_python(src, flags, cleanup)) return false; - if constexpr (is_pointer_v) { - static_assert(Caster::IsClass, - "Binding 'optional' requires that 'T' can also be bound by nanobind."); - value = caster.operator cast_t(); - } else if constexpr (Caster::IsClass) { - value = caster.operator cast_t(); - } else { - value = std::move(caster).operator cast_t(); - } + static_assert( + !std::is_pointer_v || is_base_caster_v, + "Binding ``optional`` 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(); - template - 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 static handle from_cpp(T_ &&value, rv_policy policy, cleanup_list *cleanup) noexcept { if (!value) return none().release(); + return Caster::from_cpp(forward_like(*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 struct type_caster> { using Caster1 = make_caster; using Caster2 = make_caster; - /** - * \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 using Cast = default_cast_t; - * // or, even better: - * template using Cast = movable_cast_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 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 struct type_caster> { } /// Return the constructed tuple by copying from the sub-casters - explicit operator Value() & { + explicit operator Value() { return Value(caster1.operator cast_t(), caster2.operator cast_t()); } - // Return the constructed type by moving from the sub-casters - explicit operator Value() && { - return Value(((Caster1 &&) caster1).operator cast_t(), - ((Caster2 &&) caster2).operator cast_t()); - } - 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 &&ptr, } template struct type_caster> { - using Td = std::decay_t; - using Value = std::shared_ptr; - using Caster = make_caster; - static_assert(Caster::IsClass, - "Binding 'shared_ptr' 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; + using Td = std::decay_t; - template using Cast = movable_cast_t; + NB_TYPE_CASTER(std::shared_ptr, Caster::Name) - Value value; + static_assert(is_base_caster_v, + "Conversion of ``shared_ptr`` 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 struct type_caster> { 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 struct type_caster> { 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 struct type_caster> { using Value = std::tuple; using Indices = std::make_index_sequence; - static constexpr bool IsClass = false; static constexpr auto Name = const_name(NB_TYPING_TUPLE "[") + concat(make_caster::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 using Cast = Value; bool from_python(handle src, uint8_t flags, @@ -35,7 +38,7 @@ template struct type_caster> { } template - 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) noexcept { (void) src; (void) flags; (void) cleanup; @@ -66,7 +69,7 @@ template struct type_caster> { } template - 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) noexcept { (void) value; (void) policy; (void) cleanup; @@ -86,24 +89,12 @@ template struct type_caster> { 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 - NB_INLINE Value cast_impl(std::index_sequence) & { + template Value cast_impl(std::index_sequence) { return Value(std::get(casters).operator cast_t()...); } - template - NB_INLINE Value cast_impl(std::index_sequence) && { - return Value(((make_caster &&) std::get(casters)).operator cast_t()...); - } - std::tuple...> 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 struct type_caster> { + static constexpr bool IsClass = true; using Value = std::unique_ptr; using Caster = make_caster; @@ -53,17 +54,18 @@ struct type_caster> { static constexpr bool IsNanobindDeleter = std::is_same_v>; - static_assert(Caster::IsClass, - "Binding 'std::unique_ptr' 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, + "Conversion of ``unique_ptr`` 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 requires that 'Deleter' is either " - "'std::default_delete' or 'nanobind::deleter'"); + "Binding std::unique_ptr requires that " + "'Deleter' is either 'std::default_delete' or " + "'nanobind::deleter'"); static constexpr auto Name = Caster::Name; - static constexpr bool IsClass = true; template 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, Ts3...> template struct remove_opt_mono> : concat_variant, std::variant<>, std::variant>...> {}; -template <> -struct type_caster { +template <> struct type_caster { 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 { } }; -template -class type_caster> { - template using Caster = make_caster>; +template struct type_caster> { + NB_TYPE_CASTER(std::variant, + const_name("Union[") + concat(make_caster::Name...) + const_name("]")); template - bool variadic_caster(const handle &src, uint8_t flags, cleanup_list *cleanup) { - Caster caster; - if (!caster.from_python(src, flags, cleanup)) - return false; - - if constexpr (is_pointer_v) { - static_assert(Caster::IsClass, - "Binding 'variant' requires that 'T' can also be bound by nanobind."); - value = caster.operator cast_t(); - } else if constexpr (Caster::IsClass) { - // Non-pointer classes do not accept a null pointer - if (src.is_none()) - return false; - value = caster.operator cast_t(); - } else { - value = std::move(caster).operator cast_t(); - } - return true; - } + bool try_variant(const handle &src, uint8_t flags, cleanup_list *cleanup) { + using CasterT = make_caster; -public: - using Value = std::variant; + static_assert( + !std::is_pointer_v || is_base_caster_v, + "Binding ``variant`` 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::Name...) + const_name("]"); - static constexpr bool IsClass = false; + CasterT caster; - template using Cast = movable_cast_t; + if (!caster.from_python(src, flags, cleanup)) + return false; - Value value; + value = caster.operator cast_t(); - bool from_python(handle src, uint8_t flags, cleanup_list *cleanup) noexcept { - return (variadic_caster(src, flags, cleanup) || ...); + return true; } - template - 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(src, flags, cleanup) || ...); } template static handle from_cpp(T &&value, rv_policy policy, cleanup_list *cleanup) noexcept { - return std::visit([&](auto &&v) { - return Caster::from_cpp(std::forward(v), policy, cleanup); - }, std::forward(value)); + return std::visit( + [&](auto &&v) { + return make_caster::from_cpp( + std::forward(v), policy, cleanup); + }, + std::forward(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 struct type_caster> { - using Value = ref; using Caster = make_caster; - static constexpr auto Name = Caster::Name; static constexpr bool IsClass = true; - - template using Cast = movable_cast_t; - - Value value; + NB_TYPE_CASTER(ref, Caster::Name); bool from_python(handle src, uint8_t flags, cleanup_list *cleanup) noexcept { @@ -20,7 +15,6 @@ template struct type_caster> { return false; value = Value(caster.operator T *()); - return true; } @@ -28,10 +22,6 @@ template struct type_caster> { 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 #include +#include #include 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_(m, "ModelA").def(nb::init<>{}); + + /// Issue #307: move constructor unexpectedly called + struct Example { std::string text; }; + + nb::class_(m, "Example") + .def(nb::init()) + .def("__repr__", + [](const Example& e) { + return std::string("Example(\"") + e.text + "\")"; + }); + + m.def("process", + [](const std::vector& 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 x) { + m.def("vec_movable_in_value", [](std::vector 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 &x) { + m.def("vec_movable_in_lvalue_ref", [](std::vector &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 &&x) { + m.def("vec_movable_in_rvalue_ref", [](std::vector &&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 x) { + m.def("vec_movable_in_ptr_2", [](std::vector 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 )