From bb02ebc654c87d7b562469d686564e8891f102ac Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Tue, 12 Sep 2023 16:46:00 -0700 Subject: [PATCH] Backport of https://github.com/google/pywrapcc/pull/30064 This is a very simple change in support of the PyCLIF-pybind11 integration work: Merely an extra `if` to pre-empt `TypeError: pybind11::init(): factory function returned nullptr`. This allows the factory function to set a custom Python error instead. Manually written factory functions could `throw error_already_set()` directly, but in the context of PyCLIF that is very difficult (compared to this very simple PR), because the factory function can be completely unaware of pybind11. --- include/pybind11/detail/init.h | 3 +++ tests/test_factory_constructors.cpp | 10 ++++++++++ tests/test_factory_constructors.py | 7 +++++++ 3 files changed, 20 insertions(+) diff --git a/include/pybind11/detail/init.h b/include/pybind11/detail/init.h index e21171688c..0e9b302f84 100644 --- a/include/pybind11/detail/init.h +++ b/include/pybind11/detail/init.h @@ -38,6 +38,9 @@ PYBIND11_NAMESPACE_BEGIN(initimpl) inline void no_nullptr(void *ptr) { if (!ptr) { + if (PyErr_Occurred()) { + throw error_already_set(); + } throw type_error("pybind11::init(): factory function returned nullptr"); } } diff --git a/tests/test_factory_constructors.cpp b/tests/test_factory_constructors.cpp index a387cd2e76..d1ab93d103 100644 --- a/tests/test_factory_constructors.cpp +++ b/tests/test_factory_constructors.cpp @@ -412,6 +412,16 @@ TEST_SUBMODULE(factory_constructors, m) { "__init__", [](NoisyAlloc &a, int i, const std::string &) { new (&a) NoisyAlloc(i); }); }); + struct FactoryErrorAlreadySet {}; + py::class_(m, "FactoryErrorAlreadySet") + .def(py::init([](bool set_error) -> FactoryErrorAlreadySet * { + if (!set_error) { + return new FactoryErrorAlreadySet(); + } + py::set_error(PyExc_ValueError, "factory sets error and returns nullptr"); + return nullptr; + })); + // static_assert testing (the following def's should all fail with appropriate compilation // errors): #if 0 diff --git a/tests/test_factory_constructors.py b/tests/test_factory_constructors.py index a9004cbf67..1b326fdaec 100644 --- a/tests/test_factory_constructors.py +++ b/tests/test_factory_constructors.py @@ -514,3 +514,10 @@ def __init__(self, bad): str(excinfo.value) == "__init__(self, ...) called with invalid or missing `self` argument" ) + + +def test_factory_error_already_set(): + obj = m.FactoryErrorAlreadySet(False) + assert isinstance(obj, m.FactoryErrorAlreadySet) + with pytest.raises(ValueError, match="factory sets error and returns nullptr"): + m.FactoryErrorAlreadySet(True)