Skip to content

Commit e328bd6

Browse files
committed
Add movable cast support to type casters
This commit allows type_casters to allow their local values to be moved away, rather than copied, when the type caster instance itself is an rvalue. This only applies (automatically) to type casters using PYBIND11_TYPE_CASTER; the generic type type casters don't own their own pointer, and various value casters (e.g. std::string, std::pair, arithmetic types) already cast to an rvalue (i.e. they return by value). This updates various calling code to attempt to get a movable value whenever the value is itself coming from a type caster about to be destroyed: for example, when constructing an std::pair or various stl.h containers. For types that don't support value moving, the cast_op falls back to an lvalue cast. There wasn't an obvious place to add the tests, so I added them to test_copy_move_policies, but also renamed it to drop the _policies as it now tests more than just policies.
1 parent 4567f1f commit e328bd6

9 files changed

+340
-92
lines changed

include/pybind11/cast.h

Lines changed: 48 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -374,11 +374,31 @@ class type_caster_generic {
374374
object temp;
375375
};
376376

377-
/* Determine suitable casting operator */
377+
/** Determine suitable casting operator for pointer-or-lvalue-casting type casters. The type caster
378+
* needs to provide `operator T*()` and `operator T&()` operators.
379+
*
380+
* If the type supports moving the value away via an `operator T&&() &&` method, it should use
381+
* `movable_cast_op_type` instead.
382+
*/
378383
template <typename T>
379-
using cast_op_type = typename std::conditional<std::is_pointer<typename std::remove_reference<T>::type>::value,
380-
typename std::add_pointer<intrinsic_t<T>>::type,
381-
typename std::add_lvalue_reference<intrinsic_t<T>>::type>::type;
384+
using cast_op_type =
385+
conditional_t<std::is_pointer<typename std::remove_reference<T>::type>::value,
386+
typename std::add_pointer<intrinsic_t<T>>::type,
387+
typename std::add_lvalue_reference<intrinsic_t<T>>::type>;
388+
389+
/** Determine suitable casting operator for a type caster with a movable value. Such a type caster
390+
* needs to provide `operator T*()`, `operator T&()`, and `operator T&&() &&`. The latter will be
391+
* called in appropriate contexts where the value can be moved rather than copied.
392+
*
393+
* These operator are automatically provided when using the PYBIND11_TYPE_CASTER macro.
394+
*/
395+
template <typename T>
396+
using movable_cast_op_type =
397+
conditional_t<std::is_pointer<typename std::remove_reference<T>::type>::value,
398+
typename std::add_pointer<intrinsic_t<T>>::type,
399+
conditional_t<std::is_rvalue_reference<T>::value,
400+
typename std::add_rvalue_reference<intrinsic_t<T>>::type,
401+
typename std::add_lvalue_reference<intrinsic_t<T>>::type>>;
382402

383403
// std::is_copy_constructible isn't quite enough: it lets std::vector<T> (and similar) through when
384404
// T is non-copyable, but code containing such a copy constructor fails to actually compile.
@@ -455,7 +475,7 @@ template <typename type> class type_caster_base : public type_caster_generic {
455475
nullptr, nullptr, holder);
456476
}
457477

458-
template <typename T> using cast_op_type = pybind11::detail::cast_op_type<T>;
478+
template <typename T> using cast_op_type = cast_op_type<T>;
459479

460480
operator itype*() { return (type *) value; }
461481
operator itype&() { if (!value) throw reference_cast_error(); return *((itype *) value); }
@@ -491,8 +511,10 @@ template <typename type> using make_caster = type_caster<intrinsic_t<type>>;
491511
template <typename T> typename make_caster<T>::template cast_op_type<T> cast_op(make_caster<T> &caster) {
492512
return caster.operator typename make_caster<T>::template cast_op_type<T>();
493513
}
494-
template <typename T> typename make_caster<T>::template cast_op_type<T> cast_op(make_caster<T> &&caster) {
495-
return cast_op<T>(caster);
514+
template <typename T> typename make_caster<T>::template cast_op_type<typename std::add_rvalue_reference<T>::type>
515+
cast_op(make_caster<T> &&caster) {
516+
return std::move(caster).operator
517+
typename make_caster<T>::template cast_op_type<typename std::add_rvalue_reference<T>::type>();
496518
}
497519

498520
template <typename type> class type_caster<std::reference_wrapper<type>> : public type_caster_base<type> {
@@ -515,7 +537,8 @@ template <typename type> class type_caster<std::reference_wrapper<type>> : publi
515537
} \
516538
operator type*() { return &value; } \
517539
operator type&() { return value; } \
518-
template <typename _T> using cast_op_type = pybind11::detail::cast_op_type<_T>
540+
operator type&&() && { return std::move(value); } \
541+
template <typename _T> using cast_op_type = pybind11::detail::movable_cast_op_type<_T>
519542

520543

521544
template <typename CharT> using is_std_char_type = any_of<
@@ -885,9 +908,8 @@ template <typename T1, typename T2> class type_caster<std::pair<T1, T2>> {
885908

886909
template <typename T> using cast_op_type = type;
887910

888-
operator type() {
889-
return type(cast_op<T1>(first), cast_op<T2>(second));
890-
}
911+
operator type() & { return type(cast_op<T1>(first), cast_op<T2>(second)); }
912+
operator type() && { return type(cast_op<T1>(std::move(first)), cast_op<T2>(std::move(second))); }
891913
protected:
892914
make_caster<T1> first;
893915
make_caster<T2> second;
@@ -918,17 +940,21 @@ template <typename... Tuple> class type_caster<std::tuple<Tuple...>> {
918940

919941
template <typename T> using cast_op_type = type;
920942

921-
operator type() { return implicit_cast(indices{}); }
943+
operator type() & { return implicit_cast(indices{}); }
944+
operator type() && { return std::move(*this).implicit_cast(indices{}); }
922945

923946
protected:
924947
template <size_t... Is>
925-
type implicit_cast(index_sequence<Is...>) { return type(cast_op<Tuple>(std::get<Is>(value))...); }
948+
type implicit_cast(index_sequence<Is...>) & { return type(cast_op<Tuple>(std::get<Is>(subcasters))...); }
949+
template <size_t... Is>
950+
type implicit_cast(index_sequence<Is...>) && { return type(cast_op<Tuple>(std::move(std::get<Is>(subcasters)))...); }
951+
926952

927953
static constexpr bool load_impl(const sequence &, bool, index_sequence<>) { return true; }
928954

929955
template <size_t... Is>
930956
bool load_impl(const sequence &seq, bool convert, index_sequence<Is...>) {
931-
for (bool r : {std::get<Is>(value).load(seq[Is], convert)...})
957+
for (bool r : {std::get<Is>(subcasters).load(seq[Is], convert)...})
932958
if (!r)
933959
return false;
934960
return true;
@@ -953,7 +979,7 @@ template <typename... Tuple> class type_caster<std::tuple<Tuple...>> {
953979
return result.release();
954980
}
955981

956-
std::tuple<make_caster<Tuple>...> value;
982+
std::tuple<make_caster<Tuple>...> subcasters;
957983
};
958984

959985
/// Helper class which abstracts away certain actions. Users can provide specializations for
@@ -1458,13 +1484,13 @@ class argument_loader {
14581484
}
14591485

14601486
template <typename Return, typename Guard, typename Func>
1461-
enable_if_t<!std::is_void<Return>::value, Return> call(Func &&f) {
1462-
return call_impl<Return>(std::forward<Func>(f), indices{}, Guard{});
1487+
enable_if_t<!std::is_void<Return>::value, Return> call(Func &&f) && {
1488+
return std::move(*this).template call_impl<Return>(std::forward<Func>(f), indices{}, Guard{});
14631489
}
14641490

14651491
template <typename Return, typename Guard, typename Func>
1466-
enable_if_t<std::is_void<Return>::value, void_type> call(Func &&f) {
1467-
call_impl<Return>(std::forward<Func>(f), indices{}, Guard{});
1492+
enable_if_t<std::is_void<Return>::value, void_type> call(Func &&f) && {
1493+
std::move(*this).template call_impl<Return>(std::forward<Func>(f), indices{}, Guard{});
14681494
return void_type();
14691495
}
14701496

@@ -1474,18 +1500,18 @@ class argument_loader {
14741500

14751501
template <size_t... Is>
14761502
bool load_impl_sequence(function_call &call, index_sequence<Is...>) {
1477-
for (bool r : {std::get<Is>(value).load(call.args[Is], call.args_convert[Is])...})
1503+
for (bool r : {std::get<Is>(argcasters).load(call.args[Is], call.args_convert[Is])...})
14781504
if (!r)
14791505
return false;
14801506
return true;
14811507
}
14821508

14831509
template <typename Return, typename Func, size_t... Is, typename Guard>
14841510
Return call_impl(Func &&f, index_sequence<Is...>, Guard &&) {
1485-
return std::forward<Func>(f)(cast_op<Args>(std::get<Is>(value))...);
1511+
return std::forward<Func>(f)(cast_op<Args>(std::move(std::get<Is>(argcasters)))...);
14861512
}
14871513

1488-
std::tuple<make_caster<Args>...> value;
1514+
std::tuple<make_caster<Args>...> argcasters;
14891515
};
14901516

14911517
/// Helper class which collects only positional arguments for a Python function call.

include/pybind11/eigen.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,8 @@ struct type_caster<Type, enable_if_t<is_eigen_dense_plain<Type>::value>> {
341341

342342
operator Type*() { return &value; }
343343
operator Type&() { return value; }
344-
template <typename T> using cast_op_type = cast_op_type<T>;
344+
operator Type&&() && { return std::move(value); }
345+
template <typename T> using cast_op_type = movable_cast_op_type<T>;
345346

346347
private:
347348
Type value;

include/pybind11/pybind11.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,8 @@ class cpp_function : public function {
150150
using Guard = detail::extract_guard_t<Extra...>;
151151

152152
/* Perform the function call */
153-
handle result = cast_out::cast(args_converter.template call<Return, Guard>(cap->f),
154-
policy, call.parent);
153+
handle result = cast_out::cast(
154+
std::move(args_converter).template call<Return, Guard>(cap->f), policy, call.parent);
155155

156156
/* Invoke call policy post-call hook */
157157
detail::process_attributes<Extra...>::postcall(call, result);

include/pybind11/stl.h

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,11 @@ template <typename Type, typename Key> struct set_caster {
6060
return false;
6161
auto s = reinterpret_borrow<pybind11::set>(src);
6262
value.clear();
63-
key_conv conv;
6463
for (auto entry : s) {
64+
key_conv conv;
6565
if (!conv.load(entry, convert))
6666
return false;
67-
value.insert(cast_op<Key>(conv));
67+
value.insert(cast_op<Key &&>(std::move(conv)));
6868
}
6969
return true;
7070
}
@@ -90,14 +90,14 @@ template <typename Type, typename Key, typename Value> struct map_caster {
9090
if (!isinstance<dict>(src))
9191
return false;
9292
auto d = reinterpret_borrow<dict>(src);
93-
key_conv kconv;
94-
value_conv vconv;
9593
value.clear();
9694
for (auto it : d) {
95+
key_conv kconv;
96+
value_conv vconv;
9797
if (!kconv.load(it.first.ptr(), convert) ||
9898
!vconv.load(it.second.ptr(), convert))
9999
return false;
100-
value.emplace(cast_op<Key>(kconv), cast_op<Value>(vconv));
100+
value.emplace(cast_op<Key &&>(std::move(kconv)), cast_op<Value &&>(std::move(vconv)));
101101
}
102102
return true;
103103
}
@@ -124,13 +124,13 @@ template <typename Type, typename Value> struct list_caster {
124124
if (!isinstance<sequence>(src))
125125
return false;
126126
auto s = reinterpret_borrow<sequence>(src);
127-
value_conv conv;
128127
value.clear();
129128
reserve_maybe(s, &value);
130129
for (auto it : s) {
130+
value_conv conv;
131131
if (!conv.load(it, convert))
132132
return false;
133-
value.push_back(cast_op<Value>(conv));
133+
value.push_back(cast_op<Value &&>(std::move(conv)));
134134
}
135135
return true;
136136
}
@@ -185,12 +185,12 @@ template <typename ArrayType, typename Value, bool Resizable, size_t Size = 0> s
185185
auto l = reinterpret_borrow<list>(src);
186186
if (!require_size(l.size()))
187187
return false;
188-
value_conv conv;
189188
size_t ctr = 0;
190189
for (auto it : l) {
190+
value_conv conv;
191191
if (!conv.load(it, convert))
192192
return false;
193-
value[ctr++] = cast_op<Value>(conv);
193+
value[ctr++] = cast_op<Value &&>(std::move(conv));
194194
}
195195
return true;
196196
}
@@ -249,7 +249,7 @@ template<typename T> struct optional_caster {
249249
if (!inner_caster.load(src, convert))
250250
return false;
251251

252-
value.emplace(cast_op<typename T::value_type>(inner_caster));
252+
value.emplace(cast_op<typename T::value_type &&>(std::move(inner_caster)));
253253
return true;
254254
}
255255

tests/CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ set(PYBIND11_TEST_FILES
3333
test_chrono.cpp
3434
test_class_args.cpp
3535
test_constants_and_functions.cpp
36-
test_copy_move_policies.cpp
36+
test_copy_move.cpp
3737
test_docstring_options.cpp
3838
test_eigen.cpp
3939
test_enum.cpp

0 commit comments

Comments
 (0)