From 2ac30ec41114caa0b76f13d025711e794dbb8a24 Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Thu, 25 Nov 2021 19:30:03 +0000 Subject: [PATCH] err: tweak names, inlining and docs --- CHANGELOG.md | 7 + guide/src/exception.md | 10 +- guide/src/migration.md | 2 +- guide/src/python_from_rust.md | 2 +- pyo3-macros-backend/src/from_pyobject.rs | 8 +- src/conversions/anyhow.rs | 4 +- src/conversions/eyre.rs | 2 +- src/derive_utils.rs | 4 +- src/err/mod.rs | 242 ++++++++++++++++------- src/exceptions.rs | 12 +- src/types/string.rs | 12 +- src/types/traceback.rs | 4 +- 12 files changed, 207 insertions(+), 102 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 73417797..01c834d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - `PyType::is_subclass`, `PyErr::is_instance` and `PyAny::is_instance` now operate run-time type object instead of a type known at compile-time. The old behavior is still available as `PyType::is_subclass_of`, `PyErr::is_instance_of` and `PyAny::is_instance_of`. [#1985](https://github.com/PyO3/pyo3/pull/1985) +- Rename some methods on `PyErr` (the old names are just marked deprecated for now): [#2026](https://github.com/PyO3/pyo3/pull/2026) + - `pytype` -> `get_type` + - `pvalue` -> `value` (and deprecate equivalent `instance`) + - `ptraceback` -> `traceback` + - `from_instance` -> `from_value` + - `into_instance` -> `into_value` ### Removed @@ -34,6 +40,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fix undefined symbol for `PyObject_HasAttr` on PyPy. [#2025](https://github.com/PyO3/pyo3/pull/2025) +- Fix memory leak in `PyErr::into_value`. [#2026](https://github.com/PyO3/pyo3/pull/2026) ## [0.15.1] - 2021-11-19 diff --git a/guide/src/exception.md b/guide/src/exception.md index 8806c164..251d7f85 100644 --- a/guide/src/exception.md +++ b/guide/src/exception.md @@ -90,10 +90,10 @@ Python::with_gil(|py| { }); ``` -If you already have a Python exception instance, you can simply call [`PyErr::from_instance`]. +If you already have a Python exception object, you can simply call [`PyErr::from_value`]. ```rust,ignore -PyErr::from_instance(py, err).restore(py); +PyErr::from_value(py, err).restore(py); ``` @@ -134,7 +134,7 @@ which is an alias for the type `Result`. A [`PyErr`] represents a Python exception. Errors within the PyO3 library are also exposed as Python exceptions. -If your code has a custom error type, adding an implementation of `std::convert::From for PyErr` +If your code has a custom error type, adding an implementation of `std::convert::From for PyErr` is usually enough. PyO3 will then automatically convert your error to a Python exception when needed. The following code snippet defines a Rust error named `CustomIOError`. In its `From for PyErr` @@ -188,7 +188,7 @@ fn main() { ``` This has been implemented for most of Rust's standard library errors, so that you can use the `?` -("try") operator with them. The following code snippet will raise a `ValueError` in Python if +("try") operator with them. The following code snippet will raise a `ValueError` in Python if `String::parse()` returns an error. ```rust @@ -254,6 +254,6 @@ defines exceptions for several standard library modules. [`PyErr`]: {{#PYO3_DOCS_URL}}/pyo3/struct.PyErr.html [`PyResult`]: {{#PYO3_DOCS_URL}}/pyo3/type.PyResult.html -[`PyErr::from_instance`]: {{#PYO3_DOCS_URL}}/pyo3/struct.PyErr.html#method.from_instance +[`PyErr::from_value`]: {{#PYO3_DOCS_URL}}/pyo3/struct.PyErr.html#method.from_value [`PyAny::is_instance`]: {{#PYO3_DOCS_URL}}/pyo3/struct.PyAny.html#method.is_instance [`PyAny::is_instance_of`]: {{#PYO3_DOCS_URL}}/pyo3/struct.PyAny.html#method.is_instance_of diff --git a/guide/src/migration.md b/guide/src/migration.md index 5431d009..4c298bf2 100644 --- a/guide/src/migration.md +++ b/guide/src/migration.md @@ -170,7 +170,7 @@ let err: PyErr = TypeError::py_err("error message"); After: -```rust +```rust,ignore # use pyo3::{PyErr, PyResult, Python, type_object::PyTypeObject}; # use pyo3::exceptions::{PyBaseException, PyTypeError}; # Python::with_gil(|py| -> PyResult<()> { diff --git a/guide/src/python_from_rust.md b/guide/src/python_from_rust.md index f5648213..8b3e9d55 100644 --- a/guide/src/python_from_rust.md +++ b/guide/src/python_from_rust.md @@ -280,7 +280,7 @@ class House(object): Err(e) => { house.call_method1( "__exit__", - (e.ptype(py), e.pvalue(py), e.ptraceback(py)) + (e.get_type(py), e.value(py), e.traceback(py)) ).unwrap(); } } diff --git a/pyo3-macros-backend/src/from_pyobject.rs b/pyo3-macros-backend/src/from_pyobject.rs index 5f4811eb..de6fedc1 100644 --- a/pyo3-macros-backend/src/from_pyobject.rs +++ b/pyo3-macros-backend/src/from_pyobject.rs @@ -61,9 +61,9 @@ impl<'a> Enum<'a> { match maybe_ret { ok @ ::std::result::Result::Ok(_) => return ok, - ::std::result::Result::Err(inner) => { + ::std::result::Result::Err(err) => { let py = ::pyo3::PyNativeType::py(obj); - err_reasons.push_str(&::std::format!("{}\n", inner.instance(py).str()?)); + err_reasons.push_str(&::std::format!("{}\n", err.value(py).str()?)); } } ); @@ -221,11 +221,11 @@ impl<'a> Container<'a> { format!("failed to extract inner field of {}", quote!(#self_ty)) }; quote!( - ::std::result::Result::Ok(#self_ty(obj.extract().map_err(|inner| { + ::std::result::Result::Ok(#self_ty(obj.extract().map_err(|err| { let py = ::pyo3::PyNativeType::py(obj); let err_msg = ::std::format!("{}: {}", #error_msg, - inner.instance(py).str().unwrap()); + err.value(py).str().unwrap()); ::pyo3::exceptions::PyTypeError::new_err(err_msg) })?)) ) diff --git a/src/conversions/anyhow.rs b/src/conversions/anyhow.rs index 7dd733d7..959ee347 100644 --- a/src/conversions/anyhow.rs +++ b/src/conversions/anyhow.rs @@ -144,7 +144,7 @@ mod test_anyhow { Python::with_gil(|py| { let locals = [("err", pyerr)].into_py_dict(py); let pyerr = py.run("raise err", None, Some(locals)).unwrap_err(); - assert_eq!(pyerr.pvalue(py).to_string(), expected_contents); + assert_eq!(pyerr.value(py).to_string(), expected_contents); }) } @@ -161,7 +161,7 @@ mod test_anyhow { Python::with_gil(|py| { let locals = [("err", pyerr)].into_py_dict(py); let pyerr = py.run("raise err", None, Some(locals)).unwrap_err(); - assert_eq!(pyerr.pvalue(py).to_string(), expected_contents); + assert_eq!(pyerr.value(py).to_string(), expected_contents); }) } } diff --git a/src/conversions/eyre.rs b/src/conversions/eyre.rs index 172f18c5..38305ce5 100644 --- a/src/conversions/eyre.rs +++ b/src/conversions/eyre.rs @@ -145,7 +145,7 @@ mod tests { Python::with_gil(|py| { let locals = [("err", pyerr)].into_py_dict(py); let pyerr = py.run("raise err", None, Some(locals)).unwrap_err(); - assert_eq!(pyerr.pvalue(py).to_string(), expected_contents); + assert_eq!(pyerr.value(py).to_string(), expected_contents); }) } } diff --git a/src/derive_utils.rs b/src/derive_utils.rs index 6d79b2c4..0fc18ae2 100644 --- a/src/derive_utils.rs +++ b/src/derive_utils.rs @@ -273,9 +273,9 @@ impl FunctionDescription { /// Add the argument name to the error message of an error which occurred during argument extraction pub fn argument_extraction_error(py: Python, arg_name: &str, error: PyErr) -> PyErr { - if error.ptype(py) == py.get_type::() { + if error.is_instance_of::(py) { let reason = error - .instance(py) + .value(py) .str() .unwrap_or_else(|_| PyString::new(py, "")); PyTypeError::new_err(format!("argument '{}': {}", arg_name, reason)) diff --git a/src/err/mod.rs b/src/err/mod.rs index 004c0a65..3ae35b95 100644 --- a/src/err/mod.rs +++ b/src/err/mod.rs @@ -7,7 +7,9 @@ use crate::{ exceptions::{self, PyBaseException}, ffi, }; -use crate::{AsPyPointer, IntoPy, Py, PyAny, PyObject, Python, ToBorrowedObject, ToPyObject}; +use crate::{ + AsPyPointer, IntoPy, IntoPyPointer, Py, PyAny, PyObject, Python, ToBorrowedObject, ToPyObject, +}; use std::borrow::Cow; use std::cell::UnsafeCell; use std::ffi::CString; @@ -21,7 +23,14 @@ mod impls; pub use err_state::PyErrArguments; use err_state::{boxed_args, PyErrState, PyErrStateNormalized}; -/// Represents a Python exception that was raised. +/// Represents a Python exception. +/// +/// Python exceptions can be raised in a "lazy" fashion, where the full Python object for the +/// exception is not created until needed. The process of creating the full object is known +/// as "normalization". An exception which has not yet been created is known as "unnormalized". +/// +/// This struct builds upon that design, supporting all lazily-created Python exceptions and also +/// supporting exceptions lazily-created from Rust. pub struct PyErr { // Safety: can only hand out references when in the "normalized" state. Will never change // after normalization. @@ -56,15 +65,21 @@ impl<'a> PyDowncastError<'a> { impl PyErr { /// Creates a new PyErr of type `T`. /// - /// `value` can be: - /// * 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)` + /// `args` can be: + /// * a tuple: the exception instance will be created using the equivalent to the Python + /// expression `T(*tuple)` + /// * any other value: the exception instance will be created using the equivalent to the Python + /// expression `T(value)` /// - /// Note: if `value` is not `Send` or `Sync`, consider using `PyErr::from_instance` instead. + /// This error will be stored in an unnormalized state. This avoids the need for the Python GIL + /// to be held, but requires `args` to be `Send` and `Sync`. If `args` is not `Send` or `Sync`, + /// consider using [`PyErr::from_value`] instead. /// - /// Panics if `T` is not a Python class derived from `BaseException`. + /// If an error occurs during normalization (for example if `T` is not a Python type which + /// extends from `BaseException`), then a different error may be produced during normalization. + /// + /// # Example /// - /// Example: /// ```ignore /// return Err(PyErr::new::("Error message")); /// ``` @@ -73,6 +88,7 @@ impl PyErr { /// ```ignore /// return Err(exceptions::PyTypeError::new_err("Error message")); /// ``` + #[inline] pub fn new(args: A) -> PyErr where T: PyTypeObject, @@ -84,11 +100,17 @@ impl PyErr { }) } - /// Constructs a new error, with the usual lazy initialization of Python exceptions. + /// Constructs a new PyErr from the given Python type and arguments. /// - /// `exc` is the exception type; usually one of the standard exceptions + /// `ty` is the exception type; usually one of the standard exceptions /// like `exceptions::PyRuntimeError`. - /// `args` is the a tuple of arguments to pass to the exception constructor. + /// + /// `args` is either a tuple or a single value, with the same meaning as in [`PyErr::new`]. + /// + /// If an error occurs during normalization (for example if `T` is not a Python type which + /// extends from `BaseException`), then a different error may be produced during normalization. + /// + /// This error will be stored in an unnormalized state. pub fn from_type(ty: &PyType, args: A) -> PyErr where A: PyErrArguments + Send + Sync + 'static, @@ -105,32 +127,34 @@ impl PyErr { /// Creates a new PyErr. /// - /// `obj` must be an Python exception instance, the PyErr will use that instance. - /// If `obj` is a Python exception type object, the PyErr will (lazily) create a new - /// instance of that type. - /// Otherwise, a `TypeError` is created instead. + /// If `obj` is a Python exception object, the PyErr will contain that object. The error will be + /// in a normalized state. + /// + /// If `obj` is a Python exception type object, this is equivalent to `PyErr::from_type(obj, ())`. + /// + /// Otherwise, a `TypeError` is created. /// /// # Examples /// ```rust /// use pyo3::{exceptions::PyTypeError, types::PyType, IntoPy, PyErr, Python}; /// Python::with_gil(|py| { - /// // Case #1: Exception instance - /// let err = PyErr::from_instance(PyTypeError::new_err("some type error").instance(py)); + /// // Case #1: Exception object + /// let err = PyErr::from_value(PyTypeError::new_err("some type error").value(py)); /// assert_eq!(err.to_string(), "TypeError: some type error"); /// /// // Case #2: Exception type - /// let err = PyErr::from_instance(PyType::new::(py)); + /// let err = PyErr::from_value(PyType::new::(py)); /// assert_eq!(err.to_string(), "TypeError: "); /// /// // Case #3: Invalid exception value - /// let err = PyErr::from_instance("foo".into_py(py).as_ref(py)); + /// let err = PyErr::from_value("foo".into_py(py).as_ref(py)); /// assert_eq!( /// err.to_string(), /// "TypeError: exceptions must derive from BaseException" /// ); /// }); /// ``` - pub fn from_instance(obj: &PyAny) -> PyErr { + pub fn from_value(obj: &PyAny) -> PyErr { let ptr = obj.as_ptr(); let state = if unsafe { ffi::PyExceptionInstance_Check(ptr) } != 0 { @@ -152,7 +176,7 @@ impl PyErr { PyErr::from_state(state) } - /// Get the type of this exception object. + /// Returns the type of this exception. /// /// The object will be normalized first if needed. /// @@ -162,14 +186,14 @@ impl PyErr { /// /// Python::with_gil(|py| { /// let err: PyErr = PyTypeError::new_err(("some type error",)); - /// assert_eq!(err.ptype(py), PyType::new::(py)); + /// assert_eq!(err.get_type(py), PyType::new::(py)); /// }); /// ``` - pub fn ptype<'py>(&'py self, py: Python<'py>) -> &'py PyType { + pub fn get_type<'py>(&'py self, py: Python<'py>) -> &'py PyType { self.normalized(py).ptype.as_ref(py) } - /// Get the value of this exception object. + /// Returns the value of this exception. /// /// The object will be normalized first if needed. /// @@ -181,14 +205,22 @@ impl PyErr { /// Python::with_gil(|py| { /// let err: PyErr = PyTypeError::new_err(("some type error",)); /// assert!(err.is_instance_of::(py)); - /// assert_eq!(err.pvalue(py).to_string(), "some type error"); + /// assert_eq!(err.value(py).to_string(), "some type error"); /// }); /// ``` - pub fn pvalue<'py>(&'py self, py: Python<'py>) -> &'py PyBaseException { + pub fn value<'py>(&'py self, py: Python<'py>) -> &'py PyBaseException { self.normalized(py).pvalue.as_ref(py) } - /// Get the value of this exception object. + /// Consumes self to take ownership of the exception value contained in this error. + pub fn into_value(self, py: Python) -> Py { + // NB technically this causes one reference count increase and decrease in quick succession + // on pvalue, but it's probably not worth optimizing this right now for the additional code + // complexity. + self.normalized(py).pvalue.clone_ref(py) + } + + /// Returns the traceback of this exception object. /// /// The object will be normalized first if needed. /// @@ -198,10 +230,10 @@ impl PyErr { /// /// Python::with_gil(|py| { /// let err = PyTypeError::new_err(("some type error",)); - /// assert_eq!(err.ptraceback(py), None); + /// assert_eq!(err.traceback(py), None); /// }); /// ``` - pub fn ptraceback<'py>(&'py self, py: Python<'py>) -> Option<&'py PyTraceback> { + pub fn traceback<'py>(&'py self, py: Python<'py>) -> Option<&'py PyTraceback> { self.normalized(py) .ptraceback .as_ref() @@ -221,8 +253,8 @@ impl PyErr { /// callback) then this function will resume the panic. /// /// Use this function when it is not known if an error should be present. If the error is - /// expected to have been set, for example from [PyErr::occurred] or by an error return value - /// from a C FFI function, use [PyErr::fetch]. + /// expected to have been set, for example from [`PyErr::occurred`] or by an error return value + /// from a C FFI function, use [`PyErr::fetch`]. pub fn take(py: Python) -> Option { let (ptype, pvalue, ptraceback) = unsafe { let mut ptype: *mut ffi::PyObject = std::ptr::null_mut(); @@ -267,7 +299,6 @@ impl PyErr { eprintln!("Python stack trace below:"); unsafe { - use crate::conversion::IntoPyPointer; ffi::PyErr_Restore(ptype.into_ptr(), pvalue.into_ptr(), ptraceback.into_ptr()); ffi::PyErr_PrintEx(0); } @@ -361,16 +392,18 @@ impl PyErr { T: ToBorrowedObject, { exc.with_borrowed_ptr(py, |exc| unsafe { - ffi::PyErr_GivenExceptionMatches(self.ptype_ptr(py), exc) != 0 + ffi::PyErr_GivenExceptionMatches(self.type_ptr(py), exc) != 0 }) } /// Returns true if the current exception is instance of `T`. + #[inline] pub fn is_instance(&self, py: Python, typ: &PyType) -> bool { - unsafe { ffi::PyErr_GivenExceptionMatches(self.ptype_ptr(py), typ.as_ptr()) != 0 } + unsafe { ffi::PyErr_GivenExceptionMatches(self.type_ptr(py), typ.as_ptr()) != 0 } } /// Returns true if the current exception is instance of `T`. + #[inline] pub fn is_instance_of(&self, py: Python) -> bool where T: PyTypeObject, @@ -378,18 +411,6 @@ impl PyErr { self.is_instance(py, T::type_object(py)) } - /// 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) - } - - /// 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] @@ -426,11 +447,12 @@ impl PyErr { /// Python::with_gil(|py| { /// let err: PyErr = PyTypeError::new_err(("some type error",)); /// let err_clone = err.clone_ref(py); - /// assert_eq!(err.ptype(py), err_clone.ptype(py)); - /// assert_eq!(err.pvalue(py), err_clone.pvalue(py)); - /// assert_eq!(err.ptraceback(py), err_clone.ptraceback(py)); + /// assert_eq!(err.get_type(py), err_clone.get_type(py)); + /// assert_eq!(err.value(py), err_clone.value(py)); + /// assert_eq!(err.traceback(py), err_clone.traceback(py)); /// }); /// ``` + #[inline] pub fn clone_ref(&self, py: Python) -> PyErr { PyErr::from_state(PyErrState::Normalized(self.normalized(py).clone())) } @@ -438,25 +460,23 @@ impl PyErr { /// Return the cause (either an exception instance, or None, set by `raise ... from ...`) /// associated with the exception, as accessible from Python through `__cause__`. pub fn cause(&self, py: Python) -> Option { - let ptr = unsafe { ffi::PyException_GetCause(self.pvalue(py).as_ptr()) }; + let ptr = unsafe { ffi::PyException_GetCause(self.value(py).as_ptr()) }; let obj = unsafe { py.from_owned_ptr_or_opt::(ptr) }; - obj.map(Self::from_instance) + obj.map(Self::from_value) } /// Set the cause associated with the exception, pass `None` to clear it. pub fn set_cause(&self, py: Python, cause: Option) { - if let Some(cause) = cause { - let cause = cause.into_instance(py); - unsafe { - ffi::PyException_SetCause(self.pvalue(py).as_ptr(), cause.as_ptr()); - } - } else { - unsafe { - ffi::PyException_SetCause(self.pvalue(py).as_ptr(), std::ptr::null_mut()); - } + unsafe { + // PyException_SetCause _steals_ a reference to cause, so must use .into_ptr() + ffi::PyException_SetCause( + self.value(py).as_ptr(), + cause.map_or(std::ptr::null_mut(), |err| err.into_value(py).into_ptr()), + ); } } + #[inline] fn from_state(state: PyErrState) -> PyErr { PyErr { state: UnsafeCell::new(Some(state)), @@ -464,7 +484,7 @@ impl PyErr { } /// Returns borrowed reference to this Err's type - fn ptype_ptr(&self, py: Python) -> *mut ffi::PyObject { + fn type_ptr(&self, py: Python) -> *mut ffi::PyObject { match unsafe { &*self.state.get() } { // In lazy type case, normalize before returning ptype in case the type is not a valid // exception type. @@ -476,17 +496,26 @@ impl PyErr { } } + #[inline] fn normalized(&self, py: Python) -> &PyErrStateNormalized { + if let Some(PyErrState::Normalized(n)) = unsafe { + // Safety: self.state will never be written again once normalized. + &*self.state.get() + } { + return n; + } + + self.make_normalized(py) + } + + #[cold] + fn make_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() @@ -509,15 +538,66 @@ impl PyErr { } } } + + /// Deprecated name for [`PyErr::get_type`]. + #[deprecated( + since = "0.16.0", + note = "Use err.get_type(py) instead of err.ptype(py)" + )] + pub fn ptype<'py>(&'py self, py: Python<'py>) -> &'py PyType { + self.get_type(py) + } + + /// Deprecated name for [`PyErr::value`]. + #[deprecated(since = "0.16.0", note = "Use err.value(py) instead of err.pvalue(py)")] + pub fn pvalue<'py>(&'py self, py: Python<'py>) -> &'py PyBaseException { + self.value(py) + } + + /// Deprecated name for [`PyErr::traceback`]. + #[deprecated( + since = "0.16.0", + note = "Use err.traceback(py) instead of err.ptraceback(py)" + )] + pub fn ptraceback<'py>(&'py self, py: Python<'py>) -> Option<&'py PyTraceback> { + self.traceback(py) + } + + /// Deprecated name for [`PyErr::value`]. + #[deprecated( + since = "0.16.0", + note = "Use err.value(py) instead of err.instance(py)" + )] + pub fn instance<'py>(&'py self, py: Python<'py>) -> &'py PyBaseException { + self.value(py) + } + + /// Deprecated name for [`PyErr::from_value`]. + #[deprecated( + since = "0.16.0", + note = "Use err.from_value(py, obj) instead of err.from_instance(py, obj)" + )] + pub fn from_instance(value: &PyAny) -> PyErr { + PyErr::from_value(value) + } + + /// Deprecated name for [`PyErr::into_value`]. + #[deprecated( + since = "0.16.0", + note = "Use err.into_value(py) instead of err.into_instance(py)" + )] + pub fn into_instance(self, py: Python) -> Py { + self.into_value(py) + } } impl std::fmt::Debug for PyErr { fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> { Python::with_gil(|py| { f.debug_struct("PyErr") - .field("type", self.ptype(py)) - .field("value", self.pvalue(py)) - .field("traceback", &self.ptraceback(py)) + .field("type", self.get_type(py)) + .field("value", self.value(py)) + .field("traceback", &self.traceback(py)) .finish() }) } @@ -526,10 +606,10 @@ impl std::fmt::Debug for PyErr { impl std::fmt::Display for PyErr { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { Python::with_gil(|py| { - let instance = self.instance(py); - let type_name = instance.get_type().name().map_err(|_| std::fmt::Error)?; + let value = self.value(py); + let type_name = value.get_type().name().map_err(|_| std::fmt::Error)?; write!(f, "{}", type_name)?; - if let Ok(s) = instance.str() { + if let Ok(s) = value.str() { write!(f, ": {}", &s.to_string_lossy()) } else { write!(f, ": ") @@ -542,7 +622,7 @@ impl std::error::Error for PyErr {} impl IntoPy for PyErr { fn into_py(self, py: Python) -> PyObject { - self.into_instance(py).into() + self.into_value(py).into() } } @@ -749,4 +829,22 @@ mod tests { assert_eq!(cause.to_string(), "ValueError: orange"); }); } + + #[allow(deprecated)] + #[test] + fn deprecations() { + let err = exceptions::PyValueError::new_err("an error"); + Python::with_gil(|py| { + assert_eq!(err.ptype(py), err.get_type(py)); + assert_eq!(err.pvalue(py), err.value(py)); + assert_eq!(err.instance(py), err.value(py)); + assert_eq!(err.ptraceback(py), err.traceback(py)); + + assert_eq!( + err.clone_ref(py).into_instance(py).as_ref(py), + err.value(py) + ); + assert_eq!(PyErr::from_instance(err.value(py)).value(py), err.value(py)); + }); + } } diff --git a/src/exceptions.rs b/src/exceptions.rs index 4edfae91..d930a0ed 100644 --- a/src/exceptions.rs +++ b/src/exceptions.rs @@ -21,7 +21,7 @@ macro_rules! impl_exception_boilerplate { impl ::std::convert::From<&$name> for $crate::PyErr { #[inline] fn from(err: &$name) -> $crate::PyErr { - $crate::PyErr::from_instance(err) + $crate::PyErr::from_value(err) } } @@ -613,7 +613,7 @@ macro_rules! test_exception { assert!(err.is_instance_of::<$exc_ty>(py)); - let value: &$exc_ty = err.instance(py).downcast().unwrap(); + let value: &$exc_ty = err.value(py).downcast().unwrap(); assert!(value.source().is_none()); err.set_cause(py, Some($crate::exceptions::PyValueError::new_err("a cause"))); @@ -755,7 +755,7 @@ mod tests { let exc = py .run("raise Exception('banana')", None, None) .expect_err("raising should have given us an error") - .into_instance(py) + .into_value(py) .into_ref(py); assert_eq!( format!("{:?}", exc), @@ -770,7 +770,7 @@ mod tests { let exc = py .run("raise Exception('banana')", None, None) .expect_err("raising should have given us an error") - .into_instance(py) + .into_value(py) .into_ref(py); assert_eq!( exc.to_string(), @@ -791,7 +791,7 @@ mod tests { None, ) .expect_err("raising should have given us an error") - .into_instance(py) + .into_value(py) .into_ref(py); assert_eq!(format!("{:?}", exc), "Exception('banana')"); @@ -861,7 +861,7 @@ mod tests { test_exception!(PyUnicodeDecodeError, |py| { let invalid_utf8 = b"fo\xd8o"; let err = std::str::from_utf8(invalid_utf8).expect_err("should be invalid utf8"); - PyErr::from_instance(PyUnicodeDecodeError::new_utf8(py, invalid_utf8, err).unwrap()) + PyErr::from_value(PyUnicodeDecodeError::new_utf8(py, invalid_utf8, err).unwrap()) }); test_exception!(PyUnicodeEncodeError, |py: Python<'_>| { py.eval("chr(40960).encode('ascii')", None, None) diff --git a/src/types/string.rs b/src/types/string.rs index 28eb89f6..22496e26 100644 --- a/src/types/string.rs +++ b/src/types/string.rs @@ -73,7 +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_instance(PyUnicodeDecodeError::new_utf8( + Err(e) => Err(crate::PyErr::from_value(PyUnicodeDecodeError::new_utf8( py, data, e, )?)), }, @@ -83,7 +83,7 @@ impl<'a> PyStringData<'a> { let mut message = e.to_string().as_bytes().to_vec(); message.push(0); - Err(crate::PyErr::from_instance(PyUnicodeDecodeError::new( + Err(crate::PyErr::from_value(PyUnicodeDecodeError::new( py, CStr::from_bytes_with_nul(b"utf-16\0").unwrap(), self.as_bytes(), @@ -94,7 +94,7 @@ impl<'a> PyStringData<'a> { }, 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_instance(PyUnicodeDecodeError::new( + None => Err(crate::PyErr::from_value(PyUnicodeDecodeError::new( py, CStr::from_bytes_with_nul(b"utf-32\0").unwrap(), self.as_bytes(), @@ -504,7 +504,7 @@ mod tests { let data = unsafe { s.data().unwrap() }; assert_eq!(data, PyStringData::Ucs1(b"f\xfe")); let err = data.to_string(py).unwrap_err(); - assert_eq!(err.ptype(py), PyUnicodeDecodeError::type_object(py)); + assert_eq!(err.get_type(py), PyUnicodeDecodeError::type_object(py)); assert!(err .to_string() .contains("'utf-8' codec can't decode byte 0xfe in position 1")); @@ -546,7 +546,7 @@ mod tests { let data = unsafe { s.data().unwrap() }; assert_eq!(data, PyStringData::Ucs2(&[0xff22, 0xd800])); let err = data.to_string(py).unwrap_err(); - assert_eq!(err.ptype(py), PyUnicodeDecodeError::type_object(py)); + assert_eq!(err.get_type(py), PyUnicodeDecodeError::type_object(py)); assert!(err .to_string() .contains("'utf-16' codec can't decode bytes in position 0-3")); @@ -585,7 +585,7 @@ mod tests { let data = unsafe { s.data().unwrap() }; assert_eq!(data, PyStringData::Ucs4(&[0x20000, 0xd800])); let err = data.to_string(py).unwrap_err(); - assert_eq!(err.ptype(py), PyUnicodeDecodeError::type_object(py)); + assert_eq!(err.get_type(py), PyUnicodeDecodeError::type_object(py)); assert!(err .to_string() .contains("'utf-32' codec can't decode bytes in position 0-7")); diff --git a/src/types/traceback.rs b/src/types/traceback.rs index b83f2e93..6d799a70 100644 --- a/src/types/traceback.rs +++ b/src/types/traceback.rs @@ -33,7 +33,7 @@ impl PyTraceback { /// .run("raise Exception('banana')", None, None) /// .expect_err("raise will create a Python error"); /// - /// let traceback = err.ptraceback(py).expect("raised exception will have a traceback"); + /// let traceback = err.traceback(py).expect("raised exception will have a traceback"); /// assert_eq!( /// format!("{}{}", traceback.format()?, err), /// "\ @@ -74,7 +74,7 @@ mod tests { .expect_err("raising should have given us an error"); assert_eq!( - err.ptraceback(py).unwrap().format().unwrap(), + err.traceback(py).unwrap().format().unwrap(), "Traceback (most recent call last):\n File \"\", line 1, in \n" ); })