From e9c6fe1a6e82aa5c93327bb93a90f39e792efe61 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 10 Jun 2024 19:03:59 -0700 Subject: [PATCH] Revert "[smart_holder] Auto select return value policy for clif_automatic (#4381)" This reverts commit c1f14f05117fd6884c23253378b6d2e3118f0a16. Conflicts resolved in: include/pybind11/detail/smart_holder_type_casters.h tests/test_return_value_policy_override.py --- .../detail/smart_holder_type_casters.h | 19 +- tests/test_return_value_policy_override.cpp | 241 ------------------ tests/test_return_value_policy_override.py | 34 --- 3 files changed, 2 insertions(+), 292 deletions(-) diff --git a/include/pybind11/detail/smart_holder_type_casters.h b/include/pybind11/detail/smart_holder_type_casters.h index ba8b9af3b..3ab11cf8a 100644 --- a/include/pybind11/detail/smart_holder_type_casters.h +++ b/include/pybind11/detail/smart_holder_type_casters.h @@ -753,8 +753,7 @@ struct smart_holder_type_caster : smart_holder_type_caster_load, static handle cast(T const &src, return_value_policy policy, handle parent) { // type_caster_base BEGIN if (policy == return_value_policy::automatic - || policy == return_value_policy::automatic_reference - || policy == return_value_policy::_clif_automatic) { + || policy == return_value_policy::automatic_reference) { policy = return_value_policy::copy; } return cast(&src, policy, parent); @@ -762,21 +761,11 @@ struct smart_holder_type_caster : smart_holder_type_caster_load, } static handle cast(T &src, return_value_policy policy, handle parent) { - if (policy == return_value_policy::_clif_automatic) { - if (is_move_constructible::value) { - policy = return_value_policy::move; - } else { - policy = return_value_policy::automatic; - } - } return cast(const_cast(src), policy, parent); // Mutbl2Const } static handle cast(T const *src, return_value_policy policy, handle parent) { auto st = type_caster_base::src_and_type(src); - if (policy == return_value_policy::_clif_automatic) { - policy = return_value_policy::copy; - } return cast_const_raw_ptr( // Originally type_caster_generic::cast. st.first, policy, @@ -787,9 +776,6 @@ struct smart_holder_type_caster : smart_holder_type_caster_load, } static handle cast(T *src, return_value_policy policy, handle parent) { - if (policy == return_value_policy::_clif_automatic) { - policy = return_value_policy::reference; - } return cast(const_cast(src), policy, parent); // Mutbl2Const } @@ -1006,8 +992,7 @@ struct smart_holder_type_caster> : smart_holder_type_caste if (policy != return_value_policy::automatic && policy != return_value_policy::automatic_reference && policy != return_value_policy::reference_internal - && policy != return_value_policy::move - && policy != return_value_policy::_clif_automatic) { + && policy != return_value_policy::move) { // SMART_HOLDER_WIP: IMPROVABLE: Error message. throw cast_error("Invalid return_value_policy for unique_ptr."); } diff --git a/tests/test_return_value_policy_override.cpp b/tests/test_return_value_policy_override.cpp index 5d6a51b7c..130f99055 100644 --- a/tests/test_return_value_policy_override.cpp +++ b/tests/test_return_value_policy_override.cpp @@ -1,166 +1,12 @@ -#include - #include "pybind11_tests.h" namespace test_return_value_policy_override { struct some_type {}; -// cp = copyable, mv = movable, 1 = yes, 0 = no -struct type_cp1_mv1 { - std::string mtxt; - explicit type_cp1_mv1(const std::string &mtxt_) : mtxt(mtxt_) {} - type_cp1_mv1(const type_cp1_mv1 &other) { mtxt = other.mtxt + "_CpCtor"; } - type_cp1_mv1(type_cp1_mv1 &&other) noexcept { mtxt = other.mtxt + "_MvCtor"; } - type_cp1_mv1 &operator=(const type_cp1_mv1 &other) { - mtxt = other.mtxt + "_CpCtor"; - return *this; - } - type_cp1_mv1 &operator=(type_cp1_mv1 &&other) noexcept { - mtxt = other.mtxt + "_MvCtor"; - return *this; - } -}; - -// nocopy -struct type_cp0_mv1 { - std::string mtxt; - explicit type_cp0_mv1(const std::string &mtxt_) : mtxt(mtxt_) {} - type_cp0_mv1(const type_cp0_mv1 &) = delete; - type_cp0_mv1(type_cp0_mv1 &&other) noexcept { mtxt = other.mtxt + "_MvCtor"; } - type_cp0_mv1 &operator=(const type_cp0_mv1 &) = delete; - type_cp0_mv1 &operator=(type_cp0_mv1 &&other) noexcept { - mtxt = other.mtxt + "_MvCtor"; - return *this; - } -}; - -// nomove -struct type_cp1_mv0 { - std::string mtxt; - explicit type_cp1_mv0(const std::string &mtxt_) : mtxt(mtxt_) {} - type_cp1_mv0(const type_cp1_mv0 &other) { mtxt = other.mtxt + "_CpCtor"; } - type_cp1_mv0(type_cp1_mv0 &&other) = delete; - type_cp1_mv0 &operator=(const type_cp1_mv0 &other) { - mtxt = other.mtxt + "_CpCtor"; - return *this; - } - type_cp1_mv0 &operator=(type_cp1_mv0 &&other) = delete; -}; - -// nocopy_nomove -struct type_cp0_mv0 { - std::string mtxt; - explicit type_cp0_mv0(const std::string &mtxt_) : mtxt(mtxt_) {} - type_cp0_mv0(const type_cp0_mv0 &) = delete; - type_cp0_mv0(type_cp0_mv0 &&other) = delete; - type_cp0_mv0 &operator=(const type_cp0_mv0 &other) = delete; - type_cp0_mv0 &operator=(type_cp0_mv0 &&other) = delete; -}; - -type_cp1_mv1 return_value() { return type_cp1_mv1{"value"}; } - -type_cp1_mv1 *return_pointer() { - static type_cp1_mv1 value("pointer"); - return &value; -} - -const type_cp1_mv1 *return_const_pointer() { - static type_cp1_mv1 value("const_pointer"); - return &value; -} - -type_cp1_mv1 &return_reference() { - static type_cp1_mv1 value("reference"); - return value; -} - -const type_cp1_mv1 &return_const_reference() { - static type_cp1_mv1 value("const_reference"); - return value; -} - -std::shared_ptr return_shared_pointer() { - return std::make_shared("shared_pointer"); -} - -std::unique_ptr return_unique_pointer() { - return std::unique_ptr(new type_cp1_mv1("unique_pointer")); -} - -type_cp0_mv1 return_value_nocopy() { return type_cp0_mv1{"value_nocopy"}; } - -type_cp0_mv1 *return_pointer_nocopy() { - static type_cp0_mv1 value("pointer_nocopy"); - return &value; -} - -const type_cp0_mv1 *return_const_pointer_nocopy() { - static type_cp0_mv1 value("const_pointer_nocopy"); - return &value; -} - -type_cp0_mv1 &return_reference_nocopy() { - static type_cp0_mv1 value("reference_nocopy"); - return value; -} - -std::shared_ptr return_shared_pointer_nocopy() { - return std::make_shared("shared_pointer_nocopy"); -} - -std::unique_ptr return_unique_pointer_nocopy() { - return std::unique_ptr(new type_cp0_mv1("unique_pointer_nocopy")); -} - -type_cp1_mv0 *return_pointer_nomove() { - static type_cp1_mv0 value("pointer_nomove"); - return &value; -} - -const type_cp1_mv0 *return_const_pointer_nomove() { - static type_cp1_mv0 value("const_pointer_nomove"); - return &value; -} - -type_cp1_mv0 &return_reference_nomove() { - static type_cp1_mv0 value("reference_nomove"); - return value; -} - -const type_cp1_mv0 &return_const_reference_nomove() { - static type_cp1_mv0 value("const_reference_nomove"); - return value; -} - -std::shared_ptr return_shared_pointer_nomove() { - return std::make_shared("shared_pointer_nomove"); -} - -std::unique_ptr return_unique_pointer_nomove() { - return std::unique_ptr(new type_cp1_mv0("unique_pointer_nomove")); -} - -type_cp0_mv0 *return_pointer_nocopy_nomove() { - static type_cp0_mv0 value("pointer_nocopy_nomove"); - return &value; -} - -std::shared_ptr return_shared_pointer_nocopy_nomove() { - return std::make_shared("shared_pointer_nocopy_nomove"); -} - -std::unique_ptr return_unique_pointer_nocopy_nomove() { - return std::unique_ptr(new type_cp0_mv0("unique_pointer_nocopy_nomove")); -} - } // namespace test_return_value_policy_override using test_return_value_policy_override::some_type; -using test_return_value_policy_override::type_cp0_mv0; -using test_return_value_policy_override::type_cp0_mv1; -using test_return_value_policy_override::type_cp1_mv0; -using test_return_value_policy_override::type_cp1_mv1; namespace pybind11 { namespace detail { @@ -205,11 +51,6 @@ struct type_caster : type_caster_base { } // namespace detail } // namespace pybind11 -PYBIND11_SMART_HOLDER_TYPE_CASTERS(type_cp1_mv1) -PYBIND11_SMART_HOLDER_TYPE_CASTERS(type_cp0_mv1) -PYBIND11_SMART_HOLDER_TYPE_CASTERS(type_cp1_mv0) -PYBIND11_SMART_HOLDER_TYPE_CASTERS(type_cp0_mv0) - TEST_SUBMODULE(return_value_policy_override, m) { m.def("return_value_with_default_policy", []() { return some_type(); }); m.def( @@ -238,86 +79,4 @@ TEST_SUBMODULE(return_value_policy_override, m) { return &value; }, py::return_value_policy::_clif_automatic); - - py::classh(m, "type_cp1_mv1") - .def(py::init()) - .def_readonly("mtxt", &type_cp1_mv1::mtxt); - m.def("return_value", - &test_return_value_policy_override::return_value, - py::return_value_policy::_clif_automatic); - m.def("return_pointer", - &test_return_value_policy_override::return_pointer, - py::return_value_policy::_clif_automatic); - m.def("return_const_pointer", - &test_return_value_policy_override::return_const_pointer, - py::return_value_policy::_clif_automatic); - m.def("return_reference", - &test_return_value_policy_override::return_reference, - py::return_value_policy::_clif_automatic); - m.def("return_const_reference", - &test_return_value_policy_override::return_const_reference, - py::return_value_policy::_clif_automatic); - m.def("return_unique_pointer", - &test_return_value_policy_override::return_unique_pointer, - py::return_value_policy::_clif_automatic); - m.def("return_shared_pointer", - &test_return_value_policy_override::return_shared_pointer, - py::return_value_policy::_clif_automatic); - - py::classh(m, "type_cp0_mv1") - .def(py::init()) - .def_readonly("mtxt", &type_cp0_mv1::mtxt); - m.def("return_value_nocopy", - &test_return_value_policy_override::return_value_nocopy, - py::return_value_policy::_clif_automatic); - m.def("return_pointer_nocopy", - &test_return_value_policy_override::return_pointer_nocopy, - py::return_value_policy::_clif_automatic); - m.def("return_const_pointer_nocopy", - &test_return_value_policy_override::return_const_pointer_nocopy, - py::return_value_policy::_clif_automatic); - m.def("return_reference_nocopy", - &test_return_value_policy_override::return_reference_nocopy, - py::return_value_policy::_clif_automatic); - m.def("return_shared_pointer_nocopy", - &test_return_value_policy_override::return_shared_pointer_nocopy, - py::return_value_policy::_clif_automatic); - m.def("return_unique_pointer_nocopy", - &test_return_value_policy_override::return_unique_pointer_nocopy, - py::return_value_policy::_clif_automatic); - - py::classh(m, "type_cp1_mv0") - .def(py::init()) - .def_readonly("mtxt", &type_cp1_mv0::mtxt); - m.def("return_pointer_nomove", - &test_return_value_policy_override::return_pointer_nomove, - py::return_value_policy::_clif_automatic); - m.def("return_const_pointer_nomove", - &test_return_value_policy_override::return_const_pointer_nomove, - py::return_value_policy::_clif_automatic); - m.def("return_reference_nomove", - &test_return_value_policy_override::return_reference_nomove, - py::return_value_policy::_clif_automatic); - m.def("return_const_reference_nomove", - &test_return_value_policy_override::return_const_reference_nomove, - py::return_value_policy::_clif_automatic); - m.def("return_shared_pointer_nomove", - &test_return_value_policy_override::return_shared_pointer_nomove, - py::return_value_policy::_clif_automatic); - m.def("return_unique_pointer_nomove", - &test_return_value_policy_override::return_unique_pointer_nomove, - py::return_value_policy::_clif_automatic); - - py::classh(m, "type_cp0_mv0") - .def(py::init()) - .def_readonly("mtxt", &type_cp0_mv0::mtxt); - m.def("return_pointer_nocopy_nomove", - &test_return_value_policy_override::return_pointer_nocopy_nomove, - py::return_value_policy::_clif_automatic); - m.def("return_shared_pointer_nocopy_nomove", - &test_return_value_policy_override::return_shared_pointer_nocopy_nomove, - py::return_value_policy::_clif_automatic); - m.def("return_unique_pointer_nocopy_nomove", - &test_return_value_policy_override::return_unique_pointer_nocopy_nomove, - py::return_value_policy::_clif_automatic); } diff --git a/tests/test_return_value_policy_override.py b/tests/test_return_value_policy_override.py index 27c769421..36cb5bbf0 100644 --- a/tests/test_return_value_policy_override.py +++ b/tests/test_return_value_policy_override.py @@ -1,7 +1,3 @@ -import re - -import pytest - from pybind11_tests import return_value_policy_override as m @@ -15,33 +11,3 @@ def test_return_pointer(): assert m.return_pointer_with_default_policy() == "automatic" assert m.return_pointer_with_policy_move() == "move" assert m.return_pointer_with_policy_clif_automatic() == "_clif_automatic" - - -@pytest.mark.parametrize( - ("func", "expected"), - [ - (m.return_value, "value(_MvCtor)*_MvCtor"), - (m.return_pointer, "pointer"), - (m.return_const_pointer, "const_pointer_CpCtor"), - (m.return_reference, "reference_MvCtor"), - (m.return_const_reference, "const_reference_CpCtor"), - (m.return_unique_pointer, "unique_pointer"), - (m.return_shared_pointer, "shared_pointer"), - (m.return_value_nocopy, "value_nocopy(_MvCtor)*_MvCtor"), - (m.return_pointer_nocopy, "pointer_nocopy"), - (m.return_reference_nocopy, "reference_nocopy_MvCtor"), - (m.return_unique_pointer_nocopy, "unique_pointer_nocopy"), - (m.return_shared_pointer_nocopy, "shared_pointer_nocopy"), - (m.return_pointer_nomove, "pointer_nomove"), - (m.return_const_pointer_nomove, "const_pointer_nomove_CpCtor"), - (m.return_reference_nomove, "reference_nomove_CpCtor"), - (m.return_const_reference_nomove, "const_reference_nomove_CpCtor"), - (m.return_unique_pointer_nomove, "unique_pointer_nomove"), - (m.return_shared_pointer_nomove, "shared_pointer_nomove"), - (m.return_pointer_nocopy_nomove, "pointer_nocopy_nomove"), - (m.return_unique_pointer_nocopy_nomove, "unique_pointer_nocopy_nomove"), - (m.return_shared_pointer_nocopy_nomove, "shared_pointer_nocopy_nomove"), - ], -) -def test_clif_automatic_return_value_policy_override(func, expected): - assert re.match(expected, func().mtxt)