error_already_set::what() is now constructed lazily

Prior to this commit throwing error_already_set was expensive due to the
eager construction of the error string (which required traversing the
Python stack). See pybind#1853 for more context and an alternative take on the
issue.

Author: superbobry

include/pybind11/cast.h

@@ -403,38 +403,36 @@ PYBIND11_NOINLINE inline bool isinstance_generic(handle obj, const std::type_inf
     return isinstance(obj, type);
 }
 
-PYBIND11_NOINLINE inline std::string error_string() {
-    if (!PyErr_Occurred()) {
+PYBIND11_NOINLINE inline std::string error_string(PyObject* type, PyObject* value, PyObject *trace) {
+    if (!type && !value && !trace) {
         PyErr_SetString(PyExc_RuntimeError, "Unknown internal error occurred");
         return "Unknown internal error occurred";
     }
 
-    error_scope scope; // Preserve error state
-
+    // TODO(superbobry): is it safe to assume that exception has been
+    // normalized by the caller?
     std::string errorString;
-    if (scope.type) {
-        errorString += handle(scope.type).attr("__name__").cast<std::string>();
+    if (type) {
+        errorString += handle(type).attr("__name__").cast<std::string>();
         errorString += ": ";
     }
-    if (scope.value)
-        errorString += (std::string) str(scope.value);
-
-    PyErr_NormalizeException(&scope.type, &scope.value, &scope.trace);
+    if (value)
+        errorString += str(value).cast<std::string>();
 
 #if PY_MAJOR_VERSION >= 3
-    if (scope.trace != nullptr)
-        PyException_SetTraceback(scope.value, scope.trace);
+    if (trace)
+        PyException_SetTraceback(value, trace);
 #endif
 
 #if !defined(PYPY_VERSION)
-    if (scope.trace) {
-        PyTracebackObject *trace = (PyTracebackObject *) scope.trace;
+    if (trace) {
+        PyTracebackObject *tb = (PyTracebackObject *) trace;
 
         /* Get the deepest trace possible */
-        while (trace->tb_next)
-            trace = trace->tb_next;
+        while (tb->tb_next)
+            tb = tb->tb_next;
 
-        PyFrameObject *frame = trace->tb_frame;
+        PyFrameObject *frame = tb->tb_frame;
         errorString += "\n\nAt:\n";
         while (frame) {
             int lineno = PyFrame_GetLineNumber(frame);
@@ -450,6 +448,12 @@ PYBIND11_NOINLINE inline std::string error_string() {
     return errorString;
 }
 
+PYBIND11_NOINLINE inline std::string error_string() {
+    error_scope scope; // Preserve error state
+    PyErr_NormalizeException(&scope.type, &scope.value, &scope.trace);
+    return error_string(scope.type, scope.value, scope.trace);
+}
+
 PYBIND11_NOINLINE inline handle get_object_handle(const void *ptr, const detail::type_info *type ) {
     auto &instances = get_internals().registered_instances;
     auto range = instances.equal_range(ptr);

include/pybind11/pytypes.h

@@ -313,17 +313,18 @@ template <typename T> T reinterpret_steal(handle h) { return {h, object::stolen_
 
 NAMESPACE_BEGIN(detail)
 inline std::string error_string();
+inline std::string error_string(PyObject*, PyObject*, PyObject*);
 NAMESPACE_END(detail)
 
 /// Fetch and hold an error which was already set in Python.  An instance of this is typically
 /// thrown to propagate python-side errors back through C++ which can either be caught manually or
 /// else falls back to the function dispatcher (which then raises the captured error back to
 /// python).
-class error_already_set : public std::runtime_error {
+class error_already_set : public std::exception {
 public:
     /// Constructs a new exception from the current Python error indicator, if any.  The current
     /// Python error indicator will be cleared.
-    error_already_set() : std::runtime_error(detail::error_string()) {
+    error_already_set() : std::exception() {
         PyErr_Fetch(&m_type.ptr(), &m_value.ptr(), &m_trace.ptr());
     }
 
@@ -332,10 +333,22 @@ class error_already_set : public std::runtime_error {
 
     inline ~error_already_set();
 
+    virtual const char* what() const noexcept {
+        if (m_lazy_what.empty()) {
+            PyErr_NormalizeException(&m_type.ptr(), &m_value.ptr(), &m_trace.ptr());
+            m_lazy_what = detail::error_string(m_type.ptr(), m_value.ptr(), m_trace.ptr());
+        }
+        return m_lazy_what.c_str();
+    }
+
     /// 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).
-    void restore() { PyErr_Restore(m_type.release().ptr(), m_value.release().ptr(), m_trace.release().ptr()); }
+    void restore() {
+        what();  // Force-build `.what()`.
+        if (m_type || m_value || m_trace)
+            PyErr_Restore(m_type.release().ptr(), m_value.release().ptr(), m_trace.release().ptr());
+    }
 
     // Does nothing; provided for backwards compatibility.
     PYBIND11_DEPRECATED("Use of error_already_set.clear() is deprecated")
@@ -351,7 +364,8 @@ class error_already_set : public std::runtime_error {
     const object& trace() const { return m_trace; }
 
 private:
-    object m_type, m_value, m_trace;
+    mutable std::string m_lazy_what;
+    mutable object m_type, m_value, m_trace;
 };
 
 /** \defgroup python_builtins _

tests/test_exceptions.cpp

@@ -157,7 +157,7 @@ TEST_SUBMODULE(exceptions, m) {
             PyErr_SetString(PyExc_ValueError, "foo");
         try {
             throw py::error_already_set();
-        } catch (const std::runtime_error& e) {
+        } catch (const py::error_already_set& e) {
             if ((err && e.what() != std::string("ValueError: foo")) ||
                 (!err && e.what() != std::string("Unknown internal error occurred")))
             {