From 7c03db7b4ede021e2ba74860b4dee8ad617ef24d Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Wed, 3 Sep 2025 20:13:38 +0200 Subject: [PATCH] use custom error type in pyclass extraction --- newsfragments/5413.changed.md | 1 + src/conversion.rs | 36 +++++++++------ src/pycell.rs | 12 +++++ src/pyclass.rs | 4 +- src/pyclass/guard.rs | 87 +++++++++++++++++++++++++++++++++-- 5 files changed, 120 insertions(+), 20 deletions(-) create mode 100644 newsfragments/5413.changed.md diff --git a/newsfragments/5413.changed.md b/newsfragments/5413.changed.md new file mode 100644 index 00000000000..74cec2205c8 --- /dev/null +++ b/newsfragments/5413.changed.md @@ -0,0 +1 @@ +`PyClassGuard(Mut)` and `PyRef(Mut)` extraction now returns an opaque Rust error \ No newline at end of file diff --git a/src/conversion.rs b/src/conversion.rs index 3fdb0c47060..0462844f230 100644 --- a/src/conversion.rs +++ b/src/conversion.rs @@ -3,8 +3,11 @@ use crate::err::PyResult; #[cfg(feature = "experimental-inspect")] use crate::inspect::types::TypeInfo; use crate::pyclass::boolean_struct::False; +use crate::pyclass::{PyClassGuardError, PyClassGuardMutError}; use crate::types::PyTuple; -use crate::{Borrowed, Bound, BoundObject, Py, PyAny, PyClass, PyErr, PyRef, PyRefMut, Python}; +use crate::{ + Borrowed, Bound, BoundObject, Py, PyAny, PyClass, PyClassGuard, PyErr, PyRef, PyRefMut, Python, +}; use std::convert::Infallible; /// Defines a conversion from a Rust type to a Python object, which may fail. @@ -460,46 +463,51 @@ pub trait FromPyObject<'a, 'py>: Sized { pub trait FromPyObjectOwned<'py>: for<'a> FromPyObject<'a, 'py> {} impl<'py, T> FromPyObjectOwned<'py> for T where T: for<'a> FromPyObject<'a, 'py> {} -impl FromPyObject<'_, '_> for T +impl<'a, 'py, T> FromPyObject<'a, 'py> for T where T: PyClass + Clone, { - type Error = PyErr; + type Error = PyClassGuardError<'a, 'py>; #[cfg(feature = "experimental-inspect")] const INPUT_TYPE: &'static str = ::TYPE_NAME; - fn extract(obj: Borrowed<'_, '_, PyAny>) -> Result { - let bound = obj.cast::()?; - Ok(bound.try_borrow()?.clone()) + fn extract(obj: Borrowed<'a, 'py, PyAny>) -> Result { + Ok(obj.extract::>()?.clone()) } } -impl<'py, T> FromPyObject<'_, 'py> for PyRef<'py, T> +impl<'a, 'py, T> FromPyObject<'a, 'py> for PyRef<'py, T> where T: PyClass, { - type Error = PyErr; + type Error = PyClassGuardError<'a, 'py>; #[cfg(feature = "experimental-inspect")] const INPUT_TYPE: &'static str = ::TYPE_NAME; - fn extract(obj: Borrowed<'_, 'py, PyAny>) -> Result { - obj.cast::()?.try_borrow().map_err(Into::into) + fn extract(obj: Borrowed<'a, 'py, PyAny>) -> Result { + obj.cast::() + .map_err(|e| PyClassGuardError(Some(e)))? + .try_borrow() + .map_err(|_| PyClassGuardError(None)) } } -impl<'py, T> FromPyObject<'_, 'py> for PyRefMut<'py, T> +impl<'a, 'py, T> FromPyObject<'a, 'py> for PyRefMut<'py, T> where T: PyClass, { - type Error = PyErr; + type Error = PyClassGuardMutError<'a, 'py>; #[cfg(feature = "experimental-inspect")] const INPUT_TYPE: &'static str = ::TYPE_NAME; - fn extract(obj: Borrowed<'_, 'py, PyAny>) -> Result { - obj.cast::()?.try_borrow_mut().map_err(Into::into) + fn extract(obj: Borrowed<'a, 'py, PyAny>) -> Result { + obj.cast::() + .map_err(|e| PyClassGuardMutError(Some(e)))? + .try_borrow_mut() + .map_err(|_| PyClassGuardMutError(None)) } } diff --git a/src/pycell.rs b/src/pycell.rs index fed6b69013f..c1f9606e40c 100644 --- a/src/pycell.rs +++ b/src/pycell.rs @@ -687,6 +687,12 @@ pub struct PyBorrowError { _private: (), } +impl PyBorrowError { + pub(crate) fn new() -> Self { + Self { _private: () } + } +} + impl fmt::Debug for PyBorrowError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("PyBorrowError").finish() @@ -712,6 +718,12 @@ pub struct PyBorrowMutError { _private: (), } +impl PyBorrowMutError { + pub(crate) fn new() -> Self { + Self { _private: () } + } +} + impl fmt::Debug for PyBorrowMutError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("PyBorrowMutError").finish() diff --git a/src/pyclass.rs b/src/pyclass.rs index cd21f8876ed..f5a3fd5e4eb 100644 --- a/src/pyclass.rs +++ b/src/pyclass.rs @@ -9,7 +9,9 @@ mod guard; pub(crate) use self::create_type_object::{create_type_object, PyClassTypeObject}; pub use self::gc::{PyTraverseError, PyVisit}; -pub use self::guard::{PyClassGuard, PyClassGuardMap, PyClassGuardMut}; +pub use self::guard::{ + PyClassGuard, PyClassGuardError, PyClassGuardMap, PyClassGuardMut, PyClassGuardMutError, +}; /// Types that can be used as Python classes. /// diff --git a/src/pyclass/guard.rs b/src/pyclass/guard.rs index 315b5fe6e0a..cc2b949e42e 100644 --- a/src/pyclass/guard.rs +++ b/src/pyclass/guard.rs @@ -2,8 +2,9 @@ use crate::impl_::pycell::{PyClassObject, PyClassObjectLayout as _}; use crate::pycell::PyBorrowMutError; use crate::pycell::{impl_::PyClassBorrowChecker, PyBorrowError}; use crate::pyclass::boolean_struct::False; -use crate::{ffi, Borrowed, FromPyObject, IntoPyObject, Py, PyClass, PyErr}; +use crate::{ffi, Borrowed, DowncastError, FromPyObject, IntoPyObject, Py, PyClass, PyErr}; use std::convert::Infallible; +use std::fmt; use std::marker::PhantomData; use std::ops::{Deref, DerefMut}; use std::ptr::NonNull; @@ -277,10 +278,15 @@ impl Deref for PyClassGuard<'_, T> { } impl<'a, 'py, T: PyClass> FromPyObject<'a, 'py> for PyClassGuard<'a, T> { - type Error = PyErr; + type Error = PyClassGuardError<'a, 'py>; fn extract(obj: Borrowed<'a, 'py, crate::PyAny>) -> Result { - Self::try_from_class_object(obj.cast()?.get_class_object()).map_err(Into::into) + Self::try_from_class_object( + obj.cast() + .map_err(|e| PyClassGuardError(Some(e)))? + .get_class_object(), + ) + .map_err(|_| PyClassGuardError(None)) } } @@ -327,6 +333,39 @@ unsafe impl crate::marker::Ungil for PyClassGuard<'_, T> {} unsafe impl Send for PyClassGuard<'_, T> {} unsafe impl Sync for PyClassGuard<'_, T> {} +/// Custom error type for extracting a [PyClassGuard] +pub struct PyClassGuardError<'a, 'py>(pub(crate) Option>); + +impl fmt::Debug for PyClassGuardError<'_, '_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if let Some(e) = &self.0 { + write!(f, "{e:?}") + } else { + write!(f, "{:?}", PyBorrowError::new()) + } + } +} + +impl fmt::Display for PyClassGuardError<'_, '_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if let Some(e) = &self.0 { + write!(f, "{e}") + } else { + write!(f, "{}", PyBorrowError::new()) + } + } +} + +impl From> for PyErr { + fn from(value: PyClassGuardError<'_, '_>) -> Self { + if let Some(e) = value.0 { + e.into() + } else { + PyBorrowError::new().into() + } + } +} + /// A wrapper type for a mutably borrowed value from a `PyClass` /// /// # When *not* to use [`PyClassGuardMut`] @@ -642,10 +681,15 @@ impl> DerefMut for PyClassGuardMut<'_, T> { } impl<'a, 'py, T: PyClass> FromPyObject<'a, 'py> for PyClassGuardMut<'a, T> { - type Error = PyErr; + type Error = PyClassGuardMutError<'a, 'py>; fn extract(obj: Borrowed<'a, 'py, crate::PyAny>) -> Result { - Self::try_from_class_object(obj.cast()?.get_class_object()).map_err(Into::into) + Self::try_from_class_object( + obj.cast() + .map_err(|e| PyClassGuardMutError(Some(e)))? + .get_class_object(), + ) + .map_err(|_| PyClassGuardMutError(None)) } } @@ -690,6 +734,39 @@ unsafe impl> crate::marker::Ungil for PyClassGuardMut unsafe impl + Send + Sync> Send for PyClassGuardMut<'_, T> {} unsafe impl + Sync> Sync for PyClassGuardMut<'_, T> {} +/// Custom error type for extracting a [PyClassGuardMut] +pub struct PyClassGuardMutError<'a, 'py>(pub(crate) Option>); + +impl fmt::Debug for PyClassGuardMutError<'_, '_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if let Some(e) = &self.0 { + write!(f, "{e:?}") + } else { + write!(f, "{:?}", PyBorrowMutError::new()) + } + } +} + +impl fmt::Display for PyClassGuardMutError<'_, '_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if let Some(e) = &self.0 { + write!(f, "{e}") + } else { + write!(f, "{}", PyBorrowMutError::new()) + } + } +} + +impl From> for PyErr { + fn from(value: PyClassGuardMutError<'_, '_>) -> Self { + if let Some(e) = value.0 { + e.into() + } else { + PyBorrowMutError::new().into() + } + } +} + /// Wraps a borrowed reference `U` to a value stored inside of a pyclass `T` /// /// See [`PyClassGuard::map`] and [`PyClassGuardMut::map`]