THIS IS THE ISSUE: Revert "stl.h: propagate return value policies to type-specific casters (pybind#1455)"

This reverts commit cbd16a8.

Author: EricCousineau-TRI

include/pybind11/cast.h

@@ -1869,13 +1869,9 @@ template <typename type> using cast_is_temporary_value_reference = bool_constant
 
 // 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.
+// with.  Some classes (most notably Eigen::Ref and related) need to avoid this, and so can do so by
+// specializing this struct.
 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;

include/pybind11/eigen.h

@@ -458,6 +458,14 @@ struct type_caster<Type, enable_if_t<is_eigen_dense_plain<Type>::value>> {
     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:

include/pybind11/pybind11.h

@@ -148,7 +148,7 @@ class cpp_function : public function {
             capture *cap = const_cast<capture *>(reinterpret_cast<const capture *>(data));
 
             /* Override policy for rvalues -- usually to enforce rvp::move on an rvalue */
-            return_value_policy policy = return_value_policy_override<Return>::policy(call.func.policy);
+            const auto 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...>;

include/pybind11/stl.h

@@ -83,7 +83,6 @@ template <typename Type, typename Key> struct set_caster {
 
     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));
@@ -119,11 +118,9 @@ template <typename Type, typename Key, typename Value> struct map_caster {
     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_key, parent));
-            auto value = reinterpret_steal<object>(value_conv::cast(forward_like<T>(kv.second), policy_value, parent));
+            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));
             if (!key || !value)
                 return handle();
             d[key] = value;
@@ -161,7 +158,6 @@ template <typename Type, typename Value> struct list_caster {
 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) {
@@ -256,7 +252,6 @@ template<typename T> struct optional_caster {
     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);
     }
 
@@ -361,7 +356,6 @@ struct variant_caster<V<Ts...>> {
 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) {

tests/test_stl.cpp

@@ -8,7 +8,6 @@
 */
 
 #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
@@ -236,21 +235,4 @@ TEST_SUBMODULE(stl, m) {
 
     // 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);
 }

tests/test_stl.py

@@ -2,7 +2,6 @@
 
 from pybind11_tests import stl as m
 from pybind11_tests import UserType
-from pybind11_tests import ConstructorStats
 
 
 def test_vector(doc):
@@ -199,12 +198,3 @@ def test_missing_header_message():
     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