Use rvalue subcasting when casting an rvalue container
This updates the std::tuple, std::pair and `stl.h` type casters to
forward their contained value according to whether the container being
cast is an lvalue or rvalue reference. This fixes an issue where
subcaster casts were always called with a const lvalue which meant
nested type casters didn't have the desired `cast()` overload invoked.
For example, this caused Eigen values in a tuple to end up with a
readonly flag (issue #935) and made it impossible to return a container
of move-only types (issue #853).
This fixes both issues by adding templated universal reference `cast()`
methods to the various container types that forward container elements
according to the container reference type.
diff --git a/tests/pybind11_tests.h b/tests/pybind11_tests.h
index dd8d159..18672cd 100644
--- a/tests/pybind11_tests.h
+++ b/tests/pybind11_tests.h
@@ -1,6 +1,11 @@
#pragma once
#include <pybind11/pybind11.h>
+#if defined(_MSC_VER) && _MSC_VER < 1910
+// We get some really long type names here which causes MSVC 2015 to emit warnings
+# pragma warning(disable: 4503) // warning C4503: decorated name length exceeded, name was truncated
+#endif
+
namespace py = pybind11;
using namespace pybind11::literals;
@@ -43,3 +48,17 @@
IncType &operator=(const IncType &) = delete;
IncType &operator=(IncType &&) = delete;
};
+
+/// Custom cast-only type that casts to a string "rvalue" or "lvalue" depending on the cast context.
+/// Used to test recursive casters (e.g. std::tuple, stl containers).
+struct RValueCaster {};
+NAMESPACE_BEGIN(pybind11)
+NAMESPACE_BEGIN(detail)
+template<> class type_caster<RValueCaster> {
+public:
+ PYBIND11_TYPE_CASTER(RValueCaster, _("RValueCaster"));
+ static handle cast(RValueCaster &&, return_value_policy, handle) { return py::str("rvalue").release(); }
+ static handle cast(const RValueCaster &, return_value_policy, handle) { return py::str("lvalue").release(); }
+};
+NAMESPACE_END(detail)
+NAMESPACE_END(pybind11)
diff --git a/tests/test_builtin_casters.cpp b/tests/test_builtin_casters.cpp
index 6733402..bf972e1 100644
--- a/tests/test_builtin_casters.cpp
+++ b/tests/test_builtin_casters.cpp
@@ -86,6 +86,16 @@
return std::make_tuple(std::get<2>(input), std::get<1>(input), std::get<0>(input));
}, "Return a triple in reversed order");
m.def("empty_tuple", []() { return std::tuple<>(); });
+ static std::pair<RValueCaster, RValueCaster> lvpair;
+ static std::tuple<RValueCaster, RValueCaster, RValueCaster> lvtuple;
+ static std::pair<RValueCaster, std::tuple<RValueCaster, std::pair<RValueCaster, RValueCaster>>> lvnested;
+ m.def("rvalue_pair", []() { return std::make_pair(RValueCaster{}, RValueCaster{}); });
+ m.def("lvalue_pair", []() -> const decltype(lvpair) & { return lvpair; });
+ m.def("rvalue_tuple", []() { return std::make_tuple(RValueCaster{}, RValueCaster{}, RValueCaster{}); });
+ m.def("lvalue_tuple", []() -> const decltype(lvtuple) & { return lvtuple; });
+ m.def("rvalue_nested", []() {
+ return std::make_pair(RValueCaster{}, std::make_tuple(RValueCaster{}, std::make_pair(RValueCaster{}, RValueCaster{}))); });
+ m.def("lvalue_nested", []() -> const decltype(lvnested) & { return lvnested; });
// test_builtins_cast_return_none
m.def("return_none_string", []() -> std::string * { return nullptr; });
diff --git a/tests/test_builtin_casters.py b/tests/test_builtin_casters.py
index 32eba45..d7d49b6 100644
--- a/tests/test_builtin_casters.py
+++ b/tests/test_builtin_casters.py
@@ -201,6 +201,13 @@
Return a triple in reversed order
"""
+ assert m.rvalue_pair() == ("rvalue", "rvalue")
+ assert m.lvalue_pair() == ("lvalue", "lvalue")
+ assert m.rvalue_tuple() == ("rvalue", "rvalue", "rvalue")
+ assert m.lvalue_tuple() == ("lvalue", "lvalue", "lvalue")
+ assert m.rvalue_nested() == ("rvalue", ("rvalue", ("rvalue", "rvalue")))
+ assert m.lvalue_nested() == ("lvalue", ("lvalue", ("lvalue", "lvalue")))
+
def test_builtins_cast_return_none():
"""Casters produced with PYBIND11_TYPE_CASTER() should convert nullptr to None"""
diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp
index 9762fb9..839c1af 100644
--- a/tests/test_stl.cpp
+++ b/tests/test_stl.cpp
@@ -61,6 +61,46 @@
return set.count("key1") && set.count("key2") && set.count("key3");
});
+ // test_recursive_casting
+ m.def("cast_rv_vector", []() { return std::vector<RValueCaster>{2}; });
+ m.def("cast_rv_array", []() { return std::array<RValueCaster, 3>(); });
+ // NB: map and set keys are `const`, so while we technically do move them (as `const Type &&`),
+ // casters don't typically do anything with that, which means they fall to the `const Type &`
+ // caster.
+ m.def("cast_rv_map", []() { return std::unordered_map<std::string, RValueCaster>{{"a", RValueCaster{}}}; });
+ m.def("cast_rv_nested", []() {
+ std::vector<std::array<std::list<std::unordered_map<std::string, RValueCaster>>, 2>> v;
+ v.emplace_back(); // add an array
+ v.back()[0].emplace_back(); // add a map to the array
+ v.back()[0].back().emplace("b", RValueCaster{});
+ v.back()[0].back().emplace("c", RValueCaster{});
+ v.back()[1].emplace_back(); // add a map to the array
+ v.back()[1].back().emplace("a", RValueCaster{});
+ return v;
+ });
+ static std::vector<RValueCaster> lvv{2};
+ static std::array<RValueCaster, 2> lva;
+ static std::unordered_map<std::string, RValueCaster> lvm{{"a", RValueCaster{}}, {"b", RValueCaster{}}};
+ static std::unordered_map<std::string, std::vector<std::list<std::array<RValueCaster, 2>>>> lvn;
+ lvn["a"].emplace_back(); // add a list
+ lvn["a"].back().emplace_back(); // add an array
+ lvn["a"].emplace_back(); // another list
+ lvn["a"].back().emplace_back(); // add an array
+ lvn["b"].emplace_back(); // add a list
+ lvn["b"].back().emplace_back(); // add an array
+ lvn["b"].back().emplace_back(); // add another array
+ m.def("cast_lv_vector", []() -> const decltype(lvv) & { return lvv; });
+ m.def("cast_lv_array", []() -> const decltype(lva) & { return lva; });
+ m.def("cast_lv_map", []() -> const decltype(lvm) & { return lvm; });
+ m.def("cast_lv_nested", []() -> const decltype(lvn) & { return lvn; });
+ // #853:
+ m.def("cast_unique_ptr_vector", []() {
+ std::vector<std::unique_ptr<UserType>> v;
+ v.emplace_back(new UserType{7});
+ v.emplace_back(new UserType{42});
+ return v;
+ });
+
struct MoveOutContainer {
struct Value { int value; };
diff --git a/tests/test_stl.py b/tests/test_stl.py
index fdbca0d..0f01982 100644
--- a/tests/test_stl.py
+++ b/tests/test_stl.py
@@ -58,6 +58,25 @@
assert doc(m.load_set) == "load_set(arg0: Set[str]) -> bool"
+def test_recursive_casting():
+ """Tests that stl casters preserve lvalue/rvalue context for container values"""
+ assert m.cast_rv_vector() == ["rvalue", "rvalue"]
+ assert m.cast_lv_vector() == ["lvalue", "lvalue"]
+ assert m.cast_rv_array() == ["rvalue", "rvalue", "rvalue"]
+ assert m.cast_lv_array() == ["lvalue", "lvalue"]
+ assert m.cast_rv_map() == {"a": "rvalue"}
+ assert m.cast_lv_map() == {"a": "lvalue", "b": "lvalue"}
+ assert m.cast_rv_nested() == [[[{"b": "rvalue", "c": "rvalue"}], [{"a": "rvalue"}]]]
+ assert m.cast_lv_nested() == {
+ "a": [[["lvalue", "lvalue"]], [["lvalue", "lvalue"]]],
+ "b": [[["lvalue", "lvalue"], ["lvalue", "lvalue"]]]
+ }
+
+ # Issue #853 test case:
+ z = m.cast_unique_ptr_vector()
+ assert z[0].value == 7 and z[1].value == 42
+
+
def test_move_out_container():
"""Properties use the `reference_internal` policy by default. If the underlying function
returns an rvalue, the policy is automatically changed to `move` to avoid referencing
diff --git a/tests/test_stl_binders.cpp b/tests/test_stl_binders.cpp
index f636c0b..22ba16e 100644
--- a/tests/test_stl_binders.cpp
+++ b/tests/test_stl_binders.cpp
@@ -15,11 +15,6 @@
#include <deque>
#include <unordered_map>
-#ifdef _MSC_VER
-// We get some really long type names here which causes MSVC to emit warnings
-# pragma warning(disable: 4503) // warning C4503: decorated name length exceeded, name was truncated
-#endif
-
class El {
public:
El() = delete;