Skip to content

Commit

Permalink
deprecate PyWeakRefMethods::get_object
Browse files Browse the repository at this point in the history
  • Loading branch information
@davidhewitt
davidhewitt committed Oct 4, 2024
1 parent e9a1b9b commit 67dd727
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 37 deletions.
33 changes: 18 additions & 15 deletions src/types/weakref/anyref.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::types::{
any::{PyAny, PyAnyMethods},
PyNone,
};
use crate::{ffi, Bound};
use crate::{ffi, Bound, Python};

/// Represents any Python `weakref` reference.
///
Expand Down Expand Up @@ -315,20 +315,12 @@ pub trait PyWeakrefMethods<'py> {
///
/// # Panics
/// This function panics is the current object is invalid.
/// If used propperly this is never the case. (NonNull and actually a weakref type)
/// If used properly this is never the case. (NonNull and actually a weakref type)
///
/// [`PyWeakref_GetRef`]: https://docs.python.org/3/c-api/weakref.html#c.PyWeakref_GetRef
/// [`weakref.ReferenceType`]: https://docs.python.org/3/library/weakref.html#weakref.ReferenceType
/// [`weakref.ref`]: https://docs.python.org/3/library/weakref.html#weakref.ref
fn upgrade(&self) -> Option<Bound<'py, PyAny>> {
let object = self.get_object();

if object.is_none() {
None
} else {
Some(object)
}
}
fn upgrade(&self) -> Option<Bound<'py, PyAny>>;

/// Retrieve to a Bound object pointed to by the weakref.
///
Expand Down Expand Up @@ -386,16 +378,25 @@ pub trait PyWeakrefMethods<'py> {
/// [`PyWeakref_GetRef`]: https://docs.python.org/3/c-api/weakref.html#c.PyWeakref_GetRef
/// [`weakref.ReferenceType`]: https://docs.python.org/3/library/weakref.html#weakref.ReferenceType
/// [`weakref.ref`]: https://docs.python.org/3/library/weakref.html#weakref.ref
fn get_object(&self) -> Bound<'py, PyAny>;
#[deprecated(since = "0.23.0", note = "Use `upgrade` instead")]
fn get_object(&self) -> Bound<'py, PyAny> {
self.upgrade().unwrap_or_else(|| {
// Safety: upgrade() returns `Bound<'py, PyAny>` with a lifetime `'py` if it exists, we
// can safely assume the same lifetime here.
PyNone::get(unsafe { Python::assume_gil_acquired() })
.to_owned()
.into_any()
})
}
}

impl<'py> PyWeakrefMethods<'py> for Bound<'py, PyWeakref> {
fn get_object(&self) -> Bound<'py, PyAny> {
fn upgrade(&self) -> Option<Bound<'py, PyAny>> {
let mut obj: *mut ffi::PyObject = std::ptr::null_mut();
match unsafe { ffi::compat::PyWeakref_GetRef(self.as_ptr(), &mut obj) } {
std::os::raw::c_int::MIN..=-1 => panic!("The 'weakref' weak reference instance should be valid (non-null and actually a weakref reference)"),
0 => PyNone::get(self.py()).to_owned().into_any(),
1..=std::os::raw::c_int::MAX => unsafe { obj.assume_owned(self.py()) },
0 => None,
1..=std::os::raw::c_int::MAX => Some(unsafe { obj.assume_owned(self.py()) }),
}
}
}
Expand Down Expand Up @@ -545,6 +546,7 @@ mod tests {
}

#[test]
#[allow(deprecated)]
fn test_weakref_get_object() -> PyResult<()> {
fn inner(
create_reference: impl for<'py> FnOnce(
Expand Down Expand Up @@ -696,6 +698,7 @@ mod tests {
}

#[test]
#[allow(deprecated)]
fn test_weakref_get_object() -> PyResult<()> {
fn inner(
create_reference: impl for<'py> FnOnce(
Expand Down
31 changes: 17 additions & 14 deletions src/types/weakref/proxy.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::err::PyResult;
use crate::ffi_ptr_ext::FfiPtrExt;
use crate::py_result_ext::PyResultExt;
use crate::type_object::PyTypeCheck;
use crate::types::{any::PyAny, PyNone};
use crate::types::any::PyAny;
use crate::{ffi, Bound, ToPyObject};

use super::PyWeakrefMethods;
Expand Down Expand Up @@ -182,12 +182,12 @@ impl PyWeakrefProxy {
}

impl<'py> PyWeakrefMethods<'py> for Bound<'py, PyWeakrefProxy> {
fn get_object(&self) -> Bound<'py, PyAny> {
fn upgrade(&self) -> Option<Bound<'py, PyAny>> {
let mut obj: *mut ffi::PyObject = std::ptr::null_mut();
match unsafe { ffi::compat::PyWeakref_GetRef(self.as_ptr(), &mut obj) } {
std::os::raw::c_int::MIN..=-1 => panic!("The 'weakref.ProxyType' (or `weakref.CallableProxyType`) instance should be valid (non-null and actually a weakref reference)"),
0 => PyNone::get(self.py()).to_owned().into_any(),
1..=std::os::raw::c_int::MAX => unsafe { obj.assume_owned(self.py()) },
0 => None,
1..=std::os::raw::c_int::MAX => Some(unsafe { obj.assume_owned(self.py()) }),
}
}
}
Expand Down Expand Up @@ -273,7 +273,7 @@ mod tests {
let reference = PyWeakrefProxy::new(&object)?;

assert!(!reference.is(&object));
assert!(reference.get_object().is(&object));
assert!(reference.upgrade().unwrap().is(&object));

#[cfg(not(Py_LIMITED_API))]
assert_eq!(
Expand Down Expand Up @@ -301,7 +301,7 @@ mod tests {

drop(object);

assert!(reference.get_object().is_none());
assert!(reference.upgrade().is_none());
assert!(reference
.getattr("__class__")
.err()
Expand Down Expand Up @@ -416,11 +416,11 @@ mod tests {
let object = class.call0()?;
let reference = PyWeakrefProxy::new(&object)?;

assert!(reference.get_object().is(&object));
assert!(reference.upgrade().unwrap().is(&object));

drop(object);

assert!(reference.get_object().is_none());
assert!(reference.upgrade().is_none());

Ok(())
})
Expand All @@ -444,7 +444,7 @@ mod tests {
let reference = PyWeakrefProxy::new(&object)?;

assert!(!reference.is(&object));
assert!(reference.get_object().is(&object));
assert!(reference.upgrade().unwrap().is(&object));
#[cfg(not(Py_LIMITED_API))]
assert_eq!(
reference.get_type().to_string(),
Expand Down Expand Up @@ -474,7 +474,7 @@ mod tests {

drop(object);

assert!(reference.get_object().is_none());
assert!(reference.upgrade().is_none());
assert!(reference
.getattr("__class__")
.err()
Expand Down Expand Up @@ -574,6 +574,7 @@ mod tests {
}

#[test]
#[allow(deprecated)]
fn test_weakref_get_object() -> PyResult<()> {
Python::with_gil(|py| {
let object = Py::new(py, WeakrefablePyClass {})?;
Expand Down Expand Up @@ -623,7 +624,7 @@ mod tests {
let reference = PyWeakrefProxy::new(&object)?;

assert!(!reference.is(&object));
assert!(reference.get_object().is(&object));
assert!(reference.upgrade().unwrap().is(&object));
#[cfg(not(Py_LIMITED_API))]
assert_eq!(reference.get_type().to_string(), CLASS_NAME);

Expand All @@ -640,7 +641,7 @@ mod tests {

drop(object);

assert!(reference.get_object().is_none());
assert!(reference.upgrade().is_none());
assert!(reference
.getattr("__class__")
.err()
Expand Down Expand Up @@ -747,6 +748,7 @@ mod tests {
}

#[test]
#[allow(deprecated)]
fn test_weakref_get_object() -> PyResult<()> {
Python::with_gil(|py| {
let class = get_type(py)?;
Expand Down Expand Up @@ -788,7 +790,7 @@ mod tests {
let reference = PyWeakrefProxy::new(&object)?;

assert!(!reference.is(&object));
assert!(reference.get_object().is(&object));
assert!(reference.upgrade().unwrap().is(&object));
#[cfg(not(Py_LIMITED_API))]
assert_eq!(reference.get_type().to_string(), CLASS_NAME);

Expand All @@ -808,7 +810,7 @@ mod tests {

drop(object);

assert!(reference.get_object().is_none());
assert!(reference.upgrade().is_none());
assert!(reference
.getattr("__class__")
.err()
Expand Down Expand Up @@ -906,6 +908,7 @@ mod tests {
}

#[test]
#[allow(deprecated)]
fn test_weakref_get_object() -> PyResult<()> {
Python::with_gil(|py| {
let object = Py::new(py, WeakrefablePyClass {})?;
Expand Down
18 changes: 10 additions & 8 deletions src/types/weakref/reference.rs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::err::PyResult;
use crate::ffi_ptr_ext::FfiPtrExt;
use crate::py_result_ext::PyResultExt;
use crate::types::{any::PyAny, PyNone};
use crate::types::any::PyAny;
use crate::{ffi, Bound, ToPyObject};

#[cfg(any(PyPy, GraalPy, Py_LIMITED_API))]
Expand Down Expand Up @@ -191,12 +191,12 @@ impl PyWeakrefReference {
}

impl<'py> PyWeakrefMethods<'py> for Bound<'py, PyWeakrefReference> {
fn get_object(&self) -> Bound<'py, PyAny> {
fn upgrade(&self) -> Option<Bound<'py, PyAny>> {
let mut obj: *mut ffi::PyObject = std::ptr::null_mut();
match unsafe { ffi::compat::PyWeakref_GetRef(self.as_ptr(), &mut obj) } {
std::os::raw::c_int::MIN..=-1 => panic!("The 'weakref.ReferenceType' instance should be valid (non-null and actually a weakref reference)"),
0 => PyNone::get(self.py()).to_owned().into_any(),
1..=std::os::raw::c_int::MAX => unsafe { obj.assume_owned(self.py()) },
0 => None,
1..=std::os::raw::c_int::MAX => Some(unsafe { obj.assume_owned(self.py()) }),
}
}
}
Expand Down Expand Up @@ -268,7 +268,7 @@ mod tests {
let reference = PyWeakrefReference::new(&object)?;

assert!(!reference.is(&object));
assert!(reference.get_object().is(&object));
assert!(reference.upgrade().unwrap().is(&object));

#[cfg(not(Py_LIMITED_API))]
assert_eq!(reference.get_type().to_string(), CLASS_NAME);
Expand All @@ -287,7 +287,7 @@ mod tests {

drop(object);

assert!(reference.get_object().is_none());
assert!(reference.upgrade().is_none());
#[cfg(not(Py_LIMITED_API))]
assert_eq!(reference.getattr("__class__")?.to_string(), CLASS_NAME);
check_repr(&reference, None)?;
Expand Down Expand Up @@ -387,6 +387,7 @@ mod tests {
}

#[test]
#[allow(deprecated)]
fn test_weakref_get_object() -> PyResult<()> {
Python::with_gil(|py| {
let class = get_type(py)?;
Expand Down Expand Up @@ -423,7 +424,7 @@ mod tests {
let reference = PyWeakrefReference::new(&object)?;

assert!(!reference.is(&object));
assert!(reference.get_object().is(&object));
assert!(reference.upgrade().unwrap().is(&object));
#[cfg(not(Py_LIMITED_API))]
assert_eq!(reference.get_type().to_string(), CLASS_NAME);

Expand All @@ -440,7 +441,7 @@ mod tests {

drop(object);

assert!(reference.get_object().is_none());
assert!(reference.upgrade().is_none());
#[cfg(not(Py_LIMITED_API))]
assert_eq!(reference.getattr("__class__")?.to_string(), CLASS_NAME);
check_repr(&reference, None)?;
Expand Down Expand Up @@ -531,6 +532,7 @@ mod tests {
}

#[test]
#[allow(deprecated)]
fn test_weakref_get_object() -> PyResult<()> {
Python::with_gil(|py| {
let object = Py::new(py, WeakrefablePyClass {})?;
Expand Down

0 comments on commit 67dd727

Please sign in to comment.