Skip to content

Commit

Permalink
add Bound::from_owned_ptr_unchecked (crate-private for now) (#4596)
Browse files Browse the repository at this point in the history
* add `Bound::from_owned_ptr_unchecked` (crate-private for now)

* Update src/instance.rs

Co-authored-by: Icxolu <10486322+Icxolu@users.noreply.github.com>

---------

Co-authored-by: Icxolu <10486322+Icxolu@users.noreply.github.com>
  • Loading branch information
davidhewitt and Icxolu authored Oct 4, 2024
1 parent c766108 commit 8b049d1
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 6 deletions.
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
/// - `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

0 comments on commit 8b049d1

Please sign in to comment.