add Borrowed::as_ptr
#4520
add Borrowed::as_ptr
#4520
Conversation
There was a problem hiding this comment.
I noticed there are a few places in
src/types/any.rswhere we were creating aBorrowed<'_, '_, PyAny>, then temporarily deref-ing it to&Bound<'_, PyAny>, then immediately calling.as_ptr().
I wasn't sure how much we wanted to sprinkle Borrowed around in my initial conversion, that's why I opted for &Bound initially. Will take this into account in the other trait bound migrations.
| /// The reference is borrowed; callers should not decrease the reference count | ||
| /// when they are finished with the pointer. | ||
| #[inline] | ||
| pub fn as_ptr(self) -> *mut ffi::PyObject { |
There was a problem hiding this comment.
A bit unrelated, but is there a particular reason why we didn't opt for NonNull<PyObject> for these pointer methods?
There was a problem hiding this comment.
I think it's a historical thing from before I was involved with PyO3. Given that mostly these pointers are just immediately passed to FFI, maybe it was more convenient? We could add non-null variants for cases where these pointers get handled by Rust I guess...
Following the addition of the
IntoPyObjecttrait, I noticed there are a few places insrc/types/any.rswhere we were creating aBorrowed<'_, '_, PyAny>, then temporarily deref-ing it to&Bound<'_, PyAny>, then immediately calling.as_ptr().This PR adds
Borrowed::as_ptrto avoid the need to go through the deref. I doubt this will actually change performance, but it'll at least make less work for the optimizer to chew on.While working through this I found a few cases of unnecessary calls to
.as_borrowed()(probably left over from GIL Ref migration work), which I removed at the same time.