-
-
Notifications
You must be signed in to change notification settings - Fork 53
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
Improve API/implementation for HPyCapsule_Destructor #378
Conversation
LGTM, but will defer to @antocuni |
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.
LGTM, apart the name of the macro: the idea of the various existing HPyDef_*
macros is that each of them defines a static variable whose C type is actually HPyDef
.
HPyDef_CAPSULE_DESTRUCTOR
does something different, so it should be named differently; it belongs to the HPyCapsule
"namespace", so what about HPyCapsule_DESTRUCTOR
?
Okay, I'm not objected to rename it. I did that for consistency but didn't have in mind that the |
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.
LGTM, thanks
This resolved #377.
In this PR, I'm introducing macro
HPyDef_CAPSULE_DESTRUCTOR
(much likeHPyDev_METH
macro) and removingHPyCapsule_GetDestructor
. With this, we can significantly simplify our capsule implementation because we never need to hand out the HPy capsule destructor function while the new macro will generate an appropriate CPython trampoline.Removing
HPyCapsule_GetDestructor
should be fine since I didn't find a single usage ofPyCapsule_GetDestructor
in the C code of the top4000 packages.