From 3fd8f6bd17d8b47a4cb4fc7ef0f627873fb47b1a Mon Sep 17 00:00:00 2001 From: Lily Foote Date: Wed, 21 Feb 2024 19:27:41 +0000 Subject: [PATCH 1/4] Implement `From>` for PyErr --- src/err/mod.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/err/mod.rs b/src/err/mod.rs index ab39e8cd46f..e64cd420a3f 100644 --- a/src/err/mod.rs +++ b/src/err/mod.rs @@ -982,6 +982,13 @@ impl PyErrArguments for PyDowncastErrorArguments { } } +impl<'py, T> std::convert::From> for PyErr { + #[inline] + fn from(err: Bound<'py, T>) -> PyErr { + PyErr::from_value_bound(err.into_any()) + } +} + /// Convert `PyDowncastError` to Python `TypeError`. impl<'a> std::convert::From> for PyErr { fn from(err: PyDowncastError<'_>) -> PyErr { From 481fcaf6eb91e752771950eff13f092ab81ee889 Mon Sep 17 00:00:00 2001 From: Lily Foote Date: Wed, 21 Feb 2024 19:37:53 +0000 Subject: [PATCH 2/4] Replace PyErr::from_value_bound calls with .into --- src/coroutine.rs | 2 +- src/exceptions.rs | 2 +- src/tests/common.rs | 4 ++-- src/types/string.rs | 40 +++++++++++++++++----------------------- src/types/traceback.rs | 2 +- 5 files changed, 22 insertions(+), 28 deletions(-) diff --git a/src/coroutine.rs b/src/coroutine.rs index b24197ad593..05240d70e64 100644 --- a/src/coroutine.rs +++ b/src/coroutine.rs @@ -78,7 +78,7 @@ impl Coroutine { (Some(exc), Some(cb)) => cb.throw(exc), (Some(exc), None) => { self.close(); - return Err(PyErr::from_value_bound(exc.into_bound(py))); + return Err(exc.into_bound(py).into()); } (None, _) => {} } diff --git a/src/exceptions.rs b/src/exceptions.rs index da48475e0d6..b38f1ea293a 100644 --- a/src/exceptions.rs +++ b/src/exceptions.rs @@ -1072,7 +1072,7 @@ mod tests { ); // Restoring should preserve the same error - let e = PyErr::from_value_bound(decode_err.into_any()); + let e: PyErr = decode_err.into(); e.restore(py); assert_eq!( diff --git a/src/tests/common.rs b/src/tests/common.rs index 854d73e4d7b..942ccd51c78 100644 --- a/src/tests/common.rs +++ b/src/tests/common.rs @@ -74,9 +74,9 @@ mod inner { #[pymethods(crate = "pyo3")] impl UnraisableCapture { pub fn hook(&mut self, unraisable: Bound<'_, PyAny>) { - let err = PyErr::from_value_bound(unraisable.getattr("exc_value").unwrap()); + let err = unraisable.getattr("exc_value").unwrap(); let instance = unraisable.getattr("object").unwrap(); - self.capture = Some((err, instance.into())); + self.capture = Some((err.into(), instance.into())); } } diff --git a/src/types/string.rs b/src/types/string.rs index 0a7847d2959..4a33236b47b 100644 --- a/src/types/string.rs +++ b/src/types/string.rs @@ -73,9 +73,7 @@ impl<'a> PyStringData<'a> { match self { Self::Ucs1(data) => match str::from_utf8(data) { Ok(s) => Ok(Cow::Borrowed(s)), - Err(e) => Err(crate::PyErr::from_value_bound( - PyUnicodeDecodeError::new_utf8_bound(py, data, e)?.into_any(), - )), + Err(e) => Err(PyUnicodeDecodeError::new_utf8_bound(py, data, e)?.into()), }, Self::Ucs2(data) => match String::from_utf16(data) { Ok(s) => Ok(Cow::Owned(s)), @@ -83,30 +81,26 @@ impl<'a> PyStringData<'a> { let mut message = e.to_string().as_bytes().to_vec(); message.push(0); - Err(crate::PyErr::from_value_bound( - PyUnicodeDecodeError::new_bound( - py, - CStr::from_bytes_with_nul(b"utf-16\0").unwrap(), - self.as_bytes(), - 0..self.as_bytes().len(), - CStr::from_bytes_with_nul(&message).unwrap(), - )? - .into_any(), - )) - } - }, - Self::Ucs4(data) => match data.iter().map(|&c| std::char::from_u32(c)).collect() { - Some(s) => Ok(Cow::Owned(s)), - None => Err(crate::PyErr::from_value_bound( - PyUnicodeDecodeError::new_bound( + Err(PyUnicodeDecodeError::new_bound( py, - CStr::from_bytes_with_nul(b"utf-32\0").unwrap(), + CStr::from_bytes_with_nul(b"utf-16\0").unwrap(), self.as_bytes(), 0..self.as_bytes().len(), - CStr::from_bytes_with_nul(b"error converting utf-32\0").unwrap(), + CStr::from_bytes_with_nul(&message).unwrap(), )? - .into_any(), - )), + .into()) + } + }, + Self::Ucs4(data) => match data.iter().map(|&c| std::char::from_u32(c)).collect() { + Some(s) => Ok(Cow::Owned(s)), + None => Err(PyUnicodeDecodeError::new_bound( + py, + CStr::from_bytes_with_nul(b"utf-32\0").unwrap(), + self.as_bytes(), + 0..self.as_bytes().len(), + CStr::from_bytes_with_nul(b"error converting utf-32\0").unwrap(), + )? + .into()), }, } } diff --git a/src/types/traceback.rs b/src/types/traceback.rs index b8782463367..fc4e39e62af 100644 --- a/src/types/traceback.rs +++ b/src/types/traceback.rs @@ -147,7 +147,7 @@ except Exception as e: Some(&locals), ) .unwrap(); - let err = PyErr::from_value_bound(locals.get_item("err").unwrap().unwrap()); + let err: PyErr = locals.get_item("err").unwrap().unwrap().into(); let traceback = err.value_bound(py).getattr("__traceback__").unwrap(); assert!(err.traceback_bound(py).unwrap().is(&traceback)); }) From 41c79ad499d717e2e67cd7b214171d10df874c1c Mon Sep 17 00:00:00 2001 From: Lily Foote Date: Wed, 21 Feb 2024 19:53:51 +0000 Subject: [PATCH 3/4] Fix From expected error message --- tests/ui/invalid_result_conversion.stderr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ui/invalid_result_conversion.stderr b/tests/ui/invalid_result_conversion.stderr index b3e65517e36..73ed2cba1aa 100644 --- a/tests/ui/invalid_result_conversion.stderr +++ b/tests/ui/invalid_result_conversion.stderr @@ -5,6 +5,7 @@ error[E0277]: the trait bound `PyErr: From` is not satisfied | ^^^^^^^^^^^^^ the trait `From` is not implemented for `PyErr` | = help: the following other types implement trait `From`: + >> > > > @@ -12,7 +13,6 @@ error[E0277]: the trait bound `PyErr: From` is not satisfied >> >> > - > and $N others = note: required for `MyError` to implement `Into` = note: this error originates in the attribute macro `pyfunction` (in Nightly builds, run with -Z macro-backtrace for more info) From ccde801949bf9e6f4757ac397966e08dd824aa7b Mon Sep 17 00:00:00 2001 From: Lily Foote Date: Thu, 22 Feb 2024 10:25:38 +0000 Subject: [PATCH 4/4] Add a trait bound to From> for PyErr --- src/coroutine.rs | 2 +- src/err/mod.rs | 13 ++++++++++++- src/exceptions.rs | 2 ++ src/lib.rs | 2 +- src/tests/common.rs | 4 ++-- src/types/traceback.rs | 2 +- 6 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/coroutine.rs b/src/coroutine.rs index 05240d70e64..b24197ad593 100644 --- a/src/coroutine.rs +++ b/src/coroutine.rs @@ -78,7 +78,7 @@ impl Coroutine { (Some(exc), Some(cb)) => cb.throw(exc), (Some(exc), None) => { self.close(); - return Err(exc.into_bound(py).into()); + return Err(PyErr::from_value_bound(exc.into_bound(py))); } (None, _) => {} } diff --git a/src/err/mod.rs b/src/err/mod.rs index e64cd420a3f..5e054449bc9 100644 --- a/src/err/mod.rs +++ b/src/err/mod.rs @@ -982,7 +982,18 @@ impl PyErrArguments for PyDowncastErrorArguments { } } -impl<'py, T> std::convert::From> for PyErr { +/// Python exceptions that can be converted to [`PyErr`]. +/// +/// This is used to implement [`From> for PyErr`]. +/// +/// Users should not need to implement this trait directly. It is implemented automatically in the +/// [`crate::import_exception!`] and [`crate::create_exception!`] macros. +pub trait ToPyErr {} + +impl<'py, T> std::convert::From> for PyErr +where + T: ToPyErr, +{ #[inline] fn from(err: Bound<'py, T>) -> PyErr { PyErr::from_value_bound(err.into_any()) diff --git a/src/exceptions.rs b/src/exceptions.rs index b38f1ea293a..ef55dcdd6e3 100644 --- a/src/exceptions.rs +++ b/src/exceptions.rs @@ -52,6 +52,8 @@ macro_rules! impl_exception_boilerplate { } } } + + impl $crate::ToPyErr for $name {} }; } diff --git a/src/lib.rs b/src/lib.rs index 0a0b4eae684..10f3088b7c3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -301,7 +301,7 @@ pub use crate::conversion::{AsPyPointer, FromPyObject, FromPyPointer, IntoPy, To #[allow(deprecated)] pub use crate::conversion::{PyTryFrom, PyTryInto}; pub use crate::err::{ - DowncastError, DowncastIntoError, PyDowncastError, PyErr, PyErrArguments, PyResult, + DowncastError, DowncastIntoError, PyDowncastError, PyErr, PyErrArguments, PyResult, ToPyErr, }; pub use crate::gil::GILPool; #[cfg(not(PyPy))] diff --git a/src/tests/common.rs b/src/tests/common.rs index 942ccd51c78..854d73e4d7b 100644 --- a/src/tests/common.rs +++ b/src/tests/common.rs @@ -74,9 +74,9 @@ mod inner { #[pymethods(crate = "pyo3")] impl UnraisableCapture { pub fn hook(&mut self, unraisable: Bound<'_, PyAny>) { - let err = unraisable.getattr("exc_value").unwrap(); + let err = PyErr::from_value_bound(unraisable.getattr("exc_value").unwrap()); let instance = unraisable.getattr("object").unwrap(); - self.capture = Some((err.into(), instance.into())); + self.capture = Some((err, instance.into())); } } diff --git a/src/types/traceback.rs b/src/types/traceback.rs index fc4e39e62af..b8782463367 100644 --- a/src/types/traceback.rs +++ b/src/types/traceback.rs @@ -147,7 +147,7 @@ except Exception as e: Some(&locals), ) .unwrap(); - let err: PyErr = locals.get_item("err").unwrap().unwrap().into(); + let err = PyErr::from_value_bound(locals.get_item("err").unwrap().unwrap()); let traceback = err.value_bound(py).getattr("__traceback__").unwrap(); assert!(err.traceback_bound(py).unwrap().is(&traceback)); })