fix `AsRef` and `Deref` impls on `Bound<T>` (#3879)

* fix `AsRef` and `Deref` of `Bound<T>` to `Bound<PyAny>`

* cleanup unnessesary `.as_any()` calls

* remove trait bound on `AsRef` impl

* add comment for `Deref` trait bound

* rename marker trait
This commit is contained in:
Icxolu 2024-02-22 23:38:42 +01:00 committed by GitHub
parent 9e74c858c2
commit 4f8ee96881
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 65 additions and 55 deletions

View File

@ -217,8 +217,8 @@ impl Number {
# fn main() -> PyResult<()> { # fn main() -> PyResult<()> {
# Python::with_gil(|py| { # Python::with_gil(|py| {
# let x = &Bound::new(py, Number(4))?.into_any(); # let x = &Bound::new(py, Number(4))?;
# let y = &Bound::new(py, Number(4))?.into_any(); # let y = &Bound::new(py, Number(4))?;
# assert!(x.eq(y)?); # assert!(x.eq(y)?);
# assert!(!x.ne(y)?); # assert!(!x.ne(y)?);
# Ok(()) # Ok(())

View File

@ -365,6 +365,8 @@ fn impl_class(
const _: () = { const _: () = {
use #krate as _pyo3; use #krate as _pyo3;
impl _pyo3::types::DerefToPyAny for #cls {}
#pytypeinfo_impl #pytypeinfo_impl
#py_class_impl #py_class_impl

View File

@ -4,7 +4,7 @@ use crate::pycell::{PyBorrowError, PyBorrowMutError, PyCell};
use crate::pyclass::boolean_struct::{False, True}; use crate::pyclass::boolean_struct::{False, True};
use crate::type_object::HasPyGilRef; use crate::type_object::HasPyGilRef;
use crate::types::{any::PyAnyMethods, string::PyStringMethods, typeobject::PyTypeMethods}; use crate::types::{any::PyAnyMethods, string::PyStringMethods, typeobject::PyTypeMethods};
use crate::types::{PyDict, PyString, PyTuple}; use crate::types::{DerefToPyAny, PyDict, PyString, PyTuple};
use crate::{ use crate::{
ffi, AsPyPointer, DowncastError, FromPyObject, IntoPy, PyAny, PyClass, PyClassInitializer, ffi, AsPyPointer, DowncastError, FromPyObject, IntoPy, PyAny, PyClass, PyClassInitializer,
PyRef, PyRefMut, PyTypeInfo, Python, ToPyObject, PyRef, PyRefMut, PyTypeInfo, Python, ToPyObject,
@ -366,9 +366,12 @@ fn python_format(
} }
} }
// The trait bound is needed to avoid running into the auto-deref recursion
// limit (error[E0055]), because `Bound<PyAny>` would deref into itself. See:
// https://github.com/rust-lang/rust/issues/19509
impl<'py, T> Deref for Bound<'py, T> impl<'py, T> Deref for Bound<'py, T>
where where
T: AsRef<PyAny>, T: DerefToPyAny,
{ {
type Target = Bound<'py, PyAny>; type Target = Bound<'py, PyAny>;
@ -378,10 +381,7 @@ where
} }
} }
impl<'py, T> AsRef<Bound<'py, PyAny>> for Bound<'py, T> impl<'py, T> AsRef<Bound<'py, PyAny>> for Bound<'py, T> {
where
T: AsRef<PyAny>,
{
#[inline] #[inline]
fn as_ref(&self) -> &Bound<'py, PyAny> { fn as_ref(&self) -> &Bound<'py, PyAny> {
self.as_any() self.as_any()

View File

@ -168,7 +168,7 @@ mod tests {
Python::with_gil(|py| { Python::with_gil(|py| {
assert!(PyBool::new_bound(py, true).is_true()); assert!(PyBool::new_bound(py, true).is_true());
let t = PyBool::new_bound(py, true); let t = PyBool::new_bound(py, true);
assert!(t.as_any().extract::<bool>().unwrap()); assert!(t.extract::<bool>().unwrap());
assert!(true.to_object(py).is(&*PyBool::new_bound(py, true))); assert!(true.to_object(py).is(&*PyBool::new_bound(py, true)));
}); });
} }
@ -178,7 +178,7 @@ mod tests {
Python::with_gil(|py| { Python::with_gil(|py| {
assert!(!PyBool::new_bound(py, false).is_true()); assert!(!PyBool::new_bound(py, false).is_true());
let t = PyBool::new_bound(py, false); let t = PyBool::new_bound(py, false);
assert!(!t.as_any().extract::<bool>().unwrap()); assert!(!t.extract::<bool>().unwrap());
assert!(false.to_object(py).is(&*PyBool::new_bound(py, false))); assert!(false.to_object(py).is(&*PyBool::new_bound(py, false)));
}); });
} }

View File

@ -86,6 +86,31 @@ pub mod iter {
pub use super::tuple::{BorrowedTupleIterator, BoundTupleIterator, PyTupleIterator}; pub use super::tuple::{BorrowedTupleIterator, BoundTupleIterator, PyTupleIterator};
} }
/// Python objects that have a base type.
///
/// This marks types that can be upcast into a [`PyAny`] and used in its place.
/// This essentially includes every Python object except [`PyAny`] itself.
///
/// This is used to provide the [`Deref<Target = Bound<'_, PyAny>>`](std::ops::Deref)
/// implementations for [`Bound<'_, T>`](crate::Bound).
///
/// Users should not need to implement this trait directly. It's implementation
/// is provided by the [`#[pyclass]`](macro@crate::pyclass) attribute.
///
/// ## Note
/// This is needed because the compiler currently tries to figure out all the
/// types in a deref-chain before starting to look for applicable method calls.
/// So we need to prevent [`Bound<'_, PyAny`](crate::Bound) dereferencing to
/// itself in order to avoid running into the recursion limit. This trait is
/// used to exclude this from our blanket implementation. See [this Rust
/// issue][1] for more details. If the compiler limitation gets resolved, this
/// trait will be removed.
///
/// [1]: https://github.com/rust-lang/rust/issues/19509
pub trait DerefToPyAny {
// Empty.
}
// Implementations core to all native types // Implementations core to all native types
#[doc(hidden)] #[doc(hidden)]
#[macro_export] #[macro_export]
@ -183,6 +208,8 @@ macro_rules! pyobject_native_type_named (
unsafe{&*(ob as *const $name as *const $crate::PyAny)} unsafe{&*(ob as *const $name as *const $crate::PyAny)}
} }
} }
impl $crate::types::DerefToPyAny for $name {}
}; };
); );

View File

@ -96,11 +96,11 @@ fn test_get_buffer_errors() {
) )
.unwrap(); .unwrap();
assert!(PyBuffer::<u32>::get_bound(instance.bind(py).as_any()).is_ok()); assert!(PyBuffer::<u32>::get_bound(instance.bind(py)).is_ok());
instance.borrow_mut(py).error = Some(TestGetBufferError::NullShape); instance.borrow_mut(py).error = Some(TestGetBufferError::NullShape);
assert_eq!( assert_eq!(
PyBuffer::<u32>::get_bound(instance.bind(py).as_any()) PyBuffer::<u32>::get_bound(instance.bind(py))
.unwrap_err() .unwrap_err()
.to_string(), .to_string(),
"BufferError: shape is null" "BufferError: shape is null"
@ -108,7 +108,7 @@ fn test_get_buffer_errors() {
instance.borrow_mut(py).error = Some(TestGetBufferError::NullStrides); instance.borrow_mut(py).error = Some(TestGetBufferError::NullStrides);
assert_eq!( assert_eq!(
PyBuffer::<u32>::get_bound(instance.bind(py).as_any()) PyBuffer::<u32>::get_bound(instance.bind(py))
.unwrap_err() .unwrap_err()
.to_string(), .to_string(),
"BufferError: strides is null" "BufferError: strides is null"
@ -116,7 +116,7 @@ fn test_get_buffer_errors() {
instance.borrow_mut(py).error = Some(TestGetBufferError::IncorrectItemSize); instance.borrow_mut(py).error = Some(TestGetBufferError::IncorrectItemSize);
assert_eq!( assert_eq!(
PyBuffer::<u32>::get_bound(instance.bind(py).as_any()) PyBuffer::<u32>::get_bound(instance.bind(py))
.unwrap_err() .unwrap_err()
.to_string(), .to_string(),
"BufferError: buffer contents are not compatible with u32" "BufferError: buffer contents are not compatible with u32"
@ -124,7 +124,7 @@ fn test_get_buffer_errors() {
instance.borrow_mut(py).error = Some(TestGetBufferError::IncorrectFormat); instance.borrow_mut(py).error = Some(TestGetBufferError::IncorrectFormat);
assert_eq!( assert_eq!(
PyBuffer::<u32>::get_bound(instance.bind(py).as_any()) PyBuffer::<u32>::get_bound(instance.bind(py))
.unwrap_err() .unwrap_err()
.to_string(), .to_string(),
"BufferError: buffer contents are not compatible with u32" "BufferError: buffer contents are not compatible with u32"
@ -132,7 +132,7 @@ fn test_get_buffer_errors() {
instance.borrow_mut(py).error = Some(TestGetBufferError::IncorrectAlignment); instance.borrow_mut(py).error = Some(TestGetBufferError::IncorrectAlignment);
assert_eq!( assert_eq!(
PyBuffer::<u32>::get_bound(instance.bind(py).as_any()) PyBuffer::<u32>::get_bound(instance.bind(py))
.unwrap_err() .unwrap_err()
.to_string(), .to_string(),
"BufferError: buffer contents are insufficiently aligned for u32" "BufferError: buffer contents are insufficiently aligned for u32"

View File

@ -146,13 +146,11 @@ fn test_pyref_as_base() {
#[test] #[test]
fn test_pycell_deref() { fn test_pycell_deref() {
Python::with_gil(|py| { Python::with_gil(|py| {
let cell = Bound::new(py, (SubClass {}, BaseClass { value: 120 })).unwrap(); let obj = Bound::new(py, (SubClass {}, BaseClass { value: 120 })).unwrap();
// Should be able to deref as PyAny // Should be able to deref as PyAny
// FIXME: This deref does _not_ work
assert_eq!( assert_eq!(
cell.as_any() obj.call_method0("foo")
.call_method0("foo")
.and_then(|e| e.extract::<&str>()) .and_then(|e| e.extract::<&str>())
.unwrap(), .unwrap(),
"SubClass" "SubClass"

View File

@ -22,14 +22,8 @@ fn test_cfg() {
Python::with_gil(|py| { Python::with_gil(|py| {
let cfg = CfgClass { b: 3 }; let cfg = CfgClass { b: 3 };
let py_cfg = Py::new(py, cfg).unwrap(); let py_cfg = Py::new(py, cfg).unwrap();
assert!(py_cfg.bind(py).as_any().getattr("a").is_err()); assert!(py_cfg.bind(py).getattr("a").is_err());
let b: u32 = py_cfg let b: u32 = py_cfg.bind(py).getattr("b").unwrap().extract().unwrap();
.bind(py)
.as_any()
.getattr("b")
.unwrap()
.extract()
.unwrap();
assert_eq!(b, 3); assert_eq!(b, 3);
}); });
} }

View File

@ -247,7 +247,7 @@ mod inheriting_native_type {
let item = &py.eval_bound("object()", None, None).unwrap(); let item = &py.eval_bound("object()", None, None).unwrap();
assert_eq!(item.get_refcnt(), 1); assert_eq!(item.get_refcnt(), 1);
dict_sub.bind(py).as_any().set_item("foo", item).unwrap(); dict_sub.bind(py).set_item("foo", item).unwrap();
assert_eq!(item.get_refcnt(), 2); assert_eq!(item.get_refcnt(), 2);
drop(dict_sub); drop(dict_sub);

View File

@ -123,7 +123,7 @@ fn mapping_is_not_sequence() {
PyMapping::register::<Mapping>(py).unwrap(); PyMapping::register::<Mapping>(py).unwrap();
assert!(m.bind(py).as_any().downcast::<PyMapping>().is_ok()); assert!(m.bind(py).downcast::<PyMapping>().is_ok());
assert!(m.bind(py).as_any().downcast::<PySequence>().is_err()); assert!(m.bind(py).downcast::<PySequence>().is_err());
}); });
} }

View File

@ -81,7 +81,6 @@ fn test_getattr() {
let example_py = make_example(py); let example_py = make_example(py);
assert_eq!( assert_eq!(
example_py example_py
.as_any()
.getattr("value") .getattr("value")
.unwrap() .unwrap()
.extract::<i32>() .extract::<i32>()
@ -90,7 +89,6 @@ fn test_getattr() {
); );
assert_eq!( assert_eq!(
example_py example_py
.as_any()
.getattr("special_custom_attr") .getattr("special_custom_attr")
.unwrap() .unwrap()
.extract::<i32>() .extract::<i32>()
@ -98,7 +96,6 @@ fn test_getattr() {
20, 20,
); );
assert!(example_py assert!(example_py
.as_any()
.getattr("other_attr") .getattr("other_attr")
.unwrap_err() .unwrap_err()
.is_instance_of::<PyAttributeError>(py)); .is_instance_of::<PyAttributeError>(py));
@ -109,13 +106,9 @@ fn test_getattr() {
fn test_setattr() { fn test_setattr() {
Python::with_gil(|py| { Python::with_gil(|py| {
let example_py = make_example(py); let example_py = make_example(py);
example_py example_py.setattr("special_custom_attr", 15).unwrap();
.as_any()
.setattr("special_custom_attr", 15)
.unwrap();
assert_eq!( assert_eq!(
example_py example_py
.as_any()
.getattr("special_custom_attr") .getattr("special_custom_attr")
.unwrap() .unwrap()
.extract::<i32>() .extract::<i32>()
@ -129,12 +122,8 @@ fn test_setattr() {
fn test_delattr() { fn test_delattr() {
Python::with_gil(|py| { Python::with_gil(|py| {
let example_py = make_example(py); let example_py = make_example(py);
example_py.as_any().delattr("special_custom_attr").unwrap(); example_py.delattr("special_custom_attr").unwrap();
assert!(example_py assert!(example_py.getattr("special_custom_attr").unwrap().is_none());
.as_any()
.getattr("special_custom_attr")
.unwrap()
.is_none());
}) })
} }
@ -142,7 +131,7 @@ fn test_delattr() {
fn test_str() { fn test_str() {
Python::with_gil(|py| { Python::with_gil(|py| {
let example_py = make_example(py); let example_py = make_example(py);
assert_eq!(example_py.as_any().str().unwrap().to_cow().unwrap(), "5"); assert_eq!(example_py.str().unwrap().to_cow().unwrap(), "5");
}) })
} }
@ -151,7 +140,7 @@ fn test_repr() {
Python::with_gil(|py| { Python::with_gil(|py| {
let example_py = make_example(py); let example_py = make_example(py);
assert_eq!( assert_eq!(
example_py.as_any().repr().unwrap().to_cow().unwrap(), example_py.repr().unwrap().to_cow().unwrap(),
"ExampleClass(value=5)" "ExampleClass(value=5)"
); );
}) })
@ -161,7 +150,7 @@ fn test_repr() {
fn test_hash() { fn test_hash() {
Python::with_gil(|py| { Python::with_gil(|py| {
let example_py = make_example(py); let example_py = make_example(py);
assert_eq!(example_py.as_any().hash().unwrap(), 5); assert_eq!(example_py.hash().unwrap(), 5);
}) })
} }
@ -169,9 +158,9 @@ fn test_hash() {
fn test_bool() { fn test_bool() {
Python::with_gil(|py| { Python::with_gil(|py| {
let example_py = make_example(py); let example_py = make_example(py);
assert!(example_py.as_any().is_truthy().unwrap()); assert!(example_py.is_truthy().unwrap());
example_py.borrow_mut().value = 0; example_py.borrow_mut().value = 0;
assert!(!example_py.as_any().is_truthy().unwrap()); assert!(!example_py.is_truthy().unwrap());
}) })
} }
@ -231,7 +220,7 @@ fn mapping() {
) )
.unwrap(); .unwrap();
let mapping: &Bound<'_, PyMapping> = inst.bind(py).as_any().downcast().unwrap(); let mapping: &Bound<'_, PyMapping> = inst.bind(py).downcast().unwrap();
py_assert!(py, inst, "len(inst) == 0"); py_assert!(py, inst, "len(inst) == 0");
@ -333,7 +322,7 @@ fn sequence() {
let inst = Py::new(py, Sequence { values: vec![] }).unwrap(); let inst = Py::new(py, Sequence { values: vec![] }).unwrap();
let sequence: &Bound<'_, PySequence> = inst.bind(py).as_any().downcast().unwrap(); let sequence: &Bound<'_, PySequence> = inst.bind(py).downcast().unwrap();
py_assert!(py, inst, "len(inst) == 0"); py_assert!(py, inst, "len(inst) == 0");
@ -360,16 +349,16 @@ fn sequence() {
// indices. // indices.
assert!(sequence.len().is_err()); assert!(sequence.len().is_err());
// however regular python len() works thanks to mp_len slot // however regular python len() works thanks to mp_len slot
assert_eq!(inst.bind(py).as_any().len().unwrap(), 0); assert_eq!(inst.bind(py).len().unwrap(), 0);
py_run!(py, inst, "inst.append(0)"); py_run!(py, inst, "inst.append(0)");
sequence.set_item(0, 5).unwrap(); sequence.set_item(0, 5).unwrap();
assert_eq!(inst.bind(py).as_any().len().unwrap(), 1); assert_eq!(inst.bind(py).len().unwrap(), 1);
assert_eq!(sequence.get_item(0).unwrap().extract::<u8>().unwrap(), 5); assert_eq!(sequence.get_item(0).unwrap().extract::<u8>().unwrap(), 5);
sequence.del_item(0).unwrap(); sequence.del_item(0).unwrap();
assert_eq!(inst.bind(py).as_any().len().unwrap(), 0); assert_eq!(inst.bind(py).len().unwrap(), 0);
}); });
} }