From 381ae2384eba1d80a88d8a37f5e070fabe0c4561 Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Tue, 25 Aug 2020 20:33:36 +0100 Subject: [PATCH] Implement std::error::Error for PyErr --- CHANGELOG.md | 13 + guide/src/exception.md | 10 +- pyo3-derive-backend/src/from_pyobject.rs | 4 +- src/buffer.rs | 12 +- src/callback.rs | 8 +- src/class/iter.rs | 2 +- src/class/pyasync.rs | 2 +- src/derive_utils.rs | 2 +- src/err/err_state.rs | 63 +++ src/err/impls.rs | 107 +++++ src/{err.rs => err/mod.rs} | 508 +++++++++-------------- src/exceptions.rs | 57 +-- src/lib.rs | 2 +- src/pycell.rs | 4 +- src/pyclass.rs | 2 +- src/types/any.rs | 2 +- src/types/bytearray.rs | 2 +- src/types/bytes.rs | 2 +- src/types/num.rs | 4 +- src/types/sequence.rs | 2 +- src/types/tuple.rs | 2 +- tests/test_buffer_protocol.rs | 4 +- tests/test_exceptions.rs | 2 +- tests/test_frompyobject.rs | 21 +- tests/test_mapping.rs | 4 +- tests/test_sequence.rs | 8 +- tests/test_various.rs | 2 +- 27 files changed, 451 insertions(+), 400 deletions(-) create mode 100644 src/err/err_state.rs create mode 100644 src/err/impls.rs rename src/{err.rs => err/mod.rs} (50%) diff --git a/CHANGELOG.md b/CHANGELOG.md index e3886d0e1da..0c30f49ad8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,8 +29,21 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - `PyObject` is now just a type alias for `Py`. [#1063](https://github.com/PyO3/pyo3/pull/1063) - Implement `Send + Sync` for `PyErr`. `PyErr::new`, `PyErr::from_type`, `PyException::py_err` and `PyException::into` have had these bounds added to their arguments. [#1067](https://github.com/PyO3/pyo3/pull/1067) - Change `#[pyproto]` to return NotImplemented for operators for which Python can try a reversed operation. #[1072](https://github.com/PyO3/pyo3/pull/1072) +- Rework PyErr to be compatible with the `std::error::Error` trait: [#1115](https://github.com/PyO3/pyo3/pull/1115) + - Implement `Display` and `Error` for `PyErr`. + - Add `PyErr::instance()` which returns `&PyBaseException`. + - Add `PyErr::from_err_args`. + - `PyErr`'s fields are now an implementation detail. The equivalent values can be accessed with `PyErr::ptype()`, `PyErr::pvalue()` and `PyErr::ptraceback()`. + - Change `PyErr::print()` and `PyErr::print_and_set_sys_last_vars()` to take `&self` instead of `self`. + - Remove `PyErr::into_normalized()` and `PyErr::normalize()`. + - Remove `PyErr::from_value`, `PyErr::into_normalized()` and `PyErr::normalize()`. + - Change `PyErrValue` to be a private type. + - Remove `PyException::into()` and `Into>` for `PyErr` and `PyException`. + - Rename `PyException::py_err()` to `PyException::new_err()`. + - Rename `PyUnicodeDecodeErr::new_err()` to `PyUnicodeDecodeErr::new()`. - `PyModule::add` now uses `IntoPy` instead of `ToPyObject`. #[1124](https://github.com/PyO3/pyo3/pull/1124) + ### Removed - Remove `PyString::as_bytes`. [#1023](https://github.com/PyO3/pyo3/pull/1023) - Remove `Python::register_any`. [#1023](https://github.com/PyO3/pyo3/pull/1023) diff --git a/guide/src/exception.md b/guide/src/exception.md index 3a9144f1dee..1d02b86eb43 100644 --- a/guide/src/exception.md +++ b/guide/src/exception.md @@ -76,7 +76,7 @@ If you already have a Python exception instance, you can simply call [`PyErr::fr PyErr::from_instance(py, err).restore(py); ``` -If a Rust type exists for the exception, then it is possible to use the `py_err` method. +If a Rust type exists for the exception, then it is possible to use the `new_err` method. For example, each standard exception defined in the `pyo3::exceptions` module has a corresponding Rust type, exceptions defined by [`create_exception!`] and [`import_exception!`] macro have Rust types as well. @@ -87,7 +87,7 @@ have Rust types as well. # fn check_for_error() -> bool {false} fn my_func(arg: PyObject) -> PyResult<()> { if check_for_error() { - Err(PyValueError::py_err("argument is wrong")) + Err(PyValueError::new_err("argument is wrong")) } else { Ok(()) } @@ -123,7 +123,7 @@ To check the type of an exception, you can simply do: # fn main() { # let gil = Python::acquire_gil(); # let py = gil.python(); -# let err = PyTypeError::py_err(()); +# let err = PyTypeError::new_err(()); err.is_instance::(py); # } ``` @@ -170,7 +170,7 @@ until a `Python` object is available. # } impl std::convert::From for PyErr { fn from(err: CustomIOError) -> PyErr { - PyOSError::py_err(err.to_string()) + PyOSError::new_err(err.to_string()) } } @@ -213,7 +213,7 @@ fn tell(file: &PyAny) -> PyResult { use pyo3::exceptions::*; match file.call_method0("tell") { - Err(_) => Err(io::UnsupportedOperation::py_err("not supported: tell")), + Err(_) => Err(io::UnsupportedOperation::new_err("not supported: tell")), Ok(x) => x.extract::(), } } diff --git a/pyo3-derive-backend/src/from_pyobject.rs b/pyo3-derive-backend/src/from_pyobject.rs index f7688e2b3e4..d35af6da9dd 100644 --- a/pyo3-derive-backend/src/from_pyobject.rs +++ b/pyo3-derive-backend/src/from_pyobject.rs @@ -78,7 +78,7 @@ impl<'a> Enum<'a> { .map(|s| format!("{} ({})", s.to_string_lossy(), type_name)) .unwrap_or_else(|_| type_name.to_string()); let err_msg = format!("Can't convert {} to {}", from, #error_names); - Err(::pyo3::exceptions::PyTypeError::py_err(err_msg)) + Err(::pyo3::exceptions::PyTypeError::new_err(err_msg)) ) } } @@ -263,7 +263,7 @@ impl<'a> Container<'a> { quote!( let s = <::pyo3::types::PyTuple as ::pyo3::conversion::PyTryFrom>::try_from(obj)?; if s.len() != #len { - return Err(::pyo3::exceptions::PyValueError::py_err(#msg)) + return Err(::pyo3::exceptions::PyValueError::new_err(#msg)) } let slice = s.as_slice(); Ok(#self_ty(#fields)) diff --git a/src/buffer.rs b/src/buffer.rs index 4cbb16e1109..2e7c401377f 100644 --- a/src/buffer.rs +++ b/src/buffer.rs @@ -156,10 +156,10 @@ pub unsafe trait Element: Copy { fn validate(b: &ffi::Py_buffer) -> PyResult<()> { // shape and stride information must be provided when we use PyBUF_FULL_RO if b.shape.is_null() { - return Err(exceptions::PyBufferError::py_err("Shape is Null")); + return Err(exceptions::PyBufferError::new_err("Shape is Null")); } if b.strides.is_null() { - return Err(exceptions::PyBufferError::py_err( + return Err(exceptions::PyBufferError::new_err( "PyBuffer: Strides is Null", )); } @@ -190,7 +190,7 @@ impl PyBuffer { { Ok(buf) } else { - Err(exceptions::PyBufferError::py_err( + Err(exceptions::PyBufferError::new_err( "Incompatible type as buffer", )) } @@ -441,7 +441,7 @@ impl PyBuffer { fn copy_to_slice_impl(&self, py: Python, target: &mut [T], fort: u8) -> PyResult<()> { if mem::size_of_val(target) != self.len_bytes() { - return Err(exceptions::PyBufferError::py_err( + return Err(exceptions::PyBufferError::new_err( "Slice length does not match buffer length.", )); } @@ -528,7 +528,7 @@ impl PyBuffer { return buffer_readonly_error(); } if mem::size_of_val(source) != self.len_bytes() { - return Err(exceptions::PyBufferError::py_err( + return Err(exceptions::PyBufferError::new_err( "Slice length does not match buffer length.", )); } @@ -564,7 +564,7 @@ impl PyBuffer { #[inline(always)] fn buffer_readonly_error() -> PyResult<()> { - Err(exceptions::PyBufferError::py_err( + Err(exceptions::PyBufferError::new_err( "Cannot write to read-only buffer.", )) } diff --git a/src/callback.rs b/src/callback.rs index 5f83cfaa96a..2f2632c63c9 100644 --- a/src/callback.rs +++ b/src/callback.rs @@ -86,7 +86,7 @@ impl IntoPyCallbackOutput for usize { if self <= (isize::MAX as usize) { Ok(self as isize) } else { - Err(PyOverflowError::py_err(())) + Err(PyOverflowError::new_err(())) } } } @@ -244,11 +244,11 @@ macro_rules! callback_body_without_convert { Err(e) => { // Try to format the error in the same way panic does if let Some(string) = e.downcast_ref::() { - Err($crate::panic::PanicException::py_err((string.clone(),))) + Err($crate::panic::PanicException::new_err((string.clone(),))) } else if let Some(s) = e.downcast_ref::<&str>() { - Err($crate::panic::PanicException::py_err((s.to_string(),))) + Err($crate::panic::PanicException::new_err((s.to_string(),))) } else { - Err($crate::panic::PanicException::py_err(( + Err($crate::panic::PanicException::new_err(( "panic from Rust code", ))) } diff --git a/src/class/iter.rs b/src/class/iter.rs index 0818feb0a44..057eea57807 100644 --- a/src/class/iter.rs +++ b/src/class/iter.rs @@ -112,7 +112,7 @@ impl IntoPyCallbackOutput<*mut ffi::PyObject> for PyIterNextOutput { fn convert(self, _py: Python) -> PyResult<*mut ffi::PyObject> { match self { IterNextOutput::Yield(o) => Ok(o.into_ptr()), - IterNextOutput::Return(opt) => Err(crate::exceptions::PyStopIteration::py_err((opt,))), + IterNextOutput::Return(opt) => Err(crate::exceptions::PyStopIteration::new_err((opt,))), } } } diff --git a/src/class/pyasync.rs b/src/class/pyasync.rs index 82548835c7f..ddf30b56d3a 100644 --- a/src/class/pyasync.rs +++ b/src/class/pyasync.rs @@ -119,7 +119,7 @@ impl IntoPyCallbackOutput<*mut ffi::PyObject> for PyIterANextOutput { match self { IterANextOutput::Yield(o) => Ok(o.into_ptr()), IterANextOutput::Return(opt) => { - Err(crate::exceptions::PyStopAsyncIteration::py_err((opt,))) + Err(crate::exceptions::PyStopAsyncIteration::new_err((opt,))) } } } diff --git a/src/derive_utils.rs b/src/derive_utils.rs index 2a736d7ebcd..ead7a42d867 100644 --- a/src/derive_utils.rs +++ b/src/derive_utils.rs @@ -43,7 +43,7 @@ pub fn parse_fn_args<'p>( let nargs = args.len(); let mut used_args = 0; macro_rules! raise_error { - ($s: expr $(,$arg:expr)*) => (return Err(PyTypeError::py_err(format!( + ($s: expr $(,$arg:expr)*) => (return Err(PyTypeError::new_err(format!( concat!("{} ", $s), fname.unwrap_or("function") $(,$arg)* )))) } diff --git a/src/err/err_state.rs b/src/err/err_state.rs new file mode 100644 index 00000000000..1ad4343249a --- /dev/null +++ b/src/err/err_state.rs @@ -0,0 +1,63 @@ +use crate::{ + exceptions::PyBaseException, ffi, types::PyType, IntoPyPointer, Py, PyObject, Python, + ToPyObject, +}; + +pub(crate) enum PyErrValue { + ToArgs(Box), + ToObject(Box), +} + +#[derive(Clone)] +pub(crate) struct PyErrStateNormalized { + pub ptype: Py, + pub pvalue: Py, + pub ptraceback: Option, +} + +pub(crate) enum PyErrState { + Lazy { + ptype: Py, + pvalue: Option, + }, + FfiTuple { + ptype: Option, + pvalue: Option, + ptraceback: Option, + }, + Normalized(PyErrStateNormalized), +} + +/// Helper conversion trait that allows to use custom arguments for exception constructor. +pub trait PyErrArguments { + /// Arguments for exception + fn arguments(&self, _: Python) -> PyObject; +} + +impl PyErrState { + pub fn into_ffi_tuple( + self, + py: Python, + ) -> (*mut ffi::PyObject, *mut ffi::PyObject, *mut ffi::PyObject) { + match self { + PyErrState::Lazy { ptype, pvalue } => { + let pvalue = match pvalue { + Some(PyErrValue::ToArgs(ob)) => ob.arguments(py).into_ptr(), + Some(PyErrValue::ToObject(ob)) => ob.to_object(py).into_ptr(), + None => std::ptr::null_mut(), + }; + (ptype.into_ptr(), pvalue, std::ptr::null_mut()) + } + PyErrState::FfiTuple { + ptype, + pvalue, + ptraceback, + } => (ptype.into_ptr(), pvalue.into_ptr(), ptraceback.into_ptr()), + PyErrState::Normalized(PyErrStateNormalized { + ptype, + pvalue, + ptraceback, + }) => (ptype.into_ptr(), pvalue.into_ptr(), ptraceback.into_ptr()), + } + } +} diff --git a/src/err/impls.rs b/src/err/impls.rs new file mode 100644 index 00000000000..05a9d612b6e --- /dev/null +++ b/src/err/impls.rs @@ -0,0 +1,107 @@ +use crate::{err::PyErrArguments, exceptions, IntoPy, PyErr, PyObject, Python}; +use std::io; + +/// Convert `PyErr` to `io::Error` +impl std::convert::From for io::Error { + fn from(err: PyErr) -> Self { + io::Error::new(io::ErrorKind::Other, format!("Python exception: {:?}", err)) + } +} + +/// Create `OSError` from `io::Error` +impl std::convert::From for PyErr { + fn from(err: io::Error) -> PyErr { + match err.kind() { + io::ErrorKind::BrokenPipe => { + PyErr::from_err_args::(err) + } + io::ErrorKind::ConnectionRefused => { + PyErr::from_err_args::(err) + } + io::ErrorKind::ConnectionAborted => { + PyErr::from_err_args::(err) + } + io::ErrorKind::ConnectionReset => { + PyErr::from_err_args::(err) + } + io::ErrorKind::Interrupted => { + PyErr::from_err_args::(err) + } + io::ErrorKind::NotFound => { + PyErr::from_err_args::(err) + } + io::ErrorKind::WouldBlock => { + PyErr::from_err_args::(err) + } + io::ErrorKind::TimedOut => PyErr::from_err_args::(err), + _ => PyErr::from_err_args::(err), + } + } +} + +impl PyErrArguments for io::Error { + fn arguments(&self, py: Python) -> PyObject { + self.to_string().into_py(py) + } +} + +impl std::convert::From> + for PyErr +{ + fn from(err: std::io::IntoInnerError) -> PyErr { + PyErr::from_err_args::(err) + } +} + +impl PyErrArguments for std::io::IntoInnerError { + fn arguments(&self, py: Python) -> PyObject { + self.to_string().into_py(py) + } +} + +impl PyErrArguments for std::convert::Infallible { + fn arguments(&self, py: Python) -> PyObject { + "Infalliable!".into_py(py) + } +} + +impl std::convert::From for PyErr { + fn from(_: std::convert::Infallible) -> PyErr { + PyErr::new::("Infalliable!") + } +} + +macro_rules! impl_to_pyerr { + ($err: ty, $pyexc: ty) => { + impl PyErrArguments for $err { + fn arguments(&self, py: Python) -> PyObject { + self.to_string().into_py(py) + } + } + + impl std::convert::From<$err> for PyErr { + fn from(err: $err) -> PyErr { + PyErr::from_err_args::<$pyexc, _>(err) + } + } + }; +} + +impl_to_pyerr!(std::array::TryFromSliceError, exceptions::PyValueError); +impl_to_pyerr!(std::num::ParseIntError, exceptions::PyValueError); +impl_to_pyerr!(std::num::ParseFloatError, exceptions::PyValueError); +impl_to_pyerr!(std::num::TryFromIntError, exceptions::PyValueError); +impl_to_pyerr!(std::str::ParseBoolError, exceptions::PyValueError); +impl_to_pyerr!(std::ffi::IntoStringError, exceptions::PyUnicodeDecodeError); +impl_to_pyerr!(std::ffi::NulError, exceptions::PyValueError); +impl_to_pyerr!(std::str::Utf8Error, exceptions::PyUnicodeDecodeError); +impl_to_pyerr!(std::string::FromUtf8Error, exceptions::PyUnicodeDecodeError); +impl_to_pyerr!( + std::string::FromUtf16Error, + exceptions::PyUnicodeDecodeError +); +impl_to_pyerr!( + std::char::DecodeUtf16Error, + exceptions::PyUnicodeDecodeError +); +impl_to_pyerr!(std::net::AddrParseError, exceptions::PyValueError); diff --git a/src/err.rs b/src/err/mod.rs similarity index 50% rename from src/err.rs rename to src/err/mod.rs index e1ade1eeb3a..2671a2ec2ba 100644 --- a/src/err.rs +++ b/src/err/mod.rs @@ -4,58 +4,40 @@ use crate::gil::ensure_gil; use crate::panic::PanicException; use crate::type_object::PyTypeObject; use crate::types::PyType; -use crate::{exceptions, ffi}; use crate::{ - AsPyPointer, FromPyPointer, IntoPy, IntoPyPointer, Py, PyAny, PyNativeType, PyObject, Python, + exceptions::{self, PyBaseException}, + ffi, +}; +use crate::{ + AsPyPointer, FromPyPointer, IntoPy, Py, PyAny, PyNativeType, PyObject, Python, ToBorrowedObject, ToPyObject, }; use libc::c_int; use std::borrow::Cow; +use std::cell::UnsafeCell; use std::ffi::CString; -use std::io; use std::os::raw::c_char; use std::ptr::NonNull; -/// Represents a `PyErr` value. -/// -/// **Caution:** -/// -/// When you construct an instance of `PyErrValue`, we highly recommend to use `from_err_args` -/// method. If you want to to construct `PyErrValue::ToArgs` directly, please do not forget to -/// call `Python::acquire_gil`. -pub enum PyErrValue { - None, - Value(PyObject), - ToArgs(Box), - ToObject(Box), -} +mod err_state; +mod impls; -impl PyErrValue { - pub fn from_err_args(value: T) -> Self - where - T: PyErrArguments + Send + Sync + 'static, - { - let _ = Python::acquire_gil(); - PyErrValue::ToArgs(Box::new(value)) - } -} +pub use err_state::PyErrArguments; +use err_state::{PyErrState, PyErrStateNormalized, PyErrValue}; /// Represents a Python exception that was raised. pub struct PyErr { - /// The type of the exception. This should be either a `PyClass` or a `PyType`. - pub ptype: Py, - - /// The value of the exception. - /// - /// This can be either an instance of `PyObject`, a tuple of arguments to be passed to - /// `ptype`'s constructor, or a single argument to be passed to `ptype`'s constructor. Call - /// `PyErr::to_object()` to get the exception instance in all cases. - pub pvalue: PyErrValue, - - /// The `PyTraceBack` object associated with the error. - pub ptraceback: Option, + // Safety: can only hand out references when in the "normalized" state. Will never change + // after normalization. + // + // The state is temporarily removed from the PyErr during normalization, to avoid + // concurrent modifications. + state: UnsafeCell>, } +unsafe impl Send for PyErr {} +unsafe impl Sync for PyErr {} + /// Represents the result of a Python call. pub type PyResult = Result; @@ -75,12 +57,6 @@ impl<'a> PyDowncastError<'a> { } } -/// Helper conversion trait that allows to use custom arguments for exception constructor. -pub trait PyErrArguments { - /// Arguments for exception - fn arguments(&self, _: Python) -> PyObject; -} - impl PyErr { /// Creates a new PyErr of type `T`. /// @@ -88,7 +64,7 @@ impl PyErr { /// * a tuple: the exception instance will be created using Python `T(*tuple)` /// * any other value: the exception instance will be created using Python `T(value)` /// - /// Note: if `value` is not `Send` or `Sync`, consider using `PyErr::from_value` instead. + /// Note: if `value` is not `Send` or `Sync`, consider using `PyErr::from_instance` instead. /// /// Panics if `T` is not a Python class derived from `BaseException`. /// @@ -97,11 +73,9 @@ impl PyErr { /// return Err(PyErr::new::("Error message")); /// ``` /// - /// In most cases, you can use a concrete exception's constructors instead: - /// the example is equivalent to + /// In most cases, you can use a concrete exception's constructor instead, which is equivalent: /// ```ignore - /// return Err(exceptions::PyTypeError::py_err("Error message")); - /// return exceptions::PyTypeError::into("Error message"); + /// return Err(exceptions::PyTypeError::new_err("Error message")); /// ``` pub fn new(value: V) -> PyErr where @@ -114,11 +88,10 @@ impl PyErr { let ty = T::type_object(py); assert_ne!(unsafe { ffi::PyExceptionClass_Check(ty.as_ptr()) }, 0); - PyErr { + PyErr::from_state(PyErrState::Lazy { ptype: ty.into(), - pvalue: PyErrValue::ToObject(Box::new(value)), - ptraceback: None, - } + pvalue: Some(PyErrValue::ToObject(Box::new(value))), + }) } /// Constructs a new error, with the usual lazy initialization of Python exceptions. @@ -130,17 +103,17 @@ impl PyErr { where A: ToPyObject + Send + Sync + 'static, { - PyErr { + PyErr::from_state(PyErrState::Lazy { ptype: exc.into(), - pvalue: PyErrValue::ToObject(Box::new(args)), - ptraceback: None, - } + pvalue: Some(PyErrValue::ToObject(Box::new(args))), + }) } - /// Creates a new PyErr of type `T`. - pub fn from_value(value: PyErrValue) -> PyErr + /// Creates a new PyErr of type `T`, with the corresponding arguments to lazily instantiate it. + pub fn from_err_args(args: A) -> PyErr where T: PyTypeObject, + A: PyErrArguments + Send + Sync + 'static, { let gil = ensure_gil(); let py = unsafe { gil.python() }; @@ -148,11 +121,10 @@ impl PyErr { let ty = T::type_object(py); assert_ne!(unsafe { ffi::PyExceptionClass_Check(ty.as_ptr()) }, 0); - PyErr { + PyErr::from_state(PyErrState::Lazy { ptype: ty.into(), - pvalue: value, - ptraceback: None, - } + pvalue: Some(PyErrValue::ToArgs(Box::new(args))), + }) } /// Creates a new PyErr. @@ -164,27 +136,80 @@ impl PyErr { pub fn from_instance(obj: &PyAny) -> PyErr { let ptr = obj.as_ptr(); - if unsafe { ffi::PyExceptionInstance_Check(ptr) } != 0 { - PyErr { + let state = if unsafe { ffi::PyExceptionInstance_Check(ptr) } != 0 { + PyErrState::Normalized(PyErrStateNormalized { ptype: unsafe { Py::from_borrowed_ptr(obj.py(), ffi::PyExceptionInstance_Class(ptr)) }, - pvalue: PyErrValue::Value(obj.into()), + pvalue: unsafe { Py::from_borrowed_ptr(obj.py(), obj.as_ptr()) }, ptraceback: None, - } + }) } else if unsafe { ffi::PyExceptionClass_Check(obj.as_ptr()) } != 0 { - PyErr { + PyErrState::Lazy { ptype: unsafe { Py::from_borrowed_ptr(obj.py(), ptr) }, - pvalue: PyErrValue::None, - ptraceback: None, + pvalue: None, } } else { - PyErr { + PyErrState::Lazy { ptype: exceptions::PyTypeError::type_object(obj.py()).into(), - pvalue: PyErrValue::ToObject(Box::new("exceptions must derive from BaseException")), - ptraceback: None, + pvalue: Some(PyErrValue::ToObject(Box::new( + "exceptions must derive from BaseException", + ))), } - } + }; + + PyErr::from_state(state) + } + + /// Get the type of this exception object. + /// + /// The object will be normalized first if needed. + /// + /// # Example + /// ```rust + /// use pyo3::{Python, PyErr, exceptions::PyTypeError, types::PyType}; + /// Python::with_gil(|py| { + /// let err = PyTypeError::new_err(("some type error",)); + /// assert_eq!(err.ptype(py), PyType::new::(py)); + /// }); + /// ``` + pub fn ptype<'py>(&'py self, py: Python<'py>) -> &'py PyType { + self.normalized(py).ptype.as_ref(py) + } + + /// Get the value of this exception object. + /// + /// The object will be normalized first if needed. + /// + /// # Example + /// ```rust + /// use pyo3::{Python, PyErr, exceptions::PyTypeError, types::PyType}; + /// Python::with_gil(|py| { + /// let err = PyTypeError::new_err(("some type error",)); + /// assert_eq!(err.pvalue(py).to_string(), "TypeError: some type error"); + /// }); + /// ``` + pub fn pvalue<'py>(&'py self, py: Python<'py>) -> &'py PyBaseException { + self.normalized(py).pvalue.as_ref(py) + } + + /// Get the value of this exception object. + /// + /// The object will be normalized first if needed. + /// + /// # Example + /// ```rust + /// use pyo3::{Python, PyErr, exceptions::PyTypeError, types::PyType}; + /// Python::with_gil(|py| { + /// let err = PyTypeError::new_err(("some type error",)); + /// assert_eq!(err.ptraceback(py), None); + /// }); + /// ``` + pub fn ptraceback<'py>(&'py self, py: Python<'py>) -> Option<&'py PyAny> { + self.normalized(py) + .ptraceback + .as_ref() + .map(|obj| obj.as_ref(py)) } /// Gets whether an error is present in the Python interpreter's global state. @@ -260,6 +285,12 @@ impl PyErr { } } + /// Create a PyErr from an ffi tuple + /// + /// # Safety + /// - `ptype` must be a pointer to valid Python exception type object. + /// - `pvalue` must be a pointer to a valid Python object, or NULL. + /// - `ptraceback` must be a pointer to a valid Python traceback object, or NULL. unsafe fn new_from_ffi_tuple( py: Python, ptype: *mut ffi::PyObject, @@ -268,36 +299,23 @@ impl PyErr { ) -> PyErr { // Note: must not panic to ensure all owned pointers get acquired correctly, // and because we mustn't panic in normalize(). - - let pvalue = if let Some(obj) = PyObject::from_owned_ptr_or_opt(py, pvalue) { - PyErrValue::Value(obj) - } else { - PyErrValue::None - }; - - let ptype = if ptype.is_null() { - ::type_object(py).into() - } else { - Py::from_owned_ptr(py, ptype) - }; - - PyErr { - ptype, - pvalue, - ptraceback: PyObject::from_owned_ptr_or_opt(py, ptraceback), - } + PyErr::from_state(PyErrState::FfiTuple { + ptype: Py::from_owned_ptr_or_opt(py, ptype), + pvalue: Py::from_owned_ptr_or_opt(py, pvalue), + ptraceback: Py::from_owned_ptr_or_opt(py, ptraceback), + }) } /// Prints a standard traceback to `sys.stderr`. - pub fn print(self, py: Python) { - self.restore(py); + pub fn print(&self, py: Python) { + self.clone_ref(py).restore(py); unsafe { ffi::PyErr_PrintEx(0) } } /// Prints a standard traceback to `sys.stderr`, and sets /// `sys.last_{type,value,traceback}` attributes to this exception's data. - pub fn print_and_set_sys_last_vars(self, py: Python) { - self.restore(py); + pub fn print_and_set_sys_last_vars(&self, py: Python) { + self.clone_ref(py).restore(py); unsafe { ffi::PyErr_PrintEx(1) } } @@ -310,7 +328,7 @@ impl PyErr { T: ToBorrowedObject, { exc.with_borrowed_ptr(py, |exc| unsafe { - ffi::PyErr_GivenExceptionMatches(self.ptype.as_ptr(), exc) != 0 + ffi::PyErr_GivenExceptionMatches(self.ptype_ptr(), exc) != 0 }) } @@ -320,79 +338,32 @@ impl PyErr { T: PyTypeObject, { unsafe { - ffi::PyErr_GivenExceptionMatches(self.ptype.as_ptr(), T::type_object(py).as_ptr()) != 0 + ffi::PyErr_GivenExceptionMatches(self.ptype_ptr(), T::type_object(py).as_ptr()) != 0 } } - /// Normalizes the error. This ensures that the exception value is an instance - /// of the exception type. - pub fn normalize(&mut self, py: Python) { - // The normalization helper function involves temporarily moving out of the &mut self, - // which requires some unsafe trickery: - unsafe { - std::ptr::write(self, std::ptr::read(self).into_normalized(py)); - } - // This is safe as long as normalized() doesn't unwind due to a panic. - } - - /// Helper function for normalizing the error by deconstructing and reconstructing the `PyErr`. - /// Must not panic for safety in `normalize()`. - fn into_normalized(self, py: Python) -> PyErr { - let PyErr { - ptype, - pvalue, - ptraceback, - } = self; - - let mut pvalue = match pvalue { - PyErrValue::None => std::ptr::null_mut(), - PyErrValue::Value(ob) => ob.into_ptr(), - PyErrValue::ToArgs(ob) => ob.arguments(py).into_ptr(), - PyErrValue::ToObject(ob) => ob.to_object(py).into_ptr(), - }; - - let mut ptype = ptype.into_ptr(); - let mut ptraceback = ptraceback.into_ptr(); - unsafe { - ffi::PyErr_NormalizeException(&mut ptype, &mut pvalue, &mut ptraceback); - PyErr::new_from_ffi_tuple(py, ptype, pvalue, ptraceback) - } + /// Retrieves the exception instance for this error. + pub fn instance<'py>(&'py self, py: Python<'py>) -> &'py PyBaseException { + self.normalized(py).pvalue.as_ref(py) } - /// Retrieves the exception instance for this error. - /// - /// This method takes `mut self` because the error might need - /// to be normalized in order to create the exception instance. - pub fn instance(mut self, py: Python) -> &exceptions::PyBaseException { - self.normalize(py); - match self.pvalue { - PyErrValue::Value(ref instance) => { - let any: &PyAny = unsafe { py.from_owned_ptr(instance.clone_ref(py).into_ptr()) }; - any.downcast() - .expect("Normalized error instance should be a BaseException") - } - PyErrValue::None => panic!("This exception is not an instance"), - _ => unreachable!(), - } + /// Consumes self to take ownership of the exception instance for this error. + pub fn into_instance(self, py: Python) -> Py { + let out = self.normalized(py).pvalue.as_ref(py).into(); + std::mem::forget(self); + out } /// Writes the error back to the Python interpreter's global state. /// This is the opposite of `PyErr::fetch()`. #[inline] pub fn restore(self, py: Python) { - let PyErr { - ptype, - pvalue, - ptraceback, - } = self; - - let pvalue = match pvalue { - PyErrValue::None => std::ptr::null_mut(), - PyErrValue::Value(ob) => ob.into_ptr(), - PyErrValue::ToArgs(ob) => ob.arguments(py).into_ptr(), - PyErrValue::ToObject(ob) => ob.to_object(py).into_ptr(), - }; - unsafe { ffi::PyErr_Restore(ptype.into_ptr(), pvalue, ptraceback.into_ptr()) } + let (ptype, pvalue, ptraceback) = self + .state + .into_inner() + .expect("Cannot restore a PyErr while normalizing it") + .into_ffi_tuple(py); + unsafe { ffi::PyErr_Restore(ptype, pvalue, ptraceback) } } /// Issues a warning message. @@ -412,62 +383,101 @@ impl PyErr { } pub fn clone_ref(&self, py: Python) -> PyErr { - let v = match self.pvalue { - PyErrValue::None => PyErrValue::None, - PyErrValue::Value(ref ob) => PyErrValue::Value(ob.clone_ref(py)), - PyErrValue::ToArgs(ref ob) => PyErrValue::Value(ob.arguments(py)), - PyErrValue::ToObject(ref ob) => PyErrValue::Value(ob.to_object(py)), - }; + PyErr::from_state(PyErrState::Normalized(self.normalized(py).clone())) + } - let t = if let Some(ref val) = self.ptraceback { - Some(val.clone_ref(py)) - } else { - None - }; + fn from_state(state: PyErrState) -> PyErr { PyErr { - ptype: self.ptype.clone_ref(py), - pvalue: v, - ptraceback: t, + state: UnsafeCell::new(Some(state)), + } + } + + /// Returns borrowed reference to this Err's type + fn ptype_ptr(&self) -> *mut ffi::PyObject { + match unsafe { &*self.state.get() } { + Some(PyErrState::Lazy { ptype, .. }) => ptype.as_ptr(), + Some(PyErrState::FfiTuple { ptype, .. }) => ptype.as_ptr(), + Some(PyErrState::Normalized(n)) => n.ptype.as_ptr(), + None => panic!("Cannot access exception type while normalizing"), + } + } + + fn normalized(&self, py: Python) -> &PyErrStateNormalized { + // This process is safe because: + // - Access is guaranteed not to be concurrent thanks to `Python` GIL token + // - Write happens only once, and then never will change again. + // - State is set to None during the normalization process, so that a second + // concurrent normalization attempt will panic before changing anything. + + if let Some(PyErrState::Normalized(n)) = unsafe { &*self.state.get() } { + return n; + } + + let state = unsafe { + (*self.state.get()) + .take() + .expect("Cannot normalize a PyErr while already normalizing it.") + }; + let (mut ptype, mut pvalue, mut ptraceback) = state.into_ffi_tuple(py); + + unsafe { + ffi::PyErr_NormalizeException(&mut ptype, &mut pvalue, &mut ptraceback); + let self_state = &mut *self.state.get(); + *self_state = Some(PyErrState::Normalized(PyErrStateNormalized { + ptype: Py::from_owned_ptr_or_opt(py, ptype) + .unwrap_or_else(|| exceptions::PySystemError::type_object(py).into()), + pvalue: Py::from_owned_ptr_or_opt(py, pvalue).unwrap_or_else(|| { + exceptions::PySystemError::new_err("Exception value missing") + .instance(py) + .into_py(py) + }), + ptraceback: PyObject::from_owned_ptr_or_opt(py, ptraceback), + })); + + match self_state { + Some(PyErrState::Normalized(n)) => n, + _ => unreachable!(), + } } } } impl std::fmt::Debug for PyErr { fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> { - f.write_str(format!("PyErr {{ type: {:?} }}", self.ptype).as_str()) + f.write_str(format!("PyErr {{ type: {:?} }}", self.ptype_ptr()).as_str()) } } -impl IntoPy for PyErr { - fn into_py(self, py: Python) -> PyObject { - self.instance(py).into() +impl std::fmt::Display for PyErr { + fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> { + Python::with_gil(|py| self.instance(py).fmt(f)) } } -impl IntoPy> for PyErr { - fn into_py(self, py: Python) -> Py { - self.instance(py).into() +impl std::error::Error for PyErr {} + +impl IntoPy for PyErr { + fn into_py(self, py: Python) -> PyObject { + self.into_instance(py).into() } } impl ToPyObject for PyErr { fn to_object(&self, py: Python) -> PyObject { - let err = self.clone_ref(py); - err.instance(py).into() + self.clone_ref(py).into_py(py) } } impl<'a> IntoPy for &'a PyErr { fn into_py(self, py: Python) -> PyObject { - let err = self.clone_ref(py); - err.instance(py).into() + self.clone_ref(py).into_py(py) } } /// Convert `PyDowncastError` to Python `TypeError`. impl<'a> std::convert::From> for PyErr { fn from(err: PyDowncastError) -> PyErr { - exceptions::PyTypeError::py_err(err.to_string()) + exceptions::PyTypeError::new_err(err.to_string()) } } @@ -487,128 +497,6 @@ impl<'a> std::fmt::Display for PyDowncastError<'a> { } } -/// Convert `PyErr` to `io::Error` -impl std::convert::From for std::io::Error { - fn from(err: PyErr) -> Self { - std::io::Error::new( - std::io::ErrorKind::Other, - format!("Python exception: {:?}", err), - ) - } -} - -/// Convert `PyErr` to `PyResult` -impl std::convert::Into> for PyErr { - fn into(self) -> PyResult { - Err(self) - } -} - -macro_rules! impl_to_pyerr { - ($err: ty, $pyexc: ty) => { - impl PyErrArguments for $err { - fn arguments(&self, py: Python) -> PyObject { - self.to_string().to_object(py) - } - } - - impl std::convert::From<$err> for PyErr { - fn from(err: $err) -> PyErr { - PyErr::from_value::<$pyexc>(PyErrValue::from_err_args(err)) - } - } - }; -} - -/// Create `OSError` from `io::Error` -impl std::convert::From for PyErr { - fn from(err: io::Error) -> PyErr { - macro_rules! err_value { - () => { - PyErrValue::from_err_args(err) - }; - } - match err.kind() { - io::ErrorKind::BrokenPipe => { - PyErr::from_value::(err_value!()) - } - io::ErrorKind::ConnectionRefused => { - PyErr::from_value::(err_value!()) - } - io::ErrorKind::ConnectionAborted => { - PyErr::from_value::(err_value!()) - } - io::ErrorKind::ConnectionReset => { - PyErr::from_value::(err_value!()) - } - io::ErrorKind::Interrupted => { - PyErr::from_value::(err_value!()) - } - io::ErrorKind::NotFound => { - PyErr::from_value::(err_value!()) - } - io::ErrorKind::WouldBlock => { - PyErr::from_value::(err_value!()) - } - io::ErrorKind::TimedOut => { - PyErr::from_value::(err_value!()) - } - _ => PyErr::from_value::(err_value!()), - } - } -} - -impl PyErrArguments for io::Error { - fn arguments(&self, py: Python) -> PyObject { - self.to_string().to_object(py) - } -} - -impl std::convert::From> - for PyErr -{ - fn from(err: std::io::IntoInnerError) -> PyErr { - PyErr::from_value::(PyErrValue::from_err_args(err)) - } -} - -impl PyErrArguments for std::io::IntoInnerError { - fn arguments(&self, py: Python) -> PyObject { - self.to_string().to_object(py) - } -} - -impl PyErrArguments for std::convert::Infallible { - fn arguments(&self, py: Python) -> PyObject { - "Infalliable!".to_object(py) - } -} - -impl std::convert::From for PyErr { - fn from(_: std::convert::Infallible) -> PyErr { - PyErr::new::("Infalliable!") - } -} - -impl_to_pyerr!(std::array::TryFromSliceError, exceptions::PyValueError); -impl_to_pyerr!(std::num::ParseIntError, exceptions::PyValueError); -impl_to_pyerr!(std::num::ParseFloatError, exceptions::PyValueError); -impl_to_pyerr!(std::num::TryFromIntError, exceptions::PyValueError); -impl_to_pyerr!(std::str::ParseBoolError, exceptions::PyValueError); -impl_to_pyerr!(std::ffi::IntoStringError, exceptions::PyUnicodeDecodeError); -impl_to_pyerr!(std::ffi::NulError, exceptions::PyValueError); -impl_to_pyerr!(std::str::Utf8Error, exceptions::PyUnicodeDecodeError); -impl_to_pyerr!(std::string::FromUtf8Error, exceptions::PyUnicodeDecodeError); -impl_to_pyerr!( - std::string::FromUtf16Error, - exceptions::PyUnicodeDecodeError -); -impl_to_pyerr!( - std::char::DecodeUtf16Error, - exceptions::PyUnicodeDecodeError -); -impl_to_pyerr!(std::net::AddrParseError, exceptions::PyValueError); - pub fn panic_after_error(_py: Python) -> ! { unsafe { ffi::PyErr_Print(); @@ -628,6 +516,7 @@ pub fn error_on_minusone(py: Python, result: c_int) -> PyResult<()> { #[cfg(test)] mod tests { + use super::PyErrState; use crate::exceptions; use crate::panic::PanicException; use crate::{PyErr, Python}; @@ -636,7 +525,7 @@ mod tests { fn set_typeerror() { let gil = Python::acquire_gil(); let py = gil.python(); - let err: PyErr = exceptions::PyTypeError::py_err(()); + let err: PyErr = exceptions::PyTypeError::new_err(()); err.restore(py); assert!(PyErr::occurred(py)); drop(PyErr::fetch(py)); @@ -654,7 +543,7 @@ mod tests { let gil = Python::acquire_gil(); let py = gil.python(); - let err: PyErr = PanicException::py_err("new panic"); + let err: PyErr = PanicException::new_err("new panic"); err.restore(py); assert!(PyErr::occurred(py)); let started_unwind = @@ -669,5 +558,8 @@ mod tests { is_send::(); is_sync::(); + + is_send::(); + is_sync::(); } } diff --git a/src/exceptions.rs b/src/exceptions.rs index be73c5d4bcc..2a5b92c1503 100644 --- a/src/exceptions.rs +++ b/src/exceptions.rs @@ -3,7 +3,7 @@ //! Exception types defined by Python. use crate::type_object::PySizedLayout; -use crate::types::{PyAny, PyTuple}; +use crate::types::PyTuple; use crate::{ffi, AsPyPointer, PyResult, Python}; use std::ffi::CStr; use std::ops; @@ -19,20 +19,11 @@ macro_rules! impl_exception_boilerplate { } } - impl std::convert::Into<$crate::PyResult> for $name { - fn into(self) -> $crate::PyResult { - $crate::PyErr::new::<$name, _>(()).into() - } - } - impl $name { - pub fn py_err(args: V) -> $crate::PyErr { - $crate::PyErr::new::<$name, V>(args) - } - pub fn into( + pub fn new_err( args: V, - ) -> $crate::PyResult { - $crate::PyErr::new::<$name, V>(args).into() + ) -> $crate::PyErr { + $crate::PyErr::new::<$name, V>(args) } } @@ -445,13 +436,13 @@ impl_native_exception!(PyIOError, IOError, PyExc_IOError); impl_native_exception!(PyWindowsError, WindowsError, PyExc_WindowsError); impl PyUnicodeDecodeError { - pub fn new_err<'p>( + pub fn new<'p>( py: Python<'p>, encoding: &CStr, input: &[u8], range: ops::Range, reason: &CStr, - ) -> PyResult<&'p PyAny> { + ) -> PyResult<&'p PyUnicodeDecodeError> { unsafe { let input: &[c_char] = &*(input as *const [u8] as *const [c_char]); py.from_owned_ptr_or_err(ffi::PyUnicodeDecodeError_Create( @@ -470,9 +461,9 @@ impl PyUnicodeDecodeError { py: Python<'p>, input: &[u8], err: std::str::Utf8Error, - ) -> PyResult<&'p PyAny> { + ) -> PyResult<&'p PyUnicodeDecodeError> { let pos = err.valid_up_to(); - PyUnicodeDecodeError::new_err( + PyUnicodeDecodeError::new( py, CStr::from_bytes_with_nul(b"utf-8\0").unwrap(), input, @@ -515,9 +506,8 @@ pub mod socket { mod test { use crate::exceptions::PyException; use crate::types::{IntoPyDict, PyDict}; - use crate::{AsPyPointer, PyErr, Python}; + use crate::{PyErr, Python}; use std::error::Error; - use std::fmt::Write; import_exception!(socket, gaierror); import_exception!(email.errors, MessageError); @@ -527,7 +517,7 @@ mod test { let gil = Python::acquire_gil(); let py = gil.python(); - let err: PyErr = gaierror::py_err(()); + let err: PyErr = gaierror::new_err(()); let socket = py .import("socket") .map_err(|e| e.print(py)) @@ -537,6 +527,7 @@ mod test { d.set_item("socket", socket) .map_err(|e| e.print(py)) .expect("could not setitem"); + d.set_item("exc", err) .map_err(|e| e.print(py)) .expect("could not setitem"); @@ -551,7 +542,7 @@ mod test { let gil = Python::acquire_gil(); let py = gil.python(); - let err: PyErr = MessageError::py_err(()); + let err: PyErr = MessageError::new_err(()); let email = py .import("email") .map_err(|e| e.print(py)) @@ -598,38 +589,30 @@ mod test { #[test] fn native_exception_display() { - let mut out = String::new(); let gil = Python::acquire_gil(); let py = gil.python(); - let err = py + let exc = py .run("raise Exception('banana')", None, None) .expect_err("raising should have given us an error") - .instance(py); - write!(&mut out, "{}", err).expect("successful format"); - assert_eq!(out, "Exception: banana"); + .into_instance(py); + assert_eq!(exc.to_string(), "Exception: banana"); } #[test] fn native_exception_chain() { - let mut out = String::new(); let gil = Python::acquire_gil(); let py = gil.python(); - let err = py + let exc = py .run( "raise Exception('banana') from TypeError('peach')", None, None, ) .expect_err("raising should have given us an error") - .instance(py); - write!(&mut out, "{}", err).expect("successful format"); - assert_eq!(out, "Exception: banana"); - out.clear(); - let convert_ref: &super::PyBaseException = - unsafe { &*(err.as_ptr() as *const _ as *const _) }; - let source = convert_ref.source().expect("cause should exist"); - write!(&mut out, "{}", source).expect("successful format"); - assert_eq!(out, "TypeError: peach"); + .into_instance(py); + assert_eq!(exc.to_string(), "Exception: banana"); + let source = exc.as_ref(py).source().expect("cause should exist"); + assert_eq!(source.to_string(), "TypeError: peach"); let source_source = source.source(); assert!(source_source.is_none(), "source_source should be None"); } diff --git a/src/lib.rs b/src/lib.rs index 10f3e768f8e..c741610802a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -138,7 +138,7 @@ pub use crate::conversion::{ AsPyPointer, FromPyObject, FromPyPointer, IntoPy, IntoPyPointer, PyTryFrom, PyTryInto, ToBorrowedObject, ToPyObject, }; -pub use crate::err::{PyDowncastError, PyErr, PyErrArguments, PyErrValue, PyResult}; +pub use crate::err::{PyDowncastError, PyErr, PyErrArguments, PyResult}; pub use crate::gil::{GILGuard, GILPool}; pub use crate::instance::{Py, PyNativeType, PyObject}; pub use crate::pycell::{PyCell, PyRef, PyRefMut}; diff --git a/src/pycell.rs b/src/pycell.rs index ef1acb92372..d0d498d2d8e 100644 --- a/src/pycell.rs +++ b/src/pycell.rs @@ -730,7 +730,7 @@ impl fmt::Display for PyBorrowError { impl From for PyErr { fn from(other: PyBorrowError) -> Self { - PyRuntimeError::py_err(other.to_string()) + PyRuntimeError::new_err(other.to_string()) } } @@ -755,6 +755,6 @@ impl fmt::Display for PyBorrowMutError { impl From for PyErr { fn from(other: PyBorrowMutError) -> Self { - PyRuntimeError::py_err(other.to_string()) + PyRuntimeError::new_err(other.to_string()) } } diff --git a/src/pyclass.rs b/src/pyclass.rs index a4c10876493..52cc3910ca1 100644 --- a/src/pyclass.rs +++ b/src/pyclass.rs @@ -218,7 +218,7 @@ where if ffi::PyType_Ready(type_object) == 0 { Ok(()) } else { - PyErr::fetch(py).into() + Err(PyErr::fetch(py)) } } } diff --git a/src/types/any.rs b/src/types/any.rs index aec69743427..cd973b0011d 100644 --- a/src/types/any.rs +++ b/src/types/any.rs @@ -163,7 +163,7 @@ impl PyAny { } else if do_compare(other, ffi::Py_GT)? { Ok(Ordering::Greater) } else { - Err(PyTypeError::py_err( + Err(PyTypeError::new_err( "PyAny::compare(): All comparisons returned false", )) } diff --git a/src/types/bytearray.rs b/src/types/bytearray.rs index 095c7564c99..ae7333825d6 100644 --- a/src/types/bytearray.rs +++ b/src/types/bytearray.rs @@ -295,7 +295,7 @@ mod test { let gil = Python::acquire_gil(); let py = gil.python(); let py_bytearray_result = PyByteArray::new_with(py, 10, |_b: &mut [u8]| { - Err(PyValueError::py_err("Hello Crustaceans!")) + Err(PyValueError::new_err("Hello Crustaceans!")) }); assert!(py_bytearray_result.is_err()); assert!(py_bytearray_result diff --git a/src/types/bytes.rs b/src/types/bytes.rs index 135b38cc025..ab5b13290a0 100644 --- a/src/types/bytes.rs +++ b/src/types/bytes.rs @@ -159,7 +159,7 @@ mod test { let gil = Python::acquire_gil(); let py = gil.python(); let py_bytes_result = PyBytes::new_with(py, 10, |_b: &mut [u8]| { - Err(PyValueError::py_err("Hello Crustaceans!")) + Err(PyValueError::new_err("Hello Crustaceans!")) }); assert!(py_bytes_result.is_err()); assert!(py_bytes_result diff --git a/src/types/num.rs b/src/types/num.rs index 14ab4b33ce4..09944d84486 100644 --- a/src/types/num.rs +++ b/src/types/num.rs @@ -40,7 +40,7 @@ macro_rules! int_fits_larger_int { fn extract(obj: &'source PyAny) -> PyResult { let val: $larger_type = obj.extract()?; <$rust_type>::try_from(val) - .map_err(|e| exceptions::PyOverflowError::py_err(e.to_string())) + .map_err(|e| exceptions::PyOverflowError::new_err(e.to_string())) } } }; @@ -86,7 +86,7 @@ macro_rules! int_fits_c_long { } }?; <$rust_type>::try_from(val) - .map_err(|e| exceptions::PyOverflowError::py_err(e.to_string())) + .map_err(|e| exceptions::PyOverflowError::new_err(e.to_string())) } } }; diff --git a/src/types/sequence.rs b/src/types/sequence.rs index 1b84352a5b0..67818de169a 100644 --- a/src/types/sequence.rs +++ b/src/types/sequence.rs @@ -361,7 +361,7 @@ where { let seq = ::try_from(obj)?; if seq.len()? as usize != slice.len() { - return Err(exceptions::PyBufferError::py_err( + return Err(exceptions::PyBufferError::new_err( "Slice length does not match buffer length.", )); } diff --git a/src/types/tuple.rs b/src/types/tuple.rs index e4c1d8d1c8c..77e7c903912 100644 --- a/src/types/tuple.rs +++ b/src/types/tuple.rs @@ -139,7 +139,7 @@ fn wrong_tuple_length(t: &PyTuple, expected_length: usize) -> PyErr { expected_length, t.len() ); - exceptions::PyValueError::py_err(msg) + exceptions::PyValueError::new_err(msg) } macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+} => { diff --git a/tests/test_buffer_protocol.rs b/tests/test_buffer_protocol.rs index c7343e19331..aca7ef1ca4e 100644 --- a/tests/test_buffer_protocol.rs +++ b/tests/test_buffer_protocol.rs @@ -21,11 +21,11 @@ struct TestBufferClass { impl PyBufferProtocol for TestBufferClass { fn bf_getbuffer(slf: PyRefMut, view: *mut ffi::Py_buffer, flags: c_int) -> PyResult<()> { if view.is_null() { - return Err(PyBufferError::py_err("View is null")); + return Err(PyBufferError::new_err("View is null")); } if (flags & ffi::PyBUF_WRITABLE) == ffi::PyBUF_WRITABLE { - return Err(PyBufferError::py_err("Object is not writable")); + return Err(PyBufferError::new_err("Object is not writable")); } unsafe { diff --git a/tests/test_exceptions.rs b/tests/test_exceptions.rs index 3726dfb7e39..2c71669a2f8 100644 --- a/tests/test_exceptions.rs +++ b/tests/test_exceptions.rs @@ -46,7 +46,7 @@ impl fmt::Display for CustomError { impl std::convert::From for PyErr { fn from(err: CustomError) -> PyErr { - exceptions::PyOSError::py_err(err.to_string()) + exceptions::PyOSError::new_err(err.to_string()) } } diff --git a/tests/test_frompyobject.rs b/tests/test_frompyobject.rs index 0fa937cc853..98f2d2b9908 100644 --- a/tests/test_frompyobject.rs +++ b/tests/test_frompyobject.rs @@ -1,7 +1,7 @@ use pyo3::exceptions::PyValueError; use pyo3::prelude::*; use pyo3::types::{PyDict, PyString, PyTuple}; -use pyo3::{PyErrValue, PyMappingProtocol}; +use pyo3::PyMappingProtocol; #[macro_use] mod common; @@ -30,7 +30,7 @@ impl PyMappingProtocol for PyA { if key == "t" { Ok("bar".into()) } else { - Err(PyValueError::py_err("Failed")) + Err(PyValueError::new_err("Failed")) } } } @@ -277,7 +277,7 @@ fn test_enum() { } } -#[derive(FromPyObject)] +#[derive(Debug, FromPyObject)] pub enum Bar { #[pyo3(annotation = "str")] A(String), @@ -294,15 +294,8 @@ fn test_err_rename() { let dict = PyDict::new(py); let f = Bar::extract(dict.as_ref()); assert!(f.is_err()); - match f { - Ok(_) => {} - Err(e) => match e.pvalue { - PyErrValue::ToObject(to) => { - let o = to.to_object(py); - let s = String::extract(o.as_ref(py)).expect("Err val is not a string"); - assert_eq!(s, "Can't convert {} (dict) to Union[str, uint, int]") - } - _ => panic!("Expected PyErrValue::ToObject"), - }, - } + assert_eq!( + f.unwrap_err().to_string(), + "TypeError: Can't convert {} (dict) to Union[str, uint, int]" + ); } diff --git a/tests/test_mapping.rs b/tests/test_mapping.rs index 35369e01b4e..2e5f42df130 100644 --- a/tests/test_mapping.rs +++ b/tests/test_mapping.rs @@ -40,7 +40,7 @@ impl PyMappingProtocol for Mapping { self.index .get(&query) .copied() - .ok_or_else(|| PyKeyError::py_err("unknown key")) + .ok_or_else(|| PyKeyError::new_err("unknown key")) } fn __setitem__(&mut self, key: String, value: usize) { @@ -49,7 +49,7 @@ impl PyMappingProtocol for Mapping { fn __delitem__(&mut self, key: String) -> PyResult<()> { if self.index.remove(&key).is_none() { - PyKeyError::py_err("unknown key").into() + Err(PyKeyError::new_err("unknown key")) } else { Ok(()) } diff --git a/tests/test_sequence.rs b/tests/test_sequence.rs index d46edfb3b45..0a4c4afd1f8 100644 --- a/tests/test_sequence.rs +++ b/tests/test_sequence.rs @@ -41,7 +41,7 @@ impl PySequenceProtocol for ByteSequence { self.elements .get(idx as usize) .copied() - .ok_or_else(|| PyIndexError::py_err("list index out of range")) + .ok_or_else(|| PyIndexError::new_err("list index out of range")) } fn __setitem__(&mut self, idx: isize, value: u8) { @@ -53,7 +53,7 @@ impl PySequenceProtocol for ByteSequence { self.elements.remove(idx as usize); Ok(()) } else { - Err(PyIndexError::py_err("list index out of range")) + Err(PyIndexError::new_err("list index out of range")) } } @@ -78,7 +78,7 @@ impl PySequenceProtocol for ByteSequence { } Ok(Self { elements }) } else { - Err(PyValueError::py_err("invalid repeat count")) + Err(PyValueError::new_err("invalid repeat count")) } } } @@ -242,7 +242,7 @@ impl PySequenceProtocol for OptionList { fn __getitem__(&self, idx: isize) -> PyResult> { match self.items.get(idx as usize) { Some(x) => Ok(*x), - None => Err(PyIndexError::py_err("Index out of bounds")), + None => Err(PyIndexError::new_err("Index out of bounds")), } } } diff --git a/tests/test_various.rs b/tests/test_various.rs index 87270c39186..49923bf299a 100644 --- a/tests/test_various.rs +++ b/tests/test_various.rs @@ -191,7 +191,7 @@ impl fmt::Display for MyError { /// Important for the automatic conversion to `PyErr`. impl From for PyErr { fn from(err: MyError) -> pyo3::PyErr { - pyo3::exceptions::PyOSError::py_err(err.to_string()) + pyo3::exceptions::PyOSError::new_err(err.to_string()) } }