-
Notifications
You must be signed in to change notification settings - Fork 769
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
Stop leaking in new_closure
#2842
Conversation
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.
Except for the question of whether we need to outer Option
layer, this looks good to me.
I still wonder whether we should dog-food our own capsule API, but maybe this is best left as a separate PR?
196eafb
to
4f5b250
Compare
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.
Now using our PyCapsule::new
😄
bors r=adamreichold |
Build succeeded: |
This is a rebase of #2690 which simplifies the
MaybeLeaked
abstraction from that PR with justCow<'static, CStr>
.This enabled me to annotate with
FIXME
all the places where we still leak; I wonder if we could potentially useGILOnceCell
in future and statics to avoid those. As those callsities are in#[pyclass]
and#[pyfunction]
these are effectively in statics anyway, but it would be nice to tidy up.