Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Explicitly export exception types. #2999

Merged
merged 9 commits into from
May 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/advanced/exceptions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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++
Expand Down
11 changes: 9 additions & 2 deletions include/pybind11/detail/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -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()); } \
Expand Down
2 changes: 2 additions & 0 deletions include/pybind11/detail/internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 8 additions & 1 deletion include/pybind11/pytypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions tests/pybind11_cross_module_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include "pybind11_tests.h"
#include "local_bindings.h"
#include "test_exceptions.h"
#include <pybind11/stl_bind.h>
#include <numeric>

Expand All @@ -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:
Expand Down
2 changes: 2 additions & 0 deletions tests/test_exceptions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(); });
}
12 changes: 12 additions & 0 deletions tests/test_exceptions.h
Original file line number Diff line number Diff line change
@@ -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()); }
};
14 changes: 14 additions & 0 deletions tests/test_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

import pytest

import env # noqa: F401

from pybind11_tests import exceptions as m
import pybind11_cross_module_tests as cm

Expand Down Expand Up @@ -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
Expand Down