From 7c10ff4327b613b6239b0c270e2aaf490b0d6b45 Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Sun, 25 Feb 2024 08:13:36 +0100 Subject: [PATCH] allow `Bound<'_, T>` in #[pymethods] `self` position (#3896) * allow `Bound<'_, T>` in #[pymethods] `self` position * rename `TryFromPyCell` -> `TryFromBoundRef` * remove unneccessary unsafe --- pyo3-macros-backend/src/method.rs | 11 +++---- pyo3-macros-backend/src/pyclass.rs | 2 +- src/impl_/pymethods.rs | 40 +++++++++++++++++++++-- src/pycell.rs | 2 +- tests/ui/invalid_pymethod_receiver.stderr | 8 ++--- 5 files changed, 49 insertions(+), 14 deletions(-) diff --git a/pyo3-macros-backend/src/method.rs b/pyo3-macros-backend/src/method.rs index f492a330..2d21c265 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,15 +204,14 @@ impl SelfType { ) }) } - SelfType::TryFromPyCell(span) => { + SelfType::TryFromBoundRef(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) ) } @@ -291,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 ce39cb01..784c39f7 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()), diff --git a/src/impl_/pymethods.rs b/src/impl_/pymethods.rs index 196766d0..9afb6699 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,10 @@ impl<'a, 'py> BoundRef<'a, 'py, PyAny> { Bound::ref_from_ptr_or_opt(py, ptr).as_ref().map(BoundRef) } + pub 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 +517,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 989d039f..71f2f7cc 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 b7a7880d..b6dd44bd 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>`