From 58ad49b05ee550bcdfa3742ca0b3488c69015855 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Wed, 11 May 2022 02:35:01 -0700 Subject: [PATCH] Various changes. --- include/pybind11/detail/type_caster_base.h | 13 +++---- include/pybind11/pybind11.h | 4 +-- include/pybind11/pytypes.h | 41 +++++++++++----------- tests/test_exceptions.cpp | 4 +-- tests/test_exceptions.py | 8 +++-- 5 files changed, 35 insertions(+), 35 deletions(-) diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index cd54de285..30c96fb5e 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -473,12 +473,12 @@ PYBIND11_NOINLINE bool isinstance_generic(handle obj, const std::type_info &tp) PYBIND11_NOINLINE std::string error_string(PyObject *exc_type, PyObject *exc_value, PyObject *exc_trace) { if (!exc_type) { - static const char *msg - = "Internal error: error_string() called without a Python error available."; - PyErr_SetString(PyExc_RuntimeError, msg); - return msg; + pybind11_fail( + "Internal error: pybind11::detail::error_string() called with exc_type == nullptr"); } + PyErr_NormalizeException(&exc_type, &exc_value, &exc_trace); + auto result = handle(exc_type).attr("__name__").cast(); result += ": "; @@ -530,10 +530,7 @@ error_string(PyObject *exc_type, PyObject *exc_value, PyObject *exc_trace) { } PYBIND11_NOINLINE std::string error_string() { - error_scope scope; // Preserve error state. - if (scope.type) { - PyErr_NormalizeException(&scope.type, &scope.value, &scope.trace); - } + error_scope scope; // Fetch error state. return error_string(scope.type, scope.value, scope.trace); } diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index cd86152e5..8493e06d3 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -2611,8 +2611,8 @@ void print(Args &&...args) { } error_already_set::~error_already_set() { - if (m_type) { - gil_scoped_acquire gil; + gil_scoped_acquire gil; + { error_scope scope; m_type.release().dec_ref(); m_value.release().dec_ref(); diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 750d912ee..b5fb24719 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -387,13 +387,15 @@ 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 - /// Python error indicator will be cleared. + /// Constructs a new exception from the current Python error indicator, or substitutes a + /// RuntimeError("Internal error: ..."). The current Python error indicator will be cleared. error_already_set() : std::runtime_error("") { - PyErr_Fetch(&m_type.ptr(), &m_value.ptr(), &m_trace.ptr()); - if (m_type) { - PyErr_NormalizeException(&m_type.ptr(), &m_value.ptr(), &m_trace.ptr()); + if (!PyErr_Occurred()) { + m_lazy_what = "Internal error: pybind11::detail::error_already_set called while " + "Python error indicator not set."; + PyErr_SetString(PyExc_RuntimeError, m_lazy_what.c_str()); } + PyErr_Fetch(&m_type.ptr(), &m_value.ptr(), &m_trace.ptr()); } error_already_set(const error_already_set &) = default; @@ -411,22 +413,22 @@ public: // Negate the if condition to test the catch(...) block below. if (m_lazy_what.empty()) { throw std::runtime_error( - "FATAL failure building pybind11::error_already_set error_string."); + "FATAL failure building pybind11::detail::error_already_set what()"); } } catch (...) { // Terminating the process, to not mask the original error by errors in the error // handling. Reporting the original error on stderr & stdout. Intentionally using // the Python C API directly, to maximize reliability. std::string msg - = "FATAL failure building pybind11::error_already_set error_string: "; + = "FATAL failure building pybind11::detail::error_already_set what(): "; if (m_type.ptr() == nullptr) { msg += "PYTHON_EXCEPTION_TYPE_IS_NULLPTR"; } else { - const char *tp_name = detail::obj_class_name(m_type.ptr()); - if (tp_name == nullptr) { - msg += "PYTHON_EXCEPTION_TP_NAME_IS_NULLPTR"; + const char *class_name = detail::obj_class_name(m_type.ptr()); + if (class_name == nullptr) { + msg += "PYTHON_EXCEPTION_CLASS_NAME_IS_NULLPTR"; } else { - msg += tp_name; + msg += class_name; } } msg += ": "; @@ -443,8 +445,8 @@ public: + '"'; } } - // Intentionally using C calls to maximize reliability (and to avoid #include - // ). + // Intentionally using C calls to maximize reliability + // (and to avoid #include ). fprintf(stderr, "%s [STDERR]\n", msg.c_str()); fflush(stderr); fprintf(stdout, "%s [STDOUT]\n", msg.c_str()); @@ -455,15 +457,12 @@ public: return m_lazy_what.c_str(); } - /// Restores the currently-held Python error, if any (which will clear the Python error - /// indicator first if already set). + /// Restores the currently-held Python error (which will clear the Python error indicator first + // if already set). void restore() { - if (m_type) { - // As long as this type is copyable, there is no point in releasing m_type, m_value, - // m_trace. - PyErr_Restore( - m_type.inc_ref().ptr(), m_value.inc_ref().ptr(), m_trace.inc_ref().ptr()); - } + // As long as this type is copyable, there is no point in releasing m_type, m_value, + // m_trace. + PyErr_Restore(m_type.inc_ref().ptr(), m_value.inc_ref().ptr(), m_trace.inc_ref().ptr()); } /// If it is impossible to raise the currently-held error, such as in a destructor, we can diff --git a/tests/test_exceptions.cpp b/tests/test_exceptions.cpp index 99eaed36a..c821feade 100644 --- a/tests/test_exceptions.cpp +++ b/tests/test_exceptions.cpp @@ -230,8 +230,8 @@ TEST_SUBMODULE(exceptions, m) { if ((err && e.what() != std::string("ValueError: foo")) || (!err && e.what() - != std::string("Internal error: error_string() called without a Python " - "error available."))) { + != std::string("Internal error: pybind11::detail::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 21520a8c1..6eb8d976a 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -14,9 +14,13 @@ def test_std_exception(msg): def test_error_already_set(msg): - with pytest.raises(SystemError) as excinfo: + with pytest.raises(RuntimeError) as excinfo: m.throw_already_set(False) - assert "without setting" in str(excinfo.value) + assert ( + msg(excinfo.value) + == "Internal error: pybind11::detail::error_already_set called" + " while Python error indicator not set." + ) with pytest.raises(ValueError) as excinfo: m.throw_already_set(True)