deprecate Py::as_ref (#3864)

* Deprecate Py::as_ref

* Reword as_ref deprecation note

Co-authored-by: David Hewitt <mail@davidhewitt.dev>

* Tidy up remaining uses of Py::as_ref

Co-authored-by: David Hewitt <mail@davidhewitt.dev>

* Pass hello into println! explicitly

---------

Co-authored-by: David Hewitt <mail@davidhewitt.dev>
This commit is contained in:
Lily Foote 2024-02-29 07:15:34 +00:00 committed by GitHub
parent 68ec6de0c9
commit 56683ed553
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 105 additions and 48 deletions

View File

@ -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);

View File

@ -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.

View File

@ -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
}
}
```

View File

@ -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)

View File

@ -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();

View File

@ -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()
})
}
}

View File

@ -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) }

View File

@ -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());
})
}
}

View File

@ -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() {

View File

@ -12,7 +12,7 @@ impl Foo {
}
fn borrow_mut_fails(foo: Py<Foo>, 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<ImmutableChild>, 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<MutableBase>) {

View File

@ -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 `<Foo as PyClass>::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::<T>::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<Frozen = False>,
| ^^^^^^^^^^^^^^ required by this bound in `PyCell::<T>::borrow_mut`
| ^^^^^^^^^^^^^^ required by this bound in `Bound::<'py, T>::borrow_mut`
error[E0271]: type mismatch resolving `<ImmutableChild as PyClass>::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::<T>::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<Frozen = False>,
| ^^^^^^^^^^^^^^ required by this bound in `PyCell::<T>::borrow_mut`
| ^^^^^^^^^^^^^^ required by this bound in `Bound::<'py, T>::borrow_mut`
error[E0271]: type mismatch resolving `<MutableBase as PyClass>::Frozen == True`
--> tests/ui/invalid_frozen_pyclass_borrow.rs:29:11