From ef13f04277f5a8b0535c5c19b855d798aa358d68 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 31 Mar 2023 16:28:47 -0700 Subject: [PATCH 01/17] Add `type_caster` (tests are still incomplete). --- include/pybind11/cast.h | 11 +++- tests/CMakeLists.txt | 1 + tests/test_type_caster_pyobject_ptr.cpp | 76 +++++++++++++++++++++++++ tests/test_type_caster_pyobject_ptr.py | 51 +++++++++++++++++ 4 files changed, 138 insertions(+), 1 deletion(-) create mode 100644 tests/test_type_caster_pyobject_ptr.cpp create mode 100644 tests/test_type_caster_pyobject_ptr.py diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 9b013bc39f..c756574c76 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1041,7 +1041,10 @@ make_caster load_type(const handle &handle) { PYBIND11_NAMESPACE_END(detail) // pytype -> C++ type -template ::value, int> = 0> +template < + typename T, + detail::enable_if_t::value && !std::is_same::value, int> + = 0> T cast(const handle &handle) { using namespace detail; static_assert(!cast_is_temporary_value_reference::value, @@ -1055,6 +1058,12 @@ T cast(const handle &handle) { return T(reinterpret_borrow(handle)); } +template , PyObject *>::value, int> = 0> +T cast(const handle &handle) { + return handle.ptr(); +} + // C++ type -> py::object template ::value, int> = 0> object cast(T &&value, diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index b1cb222b4a..44151b1c62 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -154,6 +154,7 @@ set(PYBIND11_TEST_FILES test_stl_binders test_tagbased_polymorphic test_thread + test_type_caster_pyobject_ptr test_union test_virtual_functions) diff --git a/tests/test_type_caster_pyobject_ptr.cpp b/tests/test_type_caster_pyobject_ptr.cpp new file mode 100644 index 0000000000..773dd8f09d --- /dev/null +++ b/tests/test_type_caster_pyobject_ptr.cpp @@ -0,0 +1,76 @@ +#include + +#include "pybind11_tests.h" + +namespace pybind11 { +namespace detail { + +template <> +class type_caster { +public: + static constexpr auto name = const_name("PyObject *"); + + static handle cast(PyObject *src, return_value_policy policy, handle parent) { + if (src == nullptr) { + // NEEDS TEST + throw error_already_set(); + } + if (PyErr_Occurred()) { + // NEEDS TEST + raise_from(PyExc_SystemError, "src != nullptr but PyErr_Occurred()"); + } + if (policy == return_value_policy::take_ownership) { + return src; + } + if (policy == return_value_policy::reference_internal) { + // NEEDS TEST + keep_alive_impl(src, parent); + } + Py_INCREF(src); + return src; + } + + bool load(handle src, bool) { + value = reinterpret_borrow(src); + return true; + } + + template + using cast_op_type = PyObject *; + + explicit operator PyObject *() { return value.ptr(); } + +private: + object value; +}; + +} // namespace detail +} // namespace pybind11 + +TEST_SUBMODULE(type_caster_pyobject_ptr, m) { + m.def("cast_from_PyObject_ptr", []() { + PyObject *ptr = PyLong_FromLongLong(6758L); + py::object retval = py::cast(ptr, py::return_value_policy::take_ownership); + return retval; + }); + m.def("cast_to_PyObject_ptr", [](py::handle obj) { + auto *ptr = py::cast(obj); + return bool(PyTuple_CheckExact(ptr)); + }); + + m.def( + "return_PyObject_ptr", + []() { return PyLong_FromLongLong(2314L); }, + py::return_value_policy::take_ownership); + m.def("pass_PyObject_ptr", [](const PyObject *obj) { return bool(PyTuple_CheckExact(obj)); }); + + m.def("call_callback_with_object_return", + [](const std::function &cb, int mode) { return cb(mode); }); + m.def("call_callback_with_handle_return", + [](const std::function &cb, int mode) { return cb(mode); }); + // + m.def("call_callback_with_PyObject_ptr_return", + [](const std::function &cb, int mode) { return cb(mode); }); + m.def("call_callback_with_PyObject_ptr_arg", + [](const std::function &cb, py::handle obj) { return cb(obj.ptr()); }); +} diff --git a/tests/test_type_caster_pyobject_ptr.py b/tests/test_type_caster_pyobject_ptr.py new file mode 100644 index 0000000000..79882e45d1 --- /dev/null +++ b/tests/test_type_caster_pyobject_ptr.py @@ -0,0 +1,51 @@ +import pytest + +from pybind11_tests import type_caster_pyobject_ptr as m + + +def test_cast_from_PyObject_ptr(): + assert m.cast_from_PyObject_ptr() == 6758 + + +def test_cast_to_PyObject_ptr(): + assert m.cast_to_PyObject_ptr(()) + assert not m.cast_to_PyObject_ptr({}) + + +def test_return_PyObject_ptr(): + assert m.return_PyObject_ptr() == 2314 + + +def test_pass_PyObject_ptr(): + assert m.pass_PyObject_ptr(()) + assert not m.pass_PyObject_ptr({}) + + +@pytest.mark.parametrize( + "call_callback", + [ + m.call_callback_with_object_return, + m.call_callback_with_handle_return, + m.call_callback_with_PyObject_ptr_return, + ], +) +def test_call_callback_with_object_return(call_callback): + def cb(mode): + if mode == 0: + return 10 + if mode == 1: + return "One" + raise NotImplementedError(f"Unknown mode: {mode}") + + assert call_callback(cb, 0) == 10 + assert call_callback(cb, 1) == "One" + with pytest.raises(NotImplementedError, match="Unknown mode: 2"): + call_callback(cb, 2) + + +def test_call_callback_with_PyObject_ptr_arg(): + def cb(obj): + return isinstance(obj, tuple) + + assert m.call_callback_with_PyObject_ptr_arg(cb, ()) + assert not m.call_callback_with_PyObject_ptr_arg(cb, {}) From c7c52b0d3212990a1587a1e33d6c52b93bc0769b Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sat, 1 Apr 2023 01:24:46 -0700 Subject: [PATCH 02/17] Fix oversight (`const PyObject *`). --- tests/test_type_caster_pyobject_ptr.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_type_caster_pyobject_ptr.cpp b/tests/test_type_caster_pyobject_ptr.cpp index 773dd8f09d..19816226f2 100644 --- a/tests/test_type_caster_pyobject_ptr.cpp +++ b/tests/test_type_caster_pyobject_ptr.cpp @@ -62,7 +62,7 @@ TEST_SUBMODULE(type_caster_pyobject_ptr, m) { "return_PyObject_ptr", []() { return PyLong_FromLongLong(2314L); }, py::return_value_policy::take_ownership); - m.def("pass_PyObject_ptr", [](const PyObject *obj) { return bool(PyTuple_CheckExact(obj)); }); + m.def("pass_PyObject_ptr", [](PyObject *obj) { return bool(PyTuple_CheckExact(obj)); }); m.def("call_callback_with_object_return", [](const std::function &cb, int mode) { return cb(mode); }); From f220f0c39d80cbad00d13c0fe914ede4a5eb32ee Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sat, 1 Apr 2023 10:22:46 -0700 Subject: [PATCH 03/17] Ensure `type_caster` only works for `PyObject *` --- tests/test_type_caster_pyobject_ptr.cpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/test_type_caster_pyobject_ptr.cpp b/tests/test_type_caster_pyobject_ptr.cpp index 19816226f2..89d5f59607 100644 --- a/tests/test_type_caster_pyobject_ptr.cpp +++ b/tests/test_type_caster_pyobject_ptr.cpp @@ -3,6 +3,10 @@ #include "pybind11_tests.h" namespace pybind11 { + +template +static constexpr bool is_same_ignoring_cvref = std::is_same, U>::value; + namespace detail { template <> @@ -10,6 +14,14 @@ class type_caster { public: static constexpr auto name = const_name("PyObject *"); + // This overload is purely to guard against accidents. + template , int> = 0> + static handle cast(T &&, return_value_policy, handle /*parent*/) { + static_assert(is_same_ignoring_cvref, + "Invalid C++ type T for to-Python conversion (type_caster)."); + return nullptr; // Unreachable. + } + static handle cast(PyObject *src, return_value_policy policy, handle parent) { if (src == nullptr) { // NEEDS TEST @@ -73,4 +85,11 @@ TEST_SUBMODULE(type_caster_pyobject_ptr, m) { [](const std::function &cb, int mode) { return cb(mode); }); m.def("call_callback_with_PyObject_ptr_arg", [](const std::function &cb, py::handle obj) { return cb(obj.ptr()); }); + +#ifdef PYBIND11_NO_COMPILE_SECTION // Change to ifndef for manual testing. + { + PyObject *ptr = nullptr; + (void) py::cast(*ptr); + } +#endif } From 467b237a1cfeb898eeea06d39eff321fac3edab0 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sat, 1 Apr 2023 10:27:40 -0700 Subject: [PATCH 04/17] Move `is_same_ignoring_cvref` into `detail` namespace. --- tests/test_type_caster_pyobject_ptr.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_type_caster_pyobject_ptr.cpp b/tests/test_type_caster_pyobject_ptr.cpp index 89d5f59607..c9c6323478 100644 --- a/tests/test_type_caster_pyobject_ptr.cpp +++ b/tests/test_type_caster_pyobject_ptr.cpp @@ -3,12 +3,11 @@ #include "pybind11_tests.h" namespace pybind11 { +namespace detail { template static constexpr bool is_same_ignoring_cvref = std::is_same, U>::value; -namespace detail { - template <> class type_caster { public: From 79f2b05ac6d9dfb3324500b5e0232284eded01b7 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sat, 1 Apr 2023 10:50:21 -0700 Subject: [PATCH 05/17] Add test_cast_nullptr --- tests/test_type_caster_pyobject_ptr.cpp | 10 ++++++++-- tests/test_type_caster_pyobject_ptr.py | 13 +++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/tests/test_type_caster_pyobject_ptr.cpp b/tests/test_type_caster_pyobject_ptr.cpp index c9c6323478..262673c58a 100644 --- a/tests/test_type_caster_pyobject_ptr.cpp +++ b/tests/test_type_caster_pyobject_ptr.cpp @@ -23,11 +23,9 @@ class type_caster { static handle cast(PyObject *src, return_value_policy policy, handle parent) { if (src == nullptr) { - // NEEDS TEST throw error_already_set(); } if (PyErr_Occurred()) { - // NEEDS TEST raise_from(PyExc_SystemError, "src != nullptr but PyErr_Occurred()"); } if (policy == return_value_policy::take_ownership) { @@ -85,6 +83,14 @@ TEST_SUBMODULE(type_caster_pyobject_ptr, m) { m.def("call_callback_with_PyObject_ptr_arg", [](const std::function &cb, py::handle obj) { return cb(obj.ptr()); }); + m.def("cast_nullptr", [](bool set_error) { + if (set_error) { + PyErr_SetString(PyExc_RuntimeError, "As in functioning error handling."); + } + PyObject *ptr = nullptr; + py::cast(ptr); + }); + #ifdef PYBIND11_NO_COMPILE_SECTION // Change to ifndef for manual testing. { PyObject *ptr = nullptr; diff --git a/tests/test_type_caster_pyobject_ptr.py b/tests/test_type_caster_pyobject_ptr.py index 79882e45d1..f882078071 100644 --- a/tests/test_type_caster_pyobject_ptr.py +++ b/tests/test_type_caster_pyobject_ptr.py @@ -49,3 +49,16 @@ def cb(obj): assert m.call_callback_with_PyObject_ptr_arg(cb, ()) assert not m.call_callback_with_PyObject_ptr_arg(cb, {}) + + +@pytest.mark.parametrize("set_error", [True, False]) +def test_cast_nullptr(set_error): + expected = { + True: r"As in functioning error handling\.", + False: ( + r"Internal error: pybind11::error_already_set called " + r"while Python error indicator not set\." + ), + }[set_error] + with pytest.raises(RuntimeError, match=expected): + m.cast_nullptr(set_error) From 45f915b274398af6e16fbceaef7bafb5ca6ddeab Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sat, 1 Apr 2023 10:54:35 -0700 Subject: [PATCH 06/17] Change is_same_ignoring_cvref from variable template to using. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ``` test_type_caster_pyobject_ptr.cpp:8:23: error: variable templates only available with ‘-std=c++14’ or ‘-std=gnu++14’ [-Werror] 8 | static constexpr bool is_same_ignoring_cvref = std::is_same, U>::value; | ^~~~~~~~~~~~~~~~~~~~~~ ``` --- tests/test_type_caster_pyobject_ptr.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/test_type_caster_pyobject_ptr.cpp b/tests/test_type_caster_pyobject_ptr.cpp index 262673c58a..4c2b8894e0 100644 --- a/tests/test_type_caster_pyobject_ptr.cpp +++ b/tests/test_type_caster_pyobject_ptr.cpp @@ -6,7 +6,7 @@ namespace pybind11 { namespace detail { template -static constexpr bool is_same_ignoring_cvref = std::is_same, U>::value; +using is_same_ignoring_cvref = std::is_same, U>; template <> class type_caster { @@ -14,9 +14,10 @@ class type_caster { static constexpr auto name = const_name("PyObject *"); // This overload is purely to guard against accidents. - template , int> = 0> + template ::value, int> = 0> static handle cast(T &&, return_value_policy, handle /*parent*/) { - static_assert(is_same_ignoring_cvref, + static_assert(is_same_ignoring_cvref::value, "Invalid C++ type T for to-Python conversion (type_caster)."); return nullptr; // Unreachable. } From 00cc8b4908b796f93d297795aa1e181fc9de0fd1 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sat, 1 Apr 2023 11:32:00 -0700 Subject: [PATCH 07/17] Remove `return_value_policy::reference_internal` `keep_alive` feature (because of doubts about it actually being useful). --- tests/test_type_caster_pyobject_ptr.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/tests/test_type_caster_pyobject_ptr.cpp b/tests/test_type_caster_pyobject_ptr.cpp index 4c2b8894e0..e54cc5603f 100644 --- a/tests/test_type_caster_pyobject_ptr.cpp +++ b/tests/test_type_caster_pyobject_ptr.cpp @@ -22,7 +22,7 @@ class type_caster { return nullptr; // Unreachable. } - static handle cast(PyObject *src, return_value_policy policy, handle parent) { + static handle cast(PyObject *src, return_value_policy policy, handle /*parent*/) { if (src == nullptr) { throw error_already_set(); } @@ -32,10 +32,6 @@ class type_caster { if (policy == return_value_policy::take_ownership) { return src; } - if (policy == return_value_policy::reference_internal) { - // NEEDS TEST - keep_alive_impl(src, parent); - } Py_INCREF(src); return src; } From 432de631857a5659c8e59e717f185e6988841313 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sat, 1 Apr 2023 12:08:39 -0700 Subject: [PATCH 08/17] Add missing test, fix bug (missing `throw error_already_set();`), various cosmetic changes. --- tests/test_type_caster_pyobject_ptr.cpp | 22 +++++++----- tests/test_type_caster_pyobject_ptr.py | 45 ++++++++++++++----------- 2 files changed, 40 insertions(+), 27 deletions(-) diff --git a/tests/test_type_caster_pyobject_ptr.cpp b/tests/test_type_caster_pyobject_ptr.cpp index e54cc5603f..60c920fd08 100644 --- a/tests/test_type_caster_pyobject_ptr.cpp +++ b/tests/test_type_caster_pyobject_ptr.cpp @@ -28,6 +28,7 @@ class type_caster { } if (PyErr_Occurred()) { raise_from(PyExc_SystemError, "src != nullptr but PyErr_Occurred()"); + throw error_already_set(); } if (policy == return_value_policy::take_ownership) { return src; @@ -54,40 +55,45 @@ class type_caster { } // namespace pybind11 TEST_SUBMODULE(type_caster_pyobject_ptr, m) { - m.def("cast_from_PyObject_ptr", []() { + m.def("cast_from_pyobject_ptr", []() { PyObject *ptr = PyLong_FromLongLong(6758L); py::object retval = py::cast(ptr, py::return_value_policy::take_ownership); return retval; }); - m.def("cast_to_PyObject_ptr", [](py::handle obj) { + m.def("cast_to_pyobject_ptr", [](py::handle obj) { auto *ptr = py::cast(obj); return bool(PyTuple_CheckExact(ptr)); }); m.def( - "return_PyObject_ptr", + "return_pyobject_ptr", []() { return PyLong_FromLongLong(2314L); }, py::return_value_policy::take_ownership); - m.def("pass_PyObject_ptr", [](PyObject *obj) { return bool(PyTuple_CheckExact(obj)); }); + m.def("pass_pyobject_ptr", [](PyObject *obj) { return bool(PyTuple_CheckExact(obj)); }); m.def("call_callback_with_object_return", [](const std::function &cb, int mode) { return cb(mode); }); m.def("call_callback_with_handle_return", [](const std::function &cb, int mode) { return cb(mode); }); // - m.def("call_callback_with_PyObject_ptr_return", + m.def("call_callback_with_pyobject_ptr_return", [](const std::function &cb, int mode) { return cb(mode); }); - m.def("call_callback_with_PyObject_ptr_arg", + m.def("call_callback_with_pyobject_ptr_arg", [](const std::function &cb, py::handle obj) { return cb(obj.ptr()); }); - m.def("cast_nullptr", [](bool set_error) { + m.def("cast_to_pyobject_ptr_nullptr", [](bool set_error) { if (set_error) { - PyErr_SetString(PyExc_RuntimeError, "As in functioning error handling."); + PyErr_SetString(PyExc_RuntimeError, "Reflective of healthy error handling."); } PyObject *ptr = nullptr; py::cast(ptr); }); + m.def("cast_to_pyobject_ptr_non_nullptr_with_error_set", []() { + PyErr_SetString(PyExc_RuntimeError, "Reflective of unhealthy error handling."); + py::cast(Py_None); + }); + #ifdef PYBIND11_NO_COMPILE_SECTION // Change to ifndef for manual testing. { PyObject *ptr = nullptr; diff --git a/tests/test_type_caster_pyobject_ptr.py b/tests/test_type_caster_pyobject_ptr.py index f882078071..699349523d 100644 --- a/tests/test_type_caster_pyobject_ptr.py +++ b/tests/test_type_caster_pyobject_ptr.py @@ -3,22 +3,22 @@ from pybind11_tests import type_caster_pyobject_ptr as m -def test_cast_from_PyObject_ptr(): - assert m.cast_from_PyObject_ptr() == 6758 +def test_cast_from_pyobject_ptr(): + assert m.cast_from_pyobject_ptr() == 6758 -def test_cast_to_PyObject_ptr(): - assert m.cast_to_PyObject_ptr(()) - assert not m.cast_to_PyObject_ptr({}) +def test_cast_to_pyobject_ptr(): + assert m.cast_to_pyobject_ptr(()) + assert not m.cast_to_pyobject_ptr({}) -def test_return_PyObject_ptr(): - assert m.return_PyObject_ptr() == 2314 +def test_return_pyobject_ptr(): + assert m.return_pyobject_ptr() == 2314 -def test_pass_PyObject_ptr(): - assert m.pass_PyObject_ptr(()) - assert not m.pass_PyObject_ptr({}) +def test_pass_pyobject_ptr(): + assert m.pass_pyobject_ptr(()) + assert not m.pass_pyobject_ptr({}) @pytest.mark.parametrize( @@ -26,7 +26,7 @@ def test_pass_PyObject_ptr(): [ m.call_callback_with_object_return, m.call_callback_with_handle_return, - m.call_callback_with_PyObject_ptr_return, + m.call_callback_with_pyobject_ptr_return, ], ) def test_call_callback_with_object_return(call_callback): @@ -43,22 +43,29 @@ def cb(mode): call_callback(cb, 2) -def test_call_callback_with_PyObject_ptr_arg(): +def test_call_callback_with_pyobject_ptr_arg(): def cb(obj): return isinstance(obj, tuple) - assert m.call_callback_with_PyObject_ptr_arg(cb, ()) - assert not m.call_callback_with_PyObject_ptr_arg(cb, {}) + assert m.call_callback_with_pyobject_ptr_arg(cb, ()) + assert not m.call_callback_with_pyobject_ptr_arg(cb, {}) @pytest.mark.parametrize("set_error", [True, False]) -def test_cast_nullptr(set_error): +def test_cast_to_python_nullptr(set_error): expected = { - True: r"As in functioning error handling\.", + True: r"^Reflective of healthy error handling\.$", False: ( - r"Internal error: pybind11::error_already_set called " - r"while Python error indicator not set\." + r"^Internal error: pybind11::error_already_set called " + r"while Python error indicator not set\.$" ), }[set_error] with pytest.raises(RuntimeError, match=expected): - m.cast_nullptr(set_error) + m.cast_to_pyobject_ptr_nullptr(set_error) + + +def test_cast_to_python_non_nullptr_with_error_set(): + with pytest.raises(SystemError) as excinfo: + m.cast_to_pyobject_ptr_non_nullptr_with_error_set() + assert str(excinfo.value) == "src != nullptr but PyErr_Occurred()" + assert str(excinfo.value.__cause__) == "Reflective of unhealthy error handling." From db47541a61debbf336ebab61511834d99829d128 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sat, 1 Apr 2023 12:18:29 -0700 Subject: [PATCH 09/17] Move `type_caster` from test to new include (pybind11/type_caster_pyobject_ptr.h) --- include/pybind11/cast.h | 11 ++-- include/pybind11/detail/common.h | 4 ++ include/pybind11/type_caster_pyobject_ptr.h | 57 +++++++++++++++++++++ tests/test_type_caster_pyobject_ptr.cpp | 53 +------------------ 4 files changed, 68 insertions(+), 57 deletions(-) create mode 100644 include/pybind11/type_caster_pyobject_ptr.h diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index c756574c76..6d368e473b 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1041,10 +1041,11 @@ make_caster load_type(const handle &handle) { PYBIND11_NAMESPACE_END(detail) // pytype -> C++ type -template < - typename T, - detail::enable_if_t::value && !std::is_same::value, int> - = 0> +template ::value + && !detail::is_same_ignoring_cvref::value, + int> + = 0> T cast(const handle &handle) { using namespace detail; static_assert(!cast_is_temporary_value_reference::value, @@ -1059,7 +1060,7 @@ T cast(const handle &handle) { } template , PyObject *>::value, int> = 0> + detail::enable_if_t::value, int> = 0> T cast(const handle &handle) { return handle.ptr(); } diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 129039252f..c571453350 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -661,6 +661,10 @@ template using remove_cvref_t = typename remove_cvref::type; #endif +/// Example usage: is_same_ignoring_cvref +template +using is_same_ignoring_cvref = std::is_same, U>; + /// Index sequences #if defined(PYBIND11_CPP14) using std::index_sequence; diff --git a/include/pybind11/type_caster_pyobject_ptr.h b/include/pybind11/type_caster_pyobject_ptr.h new file mode 100644 index 0000000000..5d538d20ed --- /dev/null +++ b/include/pybind11/type_caster_pyobject_ptr.h @@ -0,0 +1,57 @@ +// Copyright (c) 2023 The pybind Community. + +#pragma once + +#include "detail/common.h" +#include "detail/descr.h" +#include "cast.h" +#include "pytypes.h" + +PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) +PYBIND11_NAMESPACE_BEGIN(detail) + +template <> +class type_caster { +public: + static constexpr auto name = const_name("PyObject *"); + + // This overload is purely to guard against accidents. + template ::value, int> = 0> + static handle cast(T &&, return_value_policy, handle /*parent*/) { + static_assert(is_same_ignoring_cvref::value, + "Invalid C++ type T for to-Python conversion (type_caster)."); + return nullptr; // Unreachable. + } + + static handle cast(PyObject *src, return_value_policy policy, handle /*parent*/) { + if (src == nullptr) { + throw error_already_set(); + } + if (PyErr_Occurred()) { + raise_from(PyExc_SystemError, "src != nullptr but PyErr_Occurred()"); + throw error_already_set(); + } + if (policy == return_value_policy::take_ownership) { + return src; + } + Py_INCREF(src); + return src; + } + + bool load(handle src, bool) { + value = reinterpret_borrow(src); + return true; + } + + template + using cast_op_type = PyObject *; + + explicit operator PyObject *() { return value.ptr(); } + +private: + object value; +}; + +PYBIND11_NAMESPACE_END(detail) +PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) diff --git a/tests/test_type_caster_pyobject_ptr.cpp b/tests/test_type_caster_pyobject_ptr.cpp index 60c920fd08..71fbd383d9 100644 --- a/tests/test_type_caster_pyobject_ptr.cpp +++ b/tests/test_type_caster_pyobject_ptr.cpp @@ -1,59 +1,8 @@ #include +#include #include "pybind11_tests.h" -namespace pybind11 { -namespace detail { - -template -using is_same_ignoring_cvref = std::is_same, U>; - -template <> -class type_caster { -public: - static constexpr auto name = const_name("PyObject *"); - - // This overload is purely to guard against accidents. - template ::value, int> = 0> - static handle cast(T &&, return_value_policy, handle /*parent*/) { - static_assert(is_same_ignoring_cvref::value, - "Invalid C++ type T for to-Python conversion (type_caster)."); - return nullptr; // Unreachable. - } - - static handle cast(PyObject *src, return_value_policy policy, handle /*parent*/) { - if (src == nullptr) { - throw error_already_set(); - } - if (PyErr_Occurred()) { - raise_from(PyExc_SystemError, "src != nullptr but PyErr_Occurred()"); - throw error_already_set(); - } - if (policy == return_value_policy::take_ownership) { - return src; - } - Py_INCREF(src); - return src; - } - - bool load(handle src, bool) { - value = reinterpret_borrow(src); - return true; - } - - template - using cast_op_type = PyObject *; - - explicit operator PyObject *() { return value.ptr(); } - -private: - object value; -}; - -} // namespace detail -} // namespace pybind11 - TEST_SUBMODULE(type_caster_pyobject_ptr, m) { m.def("cast_from_pyobject_ptr", []() { PyObject *ptr = PyLong_FromLongLong(6758L); From b3930018599c986bf17a037e1a628d8943a833b6 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sat, 1 Apr 2023 12:55:42 -0700 Subject: [PATCH 10/17] Add new header file to CMakeLists.txt and tests/extra_python_package/test_files.py --- CMakeLists.txt | 3 ++- tests/extra_python_package/test_files.py | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 0d93203881..25a7d14984 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -139,7 +139,8 @@ set(PYBIND11_HEADERS include/pybind11/pytypes.h include/pybind11/stl.h include/pybind11/stl_bind.h - include/pybind11/stl/filesystem.h) + include/pybind11/stl/filesystem.h + include/pybind11/type_caster_pyobject_ptr.h) # Compare with grep and warn if mismatched if(PYBIND11_MASTER_PROJECT AND NOT CMAKE_VERSION VERSION_LESS 3.12) diff --git a/tests/extra_python_package/test_files.py b/tests/extra_python_package/test_files.py index dd6393bf0b..e5982f962e 100644 --- a/tests/extra_python_package/test_files.py +++ b/tests/extra_python_package/test_files.py @@ -43,6 +43,7 @@ "include/pybind11/pytypes.h", "include/pybind11/stl.h", "include/pybind11/stl_bind.h", + "include/pybind11/type_caster_pyobject_ptr.h", } detail_headers = { From 0ba8003d86248c959742a3e3fcf2eaaa7fd98cb5 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 3 Apr 2023 08:35:21 -0700 Subject: [PATCH 11/17] Backport changes from https://github.com/google/pywrapcc/pull/30021 to https://github.com/pybind/pybind11/pull/4601 --- include/pybind11/cast.h | 2 +- include/pybind11/type_caster_pyobject_ptr.h | 8 ++++-- tests/test_type_caster_pyobject_ptr.cpp | 18 ++++++++------ tests/test_type_caster_pyobject_ptr.py | 27 ++++++++++++--------- 4 files changed, 32 insertions(+), 23 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 6d368e473b..1e73eb3485 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1062,7 +1062,7 @@ T cast(const handle &handle) { template ::value, int> = 0> T cast(const handle &handle) { - return handle.ptr(); + return handle.inc_ref().ptr(); } // C++ type -> py::object diff --git a/include/pybind11/type_caster_pyobject_ptr.h b/include/pybind11/type_caster_pyobject_ptr.h index 5d538d20ed..3b6a7bed75 100644 --- a/include/pybind11/type_caster_pyobject_ptr.h +++ b/include/pybind11/type_caster_pyobject_ptr.h @@ -35,8 +35,12 @@ class type_caster { if (policy == return_value_policy::take_ownership) { return src; } - Py_INCREF(src); - return src; + if (policy == return_value_policy::reference + || policy == return_value_policy::automatic_reference) { + return handle(src).inc_ref(); + } + pybind11_fail("type_caster::cast(): unsupported return_value_policy: " + + std::to_string(static_cast(policy))); } bool load(handle src, bool) { diff --git a/tests/test_type_caster_pyobject_ptr.cpp b/tests/test_type_caster_pyobject_ptr.cpp index 71fbd383d9..4e476d34ea 100644 --- a/tests/test_type_caster_pyobject_ptr.cpp +++ b/tests/test_type_caster_pyobject_ptr.cpp @@ -21,14 +21,16 @@ TEST_SUBMODULE(type_caster_pyobject_ptr, m) { m.def("pass_pyobject_ptr", [](PyObject *obj) { return bool(PyTuple_CheckExact(obj)); }); m.def("call_callback_with_object_return", - [](const std::function &cb, int mode) { return cb(mode); }); - m.def("call_callback_with_handle_return", - [](const std::function &cb, int mode) { return cb(mode); }); - // - m.def("call_callback_with_pyobject_ptr_return", - [](const std::function &cb, int mode) { return cb(mode); }); - m.def("call_callback_with_pyobject_ptr_arg", - [](const std::function &cb, py::handle obj) { return cb(obj.ptr()); }); + [](const std::function &cb, int value) { return cb(value); }); + m.def( + "call_callback_with_pyobject_ptr_return", + [](const std::function &cb, int value) { return cb(value); }, + py::return_value_policy::take_ownership); + m.def( + "call_callback_with_pyobject_ptr_arg", + [](const std::function &cb, py::handle obj) { return cb(obj.ptr()); }, + py::arg("cb"), // This triggers return_value_policy::automatic_reference + py::arg("obj")); m.def("cast_to_pyobject_ptr_nullptr", [](bool set_error) { if (set_error) { diff --git a/tests/test_type_caster_pyobject_ptr.py b/tests/test_type_caster_pyobject_ptr.py index 699349523d..6271e92012 100644 --- a/tests/test_type_caster_pyobject_ptr.py +++ b/tests/test_type_caster_pyobject_ptr.py @@ -21,26 +21,29 @@ def test_pass_pyobject_ptr(): assert not m.pass_pyobject_ptr({}) +class ValueHolder: + def __init__(self, value): + self.value = value + + @pytest.mark.parametrize( "call_callback", [ m.call_callback_with_object_return, - m.call_callback_with_handle_return, m.call_callback_with_pyobject_ptr_return, ], ) def test_call_callback_with_object_return(call_callback): - def cb(mode): - if mode == 0: - return 10 - if mode == 1: - return "One" - raise NotImplementedError(f"Unknown mode: {mode}") - - assert call_callback(cb, 0) == 10 - assert call_callback(cb, 1) == "One" - with pytest.raises(NotImplementedError, match="Unknown mode: 2"): - call_callback(cb, 2) + def cb(value): + if value < 0: + raise ValueError("Raised from cb") + # Return a temporary user-defined object, to maximize sensitivity of this test. + return ValueHolder(1000 - value) + + assert call_callback(cb, 287).value == 713 + + with pytest.raises(ValueError, match="^Raised from cb$"): + call_callback(cb, -1) def test_call_callback_with_pyobject_ptr_arg(): From af1886a835096028386f307405cb3328aa057e55 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Tue, 4 Apr 2023 16:33:00 -0700 Subject: [PATCH 12/17] Fix oversight in test (to resolve a valgrind leak detection error) and add a related comment in cast.h. No production code changes. Make tests more sensitive by using `ValueHolder` instead of empty tuples and dicts. Manual leak checks with `while True:` & top command repeated for all tests. --- include/pybind11/cast.h | 4 ++++ tests/test_type_caster_pyobject_ptr.cpp | 16 +++++++++++----- tests/test_type_caster_pyobject_ptr.py | 23 ++++++++++------------- 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 1e73eb3485..6fbfcc9d4c 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1059,6 +1059,10 @@ T cast(const handle &handle) { return T(reinterpret_borrow(handle)); } +// Note that `cast(obj)` increments the reference count of `obj`. +// This is necessary for the case that `obj` is a temporary. +// It is the responsibility of the caller to ensure that the reference count +// is decremented. template ::value, int> = 0> T cast(const handle &handle) { diff --git a/tests/test_type_caster_pyobject_ptr.cpp b/tests/test_type_caster_pyobject_ptr.cpp index 4e476d34ea..e5fd6b9ac2 100644 --- a/tests/test_type_caster_pyobject_ptr.cpp +++ b/tests/test_type_caster_pyobject_ptr.cpp @@ -6,19 +6,25 @@ TEST_SUBMODULE(type_caster_pyobject_ptr, m) { m.def("cast_from_pyobject_ptr", []() { PyObject *ptr = PyLong_FromLongLong(6758L); - py::object retval = py::cast(ptr, py::return_value_policy::take_ownership); - return retval; + return py::cast(ptr, py::return_value_policy::take_ownership); }); m.def("cast_to_pyobject_ptr", [](py::handle obj) { + auto rc1 = obj.ref_count(); auto *ptr = py::cast(obj); - return bool(PyTuple_CheckExact(ptr)); + auto rc2 = obj.ref_count(); + if (rc2 != rc1 + 1) { + return -1; + } + return 100 - py::reinterpret_steal(ptr).attr("value").cast(); }); m.def( "return_pyobject_ptr", []() { return PyLong_FromLongLong(2314L); }, py::return_value_policy::take_ownership); - m.def("pass_pyobject_ptr", [](PyObject *obj) { return bool(PyTuple_CheckExact(obj)); }); + m.def("pass_pyobject_ptr", [](PyObject *ptr) { + return 200 - py::reinterpret_borrow(ptr).attr("value").cast(); + }); m.def("call_callback_with_object_return", [](const std::function &cb, int value) { return cb(value); }); @@ -28,7 +34,7 @@ TEST_SUBMODULE(type_caster_pyobject_ptr, m) { py::return_value_policy::take_ownership); m.def( "call_callback_with_pyobject_ptr_arg", - [](const std::function &cb, py::handle obj) { return cb(obj.ptr()); }, + [](const std::function &cb, py::handle obj) { return cb(obj.ptr()); }, py::arg("cb"), // This triggers return_value_policy::automatic_reference py::arg("obj")); diff --git a/tests/test_type_caster_pyobject_ptr.py b/tests/test_type_caster_pyobject_ptr.py index 6271e92012..53604954e7 100644 --- a/tests/test_type_caster_pyobject_ptr.py +++ b/tests/test_type_caster_pyobject_ptr.py @@ -3,13 +3,18 @@ from pybind11_tests import type_caster_pyobject_ptr as m +# For use as a temporary user-defined object, to maximize sensitivity of the tests below. +class ValueHolder: + def __init__(self, value): + self.value = value + + def test_cast_from_pyobject_ptr(): assert m.cast_from_pyobject_ptr() == 6758 def test_cast_to_pyobject_ptr(): - assert m.cast_to_pyobject_ptr(()) - assert not m.cast_to_pyobject_ptr({}) + assert m.cast_to_pyobject_ptr(ValueHolder(24)) == 76 def test_return_pyobject_ptr(): @@ -17,13 +22,7 @@ def test_return_pyobject_ptr(): def test_pass_pyobject_ptr(): - assert m.pass_pyobject_ptr(()) - assert not m.pass_pyobject_ptr({}) - - -class ValueHolder: - def __init__(self, value): - self.value = value + assert m.pass_pyobject_ptr(ValueHolder(82)) == 118 @pytest.mark.parametrize( @@ -37,7 +36,6 @@ def test_call_callback_with_object_return(call_callback): def cb(value): if value < 0: raise ValueError("Raised from cb") - # Return a temporary user-defined object, to maximize sensitivity of this test. return ValueHolder(1000 - value) assert call_callback(cb, 287).value == 713 @@ -48,10 +46,9 @@ def cb(value): def test_call_callback_with_pyobject_ptr_arg(): def cb(obj): - return isinstance(obj, tuple) + return 300 - obj.value - assert m.call_callback_with_pyobject_ptr_arg(cb, ()) - assert not m.call_callback_with_pyobject_ptr_arg(cb, {}) + assert m.call_callback_with_pyobject_ptr_arg(cb, ValueHolder(39)) == 261 @pytest.mark.parametrize("set_error", [True, False]) From 3391bbd8f82c2681eed9ca616caa22254efcac4c Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Tue, 4 Apr 2023 21:49:47 -0700 Subject: [PATCH 13/17] Add tests for interop with stl.h `list_caster` (No production code changes.) --- tests/test_type_caster_pyobject_ptr.cpp | 47 +++++++++++++++++++++++++ tests/test_type_caster_pyobject_ptr.py | 20 +++++++++++ 2 files changed, 67 insertions(+) diff --git a/tests/test_type_caster_pyobject_ptr.cpp b/tests/test_type_caster_pyobject_ptr.cpp index e5fd6b9ac2..cd65721efb 100644 --- a/tests/test_type_caster_pyobject_ptr.cpp +++ b/tests/test_type_caster_pyobject_ptr.cpp @@ -1,8 +1,25 @@ #include +#include #include #include "pybind11_tests.h" +#include +#include + +namespace { + +std::vector make_vector_pyobject_ptr(const py::object &ValueHolder) { + std::vector vec_obj; + for (int i = 1; i < 3; i++) { + vec_obj.push_back(ValueHolder(i * 93).release().ptr()); + } + // This vector now owns the refcounts. + return vec_obj; +} + +} // namespace + TEST_SUBMODULE(type_caster_pyobject_ptr, m) { m.def("cast_from_pyobject_ptr", []() { PyObject *ptr = PyLong_FromLongLong(6758L); @@ -51,6 +68,36 @@ TEST_SUBMODULE(type_caster_pyobject_ptr, m) { py::cast(Py_None); }); + m.def("pass_list_pyobject_ptr", [](const std::vector &vec_obj) { + int acc = 0; + for (const auto &ptr : vec_obj) { + acc = acc * 1000 + py::reinterpret_borrow(ptr).attr("value").cast(); + } + return acc; + }); + + m.def("return_list_pyobject_ptr_take_ownership", + make_vector_pyobject_ptr, + // Ownership is transferred one-by-one when the vector is converted to a Python list. + py::return_value_policy::take_ownership); + + m.def("return_list_pyobject_ptr_reference", + make_vector_pyobject_ptr, + // Ownership is not transferred. + py::return_value_policy::reference); + + m.def("dec_ref_each_pyobject_ptr", [](const std::vector &vec_obj) { + std::size_t i = 0; + for (; i < vec_obj.size(); i++) { + py::handle h(vec_obj[i]); + if (static_cast(h.ref_count()) < i) { + break; + } + h.dec_ref(); + } + return i; + }); + #ifdef PYBIND11_NO_COMPILE_SECTION // Change to ifndef for manual testing. { PyObject *ptr = nullptr; diff --git a/tests/test_type_caster_pyobject_ptr.py b/tests/test_type_caster_pyobject_ptr.py index 53604954e7..fcb084a575 100644 --- a/tests/test_type_caster_pyobject_ptr.py +++ b/tests/test_type_caster_pyobject_ptr.py @@ -69,3 +69,23 @@ def test_cast_to_python_non_nullptr_with_error_set(): m.cast_to_pyobject_ptr_non_nullptr_with_error_set() assert str(excinfo.value) == "src != nullptr but PyErr_Occurred()" assert str(excinfo.value.__cause__) == "Reflective of unhealthy error handling." + + +def test_pass_list_pyobject_ptr(): + acc = m.pass_list_pyobject_ptr([ValueHolder(842), ValueHolder(452)]) + assert acc == 842452 + + +def test_return_list_pyobject_ptr_take_ownership(): + vec_obj = m.return_list_pyobject_ptr_take_ownership(ValueHolder) + assert [e.value for e in vec_obj] == [93, 186] + + +def test_return_list_pyobject_ptr_reference(): + vec_obj = m.return_list_pyobject_ptr_reference(ValueHolder) + assert [e.value for e in vec_obj] == [93, 186] + # Commenting out the next `assert` will leak the Python references. + # An easy way to see evidence of the leaks: + # Insert `while True:` as the first line of this function and monitor the + # process RES (Resident Memory Size) with the Unix top command. + assert m.dec_ref_each_pyobject_ptr(vec_obj) == 2 From f6c7ceeb584a9a25cb308bebc37908aba886836d Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Tue, 4 Apr 2023 22:30:30 -0700 Subject: [PATCH 14/17] Bug fix in test. Minor comment enhancements. --- include/pybind11/detail/common.h | 2 +- tests/test_type_caster_pyobject_ptr.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index c571453350..1ff0df09fe 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -661,7 +661,7 @@ template using remove_cvref_t = typename remove_cvref::type; #endif -/// Example usage: is_same_ignoring_cvref +/// Example usage: is_same_ignoring_cvref::value template using is_same_ignoring_cvref = std::is_same, U>; diff --git a/tests/test_type_caster_pyobject_ptr.cpp b/tests/test_type_caster_pyobject_ptr.cpp index cd65721efb..6135ded40e 100644 --- a/tests/test_type_caster_pyobject_ptr.cpp +++ b/tests/test_type_caster_pyobject_ptr.cpp @@ -90,8 +90,8 @@ TEST_SUBMODULE(type_caster_pyobject_ptr, m) { std::size_t i = 0; for (; i < vec_obj.size(); i++) { py::handle h(vec_obj[i]); - if (static_cast(h.ref_count()) < i) { - break; + if (static_cast(h.ref_count()) < 2) { + break; // Something is badly wrong. } h.dec_ref(); } From 5ccb893972b921605472dafeffcce6712e5aee94 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Thu, 27 Apr 2023 07:06:35 -0700 Subject: [PATCH 15/17] Change `type_caster::name` to `object`, as suggested by @Skylion007 --- include/pybind11/type_caster_pyobject_ptr.h | 2 +- tests/test_type_caster_pyobject_ptr.cpp | 2 ++ tests/test_type_caster_pyobject_ptr.py | 5 +++++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/include/pybind11/type_caster_pyobject_ptr.h b/include/pybind11/type_caster_pyobject_ptr.h index 3b6a7bed75..aa914f9e15 100644 --- a/include/pybind11/type_caster_pyobject_ptr.h +++ b/include/pybind11/type_caster_pyobject_ptr.h @@ -13,7 +13,7 @@ PYBIND11_NAMESPACE_BEGIN(detail) template <> class type_caster { public: - static constexpr auto name = const_name("PyObject *"); + static constexpr auto name = const_name("object"); // See discussion under PR #4601. // This overload is purely to guard against accidents. template None"): + m.pass_pyobject_ptr_and_int(ValueHolder(101), ValueHolder(202)) From cf4c2823e5f7b67aef75db59e1e0794a0e9977c6 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Thu, 27 Apr 2023 07:43:16 -0700 Subject: [PATCH 16/17] Expand comment for the new `T cast(const handle &handle)` [`T` = `PyObject *`] --- include/pybind11/cast.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index fbb6a6d8a7..33e4c96dbf 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1060,7 +1060,11 @@ T cast(const handle &handle) { } // Note that `cast(obj)` increments the reference count of `obj`. -// This is necessary for the case that `obj` is a temporary. +// This is necessary for the case that `obj` is a temporary, and could +// not possibly be different, given +// 1. the established convention that the passed `handle` is borrowed, and +// 2. we don't want to force all generic code using `cast()` to special-case +// handling of `T` = `PyObject *` (to increment the reference count there). // It is the responsibility of the caller to ensure that the reference count // is decremented. template Date: Mon, 1 May 2023 17:21:38 -0700 Subject: [PATCH 17/17] Add `T cast(object &&obj)` overload as suggested by @Skylion007 The original suggestion leads to `error: call to 'cast' is ambiguous` (full error message below), therefore SFINAE guarding is needed. ``` clang++ -o pybind11/tests/test_type_caster_pyobject_ptr.os -c -std=c++17 -fPIC -fvisibility=hidden -O0 -g -Wall -Wextra -Wconversion -Wcast-qual -Wdeprecated -Wundef -Wnon-virtual-dtor -Wunused-result -Werror -isystem /usr/include/python3.10 -isystem /usr/include/eigen3 -DPYBIND11_STRICT_ASSERTS_CLASS_HOLDER_VS_TYPE_CASTER_MIX -DPYBIND11_ENABLE_TYPE_CASTER_ODR_GUARD_IF_AVAILABLE -DPYBIND11_TEST_BOOST -Ipybind11/include -I/usr/local/google/home/rwgk/forked/pybind11/include -I/usr/local/google/home/rwgk/clone/pybind11/include /usr/local/google/home/rwgk/forked/pybind11/tests/test_type_caster_pyobject_ptr.cpp In file included from /usr/local/google/home/rwgk/forked/pybind11/tests/test_type_caster_pyobject_ptr.cpp:1: In file included from /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/functional.h:12: In file included from /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/pybind11.h:13: In file included from /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/detail/class.h:12: In file included from /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/attr.h:14: /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/cast.h:1165:12: error: call to 'cast' is ambiguous return pybind11::cast(std::move(*this)); ^~~~~~~~~~~~~~~~~ /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/functional.h:109:70: note: in instantiation of function template specialization 'pybind11::object::cast<_object *>' requested here return hfunc.f(std::forward(args)...).template cast(); ^ /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/functional.h:103:16: note: in instantiation of member function 'pybind11::detail::type_caster>::load(pybind11::handle, bool)::func_wrapper::operator()' requested here struct func_wrapper { ^ /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/cast.h:1456:47: note: in instantiation of member function 'pybind11::detail::type_caster>::load' requested here if ((... || !std::get(argcasters).load(call.args[Is], call.args_convert[Is]))) { ^ /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/cast.h:1434:50: note: in instantiation of function template specialization 'pybind11::detail::argument_loader &, int>::load_impl_sequence<0UL, 1UL>' requested here bool load_args(function_call &call) { return load_impl_sequence(call, indices{}); } ^ /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/pybind11.h:227:33: note: in instantiation of member function 'pybind11::detail::argument_loader &, int>::load_args' requested here if (!args_converter.load_args(call)) { ^ /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/pybind11.h:101:9: note: in instantiation of function template specialization 'pybind11::cpp_function::initialize<(lambda at /usr/local/google/home/rwgk/forked/pybind11/tests/test_type_caster_pyobject_ptr.cpp:50:9), _object *, const std::function<_object *(int)> &, int, pybind11::name, pybind11::scope, pybind11::sibling, pybind11::return_value_policy>' requested here initialize( ^ /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/pybind11.h:1163:22: note: in instantiation of function template specialization 'pybind11::cpp_function::cpp_function<(lambda at /usr/local/google/home/rwgk/forked/pybind11/tests/test_type_caster_pyobject_ptr.cpp:50:9), pybind11::name, pybind11::scope, pybind11::sibling, pybind11::return_value_policy, void>' requested here cpp_function func(std::forward(f), ^ /usr/local/google/home/rwgk/forked/pybind11/tests/test_type_caster_pyobject_ptr.cpp:48:7: note: in instantiation of function template specialization 'pybind11::module_::def<(lambda at /usr/local/google/home/rwgk/forked/pybind11/tests/test_type_caster_pyobject_ptr.cpp:50:9), pybind11::return_value_policy>' requested here m.def( ^ /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/cast.h:1077:3: note: candidate function [with T = _object *, $1 = 0] T cast(object &&obj) { ^ /usr/local/google/home/rwgk/forked/pybind11/include/pybind11/cast.h:1149:1: note: candidate function [with T = _object *] cast(object &&object) { ^ 1 error generated. ``` --- include/pybind11/cast.h | 18 ++++++++++++++++-- tests/test_type_caster_pyobject_ptr.cpp | 23 ++++++++++++++++++++++- tests/test_type_caster_pyobject_ptr.py | 12 ++++++++++-- 3 files changed, 48 insertions(+), 5 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 33e4c96dbf..db39341180 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1068,10 +1068,24 @@ T cast(const handle &handle) { // It is the responsibility of the caller to ensure that the reference count // is decremented. template ::value, int> = 0> -T cast(const handle &handle) { + typename Handle, + detail::enable_if_t::value + && detail::is_same_ignoring_cvref::value, + int> + = 0> +T cast(Handle &&handle) { return handle.inc_ref().ptr(); } +// To optimize way an inc_ref/dec_ref cycle: +template ::value + && detail::is_same_ignoring_cvref::value, + int> + = 0> +T cast(Object &&obj) { + return obj.release().ptr(); +} // C++ type -> py::object template ::value, int> = 0> diff --git a/tests/test_type_caster_pyobject_ptr.cpp b/tests/test_type_caster_pyobject_ptr.cpp index c6d5965c07..1667ea1266 100644 --- a/tests/test_type_caster_pyobject_ptr.cpp +++ b/tests/test_type_caster_pyobject_ptr.cpp @@ -25,7 +25,7 @@ TEST_SUBMODULE(type_caster_pyobject_ptr, m) { PyObject *ptr = PyLong_FromLongLong(6758L); return py::cast(ptr, py::return_value_policy::take_ownership); }); - m.def("cast_to_pyobject_ptr", [](py::handle obj) { + m.def("cast_handle_to_pyobject_ptr", [](py::handle obj) { auto rc1 = obj.ref_count(); auto *ptr = py::cast(obj); auto rc2 = obj.ref_count(); @@ -34,6 +34,27 @@ TEST_SUBMODULE(type_caster_pyobject_ptr, m) { } return 100 - py::reinterpret_steal(ptr).attr("value").cast(); }); + m.def("cast_object_to_pyobject_ptr", [](py::object obj) { + py::handle hdl = obj; + auto rc1 = hdl.ref_count(); + auto *ptr = py::cast(std::move(obj)); + auto rc2 = hdl.ref_count(); + if (rc2 != rc1) { + return -1; + } + return 300 - py::reinterpret_steal(ptr).attr("value").cast(); + }); + m.def("cast_list_to_pyobject_ptr", [](py::list lst) { + // This is to cover types implicitly convertible to object. + py::handle hdl = lst; + auto rc1 = hdl.ref_count(); + auto *ptr = py::cast(std::move(lst)); + auto rc2 = hdl.ref_count(); + if (rc2 != rc1) { + return -1; + } + return 400 - static_cast(py::len(py::reinterpret_steal(ptr))); + }); m.def( "return_pyobject_ptr", diff --git a/tests/test_type_caster_pyobject_ptr.py b/tests/test_type_caster_pyobject_ptr.py index 93e734bd07..1f1ece2baf 100644 --- a/tests/test_type_caster_pyobject_ptr.py +++ b/tests/test_type_caster_pyobject_ptr.py @@ -13,8 +13,16 @@ def test_cast_from_pyobject_ptr(): assert m.cast_from_pyobject_ptr() == 6758 -def test_cast_to_pyobject_ptr(): - assert m.cast_to_pyobject_ptr(ValueHolder(24)) == 76 +def test_cast_handle_to_pyobject_ptr(): + assert m.cast_handle_to_pyobject_ptr(ValueHolder(24)) == 76 + + +def test_cast_object_to_pyobject_ptr(): + assert m.cast_object_to_pyobject_ptr(ValueHolder(43)) == 257 + + +def test_cast_list_to_pyobject_ptr(): + assert m.cast_list_to_pyobject_ptr([1, 2, 3, 4, 5]) == 395 def test_return_pyobject_ptr():