From 5efd96e7c856c7160c6fd0e0e63c52661614f042 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Thu, 27 Jul 2023 21:48:14 -0700 Subject: [PATCH 01/12] Copy clang 17 compatibility fixes from PR #4762 to a separate PR. --- include/pybind11/cast.h | 10 +++++++++- include/pybind11/pytypes.h | 10 +++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index db39341180..b3c8ebe17e 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1377,7 +1377,15 @@ inline namespace literals { /** \rst String literal version of `arg` \endrst */ -constexpr arg operator"" _a(const char *name, size_t) { return arg(name); } +constexpr arg +#if !defined(__clang__) && defined(__GNUC__) && __GNUC__ < 5 +operator"" _a // gcc 4.8.5 insists on having a space (hard error). +#else +operator""_a // clang 17 generates a deprecation warning if there is a space. +#endif + (const char *name, size_t) { + return arg(name); +} } // namespace literals PYBIND11_NAMESPACE_BEGIN(detail) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 64aad63476..580a4658ef 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1612,7 +1612,15 @@ inline namespace literals { /** \rst String literal version of `str` \endrst */ -inline str operator"" _s(const char *s, size_t size) { return {s, size}; } +inline str +#if !defined(__clang__) && defined(__GNUC__) && __GNUC__ < 5 +operator"" _s // gcc 4.8.5 insists on having a space (hard error). +#else +operator""_s // clang 17 generates a deprecation warning if there is a space. +#endif + (const char *s, size_t size) { + return {s, size}; +} } // namespace literals /// \addtogroup pytypes From 5a31824742d9454c6a24e797a3daca95a00ae024 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Wed, 2 Aug 2023 12:46:32 -0700 Subject: [PATCH 02/12] static py::exception<> -> static py::handle --- tests/test_exceptions.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/test_exceptions.cpp b/tests/test_exceptions.cpp index 854c7e6f76..46e0323889 100644 --- a/tests/test_exceptions.cpp +++ b/tests/test_exceptions.cpp @@ -110,7 +110,10 @@ TEST_SUBMODULE(exceptions, m) { []() { throw std::runtime_error("This exception was intentionally thrown."); }); // make a new custom exception and use it as a translation target - static py::exception ex(m, "MyException"); + // This is a static object, so we must leak the Python reference: + // it is undefined when the destructor will run, possibly only after + // the Python interpreter is finalized already. + static py::handle ex = py::exception(m, "MyException").release(); py::register_exception_translator([](std::exception_ptr p) { try { if (p) { @@ -118,7 +121,7 @@ TEST_SUBMODULE(exceptions, m) { } } catch (const MyException &e) { // Set MyException as the active python error - ex(e.what()); + PyErr_SetString(ex.ptr(), e.what()); } }); From 9a4eba52293f4ca60d0cbd13253f506ffe8d5413 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Wed, 2 Aug 2023 22:56:22 -0700 Subject: [PATCH 03/12] Add `py::set_error()` but also try the suggestion of @malfet (https://github.com/pytorch/pytorch/pull/106401#pullrequestreview-1559961407). --- docs/advanced/exceptions.rst | 9 ++++++--- include/pybind11/pybind11.h | 6 +++++- tests/test_exceptions.cpp | 25 +++++++++++-------------- 3 files changed, 22 insertions(+), 18 deletions(-) diff --git a/docs/advanced/exceptions.rst b/docs/advanced/exceptions.rst index 53981dc08f..6b34e1fdd7 100644 --- a/docs/advanced/exceptions.rst +++ b/docs/advanced/exceptions.rst @@ -142,14 +142,17 @@ standard python RuntimeError: .. code-block:: cpp - static py::exception exc(m, "MyCustomError"); + // This is a static object, so we must leak the Python reference: + // It is undefined when the destructor will run, possibly only after the + // Python interpreter is finalized already. + static const auto *const exc = new py::exception(m, "MyCustomError"); py::register_exception_translator([](std::exception_ptr p) { try { if (p) std::rethrow_exception(p); } catch (const MyCustomException &e) { - exc(e.what()); + (*exc)(e.what()); } catch (const OtherException &e) { - PyErr_SetString(PyExc_RuntimeError, e.what()); + py::set_error(PyExc_RuntimeError, e.what()); } }); diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 3bce1a01ba..0b2fb420d7 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -2525,6 +2525,10 @@ inline void register_local_exception_translator(ExceptionTranslator &&translator std::forward(translator)); } +inline void set_error(const handle exc, const char *message) { + PyErr_SetString(exc.ptr(), message); +} + /** * Wrapper to generate a new Python exception type. * @@ -2549,7 +2553,7 @@ class exception : public object { } // Sets the current python exception to this exception object with the given message - void operator()(const char *message) { PyErr_SetString(m_ptr, message); } + void operator()(const char *message) const { set_error(*this, message); } }; PYBIND11_NAMESPACE_BEGIN(detail) diff --git a/tests/test_exceptions.cpp b/tests/test_exceptions.cpp index 46e0323889..94981a2169 100644 --- a/tests/test_exceptions.cpp +++ b/tests/test_exceptions.cpp @@ -109,11 +109,8 @@ TEST_SUBMODULE(exceptions, m) { m.def("throw_std_exception", []() { throw std::runtime_error("This exception was intentionally thrown."); }); - // make a new custom exception and use it as a translation target - // This is a static object, so we must leak the Python reference: - // it is undefined when the destructor will run, possibly only after - // the Python interpreter is finalized already. - static py::handle ex = py::exception(m, "MyException").release(); + // Please keep in sync with docs/advanced/exceptions.rst + static const auto *const ex = new py::exception(m, "MyException"); py::register_exception_translator([](std::exception_ptr p) { try { if (p) { @@ -121,7 +118,7 @@ TEST_SUBMODULE(exceptions, m) { } } catch (const MyException &e) { // Set MyException as the active python error - PyErr_SetString(ex.ptr(), e.what()); + (*ex)(e.what()); } }); @@ -135,7 +132,7 @@ TEST_SUBMODULE(exceptions, m) { } } catch (const MyException2 &e) { // Translate this exception to a standard RuntimeError - PyErr_SetString(PyExc_RuntimeError, e.what()); + py::set_error(PyExc_RuntimeError, e.what()); } }); @@ -165,7 +162,7 @@ TEST_SUBMODULE(exceptions, m) { std::rethrow_exception(p); } } catch (const MyException6 &e) { - PyErr_SetString(PyExc_RuntimeError, e.what()); + py::set_error(PyExc_RuntimeError, e.what()); } }); @@ -225,7 +222,7 @@ TEST_SUBMODULE(exceptions, m) { m.def("throw_already_set", [](bool err) { if (err) { - PyErr_SetString(PyExc_ValueError, "foo"); + py::set_error(PyExc_ValueError, "foo"); } try { throw py::error_already_set(); @@ -241,7 +238,7 @@ TEST_SUBMODULE(exceptions, m) { } PyErr_Clear(); if (err) { - PyErr_SetString(PyExc_ValueError, "foo"); + py::set_error(PyExc_ValueError, "foo"); } throw py::error_already_set(); }); @@ -250,7 +247,7 @@ TEST_SUBMODULE(exceptions, m) { bool retval = false; try { PythonCallInDestructor set_dict_in_destructor(d); - PyErr_SetString(PyExc_ValueError, "foo"); + py::set_error(PyExc_ValueError, "foo"); throw py::error_already_set(); } catch (const py::error_already_set &) { retval = true; @@ -285,14 +282,14 @@ TEST_SUBMODULE(exceptions, m) { m.def("throw_should_be_translated_to_key_error", []() { throw shared_exception(); }); m.def("raise_from", []() { - PyErr_SetString(PyExc_ValueError, "inner"); + py::set_error(PyExc_ValueError, "inner"); py::raise_from(PyExc_ValueError, "outer"); throw py::error_already_set(); }); m.def("raise_from_already_set", []() { try { - PyErr_SetString(PyExc_ValueError, "inner"); + py::set_error(PyExc_ValueError, "inner"); throw py::error_already_set(); } catch (py::error_already_set &e) { py::raise_from(e, PyExc_ValueError, "outer"); @@ -324,7 +321,7 @@ TEST_SUBMODULE(exceptions, m) { }); m.def("test_error_already_set_double_restore", [](bool dry_run) { - PyErr_SetString(PyExc_ValueError, "Random error."); + py::set_error(PyExc_ValueError, "Random error."); py::error_already_set e; e.restore(); PyErr_Clear(); From 8e5450acb614fb21b09b6cdba1a55d64f388ba12 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Thu, 3 Aug 2023 13:36:29 -0700 Subject: [PATCH 04/12] clang 17 compatibility fixes (#4767) * Copy clang 17 compatibility fixes from PR #4762 to a separate PR. * Add gcc:13 C++20 * Add silkeh/clang:16-bullseye C++20 --- .github/workflows/ci.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 48f7c5e934..46e88132cf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -302,6 +302,9 @@ jobs: - clang: 15 std: 20 container_suffix: "-bullseye" + - clang: 16 + std: 20 + container_suffix: "-bullseye" name: "🐍 3 • Clang ${{ matrix.clang }} • C++${{ matrix.std }} • x64" container: "silkeh/clang:${{ matrix.clang }}${{ matrix.container_suffix }}" @@ -465,6 +468,7 @@ jobs: - { gcc: 10, std: 17 } - { gcc: 11, std: 20 } - { gcc: 12, std: 20 } + - { gcc: 13, std: 20 } name: "🐍 3 • GCC ${{ matrix.gcc }} • C++${{ matrix.std }}• x64" container: "gcc:${{ matrix.gcc }}" From 1142d0fe0ca88439432a5a204bdf614fd3bbb526 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 3 Aug 2023 22:11:25 -0700 Subject: [PATCH 05/12] chore(deps): update pre-commit hooks (#4770) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit updates: - [github.com/psf/black: 23.3.0 → 23.7.0](https://github.com/psf/black/compare/23.3.0...23.7.0) - [github.com/astral-sh/ruff-pre-commit: v0.0.276 → v0.0.281](https://github.com/astral-sh/ruff-pre-commit/compare/v0.0.276...v0.0.281) - [github.com/asottile/blacken-docs: 1.14.0 → 1.15.0](https://github.com/asottile/blacken-docs/compare/1.14.0...1.15.0) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .pre-commit-config.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 86ac965d96..c7f5963427 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -32,13 +32,13 @@ repos: # Black, the code formatter, natively supports pre-commit - repo: https://github.com/psf/black - rev: "23.3.0" # Keep in sync with blacken-docs + rev: "23.7.0" # Keep in sync with blacken-docs hooks: - id: black # Ruff, the Python auto-correcting linter written in Rust - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.0.276 + rev: v0.0.281 hooks: - id: ruff args: ["--fix", "--show-fixes"] @@ -84,7 +84,7 @@ repos: # Also code format the docs - repo: https://github.com/asottile/blacken-docs - rev: "1.14.0" + rev: "1.15.0" hooks: - id: blacken-docs additional_dependencies: From 7a12354c516af8896a4119d805fc2449fe1f2f15 Mon Sep 17 00:00:00 2001 From: "Keto D. Zhang" Date: Thu, 3 Aug 2023 22:14:23 -0700 Subject: [PATCH 06/12] docs: Remove upper bound on pybind11 in example pyproject.toml for setuptools (#4774) * docs: Remove upper bound on pybind11 in example pyproject.toml for setuptools * Update docs/compiling.rst --------- Co-authored-by: Henry Schreiner --- docs/compiling.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/compiling.rst b/docs/compiling.rst index 1fd098bec4..9d53904c42 100644 --- a/docs/compiling.rst +++ b/docs/compiling.rst @@ -143,7 +143,7 @@ Your ``pyproject.toml`` file will likely look something like this: .. code-block:: toml [build-system] - requires = ["setuptools>=42", "wheel", "pybind11~=2.6.1"] + requires = ["setuptools>=42", "pybind11>=2.6.1"] build-backend = "setuptools.build_meta" .. note:: From e2746ebf39e11034b9537040d7537d589d33b905 Mon Sep 17 00:00:00 2001 From: Dustin Spicuzza Date: Fri, 4 Aug 2023 01:48:57 -0400 Subject: [PATCH 07/12] Provide better type hints for a variety of generic types (#4259) * Provide better type hints for a variety of generic types * Makes better documentation * tuple, dict, list, set, function * Move to py::typing * style: pre-commit fixes * Update copyright line with correct year and actual author. The author information was copy-pasted from the git log output. --------- Co-authored-by: Ralf W. Grosse-Kunstleve Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- CMakeLists.txt | 3 +- docs/advanced/misc.rst | 29 +++++++ include/pybind11/typing.h | 97 ++++++++++++++++++++++++ tests/extra_python_package/test_files.py | 1 + tests/test_pytypes.cpp | 10 +++ tests/test_pytypes.py | 35 +++++++++ 6 files changed, 174 insertions(+), 1 deletion(-) create mode 100644 include/pybind11/typing.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 87ec103468..15fa799086 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -141,7 +141,8 @@ set(PYBIND11_HEADERS include/pybind11/stl.h include/pybind11/stl_bind.h include/pybind11/stl/filesystem.h - include/pybind11/type_caster_pyobject_ptr.h) + include/pybind11/type_caster_pyobject_ptr.h + include/pybind11/typing.h) # Compare with grep and warn if mismatched if(PYBIND11_MASTER_PROJECT AND NOT CMAKE_VERSION VERSION_LESS 3.12) diff --git a/docs/advanced/misc.rst b/docs/advanced/misc.rst index 805ec838fc..ddd7f39370 100644 --- a/docs/advanced/misc.rst +++ b/docs/advanced/misc.rst @@ -398,3 +398,32 @@ before they are used as a parameter or return type of a function: pyFoo.def(py::init()); pyBar.def(py::init()); } + +Setting inner type hints in docstrings +====================================== + +When you use pybind11 wrappers for ``list``, ``dict``, and other generic python +types, the docstring will just display the generic type. You can convey the +inner types in the docstring by using a special 'typed' version of the generic +type. + +.. code-block:: cpp + + PYBIND11_MODULE(example, m) { + m.def("pass_list_of_str", [](py::typing::List arg) { + // arg can be used just like py::list + )); + } + +The resulting docstring will be ``pass_list_of_str(arg0: list[str]) -> None``. + +The following special types are available in ``pybind11/typing.h``: + +* ``py::Tuple`` +* ``py::Dict`` +* ``py::List`` +* ``py::Set`` +* ``py::Callable`` + +.. warning:: Just like in python, these are merely hints. They don't actually + enforce the types of their contents at runtime or compile time. diff --git a/include/pybind11/typing.h b/include/pybind11/typing.h new file mode 100644 index 0000000000..74fd82eace --- /dev/null +++ b/include/pybind11/typing.h @@ -0,0 +1,97 @@ +/* + pybind11/typing.h: Convenience wrapper classes for basic Python types + with more explicit annotations. + + Copyright (c) 2023 Dustin Spicuzza + + All rights reserved. Use of this source code is governed by a + BSD-style license that can be found in the LICENSE file. +*/ + +#pragma once + +#include "detail/common.h" +#include "cast.h" +#include "pytypes.h" + +PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) +PYBIND11_NAMESPACE_BEGIN(typing) + +/* + The following types can be used to direct pybind11-generated docstrings + to have have more explicit types (e.g., `list[str]` instead of `list`). + Just use these in place of existing types. + + There is no additional enforcement of types at runtime. +*/ + +template +class Tuple : public tuple { + using tuple::tuple; +}; + +template +class Dict : public dict { + using dict::dict; +}; + +template +class List : public list { + using list::list; +}; + +template +class Set : public set { + using set::set; +}; + +template +class Callable; + +template +class Callable : public function { + using function::function; +}; + +PYBIND11_NAMESPACE_END(typing) + +PYBIND11_NAMESPACE_BEGIN(detail) + +template +struct handle_type_name> { + static constexpr auto name + = const_name("tuple[") + concat(make_caster::name...) + const_name("]"); +}; + +template <> +struct handle_type_name> { + // PEP 484 specifies this syntax for an empty tuple + static constexpr auto name = const_name("tuple[()]"); +}; + +template +struct handle_type_name> { + static constexpr auto name = const_name("dict[") + make_caster::name + const_name(", ") + + make_caster::name + const_name("]"); +}; + +template +struct handle_type_name> { + static constexpr auto name = const_name("list[") + make_caster::name + const_name("]"); +}; + +template +struct handle_type_name> { + static constexpr auto name = const_name("set[") + make_caster::name + const_name("]"); +}; + +template +struct handle_type_name> { + using retval_type = conditional_t::value, void_type, Return>; + static constexpr auto name = const_name("Callable[[") + concat(make_caster::name...) + + const_name("], ") + make_caster::name + + const_name("]"); +}; + +PYBIND11_NAMESPACE_END(detail) +PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) diff --git a/tests/extra_python_package/test_files.py b/tests/extra_python_package/test_files.py index 57387dd8bc..e3d881c0b3 100644 --- a/tests/extra_python_package/test_files.py +++ b/tests/extra_python_package/test_files.py @@ -44,6 +44,7 @@ "include/pybind11/stl.h", "include/pybind11/stl_bind.h", "include/pybind11/type_caster_pyobject_ptr.h", + "include/pybind11/typing.h", } detail_headers = { diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index b4ee642891..cb2f76c031 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -7,6 +7,8 @@ BSD-style license that can be found in the LICENSE file. */ +#include + #include "pybind11_tests.h" #include @@ -820,4 +822,12 @@ TEST_SUBMODULE(pytypes, m) { a >>= b; return a; }); + + m.def("annotate_tuple_float_str", [](const py::typing::Tuple &) {}); + m.def("annotate_tuple_empty", [](const py::typing::Tuple<> &) {}); + m.def("annotate_dict_str_int", [](const py::typing::Dict &) {}); + m.def("annotate_list_int", [](const py::typing::List &) {}); + m.def("annotate_set_str", [](const py::typing::Set &) {}); + m.def("annotate_fn", + [](const py::typing::Callable, py::str)> &) {}); } diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index eda7a20a9d..e88a9328f7 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -896,3 +896,38 @@ def test_inplace_lshift(a, b): def test_inplace_rshift(a, b): expected = a >> b assert m.inplace_rshift(a, b) == expected + + +def test_tuple_nonempty_annotations(doc): + assert ( + doc(m.annotate_tuple_float_str) + == "annotate_tuple_float_str(arg0: tuple[float, str]) -> None" + ) + + +def test_tuple_empty_annotations(doc): + assert ( + doc(m.annotate_tuple_empty) == "annotate_tuple_empty(arg0: tuple[()]) -> None" + ) + + +def test_dict_annotations(doc): + assert ( + doc(m.annotate_dict_str_int) + == "annotate_dict_str_int(arg0: dict[str, int]) -> None" + ) + + +def test_list_annotations(doc): + assert doc(m.annotate_list_int) == "annotate_list_int(arg0: list[int]) -> None" + + +def test_set_annotations(doc): + assert doc(m.annotate_set_str) == "annotate_set_str(arg0: set[str]) -> None" + + +def test_fn_annotations(doc): + assert ( + doc(m.annotate_fn) + == "annotate_fn(arg0: Callable[[list[str], str], int]) -> None" + ) From daaabcc638a5b7c8c9e07fd6bd260008bd11f349 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Thu, 3 Aug 2023 21:57:40 -0700 Subject: [PATCH 08/12] Use `py::set_error()` everywhere possible (only one special case, in common.h). Overload `py::set_error(py::handle, py::handle)`. Change back to `static py::handle exc = ... .release();` Deprecate `py::exception<>::operator()` --- docs/advanced/exceptions.rst | 18 ++++----- include/pybind11/detail/class.h | 6 +-- include/pybind11/detail/common.h | 2 +- include/pybind11/detail/internals.h | 2 +- include/pybind11/detail/type_caster_base.h | 2 +- include/pybind11/numpy.h | 4 +- include/pybind11/pybind11.h | 24 +++++------- include/pybind11/pytypes.h | 8 ++++ ...s_module_interleaved_error_already_set.cpp | 4 +- tests/pybind11_cross_module_tests.cpp | 8 ++-- tests/test_exceptions.cpp | 37 ++++++++++++++++--- tests/test_exceptions.h | 2 +- tests/test_exceptions.py | 10 ++++- tests/test_pytypes.cpp | 2 +- tests/test_type_caster_pyobject_ptr.cpp | 4 +- 15 files changed, 85 insertions(+), 48 deletions(-) diff --git a/docs/advanced/exceptions.rst b/docs/advanced/exceptions.rst index 6b34e1fdd7..616e2d7726 100644 --- a/docs/advanced/exceptions.rst +++ b/docs/advanced/exceptions.rst @@ -127,8 +127,7 @@ before a global translator is tried. Inside the translator, ``std::rethrow_exception`` should be used within a try block to re-throw the exception. One or more catch clauses to catch the appropriate exceptions should then be used with each clause using -``PyErr_SetString`` to set a Python exception or ``ex(string)`` to set -the python exception to a custom exception type (see below). +``py::set_error()`` (see below). To declare a custom Python exception type, declare a ``py::exception`` variable and use this in the associated exception translator (note: it is often useful @@ -145,12 +144,12 @@ standard python RuntimeError: // This is a static object, so we must leak the Python reference: // It is undefined when the destructor will run, possibly only after the // Python interpreter is finalized already. - static const auto *const exc = new py::exception(m, "MyCustomError"); + static py::handle exc = py::exception(m, "MyCustomError").release(); py::register_exception_translator([](std::exception_ptr p) { try { if (p) std::rethrow_exception(p); } catch (const MyCustomException &e) { - (*exc)(e.what()); + py::set_error(exc, e.what()); } catch (const OtherException &e) { py::set_error(PyExc_RuntimeError, e.what()); } @@ -171,8 +170,7 @@ section. .. note:: - Call either ``PyErr_SetString`` or a custom exception's call - operator (``exc(string)``) for every exception caught in a custom exception + Call ``py::set_error()`` for every exception caught in a custom exception translator. Failure to do so will cause Python to crash with ``SystemError: error return without exception set``. @@ -203,7 +201,7 @@ If module1 has the following translator: try { if (p) std::rethrow_exception(p); } catch (const std::invalid_argument &e) { - PyErr_SetString("module1 handled this") + py::set_error(PyExc_ArgumentError, "module1 handled this"); } } @@ -215,7 +213,7 @@ and module2 has the following similar translator: try { if (p) std::rethrow_exception(p); } catch (const std::invalid_argument &e) { - PyErr_SetString("module2 handled this") + py::set_error(PyExc_ArgumentError, "module2 handled this"); } } @@ -315,11 +313,11 @@ error protocol, which is outlined here. After calling the Python C API, if Python returns an error, ``throw py::error_already_set();``, which allows pybind11 to deal with the exception and pass it back to the Python interpreter. This includes calls to -the error setting functions such as ``PyErr_SetString``. +the error setting functions such as ``py::set_error()``. .. code-block:: cpp - PyErr_SetString(PyExc_TypeError, "C API type error demo"); + py::set_error(PyExc_TypeError, "C API type error demo"); throw py::error_already_set(); // But it would be easier to simply... diff --git a/include/pybind11/detail/class.h b/include/pybind11/detail/class.h index bc2b40c50a..3ece0643bd 100644 --- a/include/pybind11/detail/class.h +++ b/include/pybind11/detail/class.h @@ -375,7 +375,7 @@ extern "C" inline PyObject *pybind11_object_new(PyTypeObject *type, PyObject *, extern "C" inline int pybind11_object_init(PyObject *self, PyObject *, PyObject *) { PyTypeObject *type = Py_TYPE(self); std::string msg = get_fully_qualified_tp_name(type) + ": No constructor defined!"; - PyErr_SetString(PyExc_TypeError, msg.c_str()); + set_error(PyExc_TypeError, msg.c_str()); return -1; } @@ -579,7 +579,7 @@ extern "C" inline int pybind11_getbuffer(PyObject *obj, Py_buffer *view, int fla if (view) { view->obj = nullptr; } - PyErr_SetString(PyExc_BufferError, "pybind11_getbuffer(): Internal error"); + set_error(PyExc_BufferError, "pybind11_getbuffer(): Internal error"); return -1; } std::memset(view, 0, sizeof(Py_buffer)); @@ -587,7 +587,7 @@ extern "C" inline int pybind11_getbuffer(PyObject *obj, Py_buffer *view, int fla if ((flags & PyBUF_WRITABLE) == PyBUF_WRITABLE && info->readonly) { delete info; // view->obj = nullptr; // Was just memset to 0, so not necessary - PyErr_SetString(PyExc_BufferError, "Writable buffer requested for readonly storage"); + set_error(PyExc_BufferError, "Writable buffer requested for readonly storage"); return -1; } view->obj = obj; diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 7ebc01c5d9..e79f7693d0 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -399,7 +399,7 @@ PYBIND11_WARNING_POP return nullptr; \ } \ catch (const std::exception &e) { \ - PyErr_SetString(PyExc_ImportError, e.what()); \ + ::pybind11::set_error(PyExc_ImportError, e.what()); \ return nullptr; \ } diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index aaa7f8686e..228dd764b7 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -352,7 +352,7 @@ inline bool raise_err(PyObject *exc_type, const char *msg) { raise_from(exc_type, msg); return true; } - PyErr_SetString(exc_type, msg); + set_error(exc_type, msg); return false; } diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index 16387506cf..fc5f1c88b3 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -786,7 +786,7 @@ class type_caster_generic { std::string tname = rtti_type ? rtti_type->name() : cast_type.name(); detail::clean_type_id(tname); std::string msg = "Unregistered type : " + tname; - PyErr_SetString(PyExc_TypeError, msg.c_str()); + set_error(PyExc_TypeError, msg.c_str()); return {nullptr, nullptr}; } diff --git a/include/pybind11/numpy.h b/include/pybind11/numpy.h index 36077ec04d..02f65d740d 100644 --- a/include/pybind11/numpy.h +++ b/include/pybind11/numpy.h @@ -1008,7 +1008,7 @@ class array : public buffer { /// Create array from any object -- always returns a new reference static PyObject *raw_array(PyObject *ptr, int ExtraFlags = 0) { if (ptr == nullptr) { - PyErr_SetString(PyExc_ValueError, "cannot create a pybind11::array from a nullptr"); + set_error(PyExc_ValueError, "cannot create a pybind11::array from a nullptr"); return nullptr; } return detail::npy_api::get().PyArray_FromAny_( @@ -1155,7 +1155,7 @@ class array_t : public array { /// Create array from any object -- always returns a new reference static PyObject *raw_array_t(PyObject *ptr) { if (ptr == nullptr) { - PyErr_SetString(PyExc_ValueError, "cannot create a pybind11::array_t from a nullptr"); + set_error(PyExc_ValueError, "cannot create a pybind11::array_t from a nullptr"); return nullptr; } return detail::npy_api::get().PyArray_FromAny_(ptr, diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 0b2fb420d7..76edf7f1f1 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -694,9 +694,8 @@ class cpp_function : public function { if (overloads->is_constructor) { if (!parent || !PyObject_TypeCheck(parent.ptr(), (PyTypeObject *) overloads->scope.ptr())) { - PyErr_SetString( - PyExc_TypeError, - "__init__(self, ...) called with invalid or missing `self` argument"); + set_error(PyExc_TypeError, + "__init__(self, ...) called with invalid or missing `self` argument"); return nullptr; } @@ -1007,7 +1006,7 @@ class cpp_function : public function { A translator may choose to do one of the following: - - catch the exception and call PyErr_SetString or PyErr_SetObject + - catch the exception and call py::set_error() to set a standard (or custom) Python exception, or - do nothing and let the exception fall through to the next translator, or - delegate translation to the next translator by throwing a new type of exception. @@ -1023,8 +1022,7 @@ class cpp_function : public function { return nullptr; } - PyErr_SetString(PyExc_SystemError, - "Exception escaped from default exception translator!"); + set_error(PyExc_SystemError, "Exception escaped from default exception translator!"); return nullptr; } @@ -1125,7 +1123,7 @@ class cpp_function : public function { raise_from(PyExc_TypeError, msg.c_str()); return nullptr; } - PyErr_SetString(PyExc_TypeError, msg.c_str()); + set_error(PyExc_TypeError, msg.c_str()); return nullptr; } if (!result) { @@ -1138,7 +1136,7 @@ class cpp_function : public function { raise_from(PyExc_TypeError, msg.c_str()); return nullptr; } - PyErr_SetString(PyExc_TypeError, msg.c_str()); + set_error(PyExc_TypeError, msg.c_str()); return nullptr; } if (overloads->is_constructor && !self_value_and_holder.holder_constructed()) { @@ -2525,14 +2523,10 @@ inline void register_local_exception_translator(ExceptionTranslator &&translator std::forward(translator)); } -inline void set_error(const handle exc, const char *message) { - PyErr_SetString(exc.ptr(), message); -} - /** * Wrapper to generate a new Python exception type. * - * This should only be used with PyErr_SetString for now. + * This should only be used with py::set_error() for now. * It is not (yet) possible to use as a py::base. * Template type argument is reserved for future use. */ @@ -2553,6 +2547,8 @@ class exception : public object { } // Sets the current python exception to this exception object with the given message + PYBIND11_DEPRECATED("Please use py::set_error() instead " + "(https://github.com/pybind/pybind11/pull/4772)") void operator()(const char *message) const { set_error(*this, message); } }; @@ -2585,7 +2581,7 @@ register_exception_impl(handle scope, const char *name, handle base, bool isLoca try { std::rethrow_exception(p); } catch (const CppException &e) { - detail::get_exception_object()(e.what()); + set_error(detail::get_exception_object(), e.what()); } }); return ex; diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 580a4658ef..fa2d249844 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -334,6 +334,14 @@ class handle : public detail::object_api { #endif }; +inline void set_error(const handle &type, const char *message) { + PyErr_SetString(type.ptr(), message); +} + +inline void set_error(const handle &type, const handle &value) { + PyErr_SetObject(type.ptr(), value.ptr()); +} + /** \rst Holds a reference to a Python object (with reference counting) diff --git a/tests/cross_module_interleaved_error_already_set.cpp b/tests/cross_module_interleaved_error_already_set.cpp index fdd9939e45..3493a7e615 100644 --- a/tests/cross_module_interleaved_error_already_set.cpp +++ b/tests/cross_module_interleaved_error_already_set.cpp @@ -16,12 +16,12 @@ namespace { namespace py = pybind11; void interleaved_error_already_set() { - PyErr_SetString(PyExc_RuntimeError, "1st error."); + py::set_error(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."); + py::set_error(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. diff --git a/tests/pybind11_cross_module_tests.cpp b/tests/pybind11_cross_module_tests.cpp index 9379f3f259..ad68e9a54f 100644 --- a/tests/pybind11_cross_module_tests.cpp +++ b/tests/pybind11_cross_module_tests.cpp @@ -31,11 +31,11 @@ PYBIND11_MODULE(pybind11_cross_module_tests, m) { // test_exceptions.py py::register_local_exception(m, "LocalSimpleException"); m.def("raise_runtime_error", []() { - PyErr_SetString(PyExc_RuntimeError, "My runtime error"); + py::set_error(PyExc_RuntimeError, "My runtime error"); throw py::error_already_set(); }); m.def("raise_value_error", []() { - PyErr_SetString(PyExc_ValueError, "My value error"); + py::set_error(PyExc_ValueError, "My value error"); throw py::error_already_set(); }); m.def("throw_pybind_value_error", []() { throw py::value_error("pybind11 value error"); }); @@ -49,7 +49,7 @@ PYBIND11_MODULE(pybind11_cross_module_tests, m) { std::rethrow_exception(p); } } catch (const shared_exception &e) { - PyErr_SetString(PyExc_KeyError, e.what()); + py::set_error(PyExc_KeyError, e.what()); } }); @@ -60,7 +60,7 @@ PYBIND11_MODULE(pybind11_cross_module_tests, m) { std::rethrow_exception(p); } } catch (const LocalException &e) { - PyErr_SetString(PyExc_KeyError, e.what()); + py::set_error(PyExc_KeyError, e.what()); } }); diff --git a/tests/test_exceptions.cpp b/tests/test_exceptions.cpp index 94981a2169..b60504ce0d 100644 --- a/tests/test_exceptions.cpp +++ b/tests/test_exceptions.cpp @@ -25,6 +25,10 @@ class MyException : public std::exception { std::string message = ""; }; +class MyExceptionUseDeprecatedOperatorCall : public MyException { + using MyException::MyException; +}; + // A type that should be translated to a standard Python exception class MyException2 : public std::exception { public: @@ -109,8 +113,8 @@ TEST_SUBMODULE(exceptions, m) { m.def("throw_std_exception", []() { throw std::runtime_error("This exception was intentionally thrown."); }); - // Please keep in sync with docs/advanced/exceptions.rst - static const auto *const ex = new py::exception(m, "MyException"); + // PLEASE KEEP IN SYNC with docs/advanced/exceptions.rst + static py::handle ex = py::exception(m, "MyException").release(); py::register_exception_translator([](std::exception_ptr p) { try { if (p) { @@ -118,7 +122,25 @@ TEST_SUBMODULE(exceptions, m) { } } catch (const MyException &e) { // Set MyException as the active python error - (*ex)(e.what()); + py::set_error(ex, e.what()); + } + }); + + // Same as above, but using the deprecated `py::exception<>::operator()` + // We want to be sure it still works, until it's removed. + static const auto *const exd = new py::exception( + m, "MyExceptionUseDeprecatedOperatorCall"); + py::register_exception_translator([](std::exception_ptr p) { + try { + if (p) { + std::rethrow_exception(p); + } + } catch (const MyExceptionUseDeprecatedOperatorCall &e) { + PYBIND11_WARNING_PUSH + PYBIND11_WARNING_DISABLE_GCC("-Wdeprecated-declarations") + PYBIND11_WARNING_DISABLE_CLANG("-Wdeprecated-declarations") + (*exd)(e.what()); + PYBIND11_WARNING_POP } }); @@ -166,7 +188,12 @@ TEST_SUBMODULE(exceptions, m) { } }); - m.def("throws1", []() { throw MyException("this error should go to a custom type"); }); + m.def("throws1", + []() { throw MyException("this error should go to py::exception"); }); + m.def("throws1d", []() { + throw MyExceptionUseDeprecatedOperatorCall( + "this error should go to py::exception"); + }); m.def("throws2", []() { throw MyException2("this error should go to a standard Python exception"); }); m.def("throws3", []() { throw MyException3("this error cannot be translated"); }); @@ -306,7 +333,7 @@ TEST_SUBMODULE(exceptions, m) { }); m.def("error_already_set_what", [](const py::object &exc_type, const py::object &exc_value) { - PyErr_SetObject(exc_type.ptr(), exc_value.ptr()); + py::set_error(exc_type, exc_value); std::string what = py::error_already_set().what(); bool py_err_set_after_what = (PyErr_Occurred() != nullptr); PyErr_Clear(); diff --git a/tests/test_exceptions.h b/tests/test_exceptions.h index 03684b89fa..2eaa3d3d16 100644 --- a/tests/test_exceptions.h +++ b/tests/test_exceptions.h @@ -9,5 +9,5 @@ class PYBIND11_EXPORT_EXCEPTION shared_exception : public pybind11::builtin_exce public: using builtin_exception::builtin_exception; explicit shared_exception() : shared_exception("") {} - void set_error() const override { PyErr_SetString(PyExc_RuntimeError, what()); } + void set_error() const override { py::set_error(PyExc_RuntimeError, what()); } }; diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index ccac4536d6..5d8e6292aa 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -139,7 +139,15 @@ def test_custom(msg): # Can we catch a MyException? with pytest.raises(m.MyException) as excinfo: m.throws1() - assert msg(excinfo.value) == "this error should go to a custom type" + assert msg(excinfo.value) == "this error should go to py::exception" + + # Can we catch a MyExceptionUseDeprecatedOperatorCall? + with pytest.raises(m.MyExceptionUseDeprecatedOperatorCall) as excinfo: + m.throws1d() + assert ( + msg(excinfo.value) + == "this error should go to py::exception" + ) # Can we translate to standard Python exceptions? with pytest.raises(RuntimeError) as excinfo: diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index cb2f76c031..b03f5624e1 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -25,7 +25,7 @@ PyObject *conv(PyObject *o) { ret = PyFloat_FromDouble(v); } } else { - PyErr_SetString(PyExc_TypeError, "Unexpected type"); + py::set_error(PyExc_TypeError, "Unexpected type"); } return ret; } diff --git a/tests/test_type_caster_pyobject_ptr.cpp b/tests/test_type_caster_pyobject_ptr.cpp index 1667ea1266..8069f7dcd9 100644 --- a/tests/test_type_caster_pyobject_ptr.cpp +++ b/tests/test_type_caster_pyobject_ptr.cpp @@ -78,14 +78,14 @@ TEST_SUBMODULE(type_caster_pyobject_ptr, m) { m.def("cast_to_pyobject_ptr_nullptr", [](bool set_error) { if (set_error) { - PyErr_SetString(PyExc_RuntimeError, "Reflective of healthy error handling."); + py::set_error(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::set_error(PyExc_RuntimeError, "Reflective of unhealthy error handling."); py::cast(Py_None); }); From 712842aff03d5ca42afd04825ad2320707341cee Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 4 Aug 2023 10:32:06 -0700 Subject: [PATCH 09/12] Add `PYBIND11_WARNING_DISABLE` for INTEL and MSVC (and sort alphabetically). --- tests/test_exceptions.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_exceptions.cpp b/tests/test_exceptions.cpp index b60504ce0d..4d1361d8cf 100644 --- a/tests/test_exceptions.cpp +++ b/tests/test_exceptions.cpp @@ -137,8 +137,10 @@ TEST_SUBMODULE(exceptions, m) { } } catch (const MyExceptionUseDeprecatedOperatorCall &e) { PYBIND11_WARNING_PUSH - PYBIND11_WARNING_DISABLE_GCC("-Wdeprecated-declarations") PYBIND11_WARNING_DISABLE_CLANG("-Wdeprecated-declarations") + PYBIND11_WARNING_DISABLE_GCC("-Wdeprecated-declarations") + PYBIND11_WARNING_DISABLE_INTEL(10441) + PYBIND11_WARNING_DISABLE_MSVC(4996) (*exd)(e.what()); PYBIND11_WARNING_POP } From 5831733b5c5133a72f343d71b5e99278bb79eb94 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 4 Aug 2023 10:54:25 -0700 Subject: [PATCH 10/12] `PYBIND11_WARNING_DISABLE_INTEL(10441)` does not work. For ICC only, falling back to the recommended `py::set_error()` to keep the testing simple. It is troublesome to add `--diag-disable=10441` specifically for test_exceptions.cpp, even that is non-ideal because it covers the entire file, not just the one line we need it for, and the value of exercising the trivial deprecated `operator()` on this one extra platform is practically zero. --- tests/test_exceptions.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_exceptions.cpp b/tests/test_exceptions.cpp index 4d1361d8cf..7273a97cb8 100644 --- a/tests/test_exceptions.cpp +++ b/tests/test_exceptions.cpp @@ -136,13 +136,16 @@ TEST_SUBMODULE(exceptions, m) { std::rethrow_exception(p); } } catch (const MyExceptionUseDeprecatedOperatorCall &e) { +#if defined(__INTEL_COMPILER) // It is too troublesome to add --diag-disable=10441 + py::set_error(exd, e.what()); +#else PYBIND11_WARNING_PUSH PYBIND11_WARNING_DISABLE_CLANG("-Wdeprecated-declarations") PYBIND11_WARNING_DISABLE_GCC("-Wdeprecated-declarations") - PYBIND11_WARNING_DISABLE_INTEL(10441) PYBIND11_WARNING_DISABLE_MSVC(4996) (*exd)(e.what()); PYBIND11_WARNING_POP +#endif } }); From 2ddf85b8a887cf2839f2875f6aac7003b7a90567 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 4 Aug 2023 11:35:37 -0700 Subject: [PATCH 11/12] Fix silly oversight. --- tests/test_exceptions.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_exceptions.cpp b/tests/test_exceptions.cpp index 7273a97cb8..2bdfec33b0 100644 --- a/tests/test_exceptions.cpp +++ b/tests/test_exceptions.cpp @@ -137,7 +137,7 @@ TEST_SUBMODULE(exceptions, m) { } } catch (const MyExceptionUseDeprecatedOperatorCall &e) { #if defined(__INTEL_COMPILER) // It is too troublesome to add --diag-disable=10441 - py::set_error(exd, e.what()); + py::set_error(*exd, e.what()); #else PYBIND11_WARNING_PUSH PYBIND11_WARNING_DISABLE_CLANG("-Wdeprecated-declarations") From 1d4973fa09c5f8df82cd647a5a5bd0dbd507d313 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 4 Aug 2023 12:16:53 -0700 Subject: [PATCH 12/12] NVHPC 23.5.0 generates deprecation warnings. They are currently not treated as errors, but falling back to using `py::set_error()` to not have to deal with that distraction. --- tests/test_exceptions.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_exceptions.cpp b/tests/test_exceptions.cpp index 2bdfec33b0..f6d9c215c9 100644 --- a/tests/test_exceptions.cpp +++ b/tests/test_exceptions.cpp @@ -136,7 +136,9 @@ TEST_SUBMODULE(exceptions, m) { std::rethrow_exception(p); } } catch (const MyExceptionUseDeprecatedOperatorCall &e) { -#if defined(__INTEL_COMPILER) // It is too troublesome to add --diag-disable=10441 +#if defined(__INTEL_COMPILER) || defined(__NVCOMPILER) + // It is not worth the trouble dealing with warning suppressions for these compilers. + // Falling back to the recommended approach to keep the test code simple. py::set_error(*exd, e.what()); #else PYBIND11_WARNING_PUSH