From 70a7aa808db038ef9c9670f79b50fd67082d4918 Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Sun, 3 Mar 2024 15:47:25 +0100 Subject: [PATCH] deprecate the use of `PyCell` in favor of `Bound` and `Py` (#3916) * deprecate the use of `PyCell` in favor of `Bound` and `Py` * update `FromPyObject` for `T: PyClass + Clone` impl * move `PyCell` deprecation to the `gil-refs` feature gate and add a migration note --- guide/src/class.md | 1 + guide/src/types.md | 1 + pyo3-macros-backend/src/pyclass.rs | 1 + src/conversion.rs | 15 ++++++------- src/conversions/std/array.rs | 2 +- src/impl_/pymethods.rs | 7 +++--- src/instance.rs | 7 +++--- src/lib.rs | 4 +++- src/prelude.rs | 4 +++- src/pycell.rs | 22 ++++++++++++++++++- src/pyclass.rs | 7 +++--- src/types/pysuper.rs | 2 +- tests/test_buffer_protocol.rs | 10 ++++----- tests/test_class_new.rs | 4 ++-- tests/test_pyself.rs | 11 ++++++---- tests/test_super.rs | 6 ++--- tests/test_text_signature.rs | 2 +- tests/test_various.rs | 2 +- tests/ui/invalid_frozen_pyclass_borrow.rs | 2 +- tests/ui/invalid_frozen_pyclass_borrow.stderr | 6 ++--- tests/ui/pyclass_send.rs | 6 ++--- 21 files changed, 77 insertions(+), 45 deletions(-) diff --git a/guide/src/class.md b/guide/src/class.md index 3096c9f7..b9b47420 100644 --- a/guide/src/class.md +++ b/guide/src/class.md @@ -1230,6 +1230,7 @@ struct MyClass { impl pyo3::types::DerefToPyAny for MyClass {} +# #[allow(deprecated)] unsafe impl pyo3::type_object::HasPyGilRef for MyClass { type AsRefTarget = pyo3::PyCell; } diff --git a/guide/src/types.md b/guide/src/types.md index 51ea10f4..372c6c86 100644 --- a/guide/src/types.md +++ b/guide/src/types.md @@ -96,6 +96,7 @@ For a `&PyAny` object reference `any` where the underlying object is a `#[pyclas let obj: &PyAny = Py::new(py, MyClass {})?.into_ref(py); // To &PyCell with PyAny::downcast +# #[allow(deprecated)] let _: &PyCell = obj.downcast()?; // To Py (aka PyObject) with .into() diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index 5ec6ac17..734b9565 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -1281,6 +1281,7 @@ fn impl_pytypeinfo( }; quote! { + #[allow(deprecated)] unsafe impl _pyo3::type_object::HasPyGilRef for #cls { type AsRefTarget = _pyo3::PyCell; } diff --git a/src/conversion.rs b/src/conversion.rs index 986ac7c5..68633d19 100644 --- a/src/conversion.rs +++ b/src/conversion.rs @@ -6,9 +6,7 @@ use crate::pyclass::boolean_struct::False; use crate::type_object::PyTypeInfo; use crate::types::any::PyAnyMethods; use crate::types::PyTuple; -use crate::{ - ffi, gil, Bound, Py, PyAny, PyCell, PyClass, PyNativeType, PyObject, PyRef, PyRefMut, Python, -}; +use crate::{ffi, gil, Bound, Py, PyAny, PyClass, PyNativeType, PyObject, PyRef, PyRefMut, Python}; use std::ptr::NonNull; /// Returns a borrowed pointer to a Python object. @@ -265,7 +263,8 @@ where } } -impl<'py, T> FromPyObject<'py> for &'py PyCell +#[allow(deprecated)] +impl<'py, T> FromPyObject<'py> for &'py crate::PyCell where T: PyClass, { @@ -278,9 +277,9 @@ impl FromPyObject<'_> for T where T: PyClass + Clone, { - fn extract(obj: &PyAny) -> PyResult { - let cell: &PyCell = obj.downcast()?; - Ok(unsafe { cell.try_borrow_unguarded()?.clone() }) + fn extract_bound(obj: &Bound<'_, PyAny>) -> PyResult { + let bound = obj.downcast::()?; + Ok(bound.try_borrow()?.clone()) } } @@ -389,7 +388,7 @@ mod implementations { } } - impl<'v, T> PyTryFrom<'v> for PyCell + impl<'v, T> PyTryFrom<'v> for crate::PyCell where T: 'v + PyClass, { diff --git a/src/conversions/std/array.rs b/src/conversions/std/array.rs index d901ff4e..bf21244e 100644 --- a/src/conversions/std/array.rs +++ b/src/conversions/std/array.rs @@ -240,7 +240,7 @@ mod tests { let array: [Foo; 8] = [Foo, Foo, Foo, Foo, Foo, Foo, Foo, Foo]; let pyobject = array.into_py(py); let list = pyobject.downcast_bound::(py).unwrap(); - let _cell: &crate::PyCell = list.get_item(4).unwrap().extract().unwrap(); + let _bound = list.get_item(4).unwrap().downcast::().unwrap(); }); } diff --git a/src/impl_/pymethods.rs b/src/impl_/pymethods.rs index bc950baf..df89dba7 100644 --- a/src/impl_/pymethods.rs +++ b/src/impl_/pymethods.rs @@ -7,8 +7,8 @@ use crate::pycell::{PyBorrowError, PyBorrowMutError}; use crate::pyclass::boolean_struct::False; use crate::types::{any::PyAnyMethods, PyModule, PyType}; use crate::{ - ffi, Borrowed, Bound, DowncastError, Py, PyAny, PyCell, PyClass, PyClassInitializer, PyErr, - PyObject, PyRef, PyRefMut, PyResult, PyTraverseError, PyTypeCheck, PyVisit, Python, + ffi, Borrowed, Bound, DowncastError, Py, PyAny, PyClass, PyClassInitializer, PyErr, PyObject, + PyRef, PyRefMut, PyResult, PyTraverseError, PyTypeCheck, PyVisit, Python, }; use std::borrow::Cow; use std::ffi::CStr; @@ -518,7 +518,8 @@ impl<'a> From> for &'a PyModule { } } -impl<'a, 'py, T: PyClass> From> for &'a PyCell { +#[allow(deprecated)] +impl<'a, 'py, T: PyClass> From> for &'a crate::PyCell { #[inline] fn from(bound: BoundRef<'a, 'py, T>) -> Self { bound.0.as_gil_ref() diff --git a/src/instance.rs b/src/instance.rs index 66c530d5..cef06062 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -1,6 +1,6 @@ use crate::err::{self, PyDowncastError, PyErr, PyResult}; use crate::impl_::pycell::PyClassObject; -use crate::pycell::{PyBorrowError, PyBorrowMutError, PyCell}; +use crate::pycell::{PyBorrowError, PyBorrowMutError}; use crate::pyclass::boolean_struct::{False, True}; use crate::type_object::HasPyGilRef; use crate::types::{any::PyAnyMethods, string::PyStringMethods, typeobject::PyTypeMethods}; @@ -1698,11 +1698,12 @@ impl std::convert::From> for Py { } // `&PyCell` can be converted to `Py` -impl std::convert::From<&PyCell> for Py +#[allow(deprecated)] +impl std::convert::From<&crate::PyCell> for Py where T: PyClass, { - fn from(cell: &PyCell) -> Self { + fn from(cell: &crate::PyCell) -> Self { cell.as_borrowed().to_owned().unbind() } } diff --git a/src/lib.rs b/src/lib.rs index 8c0e6269..26d2ec55 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -315,7 +315,9 @@ pub use crate::gil::GILPool; pub use crate::gil::{prepare_freethreaded_python, with_embedded_python_interpreter}; pub use crate::instance::{Borrowed, Bound, Py, PyNativeType, PyObject}; pub use crate::marker::Python; -pub use crate::pycell::{PyCell, PyRef, PyRefMut}; +#[allow(deprecated)] +pub use crate::pycell::PyCell; +pub use crate::pycell::{PyRef, PyRefMut}; pub use crate::pyclass::PyClass; pub use crate::pyclass_init::PyClassInitializer; pub use crate::type_object::{PyTypeCheck, PyTypeInfo}; diff --git a/src/prelude.rs b/src/prelude.rs index dcf4fe71..ef42b270 100644 --- a/src/prelude.rs +++ b/src/prelude.rs @@ -14,7 +14,9 @@ pub use crate::conversion::{PyTryFrom, PyTryInto}; pub use crate::err::{PyErr, PyResult}; pub use crate::instance::{Borrowed, Bound, Py, PyObject}; pub use crate::marker::Python; -pub use crate::pycell::{PyCell, PyRef, PyRefMut}; +#[allow(deprecated)] +pub use crate::pycell::PyCell; +pub use crate::pycell::{PyRef, PyRefMut}; pub use crate::pyclass_init::PyClassInitializer; pub use crate::types::{PyAny, PyModule}; pub use crate::PyNativeType; diff --git a/src/pycell.rs b/src/pycell.rs index 43c75314..397e7b6a 100644 --- a/src/pycell.rs +++ b/src/pycell.rs @@ -62,7 +62,7 @@ //! ) -> *mut pyo3::ffi::PyObject { //! use :: pyo3 as _pyo3; //! _pyo3::impl_::trampoline::noargs(_slf, _args, |py, _slf| { -//! # #[allow(deprecated)] +//! # #[allow(deprecated)] //! let _cell = py //! .from_borrowed_ptr::<_pyo3::PyAny>(_slf) //! .downcast::<_pyo3::PyCell>()?; @@ -154,6 +154,7 @@ //! # pub struct Number { //! # inner: u32, //! # } +//! # #[allow(deprecated)] //! #[pyfunction] //! fn swap_numbers(a: &PyCell, b: &PyCell) { //! // Check that the pointers are unequal @@ -250,13 +251,22 @@ use self::impl_::{PyClassObject, PyClassObjectLayout}; /// ``` /// For more information on how, when and why (not) to use `PyCell` please see the /// [module-level documentation](self). +#[cfg_attr( + not(feature = "gil-refs"), + deprecated( + since = "0.21.0", + note = "`PyCell` was merged into `Bound`, use that instead; see the migration guide for more info" + ) +)] #[repr(transparent)] pub struct PyCell(PyClassObject); +#[allow(deprecated)] unsafe impl PyNativeType for PyCell { type AsRefSource = T; } +#[allow(deprecated)] impl PyCell { /// Makes a new `PyCell` on the Python heap and return the reference to it. /// @@ -478,9 +488,12 @@ impl PyCell { } } +#[allow(deprecated)] unsafe impl PyLayout for PyCell {} +#[allow(deprecated)] impl PySizedLayout for PyCell {} +#[allow(deprecated)] impl PyTypeCheck for PyCell where T: PyClass, @@ -492,18 +505,21 @@ where } } +#[allow(deprecated)] unsafe impl AsPyPointer for PyCell { fn as_ptr(&self) -> *mut ffi::PyObject { (self as *const _) as *mut _ } } +#[allow(deprecated)] impl ToPyObject for &PyCell { fn to_object(&self, py: Python<'_>) -> PyObject { unsafe { PyObject::from_borrowed_ptr(py, self.as_ptr()) } } } +#[allow(deprecated)] impl AsRef for PyCell { fn as_ref(&self) -> &PyAny { #[allow(deprecated)] @@ -513,6 +529,7 @@ impl AsRef for PyCell { } } +#[allow(deprecated)] impl Deref for PyCell { type Target = PyAny; @@ -524,6 +541,7 @@ impl Deref for PyCell { } } +#[allow(deprecated)] impl fmt::Debug for PyCell { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self.try_borrow() { @@ -748,6 +766,7 @@ impl IntoPy for &'_ PyRef<'_, T> { } } +#[allow(deprecated)] impl<'a, T: PyClass> std::convert::TryFrom<&'a PyCell> for crate::PyRef<'a, T> { type Error = PyBorrowError; fn try_from(cell: &'a crate::PyCell) -> Result { @@ -905,6 +924,7 @@ unsafe impl<'a, T: PyClass> AsPyPointer for PyRefMut<'a, T> { } } +#[allow(deprecated)] impl<'a, T: PyClass> std::convert::TryFrom<&'a PyCell> for crate::PyRefMut<'a, T> { diff --git a/src/pyclass.rs b/src/pyclass.rs index eb4a5595..b9b01cac 100644 --- a/src/pyclass.rs +++ b/src/pyclass.rs @@ -1,7 +1,7 @@ //! `PyClass` and related traits. use crate::{ - callback::IntoPyCallbackOutput, ffi, impl_::pyclass::PyClassImpl, IntoPy, PyCell, PyObject, - PyResult, PyTypeInfo, Python, + callback::IntoPyCallbackOutput, ffi, impl_::pyclass::PyClassImpl, IntoPy, PyObject, PyResult, + PyTypeInfo, Python, }; use std::{cmp::Ordering, os::raw::c_int}; @@ -15,7 +15,8 @@ pub use self::gc::{PyTraverseError, PyVisit}; /// /// The `#[pyclass]` attribute implements this trait for your Rust struct - /// you shouldn't implement this trait directly. -pub trait PyClass: PyTypeInfo> + PyClassImpl { +#[allow(deprecated)] +pub trait PyClass: PyTypeInfo> + PyClassImpl { /// Whether the pyclass is frozen. /// /// This can be enabled via `#[pyclass(frozen)]`. diff --git a/src/types/pysuper.rs b/src/types/pysuper.rs index 261a91f2..0f1a4744 100644 --- a/src/types/pysuper.rs +++ b/src/types/pysuper.rs @@ -62,7 +62,7 @@ impl PySuper { /// (SubClass {}, BaseClass::new()) /// } /// - /// fn method(self_: &PyCell) -> PyResult<&PyAny> { + /// fn method<'py>(self_: &Bound<'py, Self>) -> PyResult> { /// let super_ = self_.py_super()?; /// super_.call_method("method", (), None) /// } diff --git a/tests/test_buffer_protocol.rs b/tests/test_buffer_protocol.rs index 85ff6a40..dca90080 100644 --- a/tests/test_buffer_protocol.rs +++ b/tests/test_buffer_protocol.rs @@ -24,11 +24,11 @@ struct TestBufferClass { #[pymethods] impl TestBufferClass { unsafe fn __getbuffer__( - slf: &PyCell, + slf: Bound<'_, Self>, view: *mut ffi::Py_buffer, flags: c_int, ) -> PyResult<()> { - fill_view_from_readonly_data(view, flags, &slf.borrow().vec, slf) + fill_view_from_readonly_data(view, flags, &slf.borrow().vec, slf.into_any()) } unsafe fn __releasebuffer__(&self, view: *mut ffi::Py_buffer) { @@ -105,12 +105,12 @@ fn test_releasebuffer_unraisable_error() { #[pymethods] impl ReleaseBufferError { unsafe fn __getbuffer__( - slf: &PyCell, + slf: Bound<'_, Self>, view: *mut ffi::Py_buffer, flags: c_int, ) -> PyResult<()> { static BUF_BYTES: &[u8] = b"hello world"; - fill_view_from_readonly_data(view, flags, BUF_BYTES, slf) + fill_view_from_readonly_data(view, flags, BUF_BYTES, slf.into_any()) } unsafe fn __releasebuffer__(&self, _view: *mut ffi::Py_buffer) -> PyResult<()> { @@ -145,7 +145,7 @@ unsafe fn fill_view_from_readonly_data( view: *mut ffi::Py_buffer, flags: c_int, data: &[u8], - owner: &PyAny, + owner: Bound<'_, PyAny>, ) -> PyResult<()> { if view.is_null() { return Err(PyBufferError::new_err("View is null")); diff --git a/tests/test_class_new.rs b/tests/test_class_new.rs index 9e16631b..161c60e9 100644 --- a/tests/test_class_new.rs +++ b/tests/test_class_new.rs @@ -23,7 +23,7 @@ fn empty_class_with_new() { assert!(typeobj .call((), None) .unwrap() - .downcast::>() + .downcast::() .is_ok()); // Calling with arbitrary args or kwargs is not ok @@ -52,7 +52,7 @@ fn unit_class_with_new() { assert!(typeobj .call((), None) .unwrap() - .downcast::>() + .downcast::() .is_ok()); }); } diff --git a/tests/test_pyself.rs b/tests/test_pyself.rs index fa736f68..901f52de 100644 --- a/tests/test_pyself.rs +++ b/tests/test_pyself.rs @@ -18,15 +18,18 @@ struct Reader { #[pymethods] impl Reader { - fn clone_ref(slf: &PyCell) -> &PyCell { + fn clone_ref<'a, 'py>(slf: &'a Bound<'py, Self>) -> &'a Bound<'py, Self> { slf } - fn clone_ref_with_py<'py>(slf: &'py PyCell, _py: Python<'py>) -> &'py PyCell { + fn clone_ref_with_py<'a, 'py>( + slf: &'a Bound<'py, Self>, + _py: Python<'py>, + ) -> &'a Bound<'py, Self> { slf } - fn get_iter(slf: &PyCell, keys: Py) -> Iter { + fn get_iter(slf: &Bound<'_, Self>, keys: Py) -> Iter { Iter { - reader: slf.into(), + reader: slf.clone().unbind(), keys, idx: 0, } diff --git a/tests/test_super.rs b/tests/test_super.rs index 8dcedf80..3647e7d5 100644 --- a/tests/test_super.rs +++ b/tests/test_super.rs @@ -29,14 +29,14 @@ impl SubClass { (SubClass {}, BaseClass::new()) } - fn method(self_: &PyCell) -> PyResult<&PyAny> { + fn method<'py>(self_: &Bound<'py, Self>) -> PyResult> { let super_ = self_.py_super()?; super_.call_method("method", (), None) } - fn method_super_new(self_: &PyCell) -> PyResult<&PyAny> { + fn method_super_new<'py>(self_: &Bound<'py, Self>) -> PyResult> { #[cfg_attr(not(feature = "gil-refs"), allow(deprecated))] - let super_ = PySuper::new(self_.get_type(), self_)?; + let super_ = PySuper::new_bound(&self_.get_type(), self_)?; super_.call_method("method", (), None) } } diff --git a/tests/test_text_signature.rs b/tests/test_text_signature.rs index 6b56999c..0b93500d 100644 --- a/tests/test_text_signature.rs +++ b/tests/test_text_signature.rs @@ -367,7 +367,7 @@ fn test_methods() { let _ = a; } #[pyo3(text_signature = "($self, b)")] - fn pyself_method(_this: &PyCell, b: i32) { + fn pyself_method(_this: &Bound<'_, Self>, b: i32) { let _ = b; } #[classmethod] diff --git a/tests/test_various.rs b/tests/test_various.rs index 6f1bfd90..250f3983 100644 --- a/tests/test_various.rs +++ b/tests/test_various.rs @@ -124,7 +124,7 @@ impl PickleSupport { } pub fn __reduce__<'py>( - slf: &'py PyCell, + slf: &Bound<'py, Self>, py: Python<'py>, ) -> PyResult<(PyObject, Bound<'py, PyTuple>, PyObject)> { let cls = slf.to_object(py).getattr(py, "__class__")?; diff --git a/tests/ui/invalid_frozen_pyclass_borrow.rs b/tests/ui/invalid_frozen_pyclass_borrow.rs index aa8969ab..6379a870 100644 --- a/tests/ui/invalid_frozen_pyclass_borrow.rs +++ b/tests/ui/invalid_frozen_pyclass_borrow.rs @@ -29,7 +29,7 @@ fn py_get_of_mutable_class_fails(class: Py) { class.get(); } -fn pyclass_get_of_mutable_class_fails(class: &PyCell) { +fn pyclass_get_of_mutable_class_fails(class: &Bound<'_, MutableBase>) { class.get(); } diff --git a/tests/ui/invalid_frozen_pyclass_borrow.stderr b/tests/ui/invalid_frozen_pyclass_borrow.stderr index 1a8e4596..3acfbeb1 100644 --- a/tests/ui/invalid_frozen_pyclass_borrow.stderr +++ b/tests/ui/invalid_frozen_pyclass_borrow.stderr @@ -67,11 +67,11 @@ error[E0271]: type mismatch resolving `::Frozen == True` 33 | class.get(); | ^^^ expected `True`, found `False` | -note: required by a bound in `pyo3::PyCell::::get` - --> src/pycell.rs +note: required by a bound in `pyo3::Bound::<'py, T>::get` + --> src/instance.rs | | pub fn get(&self) -> &T | --- required by a bound in this associated function | where | T: PyClass + Sync, - | ^^^^^^^^^^^^^ required by this bound in `PyCell::::get` + | ^^^^^^^^^^^^^ required by this bound in `Bound::<'py, T>::get` diff --git a/tests/ui/pyclass_send.rs b/tests/ui/pyclass_send.rs index 53330274..2747c2cb 100644 --- a/tests/ui/pyclass_send.rs +++ b/tests/ui/pyclass_send.rs @@ -8,15 +8,15 @@ struct NotThreadSafe { fn main() { let obj = Python::with_gil(|py| { - PyCell::new(py, NotThreadSafe { data: Rc::new(5) }) + Bound::new(py, NotThreadSafe { data: Rc::new(5) }) .unwrap() - .to_object(py) + .unbind(py) }); std::thread::spawn(move || { Python::with_gil(|py| { // Uh oh, moved Rc to a new thread! - let c: &PyCell = obj.as_ref(py).downcast().unwrap(); + let c = obj.bind(py).downcast::().unwrap(); assert_eq!(*c.borrow().data, 5); })