Skip to content

Commit

Permalink
Adds set_name method of pybind11::capsule class (#3866)
Browse files Browse the repository at this point in the history
* Adds set_name method of pybind11::capsule class

This calls PyCapsule_SetName on the underlying capsule object.

modified destructors to query capsules's Name

[pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

Handle possible exception thrown by PyCapsule_GetName

Also removed accidentally reintroduced use of `const char *&`.

[pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

Fixed function name

* Introduced private static function to reuse get_name_or_throw

* added tests for capsule renaming

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* handle python error in flight

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Initialized PyObject * variables to nullptr

* use write-unraisable if PyCapsule_GetName raises

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* get_name_or_throw->get_name_no_throw

If PyCapsule_GetName raises an error we should write as unraisable
to consume it and notify user, and then restore the error in flight if any.
This way this method called from destructor would not modify interpreter
error state.

* used error_scope struct

* Renamed get_name_no_throw->get_name_in_error_scope

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
oleksandr-pavlyk and pre-commit-ci[bot] authored Apr 14, 2022
1 parent ad0de0f commit fa98804
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 3 deletions.
34 changes: 31 additions & 3 deletions include/pybind11/pytypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -1588,7 +1588,8 @@ class capsule : public object {
}
pybind11_fail("Unable to get capsule context");
}
void *ptr = PyCapsule_GetPointer(o, nullptr);
const char *name = get_name_in_error_scope(o);
void *ptr = PyCapsule_GetPointer(o, name);
if (ptr == nullptr) {
throw error_already_set();
}
Expand All @@ -1602,7 +1603,8 @@ class capsule : public object {

explicit capsule(void (*destructor)()) {
m_ptr = PyCapsule_New(reinterpret_cast<void *>(destructor), nullptr, [](PyObject *o) {
auto destructor = reinterpret_cast<void (*)()>(PyCapsule_GetPointer(o, nullptr));
const char *name = get_name_in_error_scope(o);
auto destructor = reinterpret_cast<void (*)()>(PyCapsule_GetPointer(o, name));
if (destructor == nullptr) {
throw error_already_set();
}
Expand Down Expand Up @@ -1637,7 +1639,33 @@ class capsule : public object {
}
}

const char *name() const { return PyCapsule_GetName(m_ptr); }
const char *name() const {
const char *name = PyCapsule_GetName(m_ptr);
if ((name == nullptr) && PyErr_Occurred()) {
throw error_already_set();
}
return name;
}

/// Replaces a capsule's name *without* calling the destructor on the existing one.
void set_name(const char *new_name) {
if (PyCapsule_SetName(m_ptr, new_name) != 0) {
throw error_already_set();
}
}

private:
static const char *get_name_in_error_scope(PyObject *o) {
error_scope error_guard;

const char *name = PyCapsule_GetName(o);
if ((name == nullptr) && PyErr_Occurred()) {
// write out and consume error raised by call to PyCapsule_GetName
PyErr_WriteUnraisable(o);
}

return name;
}
};

class tuple : public object {
Expand Down
20 changes: 20 additions & 0 deletions tests/test_pytypes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,13 +159,33 @@ TEST_SUBMODULE(pytypes, m) {
return py::capsule([]() { py::print("destructing capsule"); });
});

m.def("return_renamed_capsule_with_destructor", []() {
py::print("creating capsule");
auto cap = py::capsule([]() { py::print("destructing capsule"); });
static const char *capsule_name = "test_name1";
py::print("renaming capsule");
cap.set_name(capsule_name);
return cap;
});

m.def("return_capsule_with_destructor_2", []() {
py::print("creating capsule");
return py::capsule((void *) 1234, [](void *ptr) {
py::print("destructing capsule: {}"_s.format((size_t) ptr));
});
});

m.def("return_renamed_capsule_with_destructor_2", []() {
py::print("creating capsule");
auto cap = py::capsule((void *) 1234, [](void *ptr) {
py::print("destructing capsule: {}"_s.format((size_t) ptr));
});
static const char *capsule_name = "test_name2";
py::print("renaming capsule");
cap.set_name(capsule_name);
return cap;
});

m.def("return_capsule_with_name_and_destructor", []() {
auto capsule = py::capsule((void *) 12345, "pointer type description", [](PyObject *ptr) {
if (ptr) {
Expand Down
26 changes: 26 additions & 0 deletions tests/test_pytypes.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,19 @@ def test_capsule(capture):
"""
)

with capture:
a = m.return_renamed_capsule_with_destructor()
del a
pytest.gc_collect()
assert (
capture.unordered
== """
creating capsule
renaming capsule
destructing capsule
"""
)

with capture:
a = m.return_capsule_with_destructor_2()
del a
Expand All @@ -207,6 +220,19 @@ def test_capsule(capture):
"""
)

with capture:
a = m.return_renamed_capsule_with_destructor_2()
del a
pytest.gc_collect()
assert (
capture.unordered
== """
creating capsule
renaming capsule
destructing capsule: 1234
"""
)

with capture:
a = m.return_capsule_with_name_and_destructor()
del a
Expand Down

0 comments on commit fa98804

Please sign in to comment.