Skip to content
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

Expose FfiPtrExt methods as public API #3965

Open
MarshalX opened this issue Mar 18, 2024 · 2 comments
Open

Expose FfiPtrExt methods as public API #3965

MarshalX opened this issue Mar 18, 2024 · 2 comments

Comments

@MarshalX
Copy link

what i want to have in my codebase:

let ptr = ffi::PyList_New(len);
let list: Bound<'_, PyList> = ptr.assume_owned(py).downcast_into_unchecked();

what i have:

let ptr = ffi::PyList_New(len);
let list: Bound<'_, PyList> = Bound::from_owned_ptr(py, ptr).downcast_into_unchecked();

where private assume_owned is just:

#[inline]
unsafe fn assume_owned(self, py: Python<'_>) -> Bound<'_, PyAny> {
    Bound::from_owned_ptr(py, self)
}

any particular reason to make FfiPtrExt private while we have public Bound?

@davidhewitt
Copy link
Member

I could be open to making it public. Originally when we were first implementing the Bound API I was not sure how it would turn out, I think this trait has been nice.

The one thing I would like to consider before making it public, I'd like to experiment with an assume_owned_unchecked method which assumes the pointer is non nul without a runtime check. I have a suspicion there might be a few cases where it's acceptable to skip the check because the CPython API guarantees it, and it might be a slight performance win.

@davidhewitt
Copy link
Member

Another thing to consider briefly is at the moment Bound::from_owned_ptr is probably more coupled to Python API calls than it should be. If the pointer is null, the panic message will be "Python API call failed" and PyO3 will call PyErr_Print first. Probably that coupling should be removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants