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

deprecate from_borrowed_ptr methods #3915

Merged
merged 2 commits into from
Mar 1, 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
4 changes: 2 additions & 2 deletions pyo3-macros-backend/src/method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -513,15 +513,15 @@ impl<'a> FnSpec<'a> {
holders.pop().unwrap(); // does not actually use holder created by `self_arg`

quote! {{
let __guard = _pyo3::impl_::coroutine::RefGuard::<#cls>::new(py.from_borrowed_ptr::<_pyo3::types::PyAny>(_slf))?;
let __guard = _pyo3::impl_::coroutine::RefGuard::<#cls>::new(&_pyo3::impl_::pymethods::BoundRef::ref_from_ptr(py, &_slf))?;
async move { function(&__guard, #(#args),*).await }
}}
}
FnType::Fn(SelfType::Receiver { mutable: true, .. }) => {
holders.pop().unwrap(); // does not actually use holder created by `self_arg`

quote! {{
let mut __guard = _pyo3::impl_::coroutine::RefMutGuard::<#cls>::new(py.from_borrowed_ptr::<_pyo3::types::PyAny>(_slf))?;
let mut __guard = _pyo3::impl_::coroutine::RefMutGuard::<#cls>::new(&_pyo3::impl_::pymethods::BoundRef::ref_from_ptr(py, &_slf))?;
async move { function(&mut __guard, #(#args),*).await }
}}
}
Expand Down
33 changes: 33 additions & 0 deletions src/conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,7 @@ impl IntoPy<Py<PyTuple>> for () {
/// # Safety
///
/// See safety notes on individual functions.
#[deprecated(since = "0.21.0")]
pub unsafe trait FromPyPointer<'p>: Sized {
/// Convert from an arbitrary `PyObject`.
///
Expand Down Expand Up @@ -494,37 +495,69 @@ pub unsafe trait FromPyPointer<'p>: Sized {
/// # Safety
///
/// Implementations must ensure the object does not get freed during `'p` and avoid type confusion.
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(
since = "0.21.0",
note = "use `Py::from_borrowed_ptr_or_opt(py, ptr)` or `Bound::from_borrowed_ptr_or_opt(py, ptr)` instead"
)
)]
unsafe fn from_borrowed_ptr_or_opt(py: Python<'p>, ptr: *mut ffi::PyObject)
-> Option<&'p Self>;
/// Convert from an arbitrary borrowed `PyObject`.
///
/// # Safety
///
/// Relies on unsafe fn [`from_borrowed_ptr_or_opt`](#method.from_borrowed_ptr_or_opt).
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(
since = "0.21.0",
note = "use `Py::from_borrowed_ptr(py, ptr)` or `Bound::from_borrowed_ptr(py, ptr)` instead"
)
)]
unsafe fn from_borrowed_ptr_or_panic(py: Python<'p>, ptr: *mut ffi::PyObject) -> &'p Self {
#[allow(deprecated)]
Self::from_borrowed_ptr_or_opt(py, ptr).unwrap_or_else(|| err::panic_after_error(py))
}
/// Convert from an arbitrary borrowed `PyObject`.
///
/// # Safety
///
/// Relies on unsafe fn [`from_borrowed_ptr_or_opt`](#method.from_borrowed_ptr_or_opt).
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(
since = "0.21.0",
note = "use `Py::from_borrowed_ptr(py, ptr)` or `Bound::from_borrowed_ptr(py, ptr)` instead"
)
)]
unsafe fn from_borrowed_ptr(py: Python<'p>, ptr: *mut ffi::PyObject) -> &'p Self {
#[allow(deprecated)]
Self::from_borrowed_ptr_or_panic(py, ptr)
}
/// Convert from an arbitrary borrowed `PyObject`.
///
/// # Safety
///
/// Relies on unsafe fn [`from_borrowed_ptr_or_opt`](#method.from_borrowed_ptr_or_opt).
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(
since = "0.21.0",
note = "use `Py::from_borrowed_ptr_or_err(py, ptr)` or `Bound::from_borrowed_ptr_or_err(py, ptr)` instead"
)
)]
unsafe fn from_borrowed_ptr_or_err(
py: Python<'p>,
ptr: *mut ffi::PyObject,
) -> PyResult<&'p Self> {
#[allow(deprecated)]
Self::from_borrowed_ptr_or_opt(py, ptr).ok_or_else(|| err::PyErr::fetch(py))
}
}

#[allow(deprecated)]
unsafe impl<'p, T> FromPyPointer<'p> for T
where
T: 'p + crate::PyNativeType,
Expand Down
18 changes: 9 additions & 9 deletions src/impl_/coroutine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::{
coroutine::{cancel::ThrowCallback, Coroutine},
instance::Bound,
pyclass::boolean_struct::False,
types::PyString,
types::{PyAnyMethods, PyString},
IntoPy, Py, PyAny, PyCell, PyClass, PyErr, PyObject, PyResult, Python,
};

Expand Down Expand Up @@ -39,10 +39,10 @@ fn get_ptr<T: PyClass>(obj: &Py<T>) -> *mut T {
pub struct RefGuard<T: PyClass>(Py<T>);

impl<T: PyClass> RefGuard<T> {
pub fn new(obj: &PyAny) -> PyResult<Self> {
let owned: Py<T> = obj.extract()?;
mem::forget(owned.try_borrow(obj.py())?);
Ok(RefGuard(owned))
pub fn new(obj: &Bound<'_, PyAny>) -> PyResult<Self> {
let owned = obj.downcast::<T>()?;
mem::forget(owned.try_borrow()?);
Ok(RefGuard(owned.clone().unbind()))
}
}

Expand All @@ -67,10 +67,10 @@ impl<T: PyClass> Drop for RefGuard<T> {
pub struct RefMutGuard<T: PyClass<Frozen = False>>(Py<T>);

impl<T: PyClass<Frozen = False>> RefMutGuard<T> {
pub fn new(obj: &PyAny) -> PyResult<Self> {
let owned: Py<T> = obj.extract()?;
mem::forget(owned.try_borrow_mut(obj.py())?);
Ok(RefMutGuard(owned))
pub fn new(obj: &Bound<'_, PyAny>) -> PyResult<Self> {
let owned = obj.downcast::<T>()?;
mem::forget(owned.try_borrow_mut()?);
Ok(RefMutGuard(owned.clone().unbind()))
}
}

Expand Down
10 changes: 8 additions & 2 deletions src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,10 @@ impl<'py, T> Bound<'py, T> {
where
T: HasPyGilRef,
{
unsafe { self.py().from_borrowed_ptr(self.as_ptr()) }
#[allow(deprecated)]
unsafe {
self.py().from_borrowed_ptr(self.as_ptr())
}
}

/// Casts this `Bound<T>` as the corresponding "GIL Ref" type, registering the pointer on the
Expand Down Expand Up @@ -613,7 +616,10 @@ where
{
pub(crate) fn into_gil_ref(self) -> &'py T::AsRefTarget {
// Safety: self is a borrow over `'py`.
unsafe { self.py().from_borrowed_ptr(self.0.as_ptr()) }
#[allow(deprecated)]
unsafe {
self.py().from_borrowed_ptr(self.0.as_ptr())
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,9 +304,9 @@
//! [Features chapter of the guide]: https://pyo3.rs/latest/features.html#features-reference "Features Reference - PyO3 user guide"
//! [`Ungil`]: crate::marker::Ungil
pub use crate::class::*;
pub use crate::conversion::{AsPyPointer, FromPyObject, FromPyPointer, IntoPy, ToPyObject};
pub use crate::conversion::{AsPyPointer, FromPyObject, IntoPy, ToPyObject};
#[allow(deprecated)]
pub use crate::conversion::{PyTryFrom, PyTryInto};
pub use crate::conversion::{FromPyPointer, PyTryFrom, PyTryInto};
pub use crate::err::{
DowncastError, DowncastIntoError, PyDowncastError, PyErr, PyErrArguments, PyResult, ToPyErr,
};
Expand Down
42 changes: 30 additions & 12 deletions src/marker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,9 @@ use crate::types::{
PyAny, PyDict, PyEllipsis, PyModule, PyNone, PyNotImplemented, PyString, PyType,
};
use crate::version::PythonVersionInfo;
use crate::{
ffi, Bound, FromPyPointer, IntoPy, Py, PyNativeType, PyObject, PyTypeCheck, PyTypeInfo,
};
#[allow(deprecated)]
use crate::FromPyPointer;
use crate::{ffi, Bound, IntoPy, Py, PyNativeType, PyObject, PyTypeCheck, PyTypeInfo};
use std::ffi::{CStr, CString};
use std::marker::PhantomData;
use std::os::raw::c_int;
Expand Down Expand Up @@ -880,7 +880,7 @@ impl<'py> Python<'py> {
/// # Safety
///
/// Callers must ensure that ensure that the cast is valid.
#[allow(clippy::wrong_self_convention)]
#[allow(clippy::wrong_self_convention, deprecated)]
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(
Expand All @@ -892,7 +892,6 @@ impl<'py> Python<'py> {
where
T: FromPyPointer<'py>,
{
#[allow(deprecated)]
FromPyPointer::from_owned_ptr(self, ptr)
}

Expand All @@ -904,7 +903,7 @@ impl<'py> Python<'py> {
/// # Safety
///
/// Callers must ensure that ensure that the cast is valid.
#[allow(clippy::wrong_self_convention)]
#[allow(clippy::wrong_self_convention, deprecated)]
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(
Expand All @@ -916,7 +915,6 @@ impl<'py> Python<'py> {
where
T: FromPyPointer<'py>,
{
#[allow(deprecated)]
FromPyPointer::from_owned_ptr_or_err(self, ptr)
}

Expand All @@ -928,7 +926,7 @@ impl<'py> Python<'py> {
/// # Safety
///
/// Callers must ensure that ensure that the cast is valid.
#[allow(clippy::wrong_self_convention)]
#[allow(clippy::wrong_self_convention, deprecated)]
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(
Expand All @@ -940,7 +938,6 @@ impl<'py> Python<'py> {
where
T: FromPyPointer<'py>,
{
#[allow(deprecated)]
FromPyPointer::from_owned_ptr_or_opt(self, ptr)
}

Expand All @@ -951,7 +948,14 @@ impl<'py> Python<'py> {
/// # Safety
///
/// Callers must ensure that ensure that the cast is valid.
#[allow(clippy::wrong_self_convention)]
#[allow(clippy::wrong_self_convention, deprecated)]
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(
since = "0.21.0",
note = "use `Py::from_borrowed_ptr(py, ptr)` or `Bound::from_borrowed_ptr(py, ptr)` instead"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think technically the equivalent method would be on Borrowed, but I thought we might want to nudge user here to the safer option.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed I think Borrowed::from_ptr is hard to get right, as we keep finding by not using it in our own macro code!

)
)]
pub unsafe fn from_borrowed_ptr<T>(self, ptr: *mut ffi::PyObject) -> &'py T
where
T: FromPyPointer<'py>,
Expand All @@ -966,7 +970,14 @@ impl<'py> Python<'py> {
/// # Safety
///
/// Callers must ensure that ensure that the cast is valid.
#[allow(clippy::wrong_self_convention)]
#[allow(clippy::wrong_self_convention, deprecated)]
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(
since = "0.21.0",
note = "use `Py::from_borrowed_ptr_or_err(py, ptr)` or `Bound::from_borrowed_ptr_or_err(py, ptr)` instead"
)
)]
pub unsafe fn from_borrowed_ptr_or_err<T>(self, ptr: *mut ffi::PyObject) -> PyResult<&'py T>
where
T: FromPyPointer<'py>,
Expand All @@ -981,7 +992,14 @@ impl<'py> Python<'py> {
/// # Safety
///
/// Callers must ensure that ensure that the cast is valid.
#[allow(clippy::wrong_self_convention)]
#[allow(clippy::wrong_self_convention, deprecated)]
#[cfg_attr(
not(feature = "gil-refs"),
deprecated(
since = "0.21.0",
note = "use `Py::from_borrowed_ptr_or_opt(py, ptr)` or `Bound::from_borrowed_ptr_or_opt(py, ptr)` instead"
)
)]
pub unsafe fn from_borrowed_ptr_or_opt<T>(self, ptr: *mut ffi::PyObject) -> Option<&'py T>
where
T: FromPyPointer<'py>,
Expand Down
15 changes: 12 additions & 3 deletions src/pycell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
//! ) -> *mut pyo3::ffi::PyObject {
//! use :: pyo3 as _pyo3;
//! _pyo3::impl_::trampoline::noargs(_slf, _args, |py, _slf| {
//! # #[allow(deprecated)]
//! let _cell = py
//! .from_borrowed_ptr::<_pyo3::PyAny>(_slf)
//! .downcast::<_pyo3::PyCell<Number>>()?;
Expand Down Expand Up @@ -191,6 +192,8 @@
//! [guide]: https://pyo3.rs/latest/class.html#pycell-and-interior-mutability "PyCell and interior mutability"
//! [Interior Mutability]: https://doc.rust-lang.org/book/ch15-05-interior-mutability.html "RefCell<T> and the Interior Mutability Pattern - The Rust Programming Language"

#[allow(deprecated)]
use crate::conversion::FromPyPointer;
use crate::exceptions::PyRuntimeError;
use crate::ffi_ptr_ext::FfiPtrExt;
use crate::impl_::pyclass::{
Expand All @@ -205,7 +208,7 @@ use crate::type_object::{PyLayout, PySizedLayout};
use crate::types::any::PyAnyMethods;
use crate::types::PyAny;
use crate::{
conversion::{AsPyPointer, FromPyPointer, ToPyObject},
conversion::{AsPyPointer, ToPyObject},
type_object::get_tp_free,
PyTypeInfo,
};
Expand Down Expand Up @@ -573,15 +576,21 @@ impl<T: PyClass> ToPyObject for &PyCell<T> {

impl<T: PyClass> AsRef<PyAny> for PyCell<T> {
fn as_ref(&self) -> &PyAny {
unsafe { self.py().from_borrowed_ptr(self.as_ptr()) }
#[allow(deprecated)]
unsafe {
self.py().from_borrowed_ptr(self.as_ptr())
}
}
}

impl<T: PyClass> Deref for PyCell<T> {
type Target = PyAny;

fn deref(&self) -> &PyAny {
unsafe { self.py().from_borrowed_ptr(self.as_ptr()) }
#[allow(deprecated)]
unsafe {
self.py().from_borrowed_ptr(self.as_ptr())
}
}
}

Expand Down
5 changes: 4 additions & 1 deletion src/type_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,10 @@ pub unsafe trait PyTypeInfo: Sized + HasPyGilRef {
fn type_object(py: Python<'_>) -> &PyType {
// This isn't implemented in terms of `type_object_bound` because this just borrowed the
// object, for legacy reasons.
unsafe { py.from_borrowed_ptr(Self::type_object_raw(py) as _) }
#[allow(deprecated)]
unsafe {
py.from_borrowed_ptr(Self::type_object_raw(py) as _)
}
}

/// Returns the safe abstraction over the type object.
Expand Down
5 changes: 4 additions & 1 deletion src/types/boolobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ impl PyBool {
)]
#[inline]
pub fn new(py: Python<'_>, val: bool) -> &PyBool {
unsafe { py.from_borrowed_ptr(if val { ffi::Py_True() } else { ffi::Py_False() }) }
#[allow(deprecated)]
unsafe {
py.from_borrowed_ptr(if val { ffi::Py_True() } else { ffi::Py_False() })
}
}

/// Depending on `val`, returns `true` or `false`.
Expand Down
Loading