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

add Bound::from_owned_ptr_unchecked (crate-private for now) #4596

Merged
merged 2 commits into from
Oct 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions src/ffi_ptr_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,20 @@ use crate::{
};

pub(crate) trait FfiPtrExt: Sealed {
/// Assumes this pointer carries a Python reference which needs to be decref'd.
///
/// If the pointer is NULL, this function will fetch an error.
unsafe fn assume_owned_or_err(self, py: Python<'_>) -> PyResult<Bound<'_, PyAny>>;

/// Same as `assume_owned_or_err`, but doesn't fetch an error on NULL.
unsafe fn assume_owned_or_opt(self, py: Python<'_>) -> Option<Bound<'_, PyAny>>;

/// Same as `assume_owned_or_err`, but panics on NULL.
unsafe fn assume_owned(self, py: Python<'_>) -> Bound<'_, PyAny>;

/// Same as `assume_owned_or_err`, but does not check for NULL.
unsafe fn assume_owned_unchecked(self, py: Python<'_>) -> Bound<'_, PyAny>;

/// Assumes this pointer is borrowed from a parent object.
///
/// Warning: the lifetime `'a` is not bounded by the function arguments; the caller is
Expand Down Expand Up @@ -44,6 +54,11 @@ impl FfiPtrExt for *mut ffi::PyObject {
Bound::from_owned_ptr(py, self)
}

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

#[inline]
unsafe fn assume_borrowed_or_err<'a>(
self,
Expand Down
22 changes: 22 additions & 0 deletions src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,19 @@ impl<'py> Bound<'py, PyAny> {
Py::from_owned_ptr_or_err(py, ptr).map(|obj| Self(py, ManuallyDrop::new(obj)))
}

/// Constructs a new `Bound<'py, PyAny>` from a pointer without checking for null.
///
/// # Safety
///
/// - `ptr` must be a valid pointer to a Python object
davidhewitt marked this conversation as resolved.
Show resolved Hide resolved
/// - `ptr` must be a strong/owned reference
pub(crate) unsafe fn from_owned_ptr_unchecked(
py: Python<'py>,
ptr: *mut ffi::PyObject,
) -> Self {
Self(py, ManuallyDrop::new(Py::from_owned_ptr_unchecked(ptr)))
}

/// Constructs a new `Bound<'py, PyAny>` from a pointer by creating a new Python reference.
/// Panics if `ptr` is null.
///
Expand Down Expand Up @@ -1630,6 +1643,15 @@ impl<T> Py<T> {
NonNull::new(ptr).map(|nonnull_ptr| Py(nonnull_ptr, PhantomData))
}

/// Constructs a new `Py<T>` instance by taking ownership of the given FFI pointer.
///
/// # Safety
///
/// - `ptr` must be a non-null pointer to a Python object or type `T`.
pub(crate) unsafe fn from_owned_ptr_unchecked(ptr: *mut ffi::PyObject) -> Self {
Py(NonNull::new_unchecked(ptr), PhantomData)
}

/// Create a `Py<T>` instance by creating a new reference from the given FFI pointer.
///
/// # Safety
Expand Down
4 changes: 2 additions & 2 deletions src/pycell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ where
inner: unsafe {
ManuallyDrop::new(self)
.as_ptr()
.assume_owned(py)
.assume_owned_unchecked(py)
.downcast_into_unchecked()
},
}
Expand Down Expand Up @@ -587,7 +587,7 @@ where
inner: unsafe {
ManuallyDrop::new(self)
.as_ptr()
.assume_owned(py)
.assume_owned_unchecked(py)
.downcast_into_unchecked()
},
}
Expand Down
6 changes: 5 additions & 1 deletion src/types/dict.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,11 @@ impl<'py> PyDictMethods<'py> for Bound<'py, PyDict> {
} {
std::os::raw::c_int::MIN..=-1 => Err(PyErr::fetch(py)),
0 => Ok(None),
1..=std::os::raw::c_int::MAX => Ok(Some(unsafe { result.assume_owned(py) })),
1..=std::os::raw::c_int::MAX => {
// Safety: PyDict_GetItemRef positive return value means the result is a valid
// owned reference
Ok(Some(unsafe { result.assume_owned_unchecked(py) }))
}
}
}

Expand Down
4 changes: 3 additions & 1 deletion src/types/weakref/anyref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,9 @@ impl<'py> PyWeakrefMethods<'py> for Bound<'py, PyWeakref> {
match unsafe { ffi::compat::PyWeakref_GetRef(self.as_ptr(), &mut obj) } {
std::os::raw::c_int::MIN..=-1 => panic!("The 'weakref' weak reference instance should be valid (non-null and actually a weakref reference)"),
0 => PyNone::get(self.py()).to_owned().into_any(),
1..=std::os::raw::c_int::MAX => unsafe { obj.assume_owned(self.py()) },
// Safety: positive return value from `PyWeakRef_GetRef` guarantees the return value is
// a valid strong reference.
1..=std::os::raw::c_int::MAX => unsafe { obj.assume_owned_unchecked(self.py()) },
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/types/weakref/proxy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,9 @@ impl<'py> PyWeakrefMethods<'py> for Bound<'py, PyWeakrefProxy> {
match unsafe { ffi::compat::PyWeakref_GetRef(self.as_ptr(), &mut obj) } {
std::os::raw::c_int::MIN..=-1 => panic!("The 'weakref.ProxyType' (or `weakref.CallableProxyType`) instance should be valid (non-null and actually a weakref reference)"),
0 => PyNone::get(self.py()).to_owned().into_any(),
1..=std::os::raw::c_int::MAX => unsafe { obj.assume_owned(self.py()) },
// Safety: positive return value from `PyWeakRef_GetRef` guarantees the return value is
// a valid strong reference.
1..=std::os::raw::c_int::MAX => unsafe { obj.assume_owned_unchecked(self.py()) },
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/types/weakref/reference.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,9 @@ impl<'py> PyWeakrefMethods<'py> for Bound<'py, PyWeakrefReference> {
match unsafe { ffi::compat::PyWeakref_GetRef(self.as_ptr(), &mut obj) } {
std::os::raw::c_int::MIN..=-1 => panic!("The 'weakref.ReferenceType' instance should be valid (non-null and actually a weakref reference)"),
0 => PyNone::get(self.py()).to_owned().into_any(),
1..=std::os::raw::c_int::MAX => unsafe { obj.assume_owned(self.py()) },
// Safety: positive return value from `PyWeakRef_GetRef` guarantees the return value is
// a valid strong reference.
1..=std::os::raw::c_int::MAX => unsafe { obj.assume_owned_unchecked(self.py()) },
}
}
}
Expand Down
Loading