Explicitly export exception types. (#2999)

* Set visibility of exceptions to default.

Co-authored-by: XZiar <[email protected]>

* add test

* update docs

* Skip failed test.

Author: oraluben

docs/advanced/exceptions.rst

@@ -164,6 +164,10 @@ section.
     may be explicitly (re-)thrown to delegate it to the other,
     previously-declared existing exception translators.
 
+    Note that ``libc++`` and ``libstdc++`` `behave differently <https://stackoverflow.com/questions/19496643/using-clang-fvisibility-hidden-and-typeinfo-and-type-erasure/28827430>`_
+    with ``-fvisibility=hidden``. Therefore exceptions that are used across ABI boundaries need to be explicitly exported, as exercised in ``tests/test_exceptions.h``.
+    See also: "Problems with C++ exceptions" under `GCC Wiki <https://gcc.gnu.org/wiki/Visibility>`_.
+
 .. _handling_python_exceptions_cpp:
 
 Handling exceptions from Python in C++

include/pybind11/detail/common.h

@@ -723,16 +723,23 @@ using expand_side_effects = bool[];
 
 PYBIND11_NAMESPACE_END(detail)
 
+#if defined(_MSC_VER)
+#  pragma warning(push)
+#  pragma warning(disable: 4275) // warning C4275: An exported class was derived from a class that wasn't exported. Can be ignored when derived from a STL class.
+#endif
 /// C++ bindings of builtin Python exceptions
-class builtin_exception : public std::runtime_error {
+class PYBIND11_EXPORT builtin_exception : public std::runtime_error {
 public:
     using std::runtime_error::runtime_error;
     /// Set the error using the Python C API
     virtual void set_error() const = 0;
 };
+#if defined(_MSC_VER)
+#  pragma warning(pop)
+#endif
 
 #define PYBIND11_RUNTIME_EXCEPTION(name, type) \
-    class name : public builtin_exception { public: \
+    class PYBIND11_EXPORT name : public builtin_exception { public: \
         using builtin_exception::builtin_exception; \
         name() : name("") { } \
         void set_error() const override { PyErr_SetString(type, what()); } \

include/pybind11/detail/internals.h

@@ -276,6 +276,8 @@ PYBIND11_NOINLINE inline internals &get_internals() {
         // initial exception translator, below, so add another for our local exception classes.
         //
         // libstdc++ doesn't require this (types there are identified only by name)
+        // libc++ with CPython doesn't require this (types are explicitly exported)
+        // libc++ with PyPy still need it, awaiting further investigation
 #if !defined(__GLIBCXX__)
         (*internals_pp)->registered_exception_translators.push_front(&translate_local_exception);
 #endif

include/pybind11/pytypes.h

@@ -319,11 +319,15 @@ PYBIND11_NAMESPACE_BEGIN(detail)
 inline std::string error_string();
 PYBIND11_NAMESPACE_END(detail)
 
+#if defined(_MSC_VER)
+#  pragma warning(push)
+#  pragma warning(disable: 4275 4251) // warning C4275: An exported class was derived from a class that wasn't exported. Can be ignored when derived from a STL class.
+#endif
 /// 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 PYBIND11_EXPORT 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.
@@ -371,6 +375,9 @@ class error_already_set : public std::runtime_error {
 private:
     object m_type, m_value, m_trace;
 };
+#if defined(_MSC_VER)
+#  pragma warning(pop)
+#endif
 
 /** \defgroup python_builtins _
     Unless stated otherwise, the following C++ functions behave the same

tests/pybind11_cross_module_tests.cpp

@@ -9,6 +9,7 @@
 
 #include "pybind11_tests.h"
 #include "local_bindings.h"
+#include "test_exceptions.h"
 #include <pybind11/stl_bind.h>
 #include <numeric>
 
@@ -30,6 +31,13 @@ PYBIND11_MODULE(pybind11_cross_module_tests, m) {
     m.def("throw_pybind_value_error", []() { throw py::value_error("pybind11 value error"); });
     m.def("throw_pybind_type_error", []() { throw py::type_error("pybind11 type error"); });
     m.def("throw_stop_iteration", []() { throw py::stop_iteration(); });
+    py::register_exception_translator([](std::exception_ptr p) {
+      try {
+          if (p) std::rethrow_exception(p);
+      } catch (const shared_exception &e) {
+          PyErr_SetString(PyExc_KeyError, e.what());
+      }
+    });
 
     // test_local_bindings.py
     // Local to both:

tests/test_exceptions.cpp

@@ -7,6 +7,7 @@
     BSD-style license that can be found in the LICENSE file.
 */
 
+#include "test_exceptions.h"
 #include "pybind11_tests.h"
 
 // A type that should be raised as an exception in Python
@@ -228,4 +229,5 @@ TEST_SUBMODULE(exceptions, m) {
     // Test repr that cannot be displayed
     m.def("simple_bool_passthrough", [](bool x) {return x;});
 
+    m.def("throw_should_be_translated_to_key_error", []() { throw shared_exception(); });
 }

tests/test_exceptions.h

@@ -0,0 +1,12 @@
+#pragma once
+#include "pybind11_tests.h"
+#include <stdexcept>
+
+// shared exceptions for cross_module_tests
+
+class PYBIND11_EXPORT shared_exception : public pybind11::builtin_exception {
+public:
+    using builtin_exception::builtin_exception;
+    explicit shared_exception() : shared_exception("") {}
+    void set_error() const override { PyErr_SetString(PyExc_RuntimeError, what()); }
+};

tests/test_exceptions.py

@@ -3,6 +3,8 @@
 
 import pytest
 
+import env  # noqa: F401
+
 from pybind11_tests import exceptions as m
 import pybind11_cross_module_tests as cm
 
@@ -44,6 +46,18 @@ def test_cross_module_exceptions():
         cm.throw_stop_iteration()
 
 
+# TODO: FIXME
+@pytest.mark.xfail(
+    "env.PYPY and env.MACOS",
+    raises=RuntimeError,
+    reason="Expected failure with PyPy and libc++ (Issue #2847 & PR #2999)",
+)
+def test_cross_module_exception_translator():
+    with pytest.raises(KeyError):
+        # translator registered in cross_module_tests
+        m.throw_should_be_translated_to_key_error()
+
+
 def test_python_call_in_catch():
     d = {}
     assert m.python_call_in_destructor(d) is True