-
Notifications
You must be signed in to change notification settings - Fork 18
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
Trampoline Revamp #36
Conversation
Co-authored-by: Robert Kruszewski <[email protected]>
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 but leaving to Rob to take a pass
example/buffers.zig
Outdated
} | ||
|
||
pub fn __release_buffer__(self: *const Self, view: *py.PyBuffer) void { | ||
py.allocator.free(self.values); | ||
_ = self; | ||
// FIXME(ngates): ref count the buffer |
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.
Not sure what reference you mean here. The view holds a reference to an obj, and https://docs.python.org/3/c-api/buffer.html#c.PyBuffer_Release decrefs that reference AND releases the view which calls this function. This function is optional so I think that means no decrefs need to happen here, and re incref - this was helpful https://jakevdp.github.io/blog/2014/05/05/introduction-to-the-python-buffer-protocol/#Exploring-the-Buffer-Protocol-Code
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.
Ahhhhhh perfect. So we don't need release_buffer at all in this case then, since it's covered by the python object's refcnt
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.
Correct, but might be useful to have it empty given this is an example
Extends the functionality of the Trampoline to wrap/unwrap more Zig and Python types.
Notably, it includes the ability to wrap/unwrap pointers to Pydust class structs. That means e.g. returning a
*Dog
from a function.It also replaces
__init__
for__new__
. This better aligns with what is actually happening. Previously with init, we needed to allocate some memory (which was entirely undefined), then call an init function on a class. At this point, it wasn't obvious that the class struct fields with default values were not yet assigned.Whereas now, the class has the responsibility of constructing a full state struct, including defaults, that we can then assign during tp_new to the class state.