From 1c0b4b5432b1bd00c0fe8f6da3426ca8fcc7d54f Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Sat, 24 Feb 2024 16:05:44 +0100 Subject: [PATCH 1/3] allow `Bound<'_, T>` in #[pymethods] `self` position --- pyo3-macros-backend/src/method.rs | 5 ++- src/impl_/pymethods.rs | 42 +++++++++++++++++++++-- src/pycell.rs | 2 +- tests/ui/invalid_pymethod_receiver.stderr | 8 ++--- 4 files changed, 47 insertions(+), 10 deletions(-) diff --git a/pyo3-macros-backend/src/method.rs b/pyo3-macros-backend/src/method.rs index f492a330c92..bc6b890cb34 100644 --- a/pyo3-macros-backend/src/method.rs +++ b/pyo3-macros-backend/src/method.rs @@ -207,12 +207,11 @@ impl SelfType { SelfType::TryFromPyCell(span) => { error_mode.handle_error( quote_spanned! { *span => - #py.from_borrowed_ptr::<_pyo3::PyAny>(#slf).downcast::<_pyo3::PyCell<#cls>>() + _pyo3::impl_::pymethods::BoundRef::ref_from_ptr(#py, &#slf).downcast::<#cls>() .map_err(::std::convert::Into::<_pyo3::PyErr>::into) .and_then( - #[allow(clippy::useless_conversion)] // In case slf is PyCell #[allow(unknown_lints, clippy::unnecessary_fallible_conversions)] // In case slf is Py (unknown_lints can be removed when MSRV is 1.75+) - |cell| ::std::convert::TryFrom::try_from(cell).map_err(::std::convert::Into::into) + |bound| ::std::convert::TryFrom::try_from(bound).map_err(::std::convert::Into::into) ) } diff --git a/src/impl_/pymethods.rs b/src/impl_/pymethods.rs index 196766d0034..e069988101c 100644 --- a/src/impl_/pymethods.rs +++ b/src/impl_/pymethods.rs @@ -3,10 +3,12 @@ use crate::exceptions::PyStopAsyncIteration; use crate::gil::LockGIL; use crate::impl_::panic::PanicTrap; use crate::internal_tricks::extract_c_string; +use crate::pycell::{PyBorrowError, PyBorrowMutError}; +use crate::pyclass::boolean_struct::False; use crate::types::{any::PyAnyMethods, PyModule, PyType}; use crate::{ - ffi, Bound, Py, PyAny, PyCell, PyClass, PyErr, PyObject, PyResult, PyTraverseError, PyVisit, - Python, + ffi, Bound, DowncastError, Py, PyAny, PyCell, PyClass, PyErr, PyObject, PyRef, PyRefMut, + PyResult, PyTraverseError, PyTypeCheck, PyVisit, Python, }; use std::borrow::Cow; use std::ffi::CStr; @@ -490,6 +492,12 @@ impl<'a, 'py> BoundRef<'a, 'py, PyAny> { Bound::ref_from_ptr_or_opt(py, ptr).as_ref().map(BoundRef) } + pub unsafe fn downcast( + self, + ) -> Result, DowncastError<'a, 'py>> { + self.0.downcast::().map(BoundRef) + } + pub unsafe fn downcast_unchecked(self) -> BoundRef<'a, 'py, T> { BoundRef(self.0.downcast_unchecked::()) } @@ -511,6 +519,36 @@ impl<'a> From> for &'a PyModule { } } +impl<'a, 'py, T: PyClass> From> for &'a PyCell { + #[inline] + fn from(bound: BoundRef<'a, 'py, T>) -> Self { + bound.0.as_gil_ref() + } +} + +impl<'a, 'py, T: PyClass> TryFrom> for PyRef<'py, T> { + type Error = PyBorrowError; + #[inline] + fn try_from(value: BoundRef<'a, 'py, T>) -> Result { + value.0.clone().into_gil_ref().try_into() + } +} + +impl<'a, 'py, T: PyClass> TryFrom> for PyRefMut<'py, T> { + type Error = PyBorrowMutError; + #[inline] + fn try_from(value: BoundRef<'a, 'py, T>) -> Result { + value.0.clone().into_gil_ref().try_into() + } +} + +impl<'a, 'py, T> From> for Bound<'py, T> { + #[inline] + fn from(bound: BoundRef<'a, 'py, T>) -> Self { + bound.0.clone() + } +} + impl<'a, 'py, T> From> for &'a Bound<'py, T> { #[inline] fn from(bound: BoundRef<'a, 'py, T>) -> Self { diff --git a/src/pycell.rs b/src/pycell.rs index 5a42dfa514a..6c681b33fa5 100644 --- a/src/pycell.rs +++ b/src/pycell.rs @@ -654,7 +654,7 @@ impl fmt::Debug for PyCell { /// } /// # Python::with_gil(|py| { /// # let sub = Py::new(py, Child::new()).unwrap(); -/// # pyo3::py_run!(py, sub, "assert sub.format() == 'Caterpillar(base: Butterfly, cnt: 3)'"); +/// # pyo3::py_run!(py, sub, "assert sub.format() == 'Caterpillar(base: Butterfly, cnt: 4)'"); /// # }); /// ``` /// diff --git a/tests/ui/invalid_pymethod_receiver.stderr b/tests/ui/invalid_pymethod_receiver.stderr index b7a7880dbde..b6dd44bd9bf 100644 --- a/tests/ui/invalid_pymethod_receiver.stderr +++ b/tests/ui/invalid_pymethod_receiver.stderr @@ -1,8 +1,8 @@ -error[E0277]: the trait bound `i32: From<&PyCell>` is not satisfied +error[E0277]: the trait bound `i32: From>` is not satisfied --> tests/ui/invalid_pymethod_receiver.rs:8:43 | 8 | fn method_with_invalid_self_type(slf: i32, py: Python<'_>, index: u32) {} - | ^^^ the trait `From<&PyCell>` is not implemented for `i32` + | ^^^ the trait `From>` is not implemented for `i32` | = help: the following other types implement trait `From`: > @@ -11,5 +11,5 @@ error[E0277]: the trait bound `i32: From<&PyCell>` is not satisfied > > > - = note: required for `&PyCell` to implement `Into` - = note: required for `i32` to implement `TryFrom<&PyCell>` + = note: required for `BoundRef<'_, '_, MyClass>` to implement `Into` + = note: required for `i32` to implement `TryFrom>` From bf7ee7f46611d40ea91ef42c4b3e88e899f46ac0 Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Sat, 24 Feb 2024 17:24:12 +0100 Subject: [PATCH 2/3] rename `TryFromPyCell` -> `TryFromBoundRef` --- pyo3-macros-backend/src/method.rs | 6 +++--- pyo3-macros-backend/src/pyclass.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pyo3-macros-backend/src/method.rs b/pyo3-macros-backend/src/method.rs index bc6b890cb34..2d21c265683 100644 --- a/pyo3-macros-backend/src/method.rs +++ b/pyo3-macros-backend/src/method.rs @@ -151,7 +151,7 @@ impl FnType { #[derive(Clone, Debug)] pub enum SelfType { Receiver { mutable: bool, span: Span }, - TryFromPyCell(Span), + TryFromBoundRef(Span), } #[derive(Clone, Copy)] @@ -204,7 +204,7 @@ impl SelfType { ) }) } - SelfType::TryFromPyCell(span) => { + SelfType::TryFromBoundRef(span) => { error_mode.handle_error( quote_spanned! { *span => _pyo3::impl_::pymethods::BoundRef::ref_from_ptr(#py, &#slf).downcast::<#cls>() @@ -290,7 +290,7 @@ pub fn parse_method_receiver(arg: &syn::FnArg) -> Result { if let syn::Type::ImplTrait(_) = &**ty { bail_spanned!(ty.span() => IMPL_TRAIT_ERR); } - Ok(SelfType::TryFromPyCell(ty.span())) + Ok(SelfType::TryFromBoundRef(ty.span())) } } } diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index ce39cb01196..784c39f71aa 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -1188,7 +1188,7 @@ fn complex_enum_variant_field_getter<'a>( ) -> Result { let signature = crate::pyfunction::FunctionSignature::from_arguments(vec![])?; - let self_type = crate::method::SelfType::TryFromPyCell(field_span); + let self_type = crate::method::SelfType::TryFromBoundRef(field_span); let spec = FnSpec { tp: crate::method::FnType::Getter(self_type.clone()), From cb4e6e76f3838b02cdc983c5303a586c912c6b9e Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Sat, 24 Feb 2024 17:25:07 +0100 Subject: [PATCH 3/3] remove unneccessary unsafe --- src/impl_/pymethods.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/impl_/pymethods.rs b/src/impl_/pymethods.rs index e069988101c..9afb6699269 100644 --- a/src/impl_/pymethods.rs +++ b/src/impl_/pymethods.rs @@ -492,9 +492,7 @@ impl<'a, 'py> BoundRef<'a, 'py, PyAny> { Bound::ref_from_ptr_or_opt(py, ptr).as_ref().map(BoundRef) } - pub unsafe fn downcast( - self, - ) -> Result, DowncastError<'a, 'py>> { + pub fn downcast(self) -> Result, DowncastError<'a, 'py>> { self.0.downcast::().map(BoundRef) }