-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Adds set_name method of pybind11::capsule class #3866
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
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.
rwgk
left a comment
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!
Skylion007
left a comment
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_SetNameon the underlying Python object.Description
Definition of
pybind11::capsuleclass was missingset_namemethod.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.