From 56683ed553dfa413cc623425c655514b69c3992b Mon Sep 17 00:00:00 2001 From: Lily Foote Date: Thu, 29 Feb 2024 07:15:34 +0000 Subject: [PATCH] deprecate Py::as_ref (#3864) * Deprecate Py::as_ref * Reword as_ref deprecation note Co-authored-by: David Hewitt * Tidy up remaining uses of Py::as_ref Co-authored-by: David Hewitt * Pass hello into println! explicitly --------- Co-authored-by: David Hewitt --- guide/src/class.md | 1 + guide/src/memory.md | 4 +- guide/src/performance.md | 10 +++- guide/src/trait_bounds.md | 15 ++++++ guide/src/types.md | 4 ++ src/impl_/coroutine.rs | 12 ++++- src/instance.rs | 9 ++++ src/pycell/impl_.rs | 50 +++++++++---------- src/pyclass.rs | 16 +++++- tests/ui/invalid_frozen_pyclass_borrow.rs | 4 +- tests/ui/invalid_frozen_pyclass_borrow.stderr | 28 +++++------ 11 files changed, 105 insertions(+), 48 deletions(-) diff --git a/guide/src/class.md b/guide/src/class.md index 1e752700..cc254973 100644 --- a/guide/src/class.md +++ b/guide/src/class.md @@ -256,6 +256,7 @@ fn return_myclass() -> Py { let obj = return_myclass(); Python::with_gil(|py| { + #[allow(deprecated)] // as_ref is part of the deprecated "GIL Refs" API. let cell = obj.as_ref(py); // Py::as_ref returns &PyCell let obj_ref = cell.borrow(); // Get PyRef assert_eq!(obj_ref.num, 1); diff --git a/guide/src/memory.md b/guide/src/memory.md index 2f5e5d9b..46136e3f 100644 --- a/guide/src/memory.md +++ b/guide/src/memory.md @@ -171,7 +171,9 @@ let hello: Py = Python::with_gil(|py| { // Do some stuff... // Now sometime later in the program we want to access `hello`. Python::with_gil(|py| { - println!("Python says: {}", hello.as_ref(py)); + #[allow(deprecated)] // as_ref is part of the deprecated "GIL Refs" API. + let hello = hello.as_ref(py); + println!("Python says: {}", hello); }); // Now we're done with `hello`. drop(hello); // Memory *not* released here. diff --git a/guide/src/performance.md b/guide/src/performance.md index 23fb59c4..fe362bed 100644 --- a/guide/src/performance.md +++ b/guide/src/performance.md @@ -70,7 +70,11 @@ struct FooRef<'a>(&'a PyList); impl PartialEq for FooRef<'_> { fn eq(&self, other: &Foo) -> bool { - Python::with_gil(|py| self.0.len() == other.0.as_ref(py).len()) + Python::with_gil(|py| { + #[allow(deprecated)] // as_ref is part of the deprecated "GIL Refs" API. + let len = other.0.as_ref(py).len(); + self.0.len() == len + }) } } ``` @@ -88,7 +92,9 @@ impl PartialEq for FooRef<'_> { fn eq(&self, other: &Foo) -> bool { // Access to `&'a PyAny` implies access to `Python<'a>`. let py = self.0.py(); - self.0.len() == other.0.as_ref(py).len() + #[allow(deprecated)] // as_ref is part of the deprecated "GIL Refs" API. + let len = other.0.as_ref(py).len(); + self.0.len() == len } } ``` diff --git a/guide/src/trait_bounds.md b/guide/src/trait_bounds.md index e05d44e9..6dfaa2e2 100644 --- a/guide/src/trait_bounds.md +++ b/guide/src/trait_bounds.md @@ -83,6 +83,7 @@ impl Model for UserModel { Python::with_gil(|py| { let values: Vec = var.clone(); let list: PyObject = values.into_py(py); + #[allow(deprecated)] // as_ref is part of the deprecated "GIL Refs" API. let py_model = self.model.as_ref(py); py_model .call_method("set_variables", (list,), None) @@ -93,6 +94,7 @@ impl Model for UserModel { fn get_results(&self) -> Vec { println!("Rust calling Python to get the results"); Python::with_gil(|py| { + #[allow(deprecated)] // as_ref is part of the deprecated "GIL Refs" API. self.model .as_ref(py) .call_method("get_results", (), None) @@ -105,6 +107,7 @@ impl Model for UserModel { fn compute(&mut self) { println!("Rust calling Python to perform the computation"); Python::with_gil(|py| { + #[allow(deprecated)] // as_ref is part of the deprecated "GIL Refs" API. self.model .as_ref(py) .call_method("compute", (), None) @@ -183,6 +186,7 @@ This wrapper will also perform the type conversions between Python and Rust. # Python::with_gil(|py| { # let values: Vec = var.clone(); # let list: PyObject = values.into_py(py); +# #[allow(deprecated)] // as_ref is part of the deprecated "GIL Refs" API. # let py_model = self.model.as_ref(py); # py_model # .call_method("set_variables", (list,), None) @@ -193,6 +197,7 @@ This wrapper will also perform the type conversions between Python and Rust. # fn get_results(&self) -> Vec { # println!("Rust calling Python to get the results"); # Python::with_gil(|py| { +# #[allow(deprecated)] // as_ref is part of the deprecated "GIL Refs" API. # self.model # .as_ref(py) # .call_method("get_results", (), None) @@ -205,6 +210,7 @@ This wrapper will also perform the type conversions between Python and Rust. # fn compute(&mut self) { # println!("Rust calling Python to perform the computation"); # Python::with_gil(|py| { +# #[allow(deprecated)] // as_ref is part of the deprecated "GIL Refs" API. # self.model # .as_ref(py) # .call_method("compute", (), None) @@ -349,6 +355,7 @@ We used in our `get_results` method the following call that performs the type co impl Model for UserModel { fn get_results(&self) -> Vec { println!("Rust calling Python to get the results"); + #[allow(deprecated)] // as_ref is part of the deprecated "GIL Refs" API. Python::with_gil(|py| { self.model .as_ref(py) @@ -363,6 +370,7 @@ impl Model for UserModel { # Python::with_gil(|py| { # let values: Vec = var.clone(); # let list: PyObject = values.into_py(py); +# #[allow(deprecated)] // as_ref is part of the deprecated "GIL Refs" API. # let py_model = self.model.as_ref(py); # py_model # .call_method("set_variables", (list,), None) @@ -373,6 +381,7 @@ impl Model for UserModel { # fn compute(&mut self) { # println!("Rust calling Python to perform the computation"); # Python::with_gil(|py| { +# #[allow(deprecated)] // as_ref is part of the deprecated "GIL Refs" API. # self.model # .as_ref(py) # .call_method("compute", (), None) @@ -403,6 +412,7 @@ impl Model for UserModel { fn get_results(&self) -> Vec { println!("Get results from Rust calling Python"); Python::with_gil(|py| { + #[allow(deprecated)] // as_ref is part of the deprecated "GIL Refs" API. let py_result: &PyAny = self .model .as_ref(py) @@ -424,6 +434,7 @@ impl Model for UserModel { # Python::with_gil(|py| { # let values: Vec = var.clone(); # let list: PyObject = values.into_py(py); +# #[allow(deprecated)] // as_ref is part of the deprecated "GIL Refs" API. # let py_model = self.model.as_ref(py); # py_model # .call_method("set_variables", (list,), None) @@ -434,6 +445,7 @@ impl Model for UserModel { # fn compute(&mut self) { # println!("Rust calling Python to perform the computation"); # Python::with_gil(|py| { +# #[allow(deprecated)] // as_ref is part of the deprecated "GIL Refs" API. # self.model # .as_ref(py) # .call_method("compute", (), None) @@ -523,6 +535,7 @@ impl Model for UserModel { Python::with_gil(|py| { let values: Vec = var.clone(); let list: PyObject = values.into_py(py); + #[allow(deprecated)] // as_ref is part of the deprecated "GIL Refs" API. let py_model = self.model.as_ref(py); py_model .call_method("set_variables", (list,), None) @@ -533,6 +546,7 @@ impl Model for UserModel { fn get_results(&self) -> Vec { println!("Get results from Rust calling Python"); Python::with_gil(|py| { + #[allow(deprecated)] // as_ref is part of the deprecated "GIL Refs" API. let py_result: &PyAny = self .model .as_ref(py) @@ -553,6 +567,7 @@ impl Model for UserModel { fn compute(&mut self) { println!("Rust calling Python to perform the computation"); Python::with_gil(|py| { + #[allow(deprecated)] // as_ref is part of the deprecated "GIL Refs" API. self.model .as_ref(py) .call_method("compute", (), None) diff --git a/guide/src/types.md b/guide/src/types.md index f768a31f..51ea10f4 100644 --- a/guide/src/types.md +++ b/guide/src/types.md @@ -145,6 +145,7 @@ let _ = list.repr()?; let _: &PyAny = list; // To &PyAny explicitly with .as_ref() +#[allow(deprecated)] // as_ref is part of the deprecated "GIL Refs" API. let _: &PyAny = list.as_ref(); // To Py with .into() or Py::from() @@ -179,6 +180,7 @@ For a `Py`, the conversions are as below: let list: Py = PyList::empty_bound(py).unbind(); // To &PyList with Py::as_ref() (borrows from the Py) +#[allow(deprecated)] // as_ref is part of the deprecated "GIL Refs" API. let _: &PyList = list.as_ref(py); # let list_clone = list.clone(); // Because `.into_ref()` will consume `list`. @@ -202,6 +204,7 @@ For a `#[pyclass] struct MyClass`, the conversions for `Py` are below: let my_class: Py = Py::new(py, MyClass { })?; // To &PyCell with Py::as_ref() (borrows from the Py) +#[allow(deprecated)] // as_ref is part of the deprecated "GIL Refs" API. let _: &PyCell = my_class.as_ref(py); # let my_class_clone = my_class.clone(); // Because `.into_ref()` will consume `my_class`. @@ -276,6 +279,7 @@ let _ = cell.repr()?; let _: &PyAny = cell; // To &PyAny explicitly with .as_ref() +#[allow(deprecated)] // as_ref is part of the deprecated "GIL Refs" API. let _: &PyAny = cell.as_ref(); # Ok(()) # }).unwrap(); diff --git a/src/impl_/coroutine.rs b/src/impl_/coroutine.rs index c9ca4873..a59eddd0 100644 --- a/src/impl_/coroutine.rs +++ b/src/impl_/coroutine.rs @@ -56,7 +56,11 @@ impl Deref for RefGuard { impl Drop for RefGuard { fn drop(&mut self) { - Python::with_gil(|gil| self.0.as_ref(gil).release_ref()) + Python::with_gil(|gil| { + #[allow(deprecated)] + let self_ref = self.0.bind(gil); + self_ref.release_ref() + }) } } @@ -87,6 +91,10 @@ impl> DerefMut for RefMutGuard { impl> Drop for RefMutGuard { fn drop(&mut self) { - Python::with_gil(|gil| self.0.as_ref(gil).release_mut()) + Python::with_gil(|gil| { + #[allow(deprecated)] + let self_ref = self.0.bind(gil); + self_ref.release_mut() + }) } } diff --git a/src/instance.rs b/src/instance.rs index cb33ed32..65c5ee3b 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -914,6 +914,7 @@ where /// # /// Python::with_gil(|py| { /// let list: Py = PyList::empty_bound(py).into(); + /// # #[allow(deprecated)] /// let list: &PyList = list.as_ref(py); /// assert_eq!(list.len(), 0); /// }); @@ -929,10 +930,18 @@ where /// /// Python::with_gil(|py| { /// let my_class: Py = Py::new(py, MyClass {}).unwrap(); + /// # #[allow(deprecated)] /// let my_class_cell: &PyCell = my_class.as_ref(py); /// assert!(my_class_cell.try_borrow().is_ok()); /// }); /// ``` + #[cfg_attr( + not(feature = "gil-refs"), + deprecated( + since = "0.21.0", + note = "use `obj.bind(py)` instead of `obj.as_ref(py)`" + ) + )] pub fn as_ref<'py>(&'py self, _py: Python<'py>) -> &'py T::AsRefTarget { let any = self.as_ptr() as *const PyAny; unsafe { PyNativeType::unchecked_downcast(&*any) } diff --git a/src/pycell/impl_.rs b/src/pycell/impl_.rs index 62875e67..29ba7eda 100644 --- a/src/pycell/impl_.rs +++ b/src/pycell/impl_.rs @@ -284,43 +284,43 @@ mod tests { ) .unwrap(); - let mmm_cell: &PyCell = mmm.as_ref(py); + let mmm_bound: &Bound<'_, MutableChildOfMutableChildOfMutableBase> = mmm.bind(py); - let mmm_refmut = mmm_cell.borrow_mut(); + let mmm_refmut = mmm_bound.borrow_mut(); // Cannot take any other mutable or immutable borrows whilst the object is borrowed mutably - assert!(mmm_cell + assert!(mmm_bound .extract::>() .is_err()); - assert!(mmm_cell + assert!(mmm_bound .extract::>() .is_err()); - assert!(mmm_cell.extract::>().is_err()); - assert!(mmm_cell + assert!(mmm_bound.extract::>().is_err()); + assert!(mmm_bound .extract::>() .is_err()); - assert!(mmm_cell + assert!(mmm_bound .extract::>() .is_err()); - assert!(mmm_cell.extract::>().is_err()); + assert!(mmm_bound.extract::>().is_err()); // With the borrow dropped, all other borrow attempts will succeed drop(mmm_refmut); - assert!(mmm_cell + assert!(mmm_bound .extract::>() .is_ok()); - assert!(mmm_cell + assert!(mmm_bound .extract::>() .is_ok()); - assert!(mmm_cell.extract::>().is_ok()); - assert!(mmm_cell + assert!(mmm_bound.extract::>().is_ok()); + assert!(mmm_bound .extract::>() .is_ok()); - assert!(mmm_cell + assert!(mmm_bound .extract::>() .is_ok()); - assert!(mmm_cell.extract::>().is_ok()); + assert!(mmm_bound.extract::>().is_ok()); }) } @@ -335,38 +335,38 @@ mod tests { ) .unwrap(); - let mmm_cell: &PyCell = mmm.as_ref(py); + let mmm_bound: &Bound<'_, MutableChildOfMutableChildOfMutableBase> = mmm.bind(py); - let mmm_refmut = mmm_cell.borrow(); + let mmm_refmut = mmm_bound.borrow(); // Further immutable borrows are ok - assert!(mmm_cell + assert!(mmm_bound .extract::>() .is_ok()); - assert!(mmm_cell + assert!(mmm_bound .extract::>() .is_ok()); - assert!(mmm_cell.extract::>().is_ok()); + assert!(mmm_bound.extract::>().is_ok()); // Further mutable borrows are not ok - assert!(mmm_cell + assert!(mmm_bound .extract::>() .is_err()); - assert!(mmm_cell + assert!(mmm_bound .extract::>() .is_err()); - assert!(mmm_cell.extract::>().is_err()); + assert!(mmm_bound.extract::>().is_err()); // With the borrow dropped, all mutable borrow attempts will succeed drop(mmm_refmut); - assert!(mmm_cell + assert!(mmm_bound .extract::>() .is_ok()); - assert!(mmm_cell + assert!(mmm_bound .extract::>() .is_ok()); - assert!(mmm_cell.extract::>().is_ok()); + assert!(mmm_bound.extract::>().is_ok()); }) } } diff --git a/src/pyclass.rs b/src/pyclass.rs index eb4a5595..ebdc52dc 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, Bound, IntoPy, PyCell, + PyObject, PyResult, PyTypeInfo, Python, }; use std::{cmp::Ordering, os::raw::c_int}; @@ -216,6 +216,18 @@ pub trait Frozen: boolean_struct::private::Boolean {} impl Frozen for boolean_struct::True {} impl Frozen for boolean_struct::False {} +impl<'py, T: PyClass> Bound<'py, T> { + #[cfg(feature = "macros")] + pub(crate) fn release_ref(&self) { + self.get_cell().release_ref(); + } + + #[cfg(feature = "macros")] + pub(crate) fn release_mut(&self) { + self.get_cell().release_mut(); + } +} + mod tests { #[test] fn test_compare_op_matches() { diff --git a/tests/ui/invalid_frozen_pyclass_borrow.rs b/tests/ui/invalid_frozen_pyclass_borrow.rs index 1f18eab6..aa8969ab 100644 --- a/tests/ui/invalid_frozen_pyclass_borrow.rs +++ b/tests/ui/invalid_frozen_pyclass_borrow.rs @@ -12,7 +12,7 @@ impl Foo { } fn borrow_mut_fails(foo: Py, py: Python) { - let borrow = foo.as_ref(py).borrow_mut(); + let borrow = foo.bind(py).borrow_mut(); } #[pyclass(subclass)] @@ -22,7 +22,7 @@ struct MutableBase; struct ImmutableChild; fn borrow_mut_of_child_fails(child: Py, py: Python) { - let borrow = child.as_ref(py).borrow_mut(); + let borrow = child.bind(py).borrow_mut(); } fn py_get_of_mutable_class_fails(class: Py) { diff --git a/tests/ui/invalid_frozen_pyclass_borrow.stderr b/tests/ui/invalid_frozen_pyclass_borrow.stderr index 5e09d512..1a8e4596 100644 --- a/tests/ui/invalid_frozen_pyclass_borrow.stderr +++ b/tests/ui/invalid_frozen_pyclass_borrow.stderr @@ -17,34 +17,34 @@ note: required by a bound in `extract_pyclass_ref_mut` | ^^^^^^^^^^^^^^ required by this bound in `extract_pyclass_ref_mut` error[E0271]: type mismatch resolving `::Frozen == False` - --> tests/ui/invalid_frozen_pyclass_borrow.rs:15:33 + --> tests/ui/invalid_frozen_pyclass_borrow.rs:15:31 | -15 | let borrow = foo.as_ref(py).borrow_mut(); - | ^^^^^^^^^^ expected `False`, found `True` +15 | let borrow = foo.bind(py).borrow_mut(); + | ^^^^^^^^^^ expected `False`, found `True` | -note: required by a bound in `pyo3::PyCell::::borrow_mut` - --> src/pycell.rs +note: required by a bound in `pyo3::Bound::<'py, T>::borrow_mut` + --> src/instance.rs | - | pub fn borrow_mut(&self) -> PyRefMut<'_, T> + | pub fn borrow_mut(&self) -> PyRefMut<'py, T> | ---------- required by a bound in this associated function | where | T: PyClass, - | ^^^^^^^^^^^^^^ required by this bound in `PyCell::::borrow_mut` + | ^^^^^^^^^^^^^^ required by this bound in `Bound::<'py, T>::borrow_mut` error[E0271]: type mismatch resolving `::Frozen == False` - --> tests/ui/invalid_frozen_pyclass_borrow.rs:25:35 + --> tests/ui/invalid_frozen_pyclass_borrow.rs:25:33 | -25 | let borrow = child.as_ref(py).borrow_mut(); - | ^^^^^^^^^^ expected `False`, found `True` +25 | let borrow = child.bind(py).borrow_mut(); + | ^^^^^^^^^^ expected `False`, found `True` | -note: required by a bound in `pyo3::PyCell::::borrow_mut` - --> src/pycell.rs +note: required by a bound in `pyo3::Bound::<'py, T>::borrow_mut` + --> src/instance.rs | - | pub fn borrow_mut(&self) -> PyRefMut<'_, T> + | pub fn borrow_mut(&self) -> PyRefMut<'py, T> | ---------- required by a bound in this associated function | where | T: PyClass, - | ^^^^^^^^^^^^^^ required by this bound in `PyCell::::borrow_mut` + | ^^^^^^^^^^^^^^ required by this bound in `Bound::<'py, T>::borrow_mut` error[E0271]: type mismatch resolving `::Frozen == True` --> tests/ui/invalid_frozen_pyclass_borrow.rs:29:11