From 1c5265e1c263fcb194f7226cabeb93f59f9dbcd7 Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Fri, 1 Mar 2024 21:51:53 +0100 Subject: [PATCH] deprecate `from_borrowed_ptr` methods (#3915) * deprecate `from_borrowed_ptr` methods This deprecates the methods on the `Python` marker, aswell as `FromPyPointer` * use `BoundRef` to defer ref cnt inc until after the error case --- pyo3-macros-backend/src/method.rs | 4 +-- src/conversion.rs | 33 ++++++++++++++++++++++++ src/impl_/coroutine.rs | 18 ++++++------- src/instance.rs | 10 ++++++-- src/lib.rs | 4 +-- src/marker.rs | 42 ++++++++++++++++++++++--------- src/pycell.rs | 15 ++++++++--- src/type_object.rs | 5 +++- src/types/boolobject.rs | 5 +++- 9 files changed, 104 insertions(+), 32 deletions(-) diff --git a/pyo3-macros-backend/src/method.rs b/pyo3-macros-backend/src/method.rs index 6ee87fb9..42f22204 100644 --- a/pyo3-macros-backend/src/method.rs +++ b/pyo3-macros-backend/src/method.rs @@ -513,7 +513,7 @@ 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 } }} } @@ -521,7 +521,7 @@ impl<'a> FnSpec<'a> { 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 } }} } diff --git a/src/conversion.rs b/src/conversion.rs index fc407fa5..986ac7c5 100644 --- a/src/conversion.rs +++ b/src/conversion.rs @@ -425,6 +425,7 @@ impl IntoPy> for () { /// # Safety /// /// See safety notes on individual functions. +#[deprecated(since = "0.21.0")] pub unsafe trait FromPyPointer<'p>: Sized { /// Convert from an arbitrary `PyObject`. /// @@ -494,6 +495,13 @@ 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`. @@ -501,7 +509,15 @@ pub unsafe trait FromPyPointer<'p>: Sized { /// # 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`. @@ -509,7 +525,15 @@ pub unsafe trait FromPyPointer<'p>: Sized { /// # 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`. @@ -517,14 +541,23 @@ pub unsafe trait FromPyPointer<'p>: Sized { /// # 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, diff --git a/src/impl_/coroutine.rs b/src/impl_/coroutine.rs index a59eddd0..9be95ba3 100644 --- a/src/impl_/coroutine.rs +++ b/src/impl_/coroutine.rs @@ -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, }; @@ -39,10 +39,10 @@ fn get_ptr(obj: &Py) -> *mut T { pub struct RefGuard(Py); impl RefGuard { - pub fn new(obj: &PyAny) -> PyResult { - let owned: Py = obj.extract()?; - mem::forget(owned.try_borrow(obj.py())?); - Ok(RefGuard(owned)) + pub fn new(obj: &Bound<'_, PyAny>) -> PyResult { + let owned = obj.downcast::()?; + mem::forget(owned.try_borrow()?); + Ok(RefGuard(owned.clone().unbind())) } } @@ -67,10 +67,10 @@ impl Drop for RefGuard { pub struct RefMutGuard>(Py); impl> RefMutGuard { - pub fn new(obj: &PyAny) -> PyResult { - let owned: Py = obj.extract()?; - mem::forget(owned.try_borrow_mut(obj.py())?); - Ok(RefMutGuard(owned)) + pub fn new(obj: &Bound<'_, PyAny>) -> PyResult { + let owned = obj.downcast::()?; + mem::forget(owned.try_borrow_mut()?); + Ok(RefMutGuard(owned.clone().unbind())) } } diff --git a/src/instance.rs b/src/instance.rs index 65c5ee3b..bc655caa 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -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` as the corresponding "GIL Ref" type, registering the pointer on the @@ -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()) + } } } diff --git a/src/lib.rs b/src/lib.rs index c2591cec..8c0e6269 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -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, }; diff --git a/src/marker.rs b/src/marker.rs index 5609601f..9b3d7329 100644 --- a/src/marker.rs +++ b/src/marker.rs @@ -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; @@ -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( @@ -892,7 +892,6 @@ impl<'py> Python<'py> { where T: FromPyPointer<'py>, { - #[allow(deprecated)] FromPyPointer::from_owned_ptr(self, ptr) } @@ -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( @@ -916,7 +915,6 @@ impl<'py> Python<'py> { where T: FromPyPointer<'py>, { - #[allow(deprecated)] FromPyPointer::from_owned_ptr_or_err(self, ptr) } @@ -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( @@ -940,7 +938,6 @@ impl<'py> Python<'py> { where T: FromPyPointer<'py>, { - #[allow(deprecated)] FromPyPointer::from_owned_ptr_or_opt(self, ptr) } @@ -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" + ) + )] pub unsafe fn from_borrowed_ptr(self, ptr: *mut ffi::PyObject) -> &'py T where T: FromPyPointer<'py>, @@ -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(self, ptr: *mut ffi::PyObject) -> PyResult<&'py T> where T: FromPyPointer<'py>, @@ -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(self, ptr: *mut ffi::PyObject) -> Option<&'py T> where T: FromPyPointer<'py>, diff --git a/src/pycell.rs b/src/pycell.rs index 29fd1b37..9d00de95 100644 --- a/src/pycell.rs +++ b/src/pycell.rs @@ -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>()?; @@ -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 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::{ @@ -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, }; @@ -573,7 +576,10 @@ impl ToPyObject for &PyCell { impl AsRef for PyCell { 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()) + } } } @@ -581,7 +587,10 @@ impl Deref for PyCell { 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()) + } } } diff --git a/src/type_object.rs b/src/type_object.rs index 994781b3..318810b5 100644 --- a/src/type_object.rs +++ b/src/type_object.rs @@ -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. diff --git a/src/types/boolobject.rs b/src/types/boolobject.rs index 3a2f60f6..52e4c886 100644 --- a/src/types/boolobject.rs +++ b/src/types/boolobject.rs @@ -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`.