diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 370e52cff6..e5feca1f24 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -971,11 +971,19 @@ class cpp_function : public function { return nullptr; } if (!result) { - std::string msg = "Unable to convert function return value to a " - "Python type! The signature was\n\t"; - msg += it->signature; - append_note_if_missing_header_is_suspected(msg); - PyErr_SetString(PyExc_TypeError, msg.c_str()); + /* Allow the user to raise a Python exception directly using + PyErr_SetString() together with returning a null py::object, as + that may be significantly faster than throwing a C++ exception + in critical code paths. In that case we arrive here with + non-null PyErr_Occurred(), so keep that exception instead of + overwriting it with another. */ + if (!PyErr_Occurred()) { + std::string msg = "Unable to convert function return value to " + "a Python type! The signature was\n\t"; + msg += it->signature; + append_note_if_missing_header_is_suspected(msg); + PyErr_SetString(PyExc_TypeError, msg.c_str()); + } return nullptr; } if (overloads->is_constructor && !self_value_and_holder.holder_constructed()) { diff --git a/tests/test_exceptions.cpp b/tests/test_exceptions.cpp index 25adb32ed1..b712103179 100644 --- a/tests/test_exceptions.cpp +++ b/tests/test_exceptions.cpp @@ -228,6 +228,14 @@ TEST_SUBMODULE(exceptions, m) { throw py::error_already_set(); }); + m.def("raise_without_throw", [](bool return_null, bool set_error) { + if (set_error) + PyErr_SetString(PyExc_FutureWarning, "this is a robbery!"); + if (return_null) + return py::object{}; + return py::cast(5); + }, py::arg("return_null"), py::arg("set_error")); + m.def("python_call_in_destructor", [](const py::dict &d) { bool retval = false; try { diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index 3821eadaa4..7015b37479 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -3,7 +3,7 @@ import pytest -import env # noqa: F401 +import env import pybind11_cross_module_tests as cm from pybind11_tests import exceptions as m @@ -24,6 +24,32 @@ def test_error_already_set(msg): assert msg(excinfo.value) == "foo" +def test_raise_without_throw(msg): + # Not setting an error and returning a non-null object + assert m.raise_without_throw(return_null=False, set_error=False) == 5 + + # Setting an error and returning a null object is an allowed alternative to + # throwing a C++ exception + with pytest.raises(FutureWarning) as excinfo: + m.raise_without_throw(return_null=True, set_error=True) + assert msg(excinfo.value) == "this is a robbery!" + + # Setting an error and returning a non-null object is a Python system error + if not env.PY2: + with pytest.raises(SystemError) as excinfo: + m.raise_without_throw(return_null=False, set_error=True) + + # Returning a null object without error being set is not allowed either, as + # that's also the case when function return value can't be converted to a + # Python type + with pytest.raises(TypeError) as excinfo: + m.raise_without_throw(return_null=True, set_error=False) + assert msg(excinfo.value) == ( + "Unable to convert function return value to a Python type! The " + "signature was\n\t(return_null: bool, set_error: bool) -> object" + ) + + @pytest.mark.skipif("env.PY2") def test_raise_from(msg): with pytest.raises(ValueError) as excinfo: