From 1e4bd22bdca5068188d2d46136b79d18a1d1f647 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Wed, 18 May 2022 23:19:33 -0400 Subject: [PATCH 01/14] fix(cmake): support release and debug at the same time (#3948) --- tools/pybind11Tools.cmake | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/tools/pybind11Tools.cmake b/tools/pybind11Tools.cmake index 3d1503df8..5535e872f 100644 --- a/tools/pybind11Tools.cmake +++ b/tools/pybind11Tools.cmake @@ -115,17 +115,32 @@ if(PYTHON_IS_DEBUG) PROPERTY INTERFACE_COMPILE_DEFINITIONS Py_DEBUG) endif() -set_property( - TARGET pybind11::module - APPEND - PROPERTY - INTERFACE_LINK_LIBRARIES pybind11::python_link_helper - "$<$,$>:$>") +if(CMAKE_VERSION VERSION_LESS 3.11) + set_property( + TARGET pybind11::module + APPEND + PROPERTY + INTERFACE_LINK_LIBRARIES + pybind11::python_link_helper + "$<$,$>:$>" + ) -set_property( - TARGET pybind11::embed - APPEND - PROPERTY INTERFACE_LINK_LIBRARIES pybind11::pybind11 $) + set_property( + TARGET pybind11::embed + APPEND + PROPERTY INTERFACE_LINK_LIBRARIES pybind11::pybind11 $) +else() + target_link_libraries( + pybind11::module + INTERFACE + pybind11::python_link_helper + "$<$,$>:$>" + ) + + target_link_libraries(pybind11::embed INTERFACE pybind11::pybind11 + $) + +endif() function(pybind11_extension name) # The prefix and extension are provided by FindPythonLibsNew.cmake From 918d4481a4f69c7a4cbb4282acbabd28e2433039 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Fri, 20 May 2022 09:38:29 -0400 Subject: [PATCH 02/14] fix(cmake): support cross-compiles with classic Python (#3959) Signed-off-by: Henry Schreiner --- tools/FindPythonLibsNew.cmake | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/tools/FindPythonLibsNew.cmake b/tools/FindPythonLibsNew.cmake index a035428f8..a5a628fd7 100644 --- a/tools/FindPythonLibsNew.cmake +++ b/tools/FindPythonLibsNew.cmake @@ -151,26 +151,36 @@ if(NOT _PYTHON_SUCCESS MATCHES 0) return() endif() +# Can manually set values when cross-compiling +macro(_PYBIND11_GET_IF_UNDEF lst index name) + if(NOT DEFINED "${name}") + list(GET "${lst}" "${index}" "${name}") + endif() +endmacro() + # Convert the process output into a list if(WIN32) string(REGEX REPLACE "\\\\" "/" _PYTHON_VALUES ${_PYTHON_VALUES}) endif() string(REGEX REPLACE ";" "\\\\;" _PYTHON_VALUES ${_PYTHON_VALUES}) string(REGEX REPLACE "\n" ";" _PYTHON_VALUES ${_PYTHON_VALUES}) -list(GET _PYTHON_VALUES 0 _PYTHON_VERSION_LIST) -list(GET _PYTHON_VALUES 1 PYTHON_PREFIX) -list(GET _PYTHON_VALUES 2 PYTHON_INCLUDE_DIR) -list(GET _PYTHON_VALUES 3 PYTHON_SITE_PACKAGES) -list(GET _PYTHON_VALUES 4 PYTHON_MODULE_EXTENSION) -list(GET _PYTHON_VALUES 5 PYTHON_IS_DEBUG) -list(GET _PYTHON_VALUES 6 PYTHON_SIZEOF_VOID_P) -list(GET _PYTHON_VALUES 7 PYTHON_LIBRARY_SUFFIX) -list(GET _PYTHON_VALUES 8 PYTHON_LIBDIR) -list(GET _PYTHON_VALUES 9 PYTHON_MULTIARCH) +_pybind11_get_if_undef(_PYTHON_VALUES 0 _PYTHON_VERSION_LIST) +_pybind11_get_if_undef(_PYTHON_VALUES 1 PYTHON_PREFIX) +_pybind11_get_if_undef(_PYTHON_VALUES 2 PYTHON_INCLUDE_DIR) +_pybind11_get_if_undef(_PYTHON_VALUES 3 PYTHON_SITE_PACKAGES) +_pybind11_get_if_undef(_PYTHON_VALUES 4 PYTHON_MODULE_EXTENSION) +_pybind11_get_if_undef(_PYTHON_VALUES 5 PYTHON_IS_DEBUG) +_pybind11_get_if_undef(_PYTHON_VALUES 6 PYTHON_SIZEOF_VOID_P) +_pybind11_get_if_undef(_PYTHON_VALUES 7 PYTHON_LIBRARY_SUFFIX) +_pybind11_get_if_undef(_PYTHON_VALUES 8 PYTHON_LIBDIR) +_pybind11_get_if_undef(_PYTHON_VALUES 9 PYTHON_MULTIARCH) # Make sure the Python has the same pointer-size as the chosen compiler # Skip if CMAKE_SIZEOF_VOID_P is not defined -if(CMAKE_SIZEOF_VOID_P AND (NOT "${PYTHON_SIZEOF_VOID_P}" STREQUAL "${CMAKE_SIZEOF_VOID_P}")) +# This should be skipped for (non-Apple) cross-compiles (like EMSCRIPTEN) +if(NOT CMAKE_CROSSCOMPILING + AND CMAKE_SIZEOF_VOID_P + AND (NOT "${PYTHON_SIZEOF_VOID_P}" STREQUAL "${CMAKE_SIZEOF_VOID_P}")) if(PythonLibsNew_FIND_REQUIRED) math(EXPR _PYTHON_BITS "${PYTHON_SIZEOF_VOID_P} * 8") math(EXPR _CMAKE_BITS "${CMAKE_SIZEOF_VOID_P} * 8") From c42414db86d393aaea8b450d987f0f56c2c66bca Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Mon, 23 May 2022 12:26:53 -0400 Subject: [PATCH 03/14] (perf): use a rvalue cast in func_wrapper (#3966) * (perf): use an rvalue cast in func_wrapper * Try to clarify comment * Fix comment typo --- include/pybind11/functional.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/pybind11/functional.h b/include/pybind11/functional.h index 23e092e73..f2a752e9d 100644 --- a/include/pybind11/functional.h +++ b/include/pybind11/functional.h @@ -98,8 +98,8 @@ public: explicit func_wrapper(func_handle &&hf) noexcept : hfunc(std::move(hf)) {} Return operator()(Args... args) const { gil_scoped_acquire acq; - object retval(hfunc.f(std::forward(args)...)); - return retval.template cast(); + // casts the returned object as a rvalue to the return type + return object(hfunc.f(std::forward(args)...)).template cast(); } }; From 4624e8e16433f56c315abc9bfc49c449fdfe137f Mon Sep 17 00:00:00 2001 From: Maarten Baert Date: Tue, 24 May 2022 19:46:31 +0200 Subject: [PATCH 04/14] Don't return pointers to static objects with return_value_policy::take_ownership. (#3946) * Don't return pointers to static objects with return_value_policy::take_ownership. This fixes -Wfree-nonheap-object warnings produced by GCC. * Use return value policy fix instead Co-authored-by: Aaron Gokaslan --- tests/test_builtin_casters.cpp | 3 ++- tests/test_stl.cpp | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/test_builtin_casters.cpp b/tests/test_builtin_casters.cpp index 7b67515e5..6529f47d0 100644 --- a/tests/test_builtin_casters.cpp +++ b/tests/test_builtin_casters.cpp @@ -267,7 +267,8 @@ TEST_SUBMODULE(builtin_casters, m) { m.def("lvalue_nested", []() -> const decltype(lvnested) & { return lvnested; }); static std::pair int_string_pair{2, "items"}; - m.def("int_string_pair", []() { return &int_string_pair; }); + m.def( + "int_string_pair", []() { return &int_string_pair; }, py::return_value_policy::reference); // test_builtins_cast_return_none m.def("return_none_string", []() -> std::string * { return nullptr; }); diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index b56a91953..38d32fda9 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -177,7 +177,8 @@ TEST_SUBMODULE(stl, m) { [](const std::vector &v) { return v.at(0) == true && v.at(1) == false; }); // Unnumbered regression (caused by #936): pointers to stl containers aren't castable static std::vector lvv{2}; - m.def("cast_ptr_vector", []() { return &lvv; }); + m.def( + "cast_ptr_vector", []() { return &lvv; }, py::return_value_policy::reference); // test_deque m.def("cast_deque", []() { return std::deque{1}; }); From 2d4a20c8cb1009bdc47079581759230a202579e8 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Wed, 25 May 2022 12:14:07 -0400 Subject: [PATCH 05/14] chore: add missing moves for buffer_func and staticmethod in pybind11.h (#3969) * Use move converting ctor when making class staticmethod * Add missing caster move in buffer func * fix use after move * add back move to staticmethod * avoid shadowing with varname --- include/pybind11/pybind11.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index ca94aaeb3..4c392713c 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1566,7 +1566,8 @@ public: scope(*this), sibling(getattr(*this, name_, none())), extra...); - attr(cf.name()) = staticmethod(cf); + auto cf_name = cf.name(); + attr(std::move(cf_name)) = staticmethod(std::move(cf)); return *this; } @@ -1620,7 +1621,7 @@ public: if (!caster.load(obj, false)) { return nullptr; } - return new buffer_info(((capture *) ptr)->func(caster)); + return new buffer_info(((capture *) ptr)->func(std::move(caster))); }, ptr); weakref(m_ptr, cpp_function([ptr](handle wr) { From 2c549eb7aa16bf1297af337e1b022e9ed2a4dc52 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Wed, 25 May 2022 21:44:55 -0700 Subject: [PATCH 06/14] Move `PyErr_NormalizeException()` up a few lines (#3971) * Add error_already_set_what what tests, asserting the status quo. * Move PyErr_NormalizeException() up a few lines. * @pytest.mark.skipif("env.PYPY") from PR #1895 is required even for this much simpler PR * Move PyException_SetTraceback() with PyErr_NormalizeException() as suggested by @skylion007 * Insert a std::move() as suggested by @skylion007 --- include/pybind11/detail/type_caster_base.h | 11 +++-- tests/test_exceptions.cpp | 8 ++++ tests/test_exceptions.py | 50 ++++++++++++++++++++++ 3 files changed, 63 insertions(+), 6 deletions(-) diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index a7b977132..14561759b 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -478,6 +478,11 @@ PYBIND11_NOINLINE std::string error_string() { error_scope scope; // Preserve error state + PyErr_NormalizeException(&scope.type, &scope.value, &scope.trace); + if (scope.trace != nullptr) { + PyException_SetTraceback(scope.value, scope.trace); + } + std::string errorString; if (scope.type) { errorString += handle(scope.type).attr("__name__").cast(); @@ -487,12 +492,6 @@ PYBIND11_NOINLINE std::string error_string() { errorString += (std::string) str(scope.value); } - PyErr_NormalizeException(&scope.type, &scope.value, &scope.trace); - - if (scope.trace != nullptr) { - PyException_SetTraceback(scope.value, scope.trace); - } - #if !defined(PYPY_VERSION) if (scope.trace) { auto *trace = (PyTracebackObject *) scope.trace; diff --git a/tests/test_exceptions.cpp b/tests/test_exceptions.cpp index 3e9a3d771..d7b31cf74 100644 --- a/tests/test_exceptions.cpp +++ b/tests/test_exceptions.cpp @@ -299,4 +299,12 @@ TEST_SUBMODULE(exceptions, m) { std::throw_with_nested(std::runtime_error("Outer Exception")); } }); + + m.def("error_already_set_what", [](const py::object &exc_type, const py::object &exc_value) { + PyErr_SetObject(exc_type.ptr(), exc_value.ptr()); + std::string what = py::error_already_set().what(); + bool py_err_set_after_what = (PyErr_Occurred() != nullptr); + PyErr_Clear(); + return py::make_tuple(std::move(what), py_err_set_after_what); + }); } diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index 0be61804a..dc7594113 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -270,3 +270,53 @@ def test_local_translator(msg): m.throws_local_simple_error() assert not isinstance(excinfo.value, cm.LocalSimpleException) assert msg(excinfo.value) == "this mod" + + +class FlakyException(Exception): + def __init__(self, failure_point): + if failure_point == "failure_point_init": + raise ValueError("triggered_failure_point_init") + self.failure_point = failure_point + + def __str__(self): + if self.failure_point == "failure_point_str": + raise ValueError("triggered_failure_point_str") + return "FlakyException.__str__" + + +@pytest.mark.parametrize( + "exc_type, exc_value, expected_what", + ( + (ValueError, "plain_str", "ValueError: plain_str"), + (ValueError, ("tuple_elem",), "ValueError: tuple_elem"), + (FlakyException, ("happy",), "FlakyException: FlakyException.__str__"), + ), +) +def test_error_already_set_what_with_happy_exceptions( + exc_type, exc_value, expected_what +): + what, py_err_set_after_what = m.error_already_set_what(exc_type, exc_value) + assert not py_err_set_after_what + assert what == expected_what + + +@pytest.mark.skipif("env.PYPY", reason="PyErr_NormalizeException Segmentation fault") +def test_flaky_exception_failure_point_init(): + what, py_err_set_after_what = m.error_already_set_what( + FlakyException, ("failure_point_init",) + ) + assert not py_err_set_after_what + lines = what.splitlines() + # PyErr_NormalizeException replaces the original FlakyException with ValueError: + assert lines[:3] == ["ValueError: triggered_failure_point_init", "", "At:"] + # Checking the first two lines of the traceback as formatted in error_string(): + assert "test_exceptions.py(" in lines[3] + assert lines[3].endswith("): __init__") + assert lines[4].endswith("): test_flaky_exception_failure_point_init") + + +def test_flaky_exception_failure_point_str(): + # The error_already_set ctor fails due to a ValueError in error_string(): + with pytest.raises(ValueError) as excinfo: + m.error_already_set_what(FlakyException, ("failure_point_str",)) + assert str(excinfo.value) == "triggered_failure_point_str" From 8d14e666e3aab5c958169e23b619ae8b17b09ea6 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Thu, 26 May 2022 08:07:40 -0700 Subject: [PATCH 07/14] fix: avoid `catch (...)` for expected `import numpy` failures (#3974) * Replace import numpy catch (...) with catch (error_already_set) * Add missing const (not sure how those got lost). --- tests/test_numpy_array.cpp | 2 +- tests/test_numpy_dtypes.cpp | 2 +- tests/test_numpy_vectorize.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_numpy_array.cpp b/tests/test_numpy_array.cpp index 8bc77a33f..69ddbe1ef 100644 --- a/tests/test_numpy_array.cpp +++ b/tests/test_numpy_array.cpp @@ -162,7 +162,7 @@ static int data_i = 42; TEST_SUBMODULE(numpy_array, sm) { try { py::module_::import("numpy"); - } catch (...) { + } catch (const py::error_already_set &) { return; } diff --git a/tests/test_numpy_dtypes.cpp b/tests/test_numpy_dtypes.cpp index c25bc042b..6654f9ed8 100644 --- a/tests/test_numpy_dtypes.cpp +++ b/tests/test_numpy_dtypes.cpp @@ -301,7 +301,7 @@ struct B {}; TEST_SUBMODULE(numpy_dtypes, m) { try { py::module_::import("numpy"); - } catch (...) { + } catch (const py::error_already_set &) { return; } diff --git a/tests/test_numpy_vectorize.cpp b/tests/test_numpy_vectorize.cpp index eab08ec0a..dcc4c6ac2 100644 --- a/tests/test_numpy_vectorize.cpp +++ b/tests/test_numpy_vectorize.cpp @@ -22,7 +22,7 @@ double my_func(int x, float y, double z) { TEST_SUBMODULE(numpy_vectorize, m) { try { py::module_::import("numpy"); - } catch (...) { + } catch (const py::error_already_set &) { return; } From 68f80105007a574f114cfe899243bb3d96272a2e Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Fri, 27 May 2022 14:32:57 -0400 Subject: [PATCH 08/14] chore: add err guard to capsule destructor and add a move to iostream (#3958) * Add err guard to capsule destructor * only uses ostream currently * can these be noexcept * Add back header * fix for older compilers * This should at least be noexcept * Add missing move * Apparently not noexcept for old llvm --- include/pybind11/iostream.h | 2 +- include/pybind11/pytypes.h | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/include/pybind11/iostream.h b/include/pybind11/iostream.h index 12e1f19f0..1878089e3 100644 --- a/include/pybind11/iostream.h +++ b/include/pybind11/iostream.h @@ -100,7 +100,7 @@ private: if (size > remainder) { str line(pbase(), size - remainder); - pywrite(line); + pywrite(std::move(line)); pyflush(); } diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 256a2441b..5b1c60e33 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1581,6 +1581,8 @@ public: capsule(const void *value, void (*destructor)(void *)) { m_ptr = PyCapsule_New(const_cast(value), nullptr, [](PyObject *o) { + // guard if destructor called while err indicator is set + error_scope error_guard; auto destructor = reinterpret_cast(PyCapsule_GetContext(o)); if (destructor == nullptr) { if (PyErr_Occurred()) { From 748ae2270b333d7024d9ef7ffb7f2ca947b2491f Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sat, 28 May 2022 16:40:57 -0700 Subject: [PATCH 09/14] Add missing error handling to `module_::def_submodule` (#3973) * Add missing error handling to module_::def_submodule * Add test_def_submodule_failures * PyPy only: Skip test with trigger for PyModule_GetName() failure. * Reapply minor fix that accidentally got lost in transfer from PR #3964 --- include/pybind11/pybind11.h | 13 ++++++++++--- tests/test_modules.cpp | 2 ++ tests/test_modules.py | 30 ++++++++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 4c392713c..ea97ed072 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1173,9 +1173,16 @@ public: py::module_ m3 = m2.def_submodule("subsub", "A submodule of 'example.sub'"); \endrst */ module_ def_submodule(const char *name, const char *doc = nullptr) { - std::string full_name - = std::string(PyModule_GetName(m_ptr)) + std::string(".") + std::string(name); - auto result = reinterpret_borrow(PyImport_AddModule(full_name.c_str())); + const char *this_name = PyModule_GetName(m_ptr); + if (this_name == nullptr) { + throw error_already_set(); + } + std::string full_name = std::string(this_name) + '.' + name; + handle submodule = PyImport_AddModule(full_name.c_str()); + if (!submodule) { + throw error_already_set(); + } + auto result = reinterpret_borrow(submodule); if (doc && options::show_user_defined_docstrings()) { result.attr("__doc__") = pybind11::str(doc); } diff --git a/tests/test_modules.cpp b/tests/test_modules.cpp index 8aa2a4bdb..18a7ec74c 100644 --- a/tests/test_modules.cpp +++ b/tests/test_modules.cpp @@ -120,4 +120,6 @@ TEST_SUBMODULE(modules, m) { return failures; }); + + m.def("def_submodule", [](py::module_ m, const char *name) { return m.def_submodule(name); }); } diff --git a/tests/test_modules.py b/tests/test_modules.py index 15db8355e..e11d68e78 100644 --- a/tests/test_modules.py +++ b/tests/test_modules.py @@ -1,3 +1,6 @@ +import pytest + +import env from pybind11_tests import ConstructorStats from pybind11_tests import modules as m from pybind11_tests.modules import subsubmodule as ms @@ -89,3 +92,30 @@ def test_builtin_key_type(): keys = __builtins__.__dict__.keys() assert {type(k) for k in keys} == {str} + + +@pytest.mark.xfail("env.PYPY", reason="PyModule_GetName()") +def test_def_submodule_failures(): + sm = m.def_submodule(m, b"ScratchSubModuleName") # Using bytes to show it works. + assert sm.__name__ == m.__name__ + "." + "ScratchSubModuleName" + malformed_utf8 = b"\x80" + if env.PYPY: + # It is not worth the effort finding a trigger for a failure when running with PyPy. + pytest.skip("Sufficiently exercised on platforms other than PyPy.") + else: + # Meant to trigger PyModule_GetName() failure: + sm_name_orig = sm.__name__ + sm.__name__ = malformed_utf8 + try: + with pytest.raises(Exception): + # Seen with Python 3.9: SystemError: nameless module + # But we do not want to exercise the internals of PyModule_GetName(), which could + # change in future versions of Python, but a bad __name__ is very likely to cause + # some kind of failure indefinitely. + m.def_submodule(sm, b"SubSubModuleName") + finally: + # Clean up to ensure nothing gets upset by a module with an invalid __name__. + sm.__name__ = sm_name_orig # Purely precautionary. + # Meant to trigger PyImport_AddModule() failure: + with pytest.raises(UnicodeDecodeError): + m.def_submodule(sm, malformed_utf8) From 8da58da53996e9aa40d051b3b84cb3220fdbbb58 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Sat, 28 May 2022 19:58:15 -0400 Subject: [PATCH 10/14] chore: perfectly forward all make_iterator args (#3980) * Perfectly forward all make_iterator args * Try emplace back --- include/pybind11/pybind11.h | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index ea97ed072..be206e1a6 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -2333,7 +2333,7 @@ template -iterator make_iterator_impl(Iterator first, Sentinel last, Extra &&...extra) { +iterator make_iterator_impl(Iterator &&first, Sentinel &&last, Extra &&...extra) { using state = detail::iterator_state; // TODO: state captures only the types of Extra, not the values @@ -2359,7 +2359,7 @@ iterator make_iterator_impl(Iterator first, Sentinel last, Extra &&...extra) { Policy); } - return cast(state{first, last, true}); + return cast(state{std::forward(first), std::forward(last), true}); } PYBIND11_NAMESPACE_END(detail) @@ -2370,13 +2370,15 @@ template ::result_type, typename... Extra> -iterator make_iterator(Iterator first, Sentinel last, Extra &&...extra) { +iterator make_iterator(Iterator &&first, Sentinel &&last, Extra &&...extra) { return detail::make_iterator_impl, Policy, Iterator, Sentinel, ValueType, - Extra...>(first, last, std::forward(extra)...); + Extra...>(std::forward(first), + std::forward(last), + std::forward(extra)...); } /// Makes a python iterator over the keys (`.first`) of a iterator over pairs from a @@ -2386,13 +2388,15 @@ template ::result_type, typename... Extra> -iterator make_key_iterator(Iterator first, Sentinel last, Extra &&...extra) { +iterator make_key_iterator(Iterator &&first, Sentinel &&last, Extra &&...extra) { return detail::make_iterator_impl, Policy, Iterator, Sentinel, KeyType, - Extra...>(first, last, std::forward(extra)...); + Extra...>(std::forward(first), + std::forward(last), + std::forward(extra)...); } /// Makes a python iterator over the values (`.second`) of a iterator over pairs from a @@ -2402,13 +2406,15 @@ template ::result_type, typename... Extra> -iterator make_value_iterator(Iterator first, Sentinel last, Extra &&...extra) { +iterator make_value_iterator(Iterator &&first, Sentinel &&last, Extra &&...extra) { return detail::make_iterator_impl, Policy, Iterator, Sentinel, ValueType, - Extra...>(first, last, std::forward(extra)...); + Extra...>(std::forward(first), + std::forward(last), + std::forward(extra)...); } /// Makes an iterator over values of an stl container or other container supporting @@ -2467,7 +2473,7 @@ void implicitly_convertible() { }; if (auto *tinfo = detail::get_type_info(typeid(OutputType))) { - tinfo->implicit_conversions.push_back(implicit_caster); + tinfo->implicit_conversions.emplace_back(std::move(implicit_caster)); } else { pybind11_fail("implicitly_convertible: Unable to find type " + type_id()); } From de4ba92c9fe1d5d225f39cbc251e7f2616f3b58d Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Tue, 31 May 2022 11:51:13 -0700 Subject: [PATCH 11/14] Add `error_scope` to `detail::get_internals()` (#3981) * Add `error_scope` to `detail::get_internals()` * Adjust test to tolerate macOS PyPy behavior. --- include/pybind11/detail/internals.h | 1 + tests/CMakeLists.txt | 1 + ...s_module_interleaved_error_already_set.cpp | 51 +++++++++++++++++++ tests/test_exceptions.cpp | 7 +++ tests/test_exceptions.py | 9 ++++ 5 files changed, 69 insertions(+) create mode 100644 tests/cross_module_interleaved_error_already_set.cpp diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index 3f83113bd..6ca5e1482 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -415,6 +415,7 @@ PYBIND11_NOINLINE internals &get_internals() { ~gil_scoped_acquire_local() { PyGILState_Release(state); } const PyGILState_STATE state; } gil; + error_scope err_scope; PYBIND11_STR_TYPE id(PYBIND11_INTERNALS_ID); auto builtins = handle(PyEval_GetBuiltins()); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index a14b520a5..7296cd1b8 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -215,6 +215,7 @@ tests_extra_targets("test_exceptions.py;test_local_bindings.py;test_stl.py;test_ "pybind11_cross_module_tests") # And add additional targets for other tests. +tests_extra_targets("test_exceptions.py" "cross_module_interleaved_error_already_set") tests_extra_targets("test_gil_scoped.py" "cross_module_gil_utils") set(PYBIND11_EIGEN_REPO diff --git a/tests/cross_module_interleaved_error_already_set.cpp b/tests/cross_module_interleaved_error_already_set.cpp new file mode 100644 index 000000000..fdd9939e4 --- /dev/null +++ b/tests/cross_module_interleaved_error_already_set.cpp @@ -0,0 +1,51 @@ +/* + Copyright (c) 2022 Google LLC + + All rights reserved. Use of this source code is governed by a + BSD-style license that can be found in the LICENSE file. +*/ + +#include + +// This file mimics a DSO that makes pybind11 calls but does not define a PYBIND11_MODULE, +// so that the first call of cross_module_error_already_set() triggers the first call of +// pybind11::detail::get_internals(). + +namespace { + +namespace py = pybind11; + +void interleaved_error_already_set() { + PyErr_SetString(PyExc_RuntimeError, "1st error."); + try { + throw py::error_already_set(); + } catch (const py::error_already_set &) { + // The 2nd error could be conditional in a real application. + PyErr_SetString(PyExc_RuntimeError, "2nd error."); + } // Here the 1st error is destroyed before the 2nd error is fetched. + // The error_already_set dtor triggers a pybind11::detail::get_internals() + // call via pybind11::gil_scoped_acquire. + if (PyErr_Occurred()) { + throw py::error_already_set(); + } +} + +constexpr char kModuleName[] = "cross_module_interleaved_error_already_set"; + +struct PyModuleDef moduledef = { + PyModuleDef_HEAD_INIT, kModuleName, nullptr, 0, nullptr, nullptr, nullptr, nullptr, nullptr}; + +} // namespace + +extern "C" PYBIND11_EXPORT PyObject *PyInit_cross_module_interleaved_error_already_set() { + PyObject *m = PyModule_Create(&moduledef); + if (m != nullptr) { + static_assert(sizeof(&interleaved_error_already_set) == sizeof(void *), + "Function pointer must have the same size as void *"); + PyModule_AddObject( + m, + "funcaddr", + PyLong_FromVoidPtr(reinterpret_cast(&interleaved_error_already_set))); + } + return m; +} diff --git a/tests/test_exceptions.cpp b/tests/test_exceptions.cpp index d7b31cf74..8c74d7029 100644 --- a/tests/test_exceptions.cpp +++ b/tests/test_exceptions.cpp @@ -307,4 +307,11 @@ TEST_SUBMODULE(exceptions, m) { PyErr_Clear(); return py::make_tuple(std::move(what), py_err_set_after_what); }); + + m.def("test_cross_module_interleaved_error_already_set", []() { + auto cm = py::module_::import("cross_module_interleaved_error_already_set"); + auto interleaved_error_already_set + = reinterpret_cast(PyLong_AsVoidPtr(cm.attr("funcaddr").ptr())); + interleaved_error_already_set(); + }); } diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index dc7594113..4320ad181 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -320,3 +320,12 @@ def test_flaky_exception_failure_point_str(): with pytest.raises(ValueError) as excinfo: m.error_already_set_what(FlakyException, ("failure_point_str",)) assert str(excinfo.value) == "triggered_failure_point_str" + + +def test_cross_module_interleaved_error_already_set(): + with pytest.raises(RuntimeError) as excinfo: + m.test_cross_module_interleaved_error_already_set() + assert str(excinfo.value) in ( + "2nd error.", # Almost all platforms. + "RuntimeError: 2nd error.", # Some PyPy builds (seen under macOS). + ) From b24c5ed204f4e2b9599106c879106594ea3ef8c3 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Tue, 31 May 2022 11:54:33 -0700 Subject: [PATCH 12/14] Replace "Unknown internal error occurred" with a more helpful message. (#3982) --- include/pybind11/detail/type_caster_base.h | 14 ++++++++------ include/pybind11/pytypes.h | 20 ++++++++++++++------ tests/test_exceptions.cpp | 5 ++++- tests/test_exceptions.py | 5 ++++- 4 files changed, 30 insertions(+), 14 deletions(-) diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index 14561759b..dd8d03751 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -470,14 +470,16 @@ PYBIND11_NOINLINE bool isinstance_generic(handle obj, const std::type_info &tp) return isinstance(obj, type); } -PYBIND11_NOINLINE std::string error_string() { - if (!PyErr_Occurred()) { - PyErr_SetString(PyExc_RuntimeError, "Unknown internal error occurred"); - return "Unknown internal error occurred"; +PYBIND11_NOINLINE std::string error_string(const char *called) { + error_scope scope; // Fetch error state (will be restored when this function returns). + if (scope.type == nullptr) { + if (called == nullptr) { + called = "pybind11::detail::error_string()"; + } + pybind11_fail("Internal error: " + std::string(called) + + " called while Python error indicator not set."); } - error_scope scope; // Preserve error state - PyErr_NormalizeException(&scope.type, &scope.value, &scope.trace); if (scope.trace != nullptr) { PyException_SetTraceback(scope.value, scope.trace); diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 5b1c60e33..c71f1b2e9 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -360,7 +360,7 @@ T reinterpret_steal(handle h) { } PYBIND11_NAMESPACE_BEGIN(detail) -std::string error_string(); +std::string error_string(const char *called = nullptr); PYBIND11_NAMESPACE_END(detail) #if defined(_MSC_VER) @@ -375,20 +375,27 @@ PYBIND11_NAMESPACE_END(detail) /// python). class PYBIND11_EXPORT_EXCEPTION error_already_set : public std::runtime_error { public: - /// Constructs a new exception from the current Python error indicator, if any. The current + /// Constructs a new exception from the current Python error indicator. The current /// Python error indicator will be cleared. - error_already_set() : std::runtime_error(detail::error_string()) { + error_already_set() : std::runtime_error(detail::error_string("pybind11::error_already_set")) { PyErr_Fetch(&m_type.ptr(), &m_value.ptr(), &m_trace.ptr()); } + /// WARNING: The GIL must be held when this copy constructor is invoked! error_already_set(const error_already_set &) = default; error_already_set(error_already_set &&) = default; + /// WARNING: This destructor needs to acquire the Python GIL. This can lead to + /// crashes (undefined behavior) if the Python interpreter is finalizing. inline ~error_already_set() override; - /// Give the currently-held error back to Python, if any. If there is currently a Python error - /// already set it is cleared first. After this call, the current object no longer stores the - /// error variables (but the `.what()` string is still available). + /// Restores the currently-held Python error (which will clear the Python error indicator first + /// if already set). After this call, the current object no longer stores the error variables. + /// NOTE: Any copies of this object may still store the error variables. Currently there is no + // protection against calling restore() from multiple copies. + /// NOTE: This member function will always restore the normalized exception, which may or may + /// not be the original Python exception. + /// WARNING: The GIL must be held when this member function is called! void restore() { PyErr_Restore(m_type.release().ptr(), m_value.release().ptr(), m_trace.release().ptr()); } @@ -405,6 +412,7 @@ public: } /// An alternate version of `discard_as_unraisable()`, where a string provides information on /// the location of the error. For example, `__func__` could be helpful. + /// WARNING: The GIL must be held when this member function is called! void discard_as_unraisable(const char *err_context) { discard_as_unraisable(reinterpret_steal(PYBIND11_FROM_STRING(err_context))); } diff --git a/tests/test_exceptions.cpp b/tests/test_exceptions.cpp index 8c74d7029..9469b8672 100644 --- a/tests/test_exceptions.cpp +++ b/tests/test_exceptions.cpp @@ -228,7 +228,10 @@ TEST_SUBMODULE(exceptions, m) { throw py::error_already_set(); } catch (const std::runtime_error &e) { if ((err && e.what() != std::string("ValueError: foo")) - || (!err && e.what() != std::string("Unknown internal error occurred"))) { + || (!err + && e.what() + != std::string("Internal error: pybind11::error_already_set called " + "while Python error indicator not set."))) { PyErr_Clear(); throw std::runtime_error("error message mismatch"); } diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index 4320ad181..4dcbb83a3 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -16,7 +16,10 @@ def test_std_exception(msg): def test_error_already_set(msg): with pytest.raises(RuntimeError) as excinfo: m.throw_already_set(False) - assert msg(excinfo.value) == "Unknown internal error occurred" + assert ( + msg(excinfo.value) + == "Internal error: pybind11::error_already_set called while Python error indicator not set." + ) with pytest.raises(ValueError) as excinfo: m.throw_already_set(True) From 9f7b3f735aae417838001b338c9970710e9cee82 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Tue, 31 May 2022 12:59:19 -0700 Subject: [PATCH 13/14] addl unit tests for PR #3970 (#3977) * Add test_perf_accessors (to be merged into test_pytypes). * Python < 3.8 f-string compatibility * Use thread_local in inc_ref_counter() * Intentional breakage, brute-force way to quickly find out how many platforms reach the PYBIND11_HANDLE_REF_DEBUG code, with and without threads. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Remove Intentional breakage * Drop perf test, move inc_refs tests to test_pytypes * Fold in PR #3970 with `#ifdef`s * Complete test coverage for all newly added code. * Condense new unit tests via a simple local helper macro. * Remove PYBIND11_PR3970 define. See https://github.com/pybind/pybind11/pull/3977#issuecomment-1142526417 * Move static keyword first (fixes silly oversight). Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- include/pybind11/pytypes.h | 19 +++++++++++++++ tests/test_pytypes.cpp | 49 ++++++++++++++++++++++++++++++++++++++ tests/test_pytypes.py | 9 +++++++ 3 files changed, 77 insertions(+) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index c71f1b2e9..0c5690064 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -180,6 +180,10 @@ private: PYBIND11_NAMESPACE_END(detail) +#if !defined(PYBIND11_HANDLE_REF_DEBUG) && !defined(NDEBUG) +# define PYBIND11_HANDLE_REF_DEBUG +#endif + /** \rst Holds a reference to a Python object (no reference counting) @@ -209,6 +213,9 @@ public: this function automatically. Returns a reference to itself. \endrst */ const handle &inc_ref() const & { +#ifdef PYBIND11_HANDLE_REF_DEBUG + inc_ref_counter(1); +#endif Py_XINCREF(m_ptr); return *this; } @@ -244,6 +251,18 @@ public: protected: PyObject *m_ptr = nullptr; + +#ifdef PYBIND11_HANDLE_REF_DEBUG +private: + static std::size_t inc_ref_counter(std::size_t add) { + thread_local std::size_t counter = 0; + counter += add; + return counter; + } + +public: + static std::size_t inc_ref_counter() { return inc_ref_counter(0); } +#endif }; /** \rst diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index 8d296f655..ef214acc3 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -297,6 +297,55 @@ TEST_SUBMODULE(pytypes, m) { return d; }); + m.def("accessor_moves", []() { // See PR #3970 + py::list return_list; +#ifdef PYBIND11_HANDLE_REF_DEBUG + py::int_ py_int_0(0); + py::int_ py_int_42(42); + py::str py_str_count("count"); + + auto tup = py::make_tuple(0); + + py::sequence seq(tup); + + py::list lst; + lst.append(0); + +# define PYBIND11_LOCAL_DEF(...) \ + { \ + std::size_t inc_refs = py::handle::inc_ref_counter(); \ + __VA_ARGS__; \ + inc_refs = py::handle::inc_ref_counter() - inc_refs; \ + return_list.append(inc_refs); \ + } + + PYBIND11_LOCAL_DEF(tup[py_int_0]) // l-value (to have a control) + PYBIND11_LOCAL_DEF(tup[py::int_(0)]) // r-value + + PYBIND11_LOCAL_DEF(tup.attr(py_str_count)) // l-value + PYBIND11_LOCAL_DEF(tup.attr(py::str("count"))) // r-value + + PYBIND11_LOCAL_DEF(seq[py_int_0]) // l-value + PYBIND11_LOCAL_DEF(seq[py::int_(0)]) // r-value + + PYBIND11_LOCAL_DEF(seq.attr(py_str_count)) // l-value + PYBIND11_LOCAL_DEF(seq.attr(py::str("count"))) // r-value + + PYBIND11_LOCAL_DEF(lst[py_int_0]) // l-value + PYBIND11_LOCAL_DEF(lst[py::int_(0)]) // r-value + + PYBIND11_LOCAL_DEF(lst.attr(py_str_count)) // l-value + PYBIND11_LOCAL_DEF(lst.attr(py::str("count"))) // r-value + + auto lst_acc = lst[py::int_(0)]; + lst_acc = py::int_(42); // Detaches lst_acc from lst. + PYBIND11_LOCAL_DEF(lst_acc = py_int_42) // l-value + PYBIND11_LOCAL_DEF(lst_acc = py::int_(42)) // r-value +# undef PYBIND11_LOCAL_DEF +#endif + return return_list; + }); + // test_constructors m.def("default_constructors", []() { return py::dict("bytes"_a = py::bytes(), diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index 9afe62f42..b91a7e156 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -317,6 +317,15 @@ def test_accessors(): assert d["var"] == 99 +def test_accessor_moves(): + inc_refs = m.accessor_moves() + if inc_refs: + # To be changed in PR #3970: [1, 0, 1, 0, ...] + assert inc_refs == [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1] + else: + pytest.skip("Not defined: PYBIND11_HANDLE_REF_DEBUG") + + def test_constructors(): """C++ default and converting constructors are equivalent to type calls in Python""" types = [bytes, bytearray, str, bool, int, float, tuple, list, dict, set] From 58802de41bc9c78425b66c3b6f22392241aac8de Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Wed, 1 Jun 2022 15:19:13 -0400 Subject: [PATCH 14/14] perf: Add object rvalue overload for accessors. Enables reference stealing (#3970) * Add object rvalue overload for accessors. Enables reference stealing * Fix comments * Fix more comment typos * Fix bug * reorder declarations for clarity * fix another perf bug * should be static * future proof operator overloads * Fix perfect forwarding * Add a couple of tests * Remove errant include * Improve test documentation * Add dict test * add object attr tests * Optimize STL map caster and cleanup enum * Reorder to match declarations * adjust increfs * Remove comment * revert value change * add missing move --- include/pybind11/pybind11.h | 6 +++--- include/pybind11/pytypes.h | 36 ++++++++++++++++++++++++++++++------ include/pybind11/stl.h | 2 +- tests/test_pytypes.cpp | 34 ++++++++++++++++++++++++++++++++++ tests/test_pytypes.py | 31 +++++++++++++++++++++++++++++-- 5 files changed, 97 insertions(+), 12 deletions(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index be206e1a6..cfa442067 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -2069,12 +2069,12 @@ struct enum_base { str name(name_); if (entries.contains(name)) { std::string type_name = (std::string) str(m_base.attr("__name__")); - throw value_error(type_name + ": element \"" + std::string(name_) + throw value_error(std::move(type_name) + ": element \"" + std::string(name_) + "\" already exists!"); } entries[name] = std::make_pair(value, doc); - m_base.attr(name) = value; + m_base.attr(std::move(name)) = std::move(value); } PYBIND11_NOINLINE void export_values() { @@ -2610,7 +2610,7 @@ PYBIND11_NOINLINE void print(const tuple &args, const dict &kwargs) { } auto write = file.attr("write"); - write(line); + write(std::move(line)); write(kwargs.contains("end") ? kwargs["end"] : str("\n")); if (kwargs.contains("flush") && kwargs["flush"].cast()) { diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 0c5690064..27807953b 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -85,7 +85,9 @@ public: or `object` subclass causes a call to ``__setitem__``. \endrst */ item_accessor operator[](handle key) const; - /// See above (the only difference is that they key is provided as a string literal) + /// See above (the only difference is that the key's reference is stolen) + item_accessor operator[](object &&key) const; + /// See above (the only difference is that the key is provided as a string literal) item_accessor operator[](const char *key) const; /** \rst @@ -95,7 +97,9 @@ public: or `object` subclass causes a call to ``setattr``. \endrst */ obj_attr_accessor attr(handle key) const; - /// See above (the only difference is that they key is provided as a string literal) + /// See above (the only difference is that the key's reference is stolen) + obj_attr_accessor attr(object &&key) const; + /// See above (the only difference is that the key is provided as a string literal) str_attr_accessor attr(const char *key) const; /** \rst @@ -684,7 +688,7 @@ public: } template void operator=(T &&value) & { - get_cache() = reinterpret_borrow(object_or_cast(std::forward(value))); + get_cache() = ensure_object(object_or_cast(std::forward(value))); } template @@ -712,6 +716,9 @@ public: } private: + static object ensure_object(object &&o) { return std::move(o); } + static object ensure_object(handle h) { return reinterpret_borrow(h); } + object &get_cache() const { if (!cache) { cache = Policy::get(obj, key); @@ -1711,7 +1718,10 @@ public: size_t size() const { return (size_t) PyTuple_Size(m_ptr); } bool empty() const { return size() == 0; } detail::tuple_accessor operator[](size_t index) const { return {*this, index}; } - detail::item_accessor operator[](handle h) const { return object::operator[](h); } + template ::value, int> = 0> + detail::item_accessor operator[](T &&o) const { + return object::operator[](std::forward(o)); + } detail::tuple_iterator begin() const { return {*this, 0}; } detail::tuple_iterator end() const { return {*this, PyTuple_GET_SIZE(m_ptr)}; } }; @@ -1771,7 +1781,10 @@ public: } bool empty() const { return size() == 0; } detail::sequence_accessor operator[](size_t index) const { return {*this, index}; } - detail::item_accessor operator[](handle h) const { return object::operator[](h); } + template ::value, int> = 0> + detail::item_accessor operator[](T &&o) const { + return object::operator[](std::forward(o)); + } detail::sequence_iterator begin() const { return {*this, 0}; } detail::sequence_iterator end() const { return {*this, PySequence_Size(m_ptr)}; } }; @@ -1790,7 +1803,10 @@ public: size_t size() const { return (size_t) PyList_Size(m_ptr); } bool empty() const { return size() == 0; } detail::list_accessor operator[](size_t index) const { return {*this, index}; } - detail::item_accessor operator[](handle h) const { return object::operator[](h); } + template ::value, int> = 0> + detail::item_accessor operator[](T &&o) const { + return object::operator[](std::forward(o)); + } detail::list_iterator begin() const { return {*this, 0}; } detail::list_iterator end() const { return {*this, PyList_GET_SIZE(m_ptr)}; } template @@ -2090,6 +2106,10 @@ item_accessor object_api::operator[](handle key) const { return {derived(), reinterpret_borrow(key)}; } template +item_accessor object_api::operator[](object &&key) const { + return {derived(), std::move(key)}; +} +template item_accessor object_api::operator[](const char *key) const { return {derived(), pybind11::str(key)}; } @@ -2098,6 +2118,10 @@ obj_attr_accessor object_api::attr(handle key) const { return {derived(), reinterpret_borrow(key)}; } template +obj_attr_accessor object_api::attr(object &&key) const { + return {derived(), std::move(key)}; +} +template str_attr_accessor object_api::attr(const char *key) const { return {derived(), key}; } diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index 597bce61d..ab30ecac0 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -128,7 +128,7 @@ struct map_caster { if (!key || !value) { return handle(); } - d[key] = value; + d[std::move(key)] = std::move(value); } return d.release(); } diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index ef214acc3..cb81007c3 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -661,4 +661,38 @@ TEST_SUBMODULE(pytypes, m) { double v = x.get_value(); return v * v; }); + + m.def("tuple_rvalue_getter", [](const py::tuple &tup) { + // tests accessing tuple object with rvalue int + for (size_t i = 0; i < tup.size(); i++) { + auto o = py::handle(tup[py::int_(i)]); + if (!o) { + throw py::value_error("tuple is malformed"); + } + } + return tup; + }); + m.def("list_rvalue_getter", [](const py::list &l) { + // tests accessing list with rvalue int + for (size_t i = 0; i < l.size(); i++) { + auto o = py::handle(l[py::int_(i)]); + if (!o) { + throw py::value_error("list is malformed"); + } + } + return l; + }); + m.def("populate_dict_rvalue", [](int population) { + auto d = py::dict(); + for (int i = 0; i < population; i++) { + d[py::int_(i)] = py::int_(i); + } + return d; + }); + m.def("populate_obj_str_attrs", [](py::object &o, int population) { + for (int i = 0; i < population; i++) { + o.attr(py::str(py::int_(i))) = py::str(py::int_(i)); + } + return o; + }); } diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index b91a7e156..3e9d51a27 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -1,5 +1,6 @@ import contextlib import sys +import types import pytest @@ -320,8 +321,7 @@ def test_accessors(): def test_accessor_moves(): inc_refs = m.accessor_moves() if inc_refs: - # To be changed in PR #3970: [1, 0, 1, 0, ...] - assert inc_refs == [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1] + assert inc_refs == [1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0, 1, 0] else: pytest.skip("Not defined: PYBIND11_HANDLE_REF_DEBUG") @@ -707,3 +707,30 @@ def test_implementation_details(): def test_external_float_(): r1 = m.square_float_(2.0) assert r1 == 4.0 + + +def test_tuple_rvalue_getter(): + pop = 1000 + tup = tuple(range(pop)) + m.tuple_rvalue_getter(tup) + + +def test_list_rvalue_getter(): + pop = 1000 + my_list = list(range(pop)) + m.list_rvalue_getter(my_list) + + +def test_populate_dict_rvalue(): + pop = 1000 + my_dict = {i: i for i in range(pop)} + assert m.populate_dict_rvalue(pop) == my_dict + + +def test_populate_obj_str_attrs(): + pop = 1000 + o = types.SimpleNamespace(**{str(i): i for i in range(pop)}) + new_o = m.populate_obj_str_attrs(o, pop) + new_attrs = {k: v for k, v in new_o.__dict__.items() if not k.startswith("_")} + assert all(isinstance(v, str) for v in new_attrs.values()) + assert len(new_attrs) == pop