Introduce return_value_policy_pack

rwgk · rwgk · commit 517ed53efa16 · 2023-02-07T07:07:57.000-08:00
Currently only for string_caster, tuple_caster, map_caster
diff --git a/include/pybind11/attr.h b/include/pybind11/attr.h
@@ -212,8 +212,8 @@ struct function_record {
     /// Pointer to custom destructor for 'data' (if needed)
     void (*free_data)(function_record *ptr) = nullptr;
 
-    /// Return value policy associated with this function
-    return_value_policy policy = return_value_policy::automatic;
+    /// Return value options associated with this function
+    return_value_policy_pack rvpp;
 
     /// True if name == '__init__'
     bool is_constructor : 1;
@@ -407,7 +407,12 @@ struct process_attribute<char *> : process_attribute<const char *> {};
 /// Process an attribute indicating the function's return value policy
 template <>
 struct process_attribute<return_value_policy> : process_attribute_default<return_value_policy> {
-    static void init(const return_value_policy &p, function_record *r) { r->policy = p; }
+    static void init(const return_value_policy &p, function_record *r) { r->rvpp.policy = p; }
+};
+template <>
+struct process_attribute<return_value_policy_pack>
+    : process_attribute_default<return_value_policy_pack> {
+    static void init(const return_value_policy_pack &rvpp, function_record *r) { r->rvpp = rvpp; }
 };
 
 /// Process an attribute which indicates that this is an overloaded function associated with a
diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h
@@ -464,11 +464,12 @@ struct string_caster {
         return true;
     }
 
-    static handle cast(const StringType &src, return_value_policy policy, handle /* parent */) {
+    static handle
+    cast(const StringType &src, const return_value_policy_pack &rvpp, handle /* parent */) {
         const char *buffer = reinterpret_cast<const char *>(src.data());
         auto nbytes = ssize_t(src.size() * sizeof(CharT));
         handle s;
-        if (policy == return_value_policy::_return_as_bytes) {
+        if (rvpp.policy == return_value_policy::_return_as_bytes) {
             s = PyBytes_FromStringAndSize(buffer, nbytes);
         } else {
             s = decode_utfN(buffer, nbytes);
@@ -676,22 +677,22 @@ class tuple_caster {
     }
 
     template <typename T>
-    static handle cast(T &&src, return_value_policy policy, handle parent) {
-        return cast_impl(std::forward<T>(src), policy, parent, indices{});
+    static handle cast(T &&src, const return_value_policy_pack &rvpp, handle parent) {
+        return cast_impl(std::forward<T>(src), rvpp, parent, indices{});
     }
 
     // copied from the PYBIND11_TYPE_CASTER macro
     template <typename T>
-    static handle cast(T *src, return_value_policy policy, handle parent) {
+    static handle cast(T *src, const return_value_policy_pack &rvpp, handle parent) {
         if (!src) {
             return none().release();
         }
-        if (policy == return_value_policy::take_ownership) {
-            auto h = cast(std::move(*src), policy, parent);
+        if (rvpp.policy == return_value_policy::take_ownership) {
+            auto h = cast(std::move(*src), rvpp, parent);
             delete src;
             return h;
         }
-        return cast(*src, policy, parent);
+        return cast(*src, rvpp, parent);
     }
 
     static constexpr auto name
@@ -733,12 +734,14 @@ class tuple_caster {
 
     /* Implementation: Convert a C++ tuple into a Python tuple */
     template <typename T, size_t... Is>
-    static handle
-    cast_impl(T &&src, return_value_policy policy, handle parent, index_sequence<Is...>) {
-        PYBIND11_WORKAROUND_INCORRECT_MSVC_C4100(src, policy, parent);
-        PYBIND11_WORKAROUND_INCORRECT_GCC_UNUSED_BUT_SET_PARAMETER(policy, parent);
+    static handle cast_impl(T &&src,
+                            const return_value_policy_pack &rvpp,
+                            handle parent,
+                            index_sequence<Is...>) {
+        PYBIND11_WORKAROUND_INCORRECT_MSVC_C4100(src, rvpp, parent);
+        PYBIND11_WORKAROUND_INCORRECT_GCC_UNUSED_BUT_SET_PARAMETER(rvpp, parent);
         std::array<object, size> entries{{reinterpret_steal<object>(
-            make_caster<Ts>::cast(std::get<Is>(std::forward<T>(src)), policy, parent))...}};
+            make_caster<Ts>::cast(std::get<Is>(std::forward<T>(src)), rvpp.get(Is), parent))...}};
         for (const auto &entry : entries) {
             if (!entry) {
                 return handle();
diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h
@@ -560,6 +560,30 @@ enum class return_value_policy : uint8_t {
 #define PYBIND11_HAS_RETURN_VALUE_POLICY_RETURN_AS_BYTES
 #define PYBIND11_HAS_RETURN_VALUE_POLICY_CLIF_AUTOMATIC
 
+struct return_value_policy_pack {
+    return_value_policy policy = return_value_policy::automatic;
+    std::vector<return_value_policy_pack> vec_rvpp;
+
+    return_value_policy_pack() = default;
+
+    // NOLINTNEXTLINE(google-explicit-constructor)
+    return_value_policy_pack(return_value_policy policy) : policy(policy) {}
+
+    // NOLINTNEXTLINE(google-explicit-constructor)
+    return_value_policy_pack(std::initializer_list<return_value_policy_pack> vec_rvpp)
+        : vec_rvpp(vec_rvpp) {}
+
+    // NOLINTNEXTLINE(google-explicit-constructor)
+    operator return_value_policy() const { return policy; }
+
+    return_value_policy_pack get(std::size_t i) const {
+        if (vec_rvpp.empty()) {
+            return policy;
+        }
+        return vec_rvpp.at(i);
+    }
+};
+
 PYBIND11_NAMESPACE_BEGIN(detail)
 
 inline static constexpr int log2(size_t n, int k = 0) {
diff --git a/include/pybind11/detail/type_caster_odr_guard.h b/include/pybind11/detail/type_caster_odr_guard.h
@@ -117,11 +117,12 @@ struct type_caster_odr_guard : TypeCasterType {
 
     // The original author of this function is @amauryfa
     template <typename CType, typename... Arg>
-    static handle cast(CType &&src, return_value_policy policy, handle parent, Arg &&...arg) {
+    static handle
+    cast(CType &&src, const return_value_policy_pack &rvpp, handle parent, Arg &&...arg) {
         if (translation_unit_local) {
         }
         return TypeCasterType::cast(
-            std::forward<CType>(src), policy, parent, std::forward<Arg>(arg)...);
+            std::forward<CType>(src), rvpp, parent, std::forward<Arg>(arg)...);
     }
 };
 
diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h
@@ -238,17 +238,15 @@ class cpp_function : public function {
             auto *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);
+            return_value_policy_pack rvpp = call.func.rvpp;
+            rvpp.policy = return_value_policy_override<Return>::policy(rvpp.policy);
 
             /* Function scope guard -- defaults to the compile-to-nothing `void_type` */
             using Guard = extract_guard_t<Extra...>;
 
             /* Perform the function call */
-            handle result
-                = cast_out::cast(std::move(args_converter).template call<Return, Guard>(cap->f),
-                                 policy,
-                                 call.parent);
+            handle result = cast_out::cast(
+                std::move(args_converter).template call<Return, Guard>(cap->f), rvpp, call.parent);
 
             /* Invoke call policy post-call hook */
             process_attributes<Extra...>::postcall(call, result);
diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h
@@ -135,19 +135,19 @@ struct map_caster {
     }
 
     template <typename T>
-    static handle cast(T &&src, return_value_policy policy, handle parent) {
+    static handle cast(T &&src, const return_value_policy_pack &rvpp, handle parent) {
         dict d;
-        return_value_policy policy_key = policy;
-        return_value_policy policy_value = policy;
+        return_value_policy_pack rvpp_key = rvpp.get(0);
+        return_value_policy_pack rvpp_value = rvpp.get(1);
         if (!std::is_lvalue_reference<T>::value) {
-            policy_key = return_value_policy_override<Key>::policy(policy_key);
-            policy_value = return_value_policy_override<Value>::policy(policy_value);
+            rvpp_key.policy = return_value_policy_override<Key>::policy(rvpp_key.policy);
+            rvpp_value.policy = return_value_policy_override<Value>::policy(rvpp_value.policy);
         }
         for (auto &&kv : src) {
             auto key = reinterpret_steal<object>(
-                key_conv::cast(detail::forward_like<T>(kv.first), policy_key, parent));
+                key_conv::cast(detail::forward_like<T>(kv.first), rvpp_key, parent));
             auto value = reinterpret_steal<object>(
-                value_conv::cast(detail::forward_like<T>(kv.second), policy_value, parent));
+                value_conv::cast(detail::forward_like<T>(kv.second), rvpp_value, parent));
             if (!key || !value) {
                 return handle();
             }
diff --git a/tests/test_gil_scoped.py b/tests/test_gil_scoped.py
@@ -144,7 +144,9 @@ def _intentional_deadlock():
     m.intentional_deadlock()
 
 
-ALL_BASIC_TESTS_PLUS_INTENTIONAL_DEADLOCK = ALL_BASIC_TESTS + (_intentional_deadlock,)
+ALL_BASIC_TESTS_PLUS_INTENTIONAL_DEADLOCK = ALL_BASIC_TESTS[
+    :1
+]  # + (_intentional_deadlock,)
 
 
 def _run_in_process(target, *args, **kwargs):
diff --git a/tests/test_return_value_policy_pack.cpp b/tests/test_return_value_policy_pack.cpp
@@ -0,0 +1,78 @@
+#include <pybind11/stl.h>
+
+#include "pybind11_tests.h"
+
+#include <string>
+#include <utility>
+
+namespace {
+
+using PairString = std::pair<std::string, std::string>;
+
+PairString return_pair_string() { return PairString({"", ""}); }
+
+using NestedPairString = std::pair<PairString, PairString>;
+
+NestedPairString return_nested_pair_string() {
+    return NestedPairString(return_pair_string(), return_pair_string());
+}
+
+using MapString = std::map<std::string, std::string>;
+
+MapString return_map_string() { return MapString({return_pair_string()}); }
+
+using MapPairString = std::map<PairString, PairString>;
+
+MapPairString return_map_pair_string() { return MapPairString({return_nested_pair_string()}); }
+
+} // namespace
+
+TEST_SUBMODULE(return_value_policy_pack, m) {
+    auto rvpc = py::return_value_policy::_clif_automatic;
+    auto rvpb = py::return_value_policy::_return_as_bytes;
+
+    m.def("return_tuple_str_str", []() { return return_pair_string(); });
+    m.def(
+        "return_tuple_bytes_bytes", []() { return return_pair_string(); }, rvpb);
+    m.def(
+        "return_tuple_str_bytes",
+        []() { return return_pair_string(); },
+        py::return_value_policy_pack({rvpc, rvpb}));
+    m.def(
+        "return_tuple_bytes_str",
+        []() { return return_pair_string(); },
+        py::return_value_policy_pack({rvpb, rvpc}));
+
+    m.def("return_nested_tuple_str", []() { return return_nested_pair_string(); });
+    m.def(
+        "return_nested_tuple_bytes", []() { return return_nested_pair_string(); }, rvpb);
+    m.def(
+        "return_nested_tuple_str_bytes_bytes_str",
+        []() { return return_nested_pair_string(); },
+        py::return_value_policy_pack({{rvpc, rvpb}, {rvpb, rvpc}}));
+    m.def(
+        "return_nested_tuple_bytes_str_str_bytes",
+        []() { return return_nested_pair_string(); },
+        py::return_value_policy_pack({{rvpb, rvpc}, {rvpc, rvpb}}));
+
+    m.def("return_dict_str_str", []() { return return_map_string(); });
+    m.def(
+        "return_dict_bytes_bytes", []() { return return_map_string(); }, rvpb);
+    m.def(
+        "return_dict_str_bytes",
+        []() { return return_map_string(); },
+        py::return_value_policy_pack({rvpc, rvpb}));
+    m.def(
+        "return_dict_bytes_str",
+        []() { return return_map_string(); },
+        py::return_value_policy_pack({rvpb, rvpc}));
+
+    m.def(
+        "return_dict_sbbs",
+        []() { return return_map_pair_string(); },
+        py::return_value_policy_pack({{rvpc, rvpb}, {rvpb, rvpc}}));
+    m.def(
+        "return_dict_bssb",
+        []() { return return_map_pair_string(); },
+        py::return_value_policy_pack({{rvpb, rvpc}, {rvpc, rvpb}}));
+}
diff --git a/tests/test_return_value_policy_pack.py b/tests/test_return_value_policy_pack.py
@@ -0,0 +1,67 @@
+import pytest
+
+from pybind11_tests import return_value_policy_pack as m
+
+
+@pytest.mark.parametrize(
+    "func, expected",
+    [
+        (m.return_tuple_str_str, (str, str)),
+        (m.return_tuple_bytes_bytes, (bytes, bytes)),
+        (m.return_tuple_str_bytes, (str, bytes)),
+        (m.return_tuple_bytes_str, (bytes, str)),
+    ],
+)
+def test_return_pair_string(func, expected):
+    p = func()
+    assert isinstance(p, tuple)
+    assert len(p) == 2
+    assert all(isinstance(e, t) for e, t in zip(p, expected))
+
+
+@pytest.mark.parametrize(
+    "func, expected",
+    [
+        (m.return_nested_tuple_str, (str, str, str, str)),
+        (m.return_nested_tuple_bytes, (bytes, bytes, bytes, bytes)),
+        (m.return_nested_tuple_str_bytes_bytes_str, (str, bytes, bytes, str)),
+        (m.return_nested_tuple_bytes_str_str_bytes, (bytes, str, str, bytes)),
+    ],
+)
+def test_return_nested_pair_string(func, expected):
+    np = func()
+    assert isinstance(np, tuple)
+    assert len(np) == 2
+    assert all(isinstance(e, t) for e, t in zip(sum(np, ()), expected))
+
+
+@pytest.mark.parametrize(
+    "func, expected",
+    [
+        (m.return_dict_str_str, (str, str)),
+        (m.return_dict_bytes_bytes, (bytes, bytes)),
+        (m.return_dict_str_bytes, (str, bytes)),
+        (m.return_dict_bytes_str, (bytes, str)),
+    ],
+)
+def test_return_map_string(func, expected):
+    mp = func()
+    assert isinstance(mp, dict)
+    assert len(mp) == 1
+    assert all(isinstance(e, t) for e, t in zip(tuple(mp.items())[0], expected))
+
+
+@pytest.mark.parametrize(
+    "func, expected",
+    [
+        (m.return_dict_sbbs, (str, bytes, bytes, str)),
+        (m.return_dict_bssb, (bytes, str, str, bytes)),
+    ],
+)
+def test_return_map_pair_string(func, expected):
+    mp = func()
+    assert isinstance(mp, dict)
+    assert len(mp) == 1
+    assert all(
+        isinstance(e, t) for e, t in zip(sum(tuple(mp.items())[0], ()), expected)
+    )

Original file line number	Diff line number	Diff line change
`@@ -117,11 +117,12 @@ struct type_caster_odr_guard : TypeCasterType {`
`117`	`117`
`118`	`118`	`// The original author of this function is @amauryfa`
`119`	`119`	`template <typename CType, typename... Arg>`
`120`		`- static handle cast(CType &&src, return_value_policy policy, handle parent, Arg &&...arg) {`
	`120`	`+ static handle`
	`121`	`+ cast(CType &&src, const return_value_policy_pack &rvpp, handle parent, Arg &&...arg) {`
`121`	`122`	`if (translation_unit_local) {`
`122`	`123`	`}`
`123`	`124`	`return TypeCasterType::cast(`
`124`		`- std::forward<CType>(src), policy, parent, std::forward<Arg>(arg)...);`
	`125`	`+ std::forward<CType>(src), rvpp, parent, std::forward<Arg>(arg)...);`
`125`	`126`	`}`
`126`	`127`	`};`
`127`	`128`