deprecate Py::as_ref (#3864)

* Deprecate Py::as_ref * Reword as_ref deprecation note Co-authored-by: David Hewitt <[email protected]> * Tidy up remaining uses of Py::as_ref Co-authored-by: David Hewitt <[email protected]> * Pass hello into println! explicitly --------- Co-authored-by: David Hewitt <[email protected]>
PyO3 · Feb 29, 2024 · 56683ed · 56683ed
1 parent 68ec6de
commit 56683ed
Show file tree

Hide file tree

Showing 11 changed files with 105 additions and 48 deletions.
diff --git a/guide/src/class.md b/guide/src/class.md
@@ -256,6 +256,7 @@ fn return_myclass() -> Py<MyClass> {
 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<MyClass>::as_ref returns &PyCell<MyClass>
     let obj_ref = cell.borrow(); // Get PyRef<T>
     assert_eq!(obj_ref.num, 1);

diff --git a/guide/src/memory.md b/guide/src/memory.md
@@ -171,7 +171,9 @@ let hello: Py<PyString> = 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
@@ -70,7 +70,11 @@ struct FooRef<'a>(&'a PyList);
 
 impl PartialEq<Foo> 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<Foo> 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
@@ -83,6 +83,7 @@ impl Model for UserModel {
         Python::with_gil(|py| {
             let values: Vec<f64> = 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<f64> {
         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<f64> = 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<f64> {
 #      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<f64> {
         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<f64> = 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<f64> {
         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<f64> = 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<f64> = 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<f64> {
         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
@@ -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<T> with .into() or Py::from()
@@ -179,6 +180,7 @@ For a `Py<PyList>`, the conversions are as below:
 let list: Py<PyList> = 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<MyClass>` are below:
 let my_class: Py<MyClass> = Py::new(py, MyClass { })?;
 
 // To &PyCell<MyClass> with Py::as_ref() (borrows from the Py)
+#[allow(deprecated)]  // as_ref is part of the deprecated "GIL Refs" API.
 let _: &PyCell<MyClass> = 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
@@ -56,7 +56,11 @@ impl<T: PyClass> Deref for RefGuard<T> {
 
 impl<T: PyClass> Drop for RefGuard<T> {
     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<T: PyClass<Frozen = False>> DerefMut for RefMutGuard<T> {
 
 impl<T: PyClass<Frozen = False>> Drop for RefMutGuard<T> {
     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
@@ -914,6 +914,7 @@ where
     /// #
     /// Python::with_gil(|py| {
     ///     let list: Py<PyList> = 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<MyClass> = Py::new(py, MyClass {}).unwrap();
+    ///     # #[allow(deprecated)]
     ///     let my_class_cell: &PyCell<MyClass> = 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
@@ -284,43 +284,43 @@ mod tests {
             )
             .unwrap();
 
-            let mmm_cell: &PyCell<MutableChildOfMutableChildOfMutableBase> = 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::<PyRef<'_, MutableChildOfMutableChildOfMutableBase>>()
                 .is_err());
-            assert!(mmm_cell
+            assert!(mmm_bound
                 .extract::<PyRef<'_, MutableChildOfMutableBase>>()
                 .is_err());
-            assert!(mmm_cell.extract::<PyRef<'_, MutableBase>>().is_err());
-            assert!(mmm_cell
+            assert!(mmm_bound.extract::<PyRef<'_, MutableBase>>().is_err());
+            assert!(mmm_bound
                 .extract::<PyRefMut<'_, MutableChildOfMutableChildOfMutableBase>>()
                 .is_err());
-            assert!(mmm_cell
+            assert!(mmm_bound
                 .extract::<PyRefMut<'_, MutableChildOfMutableBase>>()
                 .is_err());
-            assert!(mmm_cell.extract::<PyRefMut<'_, MutableBase>>().is_err());
+            assert!(mmm_bound.extract::<PyRefMut<'_, MutableBase>>().is_err());
 
             // With the borrow dropped, all other borrow attempts will succeed
             drop(mmm_refmut);
 
-            assert!(mmm_cell
+            assert!(mmm_bound
                 .extract::<PyRef<'_, MutableChildOfMutableChildOfMutableBase>>()
                 .is_ok());
-            assert!(mmm_cell
+            assert!(mmm_bound
                 .extract::<PyRef<'_, MutableChildOfMutableBase>>()
                 .is_ok());
-            assert!(mmm_cell.extract::<PyRef<'_, MutableBase>>().is_ok());
-            assert!(mmm_cell
+            assert!(mmm_bound.extract::<PyRef<'_, MutableBase>>().is_ok());
+            assert!(mmm_bound
                 .extract::<PyRefMut<'_, MutableChildOfMutableChildOfMutableBase>>()
                 .is_ok());
-            assert!(mmm_cell
+            assert!(mmm_bound
                 .extract::<PyRefMut<'_, MutableChildOfMutableBase>>()
                 .is_ok());
-            assert!(mmm_cell.extract::<PyRefMut<'_, MutableBase>>().is_ok());
+            assert!(mmm_bound.extract::<PyRefMut<'_, MutableBase>>().is_ok());
         })
     }
 
@@ -335,38 +335,38 @@ mod tests {
             )
             .unwrap();
 
-            let mmm_cell: &PyCell<MutableChildOfMutableChildOfMutableBase> = 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::<PyRef<'_, MutableChildOfMutableChildOfMutableBase>>()
                 .is_ok());
-            assert!(mmm_cell
+            assert!(mmm_bound
                 .extract::<PyRef<'_, MutableChildOfMutableBase>>()
                 .is_ok());
-            assert!(mmm_cell.extract::<PyRef<'_, MutableBase>>().is_ok());
+            assert!(mmm_bound.extract::<PyRef<'_, MutableBase>>().is_ok());
 
             // Further mutable borrows are not ok
-            assert!(mmm_cell
+            assert!(mmm_bound
                 .extract::<PyRefMut<'_, MutableChildOfMutableChildOfMutableBase>>()
                 .is_err());
-            assert!(mmm_cell
+            assert!(mmm_bound
                 .extract::<PyRefMut<'_, MutableChildOfMutableBase>>()
                 .is_err());
-            assert!(mmm_cell.extract::<PyRefMut<'_, MutableBase>>().is_err());
+            assert!(mmm_bound.extract::<PyRefMut<'_, MutableBase>>().is_err());
 
             // With the borrow dropped, all mutable borrow attempts will succeed
             drop(mmm_refmut);
 
-            assert!(mmm_cell
+            assert!(mmm_bound
                 .extract::<PyRefMut<'_, MutableChildOfMutableChildOfMutableBase>>()
                 .is_ok());
-            assert!(mmm_cell
+            assert!(mmm_bound
                 .extract::<PyRefMut<'_, MutableChildOfMutableBase>>()
                 .is_ok());
-            assert!(mmm_cell.extract::<PyRefMut<'_, MutableBase>>().is_ok());
+            assert!(mmm_bound.extract::<PyRefMut<'_, MutableBase>>().is_ok());
         })
     }
 }
diff --git 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() {