-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Adds set_name method of pybind11::capsule class #3866
Conversation
135d912
to
df3775b
Compare
Okay, we just need to fix our custom destructors with PyCapsule_GetName() instead of assuming the name is null. |
I think it's too complicated, trying to establish a safe context for managing the lifetime of name. Maybe even impossible, because we don't name who owns the passed name (is it static, or system malloc, from a pool, etc.). To not turn this into something big, I'd just add the thin wrapper (as-is) with the suggested comment. |
15e43ef
to
cefbd50
Compare
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
651cb53
to
e2cd581
Compare
Looks good to me but (3.9-dbg (deadsnakes) • Valgrind • x64) ...
huh? |
for more information, see https://pre-commit.ci
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Oleksandr!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is actually a better solution then what I was going to suggest (as we still throw errors for if we can't get the context or the destructor is nullptr). Regardless, we can consider fixing those in a separate PR.
This calls
PyCapsule_SetName
on the underlying Python object.Description
Definition of
pybind11::capsule
class was missingset_name
method.This is useful if using pybind11 to implement DLPack protocol which requires to rename consumed capsule.
Suggested changelog entry:
pybind11::capsule::set_name added to mutate the name of the capsule instance.