stl.h: propagate return value policies to type-specific casters (#1455)
* stl.h: propagate return value policies to type-specific casters
Return value policies for containers like those handled in in 'stl.h'
are currently broken.
The problem is that detail::return_value_policy_override<C>::policy()
always returns 'move' when given a non-pointer/reference type, e.g.
'std::vector<...>'.
This is sensible behavior for custom types that are exposed via
'py::class_<>', but it does not make sense for types that are handled by
other type casters (STL containers, Eigen matrices, etc.).
This commit changes the behavior so that
detail::return_value_policy_override only becomes active when the type
caster derives from type_caster_generic.
Furthermore, the override logic is called recursively in STL type
casters to enable key/value-specific behavior.
diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h
index 2d33d81..4084d42 100644
--- a/include/pybind11/cast.h
+++ b/include/pybind11/cast.h
@@ -1606,9 +1606,13 @@
// When a value returned from a C++ function is being cast back to Python, we almost always want to
// force `policy = move`, regardless of the return value policy the function/method was declared
-// with. Some classes (most notably Eigen::Ref and related) need to avoid this, and so can do so by
-// specializing this struct.
+// with.
template <typename Return, typename SFINAE = void> struct return_value_policy_override {
+ static return_value_policy policy(return_value_policy p) { return p; }
+};
+
+template <typename Return> struct return_value_policy_override<Return,
+ detail::enable_if_t<std::is_base_of<type_caster_generic, make_caster<Return>>::value, void>> {
static return_value_policy policy(return_value_policy p) {
return !std::is_lvalue_reference<Return>::value && !std::is_pointer<Return>::value
? return_value_policy::move : p;
diff --git a/include/pybind11/eigen.h b/include/pybind11/eigen.h
index 531d070..d963d96 100644
--- a/include/pybind11/eigen.h
+++ b/include/pybind11/eigen.h
@@ -353,14 +353,6 @@
Type value;
};
-// Eigen Ref/Map classes have slightly different policy requirements, meaning we don't want to force
-// `move` when a Ref/Map rvalue is returned; we treat Ref<> sort of like a pointer (we care about
-// the underlying data, not the outer shell).
-template <typename Return>
-struct return_value_policy_override<Return, enable_if_t<is_eigen_dense_map<Return>::value>> {
- static return_value_policy policy(return_value_policy p) { return p; }
-};
-
// Base class for casting reference/map/block/etc. objects back to python.
template <typename MapType> struct eigen_map_caster {
private:
diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h
index 5ddc601..e986d00 100644
--- a/include/pybind11/pybind11.h
+++ b/include/pybind11/pybind11.h
@@ -145,7 +145,7 @@
capture *cap = const_cast<capture *>(reinterpret_cast<const capture *>(data));
/* Override policy for rvalues -- usually to enforce rvp::move on an rvalue */
- const auto policy = return_value_policy_override<Return>::policy(call.func.policy);
+ return_value_policy policy = return_value_policy_override<Return>::policy(call.func.policy);
/* Function scope guard -- defaults to the compile-to-nothing `void_type` */
using Guard = extract_guard_t<Extra...>;
diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h
index eb4c812..faa7087 100644
--- a/include/pybind11/stl.h
+++ b/include/pybind11/stl.h
@@ -83,6 +83,7 @@
template <typename T>
static handle cast(T &&src, return_value_policy policy, handle parent) {
+ policy = return_value_policy_override<Key>::policy(policy);
pybind11::set s;
for (auto &&value : src) {
auto value_ = reinterpret_steal<object>(key_conv::cast(forward_like<T>(value), policy, parent));
@@ -118,9 +119,11 @@
template <typename T>
static handle cast(T &&src, return_value_policy policy, handle parent) {
dict d;
+ return_value_policy policy_key = return_value_policy_override<Key>::policy(policy);
+ return_value_policy policy_value = return_value_policy_override<Value>::policy(policy);
for (auto &&kv : src) {
- auto key = reinterpret_steal<object>(key_conv::cast(forward_like<T>(kv.first), policy, parent));
- auto value = reinterpret_steal<object>(value_conv::cast(forward_like<T>(kv.second), policy, parent));
+ auto key = reinterpret_steal<object>(key_conv::cast(forward_like<T>(kv.first), policy_key, parent));
+ auto value = reinterpret_steal<object>(value_conv::cast(forward_like<T>(kv.second), policy_value, parent));
if (!key || !value)
return handle();
d[key] = value;
@@ -158,6 +161,7 @@
public:
template <typename T>
static handle cast(T &&src, return_value_policy policy, handle parent) {
+ policy = return_value_policy_override<Value>::policy(policy);
list l(src.size());
size_t index = 0;
for (auto &&value : src) {
@@ -252,6 +256,7 @@
static handle cast(T_ &&src, return_value_policy policy, handle parent) {
if (!src)
return none().inc_ref();
+ policy = return_value_policy_override<typename T::value_type>::policy(policy);
return value_conv::cast(*std::forward<T_>(src), policy, parent);
}
@@ -356,6 +361,7 @@
template <typename... Ts>
struct type_caster<std::variant<Ts...>> : variant_caster<std::variant<Ts...>> { };
#endif
+
NAMESPACE_END(detail)
inline std::ostream &operator<<(std::ostream &os, const handle &obj) {
diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp
index 7d53e9c..cd0985d 100644
--- a/tests/test_stl.cpp
+++ b/tests/test_stl.cpp
@@ -8,6 +8,7 @@
*/
#include "pybind11_tests.h"
+#include "constructor_stats.h"
#include <pybind11/stl.h>
// Test with `std::variant` in C++17 mode, or with `boost::variant` in C++11/14
@@ -235,4 +236,21 @@
// test_stl_pass_by_pointer
m.def("stl_pass_by_pointer", [](std::vector<int>* v) { return *v; }, "v"_a=nullptr);
+
+ class Placeholder {
+ public:
+ Placeholder() { print_created(this); }
+ Placeholder(const Placeholder &) = delete;
+ ~Placeholder() { print_destroyed(this); }
+ };
+ py::class_<Placeholder>(m, "Placeholder");
+
+ /// test_stl_vector_ownership
+ m.def("test_stl_ownership",
+ []() {
+ std::vector<Placeholder *> result;
+ result.push_back(new Placeholder());
+ return result;
+ },
+ py::return_value_policy::take_ownership);
}
diff --git a/tests/test_stl.py b/tests/test_stl.py
index 422b02c..2c5e995 100644
--- a/tests/test_stl.py
+++ b/tests/test_stl.py
@@ -2,6 +2,7 @@
from pybind11_tests import stl as m
from pybind11_tests import UserType
+from pybind11_tests import ConstructorStats
def test_vector(doc):
@@ -198,3 +199,12 @@
with pytest.raises(TypeError) as excinfo:
cm.missing_header_return()
assert expected_message in str(excinfo.value)
+
+
+def test_stl_ownership():
+ cstats = ConstructorStats.get(m.Placeholder)
+ assert cstats.alive() == 0
+ r = m.test_stl_ownership()
+ assert len(r) == 1
+ del r
+ assert cstats.alive() == 0