From a590a0f347dbb441442a4c35ac3946c8e3a25d29 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Thu, 24 Mar 2022 14:49:34 -0400 Subject: [PATCH 1/2] Make pycapsule errors better match python --- include/pybind11/numpy.h | 3 ++- include/pybind11/pytypes.h | 29 ++++++++++++++++------------- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/include/pybind11/numpy.h b/include/pybind11/numpy.h index 07dea21b4f..7624c9fbf4 100644 --- a/include/pybind11/numpy.h +++ b/include/pybind11/numpy.h @@ -1288,7 +1288,8 @@ struct npy_format_descriptor::is_array>> { static pybind11::dtype dtype() { list shape; array_info::append_extents(shape); - return pybind11::dtype::from_args(pybind11::make_tuple(base_descr::dtype(), shape)); + return pybind11::dtype::from_args( + pybind11::make_tuple(base_descr::dtype(), std::move(shape))); } }; diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index e54941485b..c563405fc6 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1574,7 +1574,7 @@ class capsule : public object { void (*destructor)(PyObject *) = nullptr) : object(PyCapsule_New(const_cast(value), name, destructor), stolen_t{}) { if (!m_ptr) { - pybind11_fail("Could not allocate capsule object!"); + throw error_already_set(); } } @@ -1582,34 +1582,39 @@ class capsule : public object { capsule(const void *value, void (*destruct)(PyObject *)) : object(PyCapsule_New(const_cast(value), nullptr, destruct), stolen_t{}) { if (!m_ptr) { - pybind11_fail("Could not allocate capsule object!"); + throw error_already_set(); } } capsule(const void *value, void (*destructor)(void *)) { m_ptr = PyCapsule_New(const_cast(value), nullptr, [](PyObject *o) { auto destructor = reinterpret_cast(PyCapsule_GetContext(o)); + if (destructor == nullptr && PyErr_Occurred()) { + throw error_already_set(); + } void *ptr = PyCapsule_GetPointer(o, nullptr); + if (ptr == nullptr && PyErr_Occurred()) { + throw error_already_set(); + } destructor(ptr); }); - if (!m_ptr) { - pybind11_fail("Could not allocate capsule object!"); - } - - if (PyCapsule_SetContext(m_ptr, (void *) destructor) != 0) { - pybind11_fail("Could not set capsule context!"); + if (!m_ptr || PyCapsule_SetContext(m_ptr, (void *) destructor) != 0) { + throw error_already_set(); } } explicit capsule(void (*destructor)()) { m_ptr = PyCapsule_New(reinterpret_cast(destructor), nullptr, [](PyObject *o) { auto destructor = reinterpret_cast(PyCapsule_GetPointer(o, nullptr)); + if (destructor == nullptr && PyErr_Occurred()) { + throw error_already_set(); + } destructor(); }); if (!m_ptr) { - pybind11_fail("Could not allocate capsule object!"); + throw error_already_set(); } } @@ -1624,8 +1629,7 @@ class capsule : public object { const auto *name = this->name(); T *result = static_cast(PyCapsule_GetPointer(m_ptr, name)); if (!result) { - PyErr_Clear(); - pybind11_fail("Unable to extract capsule contents!"); + throw error_already_set(); } return result; } @@ -1633,8 +1637,7 @@ class capsule : public object { /// Replaces a capsule's pointer *without* calling the destructor on the existing one. void set_pointer(const void *value) { if (PyCapsule_SetPointer(m_ptr, const_cast(value)) != 0) { - PyErr_Clear(); - pybind11_fail("Could not set capsule pointer"); + throw error_already_set(); } } From 16d9da59a5e59ca36a74f1eecccf2a8fa4407dfd Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Thu, 24 Mar 2022 15:13:30 -0400 Subject: [PATCH 2/2] Improve error handling --- include/pybind11/pytypes.h | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index c563405fc6..b9ec8af4e1 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -1589,11 +1589,14 @@ class capsule : public object { capsule(const void *value, void (*destructor)(void *)) { m_ptr = PyCapsule_New(const_cast(value), nullptr, [](PyObject *o) { auto destructor = reinterpret_cast(PyCapsule_GetContext(o)); - if (destructor == nullptr && PyErr_Occurred()) { - throw error_already_set(); + if (destructor == nullptr) { + if (PyErr_Occurred()) { + throw error_already_set(); + } + pybind11_fail("Unable to get capsule context"); } void *ptr = PyCapsule_GetPointer(o, nullptr); - if (ptr == nullptr && PyErr_Occurred()) { + if (ptr == nullptr) { throw error_already_set(); } destructor(ptr); @@ -1607,7 +1610,7 @@ class capsule : public object { explicit capsule(void (*destructor)()) { m_ptr = PyCapsule_New(reinterpret_cast(destructor), nullptr, [](PyObject *o) { auto destructor = reinterpret_cast(PyCapsule_GetPointer(o, nullptr)); - if (destructor == nullptr && PyErr_Occurred()) { + if (destructor == nullptr) { throw error_already_set(); } destructor();