From 7d8e6ea5b9ec0633240b48b28ab5d6bba6ec563e Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 10 Jun 2024 18:38:40 -0700 Subject: [PATCH 01/10] Revert "Add handling of `return_value_policy::_clif_automatic` in type_caster_pyobject_ptr.h (backported from https://github.com/google/pywrapcc/pull/30021)" This reverts commit bd69f7aaf206d52ce227f796e7601e5788009f7c. --- include/pybind11/type_caster_pyobject_ptr.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/include/pybind11/type_caster_pyobject_ptr.h b/include/pybind11/type_caster_pyobject_ptr.h index 7789c040..aa914f9e 100644 --- a/include/pybind11/type_caster_pyobject_ptr.h +++ b/include/pybind11/type_caster_pyobject_ptr.h @@ -32,8 +32,7 @@ class type_caster { raise_from(PyExc_SystemError, "src != nullptr but PyErr_Occurred()"); throw error_already_set(); } - if (policy == return_value_policy::take_ownership - || policy == return_value_policy::_clif_automatic) { + if (policy == return_value_policy::take_ownership) { return src; } if (policy == return_value_policy::reference From bfb5fce463fc591bce3feb0ea87c97bb78f5b5e6 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 10 Jun 2024 18:56:44 -0700 Subject: [PATCH 02/10] Revert "[smart_holder] Keep parent alive when returning raw pointers (#4609)" This reverts commit 99cf27a4f5d36d7336e0340bb5ee2eef03c00407. --- .../detail/smart_holder_type_casters.h | 6 +-- tests/test_return_value_policy_override.cpp | 46 ------------------- tests/test_return_value_policy_override.py | 15 ------ 3 files changed, 1 insertion(+), 66 deletions(-) diff --git a/include/pybind11/detail/smart_holder_type_casters.h b/include/pybind11/detail/smart_holder_type_casters.h index c5fe9cd0..ba8b9af3 100644 --- a/include/pybind11/detail/smart_holder_type_casters.h +++ b/include/pybind11/detail/smart_holder_type_casters.h @@ -788,11 +788,7 @@ 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 (parent) { - policy = return_value_policy::reference_internal; - } else { - policy = return_value_policy::reference; - } + policy = return_value_policy::reference; } return cast(const_cast(src), policy, parent); // Mutbl2Const } diff --git a/tests/test_return_value_policy_override.cpp b/tests/test_return_value_policy_override.cpp index a61bec1b..5d6a51b7 100644 --- a/tests/test_return_value_policy_override.cpp +++ b/tests/test_return_value_policy_override.cpp @@ -2,37 +2,10 @@ #include "pybind11_tests.h" -#include - namespace test_return_value_policy_override { struct some_type {}; -struct data_field { - int value = -99; - - explicit data_field(int v) : value(v) {} -}; - -struct data_fields_holder { - std::vector vec; - - explicit data_fields_holder(std::size_t vec_size) { - for (std::size_t i = 0; i < vec_size; i++) { - vec.emplace_back(13 + static_cast(i) * 11); - } - } - - data_field *vec_at(std::size_t index) { - if (index >= vec.size()) { - return nullptr; - } - return &vec[index]; - } - - const data_field *vec_at_const_ptr(std::size_t index) { return vec_at(index); } -}; - // cp = copyable, mv = movable, 1 = yes, 0 = no struct type_cp1_mv1 { std::string mtxt; @@ -183,8 +156,6 @@ std::unique_ptr return_unique_pointer_nocopy_nomove() { } // namespace test_return_value_policy_override -using test_return_value_policy_override::data_field; -using test_return_value_policy_override::data_fields_holder; 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; @@ -234,8 +205,6 @@ struct type_caster : type_caster_base { } // namespace detail } // namespace pybind11 -PYBIND11_SMART_HOLDER_TYPE_CASTERS(data_field) -PYBIND11_SMART_HOLDER_TYPE_CASTERS(data_fields_holder) PYBIND11_SMART_HOLDER_TYPE_CASTERS(type_cp1_mv1) PYBIND11_SMART_HOLDER_TYPE_CASTERS(type_cp0_mv1) PYBIND11_SMART_HOLDER_TYPE_CASTERS(type_cp1_mv0) @@ -270,21 +239,6 @@ TEST_SUBMODULE(return_value_policy_override, m) { }, py::return_value_policy::_clif_automatic); - py::classh(m, "data_field").def_readwrite("value", &data_field::value); - py::classh(m, "data_fields_holder") - .def(py::init()) - .def("vec_at", - [](const py::object &self_py, std::size_t index) { - auto *self = py::cast(self_py); - return py::cast( - self->vec_at(index), py::return_value_policy::_clif_automatic, self_py); - }) - .def("vec_at_const_ptr", [](const py::object &self_py, std::size_t index) { - auto *self = py::cast(self_py); - return py::cast( - self->vec_at_const_ptr(index), py::return_value_policy::_clif_automatic, self_py); - }); - py::classh(m, "type_cp1_mv1") .def(py::init()) .def_readonly("mtxt", &type_cp1_mv1::mtxt); diff --git a/tests/test_return_value_policy_override.py b/tests/test_return_value_policy_override.py index 213f9c20..27c76942 100644 --- a/tests/test_return_value_policy_override.py +++ b/tests/test_return_value_policy_override.py @@ -17,21 +17,6 @@ def test_return_pointer(): assert m.return_pointer_with_policy_clif_automatic() == "_clif_automatic" -def test_persistent_holder(): - h = m.data_fields_holder(2) - assert h.vec_at(0).value == 13 - assert h.vec_at(1).value == 24 - assert h.vec_at_const_ptr(0).value == 13 - assert h.vec_at_const_ptr(1).value == 24 - - -def test_temporary_holder(): - data_field = m.data_fields_holder(2).vec_at(1) - assert data_field.value == 24 - data_field = m.data_fields_holder(2).vec_at_const_ptr(1) - assert data_field.value == 24 - - @pytest.mark.parametrize( ("func", "expected"), [ 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 03/10] 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 ba8b9af3..3ab11cf8 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 5d6a51b7..130f9905 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 27c76942..36cb5bbf 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) From 7a945d8f40cf946538ca2170a7adcf6dcd1e2509 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 10 Jun 2024 19:05:21 -0700 Subject: [PATCH 04/10] Revert "cast.h return_value_policy_override _clif_automatic (#4364)" This reverts commit 8720cf94d69a3fce302c2e2f7abed464a9e485c6. --- include/pybind11/cast.h | 1 - tests/CMakeLists.txt | 1 - tests/test_return_value_policy_override.cpp | 82 --------------------- tests/test_return_value_policy_override.py | 13 ---- 4 files changed, 97 deletions(-) delete mode 100644 tests/test_return_value_policy_override.cpp delete mode 100644 tests/test_return_value_policy_override.py diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index a35894e9..97e73c1a 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1170,7 +1170,6 @@ struct return_value_policy_override< void>> { static return_value_policy policy(return_value_policy p) { return !std::is_lvalue_reference::value && !std::is_pointer::value - && p != return_value_policy::_clif_automatic ? return_value_policy::move : p; } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index d8e050ec..c5458837 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -167,7 +167,6 @@ set(PYBIND11_TEST_FILES test_pickling test_python_multiple_inheritance test_pytypes - test_return_value_policy_override test_sequences_and_iterators test_smart_ptr test_stl diff --git a/tests/test_return_value_policy_override.cpp b/tests/test_return_value_policy_override.cpp deleted file mode 100644 index 130f9905..00000000 --- a/tests/test_return_value_policy_override.cpp +++ /dev/null @@ -1,82 +0,0 @@ -#include "pybind11_tests.h" - -namespace test_return_value_policy_override { - -struct some_type {}; - -} // namespace test_return_value_policy_override - -using test_return_value_policy_override::some_type; - -namespace pybind11 { -namespace detail { - -const char *return_value_policy_name(return_value_policy policy) { - switch (policy) { - case return_value_policy::automatic: - return "automatic"; - case return_value_policy::automatic_reference: - return "automatic_reference"; - case return_value_policy::take_ownership: - return "take_ownership"; - case return_value_policy::copy: - return "copy"; - case return_value_policy::move: - return "move"; - case return_value_policy::reference: - return "reference"; - case return_value_policy::reference_internal: - return "reference_internal"; - case return_value_policy::_return_as_bytes: - return "_return_as_bytes"; - case return_value_policy::_clif_automatic: - return "_clif_automatic"; - default: - return "Expected to be unreachable."; - } -}; - -template <> -struct type_caster : type_caster_base { - - static handle cast(some_type &&, return_value_policy policy, handle /*parent*/) { - return str(std::string(return_value_policy_name(policy))).release().ptr(); - } - - static handle cast(some_type *, return_value_policy policy, handle /*parent*/) { - return str(std::string(return_value_policy_name(policy))).release().ptr(); - } -}; - -} // namespace detail -} // namespace pybind11 - -TEST_SUBMODULE(return_value_policy_override, m) { - m.def("return_value_with_default_policy", []() { return some_type(); }); - m.def( - "return_value_with_policy_copy", - []() { return some_type(); }, - py::return_value_policy::copy); - m.def( - "return_value_with_policy_clif_automatic", - []() { return some_type(); }, - py::return_value_policy::_clif_automatic); - m.def("return_pointer_with_default_policy", []() { - static some_type value; - return &value; - }); - m.def( - "return_pointer_with_policy_move", - []() { - static some_type value; - return &value; - }, - py::return_value_policy::move); - m.def( - "return_pointer_with_policy_clif_automatic", - []() { - static some_type value; - return &value; - }, - py::return_value_policy::_clif_automatic); -} diff --git a/tests/test_return_value_policy_override.py b/tests/test_return_value_policy_override.py deleted file mode 100644 index 36cb5bbf..00000000 --- a/tests/test_return_value_policy_override.py +++ /dev/null @@ -1,13 +0,0 @@ -from pybind11_tests import return_value_policy_override as m - - -def test_return_value(): - assert m.return_value_with_default_policy() == "move" - assert m.return_value_with_policy_copy() == "move" - assert m.return_value_with_policy_clif_automatic() == "_clif_automatic" - - -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" From 7460dc0ce782b74a3727375e6c06276237d4e725 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 10 Jun 2024 19:05:51 -0700 Subject: [PATCH 05/10] Revert "Add return value policy _clif_automatic (#4343)" This reverts commit 6d3a0fc319cc03ba7b5e242e76e697626670340e. --- include/pybind11/detail/common.h | 11 +---------- include/pybind11/detail/smart_holder_type_casters.h | 1 - 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 55e25f3f..6223a71a 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -554,16 +554,7 @@ enum class return_value_policy : uint8_t { but the purpose of _return_as_bytes is certain to be orthogonal, because C++ strings are always copied to Python `bytes` or `str`. NOTE: This policy is NOT available on master. */ - _return_as_bytes, - - /** This policy should only be used by PyCLIF to automatically select a - return value policy. Legacy PyCLIF automatically decides object lifetime - management based on their properties: - https://github.com/google/clif/tree/main/clif/python#pointers-references-and-object-ownership - With this policy, the return value policy selection is consistent with - legacy PyCLIF. - NOTE: This policy is NOT available on master. */ - _clif_automatic + _return_as_bytes }; #define PYBIND11_HAS_RETURN_VALUE_POLICY_RETURN_AS_BYTES diff --git a/include/pybind11/detail/smart_holder_type_casters.h b/include/pybind11/detail/smart_holder_type_casters.h index 3ab11cf8..55a2b5a0 100644 --- a/include/pybind11/detail/smart_holder_type_casters.h +++ b/include/pybind11/detail/smart_holder_type_casters.h @@ -918,7 +918,6 @@ struct smart_holder_type_caster> : smart_holder_type_caster_l break; case return_value_policy::reference_internal: case return_value_policy::_return_as_bytes: - case return_value_policy::_clif_automatic: break; } if (!src) { From e16b9eaa42624c4a6a40a98456c974fc808485e9 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 10 Jun 2024 19:07:45 -0700 Subject: [PATCH 06/10] Also remove PYBIND11_HAS_RETURN_VALUE_POLICY_CLIF_AUTOMATIC macro. --- include/pybind11/detail/common.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 6223a71a..f280854d 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -558,7 +558,6 @@ enum class return_value_policy : uint8_t { }; #define PYBIND11_HAS_RETURN_VALUE_POLICY_RETURN_AS_BYTES -#define PYBIND11_HAS_RETURN_VALUE_POLICY_CLIF_AUTOMATIC PYBIND11_NAMESPACE_BEGIN(detail) From 0de9906459c2f28e11c18ed14e2a240311e0711c Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 10 Jun 2024 19:19:37 -0700 Subject: [PATCH 07/10] Revert "[smart_holder] Add a new return value policy `return_as_bytes` (#3838)" This reverts commit 7064d43bc988e20b152f8035330e2e916e3666d5. Conflicts resolved in: include/pybind11/eigen.h tests/test_builtin_casters.cpp --- include/pybind11/cast.h | 10 ++---- include/pybind11/detail/common.h | 14 +-------- .../detail/smart_holder_type_casters.h | 3 -- include/pybind11/detail/type_caster_base.h | 4 --- tests/test_builtin_casters.cpp | 31 ------------------- tests/test_builtin_casters.py | 14 --------- tests/test_stl.cpp | 21 ------------- tests/test_stl.py | 11 ------- 8 files changed, 4 insertions(+), 104 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 97e73c1a..f1821721 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -476,15 +476,11 @@ struct string_caster { return true; } - static handle cast(const StringType &src, return_value_policy policy, handle /* parent */) { + static handle + cast(const StringType &src, return_value_policy /* policy */, handle /* parent */) { const char *buffer = reinterpret_cast(src.data()); auto nbytes = ssize_t(src.size() * sizeof(CharT)); - handle s; - if (policy == return_value_policy::_return_as_bytes) { - s = PyBytes_FromStringAndSize(buffer, nbytes); - } else { - s = decode_utfN(buffer, nbytes); - } + handle s = decode_utfN(buffer, nbytes); if (!s) { throw error_already_set(); } diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index f280854d..567eb75a 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -542,19 +542,7 @@ enum class return_value_policy : uint8_t { collected while Python is still using the child. More advanced variations of this scheme are also possible using combinations of return_value_policy::reference and the keep_alive call policy */ - reference_internal, - - /** With this policy, C++ string types are converted to Python bytes, - instead of str. This is most useful when a C++ function returns a - container-like type with nested C++ string types, and `py::bytes` cannot - be applied easily. Dictionary like types might not work, for example, - `Dict[str, bytes]`, because this policy forces all string return values - to be converted to bytes. Note that this return_value_policy is not - concerned with lifetime/ownership semantics, like the other policies, - but the purpose of _return_as_bytes is certain to be orthogonal, because - C++ strings are always copied to Python `bytes` or `str`. - NOTE: This policy is NOT available on master. */ - _return_as_bytes + reference_internal }; #define PYBIND11_HAS_RETURN_VALUE_POLICY_RETURN_AS_BYTES diff --git a/include/pybind11/detail/smart_holder_type_casters.h b/include/pybind11/detail/smart_holder_type_casters.h index 55a2b5a0..169a3659 100644 --- a/include/pybind11/detail/smart_holder_type_casters.h +++ b/include/pybind11/detail/smart_holder_type_casters.h @@ -909,15 +909,12 @@ struct smart_holder_type_caster> : smart_holder_type_caster_l break; case return_value_policy::take_ownership: throw cast_error("Invalid return_value_policy for shared_ptr (take_ownership)."); - break; case return_value_policy::copy: case return_value_policy::move: break; case return_value_policy::reference: throw cast_error("Invalid return_value_policy for shared_ptr (reference)."); - break; case return_value_policy::reference_internal: - case return_value_policy::_return_as_bytes: break; } if (!src) { diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index ec1e7395..037c430a 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -625,10 +625,6 @@ class type_caster_generic { keep_alive_impl(inst, parent); break; - case return_value_policy::_return_as_bytes: - pybind11_fail("return_value_policy::_return_as_bytes does not apply."); - break; - default: throw cast_error("unhandled return_value_policy: should not happen!"); } diff --git a/tests/test_builtin_casters.cpp b/tests/test_builtin_casters.cpp index 4177b6d4..292304a6 100644 --- a/tests/test_builtin_casters.cpp +++ b/tests/test_builtin_casters.cpp @@ -11,17 +11,10 @@ #include "pybind11_tests.h" -#include - struct ConstRefCasted { int tag; }; -struct StringAttr { - explicit StringAttr(std::string v) : value(std::move(v)) {} - std::string value; -}; - PYBIND11_NAMESPACE_BEGIN(pybind11) PYBIND11_NAMESPACE_BEGIN(detail) template <> @@ -390,29 +383,5 @@ TEST_SUBMODULE(builtin_casters, m) { m.def("takes_const_ref_wrap", [](std::reference_wrapper x) { return x.get().tag; }); - // test return_value_policy::_return_as_bytes - m.def( - "invalid_utf8_string_as_bytes", - []() { return std::string("\xba\xd0\xba\xd0"); }, - py::return_value_policy::_return_as_bytes); - m.def("invalid_utf8_string_as_str", []() { return std::string("\xba\xd0\xba\xd0"); }); - m.def( - "invalid_utf8_char_array_as_bytes", - []() { return "\xba\xd0\xba\xd0"; }, - py::return_value_policy::_return_as_bytes); - py::class_(m, "StringAttr") - .def(py::init()) - .def_property( - "value", - py::cpp_function([](StringAttr &self) { return self.value; }, - py::return_value_policy::_return_as_bytes), - py::cpp_function([](StringAttr &self, std::string v) { self.value = std::move(v); })); -#ifdef PYBIND11_HAS_STRING_VIEW - m.def( - "invalid_utf8_string_view_as_bytes", - []() { return std::string_view("\xba\xd0\xba\xd0"); }, - py::return_value_policy::_return_as_bytes); -#endif - PYBIND11_WARNING_POP } diff --git a/tests/test_builtin_casters.py b/tests/test_builtin_casters.py index 0fb5ef61..dbac1cbc 100644 --- a/tests/test_builtin_casters.py +++ b/tests/test_builtin_casters.py @@ -526,17 +526,3 @@ def test_const_ref_caster(): assert m.takes_const_ptr(x) == 5 assert m.takes_const_ref(x) == 4 assert m.takes_const_ref_wrap(x) == 4 - - -def test_return_as_bytes_policy(): - expected_return_value = b"\xba\xd0\xba\xd0" - assert m.invalid_utf8_string_as_bytes() == expected_return_value - with pytest.raises(UnicodeDecodeError): - m.invalid_utf8_string_as_str() - assert m.invalid_utf8_char_array_as_bytes() == expected_return_value - obj = m.StringAttr(expected_return_value) - assert obj.value == expected_return_value - obj.value = "123" - assert obj.value == b"123" - if hasattr(m, "has_string_view"): - assert m.invalid_utf8_string_view_as_bytes() == expected_return_value diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index 4e0dabd5..48c907ff 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -546,25 +546,4 @@ TEST_SUBMODULE(stl, m) { []() { return new std::vector(4513); }, // Without explicitly specifying `take_ownership`, this function leaks. py::return_value_policy::take_ownership); - - // test return_value_policy::_return_as_bytes - m.def( - "invalid_utf8_string_array_as_bytes", - []() { return std::array{{"\xba\xd0\xba\xd0"}}; }, - py::return_value_policy::_return_as_bytes); - m.def("invalid_utf8_string_array_as_str", - []() { return std::array{{"\xba\xd0\xba\xd0"}}; }); -#ifdef PYBIND11_HAS_OPTIONAL - m.def( - "invalid_utf8_optional_string_as_bytes", - []() { return std::optional{"\xba\xd0\xba\xd0"}; }, - py::return_value_policy::_return_as_bytes); -#endif - -#ifdef PYBIND11_TEST_VARIANT - m.def( - "invalid_utf8_variant_string_as_bytes", - []() { return variant{"\xba\xd0\xba\xd0"}; }, - py::return_value_policy::_return_as_bytes); -#endif } diff --git a/tests/test_stl.py b/tests/test_stl.py index 8a055d55..b08bd468 100644 --- a/tests/test_stl.py +++ b/tests/test_stl.py @@ -379,14 +379,3 @@ def test_return_vector_bool_raw_ptr(): v = m.return_vector_bool_raw_ptr() assert isinstance(v, list) assert len(v) == 4513 - - -def test_return_as_bytes_policy(): - expected_return_value = b"\xba\xd0\xba\xd0" - assert m.invalid_utf8_string_array_as_bytes() == [expected_return_value] - with pytest.raises(UnicodeDecodeError): - m.invalid_utf8_string_array_as_str() - if hasattr(m, "has_optional"): - assert m.invalid_utf8_optional_string_as_bytes() == expected_return_value - if hasattr(m, "load_variant"): - assert m.invalid_utf8_variant_string_as_bytes() == expected_return_value From afd612b50efbf983c758dae9bc9620aceeea6c58 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 10 Jun 2024 19:21:52 -0700 Subject: [PATCH 08/10] Also remove PYBIND11_HAS_RETURN_VALUE_POLICY_RETURN_AS_BYTES macro. --- include/pybind11/detail/common.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 567eb75a..22416b03 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -545,8 +545,6 @@ enum class return_value_policy : uint8_t { reference_internal }; -#define PYBIND11_HAS_RETURN_VALUE_POLICY_RETURN_AS_BYTES - PYBIND11_NAMESPACE_BEGIN(detail) inline static constexpr int log2(size_t n, int k = 0) { From c4df281e9d88ffd40f76d42ea824ab65afd26f9f Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 10 Jun 2024 19:23:08 -0700 Subject: [PATCH 09/10] Also remove return_value_policy::_return_as_bytes in pybind11/eigen/ --- include/pybind11/eigen/matrix.h | 3 --- include/pybind11/eigen/tensor.h | 4 ---- 2 files changed, 7 deletions(-) diff --git a/include/pybind11/eigen/matrix.h b/include/pybind11/eigen/matrix.h index eafbca14..8d4342f8 100644 --- a/include/pybind11/eigen/matrix.h +++ b/include/pybind11/eigen/matrix.h @@ -351,9 +351,6 @@ struct type_caster::value>> { return eigen_ref_array(*src); case return_value_policy::reference_internal: return eigen_ref_array(*src, parent); - case return_value_policy::_return_as_bytes: - pybind11_fail("return_value_policy::_return_as_bytes does not apply."); - break; default: throw cast_error("unhandled return_value_policy: should not happen!"); }; diff --git a/include/pybind11/eigen/tensor.h b/include/pybind11/eigen/tensor.h index 803d41be..d4ed6c0c 100644 --- a/include/pybind11/eigen/tensor.h +++ b/include/pybind11/eigen/tensor.h @@ -313,10 +313,6 @@ struct type_caster::ValidType> { writeable = !std::is_const::value; break; - case return_value_policy::_return_as_bytes: - pybind11_fail("return_value_policy::_return_as_bytes does not apply."); - break; - default: pybind11_fail("pybind11 bug in eigen.h, please file a bug report"); } From bf79caafd9ca90f8761acf6a484d9eead46bc4e5 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Tue, 11 Jun 2024 11:32:26 -0700 Subject: [PATCH 10/10] Remove `try_as_void_ptr_capsule` feature from smart_holder branch. This manual removal was informed by reviews of these commits (newest to oldest): * d930de0bca046774acf2cd0c09b1e4ef84d8c0bb * 114c0f2a3e94b759e32dad897f25fc979ef79a43 * 1c10b097a79008c8fb0107fb61831126784f8b88 * 9d4b4dffce6677a7c9a9af37a45f4436d353f5e1 * da058a2904bb5eb1a82417bf36ab2357a1af4d3f --- .../detail/smart_holder_type_casters.h | 95 +------------ tests/CMakeLists.txt | 1 - tests/test_class_sh_void_ptr_capsule.cpp | 127 ------------------ tests/test_class_sh_void_ptr_capsule.py | 100 -------------- 4 files changed, 1 insertion(+), 322 deletions(-) delete mode 100644 tests/test_class_sh_void_ptr_capsule.cpp delete mode 100644 tests/test_class_sh_void_ptr_capsule.py diff --git a/include/pybind11/detail/smart_holder_type_casters.h b/include/pybind11/detail/smart_holder_type_casters.h index 169a3659..c8404a75 100644 --- a/include/pybind11/detail/smart_holder_type_casters.h +++ b/include/pybind11/detail/smart_holder_type_casters.h @@ -36,72 +36,6 @@ struct is_smart_holder_type : std::true_type {}; // SMART_HOLDER_WIP: Needs refactoring of existing pybind11 code. inline void register_instance(instance *self, void *valptr, const type_info *tinfo); inline bool deregister_instance(instance *self, void *valptr, const type_info *tinfo); -extern "C" inline PyObject *pybind11_object_new(PyTypeObject *type, PyObject *, PyObject *); - -// Replace all occurrences of substrings in a string. -inline void replace_all(std::string &str, const std::string &from, const std::string &to) { - if (str.empty()) { - return; - } - size_t pos = 0; - while ((pos = str.find(from, pos)) != std::string::npos) { - str.replace(pos, from.length(), to); - pos += to.length(); - } -} - -inline bool type_is_pybind11_class_(PyTypeObject *type_obj) { -#if defined(PYPY_VERSION) - auto &internals = get_internals(); - return bool(internals.registered_types_py.find(type_obj) - != internals.registered_types_py.end()); -#else - return bool(type_obj->tp_new == pybind11_object_new); -#endif -} - -inline bool is_instance_method_of_type(PyTypeObject *type_obj, PyObject *attr_name) { - PyObject *descr = _PyType_Lookup(type_obj, attr_name); - return bool((descr != nullptr) && PyInstanceMethod_Check(descr)); -} - -inline object try_get_as_capsule_method(PyObject *obj, PyObject *attr_name) { - if (PyType_Check(obj)) { - return object(); - } - PyTypeObject *type_obj = Py_TYPE(obj); - bool known_callable = false; - if (type_is_pybind11_class_(type_obj)) { - if (!is_instance_method_of_type(type_obj, attr_name)) { - return object(); - } - known_callable = true; - } - PyObject *method = PyObject_GetAttr(obj, attr_name); - if (method == nullptr) { - PyErr_Clear(); - return object(); - } - if (!known_callable && PyCallable_Check(method) == 0) { - Py_DECREF(method); - return object(); - } - return reinterpret_steal(method); -} - -inline void *try_as_void_ptr_capsule_get_pointer(handle src, const char *typeid_name) { - std::string suffix = clean_type_id(typeid_name); - replace_all(suffix, "::", "_"); // Convert `a::b::c` to `a_b_c`. - replace_all(suffix, "*", ""); - object as_capsule_method = try_get_as_capsule_method(src.ptr(), str("as_" + suffix).ptr()); - if (as_capsule_method) { - object void_ptr_capsule = as_capsule_method(); - if (isinstance(void_ptr_capsule)) { - return reinterpret_borrow(void_ptr_capsule).get_pointer(); - } - } - return nullptr; -} // The modified_type_caster_generic_load_impl could replace type_caster_generic::load_impl but not // vice versa. The main difference is that the original code only propagates a reference to the @@ -176,15 +110,6 @@ class modified_type_caster_generic_load_impl { return false; } - bool try_as_void_ptr_capsule(handle src) { - unowned_void_ptr_from_void_ptr_capsule - = try_as_void_ptr_capsule_get_pointer(src, cpptype->name()); - if (unowned_void_ptr_from_void_ptr_capsule != nullptr) { - return true; - } - return false; - } - PYBIND11_NOINLINE static void *local_load(PyObject *src, const type_info *ti) { std::unique_ptr loader( new modified_type_caster_generic_load_impl(ti)); @@ -337,16 +262,12 @@ class modified_type_caster_generic_load_impl { loaded_v_h = value_and_holder(); return true; } - if (convert && cpptype && try_as_void_ptr_capsule(src)) { - return true; - } return false; } const type_info *typeinfo = nullptr; const std::type_info *cpptype = nullptr; void *unowned_void_ptr_from_direct_conversion = nullptr; - void *unowned_void_ptr_from_void_ptr_capsule = nullptr; const std::type_info *loaded_v_h_cpptype = nullptr; std::vector implicit_casts; value_and_holder loaded_v_h; @@ -499,10 +420,7 @@ struct smart_holder_type_caster_load { } T *loaded_as_raw_ptr_unowned() const { - void *void_ptr = load_impl.unowned_void_ptr_from_void_ptr_capsule; - if (void_ptr == nullptr) { - void_ptr = load_impl.unowned_void_ptr_from_direct_conversion; - } + void *void_ptr = load_impl.unowned_void_ptr_from_direct_conversion; if (void_ptr == nullptr) { if (have_holder()) { throw_if_uninitialized_or_disowned_holder(typeid(T)); @@ -531,13 +449,6 @@ struct smart_holder_type_caster_load { } std::shared_ptr loaded_as_shared_ptr(handle responsible_parent = nullptr) const { - if (load_impl.unowned_void_ptr_from_void_ptr_capsule) { - if (responsible_parent) { - return make_shared_ptr_with_responsible_parent(responsible_parent); - } - throw cast_error("Unowned pointer from a void pointer capsule cannot be converted to a" - " std::shared_ptr."); - } if (load_impl.unowned_void_ptr_from_direct_conversion != nullptr) { if (responsible_parent) { return make_shared_ptr_with_responsible_parent(responsible_parent); @@ -601,10 +512,6 @@ struct smart_holder_type_caster_load { template std::unique_ptr loaded_as_unique_ptr(const char *context = "loaded_as_unique_ptr") { - if (load_impl.unowned_void_ptr_from_void_ptr_capsule) { - throw cast_error("Unowned pointer from a void pointer capsule cannot be converted to a" - " std::unique_ptr."); - } if (load_impl.unowned_void_ptr_from_direct_conversion != nullptr) { throw cast_error("Unowned pointer from direct conversion cannot be converted to a" " std::unique_ptr."); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index c5458837..c1fb00fc 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -137,7 +137,6 @@ set(PYBIND11_TEST_FILES test_class_sh_unique_ptr_custom_deleter test_class_sh_unique_ptr_member test_class_sh_virtual_py_cpp_mix - test_class_sh_void_ptr_capsule test_classh_mock test_const_name test_constants_and_functions diff --git a/tests/test_class_sh_void_ptr_capsule.cpp b/tests/test_class_sh_void_ptr_capsule.cpp deleted file mode 100644 index db47e3c2..00000000 --- a/tests/test_class_sh_void_ptr_capsule.cpp +++ /dev/null @@ -1,127 +0,0 @@ -#include - -#include "pybind11_tests.h" - -#include - -namespace pybind11_tests { -namespace class_sh_void_ptr_capsule { - -struct Valid {}; - -struct NoConversion {}; - -struct NoCapsuleReturned {}; - -struct AsAnotherObject {}; - -// Create a void pointer capsule for test. The encapsulated object does not -// matter for this test case. -PyObject *create_test_void_ptr_capsule() { - static int just_to_have_something_to_point_to = 0; - return PyCapsule_New(static_cast(&just_to_have_something_to_point_to), "int", nullptr); -} - -int get_from_valid_capsule(const Valid *) { return 1; } - -int get_from_shared_ptr_valid_capsule(const std::shared_ptr &) { return 2; } - -int get_from_unique_ptr_valid_capsule(std::unique_ptr) { return 3; } - -int get_from_no_conversion_capsule(const NoConversion *) { return 4; } - -int get_from_no_capsule_returned(const NoCapsuleReturned *) { return 5; } - -// https://github.com/pybind/pybind11/issues/3788 -struct TypeWithGetattr { - TypeWithGetattr() = default; - int get_42() const { return 42; } -}; - -// https://github.com/pybind/pybind11/issues/3804 -struct Base1 { - int a1{}; -}; -struct Base2 { - int a2{}; -}; - -struct Base12 : Base1, Base2 { - int foo() const { return 0; } -}; - -struct Derived1 : Base12 { - int bar() const { return 1; } -}; - -struct Derived2 : Base12 { - int bar() const { return 2; } -}; - -struct UnspecBase { - virtual ~UnspecBase() = default; - virtual int Get() const { return 100; } -}; - -int PassUnspecBase(const UnspecBase &sb) { return sb.Get() + 30; } - -struct UnspecDerived : UnspecBase { - int Get() const override { return 200; } -}; - -} // namespace class_sh_void_ptr_capsule -} // namespace pybind11_tests - -using namespace pybind11_tests::class_sh_void_ptr_capsule; - -PYBIND11_SMART_HOLDER_TYPE_CASTERS(Valid) -PYBIND11_SMART_HOLDER_TYPE_CASTERS(TypeWithGetattr) -PYBIND11_SMART_HOLDER_TYPE_CASTERS(Base1) -PYBIND11_SMART_HOLDER_TYPE_CASTERS(Base2) -PYBIND11_SMART_HOLDER_TYPE_CASTERS(Base12) -PYBIND11_SMART_HOLDER_TYPE_CASTERS(Derived1) -PYBIND11_SMART_HOLDER_TYPE_CASTERS(Derived2) -PYBIND11_SMART_HOLDER_TYPE_CASTERS(UnspecBase) -PYBIND11_SMART_HOLDER_TYPE_CASTERS(UnspecDerived) - -TEST_SUBMODULE(class_sh_void_ptr_capsule, m) { - py::classh(m, "Valid"); - - m.def("get_from_valid_capsule", &get_from_valid_capsule); - m.def("get_from_shared_ptr_valid_capsule", &get_from_shared_ptr_valid_capsule); - m.def("get_from_unique_ptr_valid_capsule", &get_from_unique_ptr_valid_capsule); - m.def("get_from_no_conversion_capsule", &get_from_no_conversion_capsule); - m.def("get_from_no_capsule_returned", &get_from_no_capsule_returned); - m.def("create_test_void_ptr_capsule", []() { - PyObject *capsule = create_test_void_ptr_capsule(); - return pybind11::reinterpret_steal(capsule); - }); - - py::classh(m, "TypeWithGetattr") - .def(py::init<>()) - .def("get_42", &TypeWithGetattr::get_42) - .def("__getattr__", - [](TypeWithGetattr &, const std::string &key) { return "GetAttr: " + key; }); - - py::classh(m, "Base1"); - py::classh(m, "Base2"); - - py::classh(m, "Base12") - .def(py::init<>()) - .def("foo", &Base12::foo) - .def("__getattr__", - [](Base12 &, const std::string &key) { return "Base GetAttr: " + key; }); - - py::classh(m, "Derived1").def(py::init<>()).def("bar", &Derived1::bar); - - py::classh(m, "Derived2").def(py::init<>()).def("bar", &Derived2::bar); - - py::classh(m, "UnspecBase"); - m.def("PassUnspecBase", PassUnspecBase); - py::classh(m, "UnspecDerived") // UnspecBase NOT specified as base here. - .def(py::init<>()) - .def("as_pybind11_tests_class_sh_void_ptr_capsule_UnspecBase", [](UnspecDerived *self) { - return py::reinterpret_steal( - PyCapsule_New(static_cast(self), nullptr, nullptr)); - }); -} diff --git a/tests/test_class_sh_void_ptr_capsule.py b/tests/test_class_sh_void_ptr_capsule.py deleted file mode 100644 index 7c9824d8..00000000 --- a/tests/test_class_sh_void_ptr_capsule.py +++ /dev/null @@ -1,100 +0,0 @@ -import pytest - -from pybind11_tests import class_sh_void_ptr_capsule as m - - -class Valid: - def __init__(self): - self.capsule_generated = False - - def as_pybind11_tests_class_sh_void_ptr_capsule_Valid(self): - self.capsule_generated = True - return m.create_test_void_ptr_capsule() - - -class NoConversion: - def __init__(self): - self.capsule_generated = False - - -class NoCapsuleReturned: - def __init__(self): - self.capsule_generated = False - - def as_pybind11_tests_class_sh_void_ptr_capsule_NoCapsuleReturned( - self, - ): - pass - - -class AsAnotherObject: - def __init__(self): - self.capsule_generated = False - - def as_pybind11_tests_class_sh_void_ptr_capsule_Valid(self): - self.capsule_generated = True - return m.create_test_void_ptr_capsule() - - -@pytest.mark.parametrize( - ("ctor", "caller", "expected"), - [ - (Valid, m.get_from_valid_capsule, 1), - (AsAnotherObject, m.get_from_valid_capsule, 1), - ], -) -def test_valid_as_void_ptr_capsule_function(ctor, caller, expected): - obj = ctor() - assert caller(obj) == expected - assert obj.capsule_generated - - -@pytest.mark.parametrize( - ("ctor", "caller"), - [ - (NoConversion, m.get_from_no_conversion_capsule), - (NoCapsuleReturned, m.get_from_no_capsule_returned), - ], -) -def test_invalid_as_void_ptr_capsule_function(ctor, caller): - obj = ctor() - with pytest.raises(TypeError): - caller(obj) - assert not obj.capsule_generated - - -@pytest.mark.parametrize( - ("ctor", "caller", "pointer_type", "capsule_generated"), - [ - (AsAnotherObject, m.get_from_shared_ptr_valid_capsule, "shared_ptr", True), - (AsAnotherObject, m.get_from_unique_ptr_valid_capsule, "unique_ptr", True), - ], -) -def test_as_void_ptr_capsule_unsupported(ctor, caller, pointer_type, capsule_generated): - obj = ctor() - with pytest.raises(RuntimeError) as excinfo: - caller(obj) - assert pointer_type in str(excinfo.value) - assert obj.capsule_generated == capsule_generated - - -def test_type_with_getattr(): - obj = m.TypeWithGetattr() - assert obj.get_42() == 42 - assert obj.something == "GetAttr: something" - - -def test_multiple_inheritance_getattr(): - d1 = m.Derived1() - assert d1.foo() == 0 - assert d1.bar() == 1 - assert d1.prop1 == "Base GetAttr: prop1" - - d2 = m.Derived2() - assert d2.foo() == 0 - assert d2.bar() == 2 - assert d2.prop2 == "Base GetAttr: prop2" - - -def test_pass_unspecified_base(): - assert m.PassUnspecBase(m.UnspecDerived()) == 230