Skip to content

Commit

Permalink
fix: define (non-empty) PYBIND11_EXPORT_EXCEPTION only under macOS. (
Browse files Browse the repository at this point in the history
…#4298)

Background: #2999, #4105, #4283, #4284

In a nutshell:

* Only macOS actually needs `PYBIND11_EXPORT_EXCEPTION` (#4284).

* Evidently (#4283), under macOS `PYBIND11_EXPORT_EXCEPTION` does not run the risk of introducing ODR violations,

* but evidently (#4283) under Linux it does, in the presumably rare/unusual situation that `RTLD_GLOBAL` is used.

* Windows does no have the equivalent of `RTLD_GLOBAL`, therefore `PYBIND11_EXPORT_EXCEPTION` has no practical benefit, on the contrary, noisy warning suppression pragmas are needed, therefore it is best left empty.
  • Loading branch information
Ralf W. Grosse-Kunstleve authored Oct 31, 2022
1 parent 3a2c96b commit b1bd7f2
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 27 deletions.
9 changes: 6 additions & 3 deletions docs/advanced/exceptions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -177,9 +177,12 @@ 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>`_.
Note that ``libc++`` and ``libstdc++`` `behave differently under macOS
<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>`_.


Local vs Global Exception Translators
Expand Down
18 changes: 3 additions & 15 deletions include/pybind11/detail/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,10 @@
#endif

#if !defined(PYBIND11_EXPORT_EXCEPTION)
# ifdef __MINGW32__
// workaround for:
// error: 'dllexport' implies default visibility, but xxx has already been declared with a
// different visibility
# define PYBIND11_EXPORT_EXCEPTION
# else
# if defined(__apple_build_version__)
# define PYBIND11_EXPORT_EXCEPTION PYBIND11_EXPORT
# else
# define PYBIND11_EXPORT_EXCEPTION
# endif
#endif

Expand Down Expand Up @@ -904,22 +901,13 @@ 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 PYBIND11_EXPORT_EXCEPTION 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 PYBIND11_EXPORT_EXCEPTION name : public builtin_exception { \
Expand Down
9 changes: 0 additions & 9 deletions include/pybind11/pytypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -623,12 +623,6 @@ 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
Expand Down Expand Up @@ -688,9 +682,6 @@ class PYBIND11_EXPORT_EXCEPTION error_already_set : public std::exception {
/// crashes (undefined behavior) if the Python interpreter is finalizing.
static void m_fetched_error_deleter(detail::error_fetch_and_normalize *raw_ptr);
};
#if defined(_MSC_VER)
# pragma warning(pop)
#endif

/// Replaces the current Python error indicator with the chosen error, performing a
/// 'raise from' to indicate that the chosen error was caused by the original error.
Expand Down

0 comments on commit b1bd7f2

Please sign in to comment.