From 63d6d4c0e2cbcded23a598e91b41e3b0a5079076 Mon Sep 17 00:00:00 2001 From: Sebastian Puetz Date: Sun, 19 Jul 2020 10:44:25 +0200 Subject: [PATCH 1/2] Add type info to conversion errors. --- CHANGELOG.md | 1 + src/conversion.rs | 20 ++++++++++---------- src/err.rs | 36 ++++++++++++++++++++++++++++++------ src/python.rs | 2 +- src/types/sequence.rs | 6 +++--- 5 files changed, 45 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d7b1cef612..fc8e0bc9f24 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Add FFI definitions `Py_FinalizeEx`, `PyOS_getsig`, `PyOS_setsig`. [#1021](https://github.com/PyO3/pyo3/pull/1021) - Add `Python::with_gil` for executing a closure with the Python GIL. [#1037](https://github.com/PyO3/pyo3/pull/1037) - Implement `Debug` for `PyIterator`. [#1051](https://github.com/PyO3/pyo3/pull/1051) +- Implement type information for conversion failures. [#1050](https://github.com/PyO3/pyo3/pull/1050) ### Changed - Exception types have been renamed from e.g. `RuntimeError` to `PyRuntimeError`, and are now only accessible by `&T` or `Py` similar to other Python-native types. The old names continue to exist but are deprecated. [#1024](https://github.com/PyO3/pyo3/pull/1024) diff --git a/src/conversion.rs b/src/conversion.rs index b07fef42419..2bf98dbec34 100644 --- a/src/conversion.rs +++ b/src/conversion.rs @@ -321,10 +321,10 @@ where /// This trait is similar to `std::convert::TryFrom` pub trait PyTryFrom<'v>: Sized + PyNativeType { /// Cast from a concrete Python object type to PyObject. - fn try_from>(value: V) -> Result<&'v Self, PyDowncastError>; + fn try_from<'b, V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError<'v>>; /// Cast from a concrete Python object type to PyObject. With exact type check. - fn try_from_exact>(value: V) -> Result<&'v Self, PyDowncastError>; + fn try_from_exact<'b, V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError<'v>>; /// Cast a PyAny to a specific type of PyObject. The caller must /// have already verified the reference is for this type. @@ -358,24 +358,24 @@ impl<'v, T> PyTryFrom<'v> for T where T: PyTypeInfo + PyNativeType, { - fn try_from>(value: V) -> Result<&'v Self, PyDowncastError> { + fn try_from<'b, V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError<'v>> { let value = value.into(); unsafe { if T::is_instance(value) { Ok(Self::try_from_unchecked(value)) } else { - Err(PyDowncastError) + Err(PyDowncastError::new(value, T::NAME)) } } } - fn try_from_exact>(value: V) -> Result<&'v Self, PyDowncastError> { + fn try_from_exact<'b, V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError<'v>> { let value = value.into(); unsafe { if T::is_exact_instance(value) { Ok(Self::try_from_unchecked(value)) } else { - Err(PyDowncastError) + Err(PyDowncastError::new(value, T::NAME)) } } } @@ -390,23 +390,23 @@ impl<'v, T> PyTryFrom<'v> for PyCell where T: 'v + PyClass, { - fn try_from>(value: V) -> Result<&'v Self, PyDowncastError> { + fn try_from<'b, V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError<'v>> { let value = value.into(); unsafe { if T::is_instance(value) { Ok(Self::try_from_unchecked(value)) } else { - Err(PyDowncastError) + Err(PyDowncastError::new(value, T::NAME)) } } } - fn try_from_exact>(value: V) -> Result<&'v Self, PyDowncastError> { + fn try_from_exact<'b, V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError<'v>> { let value = value.into(); unsafe { if T::is_exact_instance(value) { Ok(Self::try_from_unchecked(value)) } else { - Err(PyDowncastError) + Err(PyDowncastError::new(value, T::NAME)) } } } diff --git a/src/err.rs b/src/err.rs index 839ca0c5e92..8400e1f5d2c 100644 --- a/src/err.rs +++ b/src/err.rs @@ -10,6 +10,7 @@ use crate::{ Python, ToBorrowedObject, ToPyObject, }; use libc::c_int; +use std::borrow::Cow; use std::ffi::CString; use std::io; use std::os::raw::c_char; @@ -56,7 +57,20 @@ pub struct PyErr { pub type PyResult = Result; /// Marker type that indicates an error while downcasting -pub struct PyDowncastError; +#[derive(Debug)] +pub struct PyDowncastError<'a> { + from: &'a PyAny, + to: Cow<'static, str>, +} + +impl<'a> PyDowncastError<'a> { + pub fn new(from: &'a PyAny, to: impl Into>) -> Self { + PyDowncastError { + from, + to: to.into(), + } + } +} /// Helper conversion trait that allows to use custom arguments for exception constructor. pub trait PyErrArguments { @@ -460,15 +474,25 @@ impl<'a> IntoPy for &'a PyErr { } /// Convert `PyDowncastError` to Python `TypeError`. -impl std::convert::From for PyErr { - fn from(_err: PyDowncastError) -> PyErr { - exceptions::PyTypeError::py_err(()) +impl<'a> std::convert::From> for PyErr { + fn from(err: PyDowncastError) -> PyErr { + exceptions::PyTypeError::py_err(err.to_string()) } } -impl<'p> std::fmt::Debug for PyDowncastError { +impl<'a> std::error::Error for PyDowncastError<'a> {} + +impl<'a> std::fmt::Display for PyDowncastError<'a> { fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> { - f.write_str("PyDowncastError") + write!( + f, + "Can't convert {} to {}", + self.from + .repr() + .map(|s| s.to_string_lossy()) + .unwrap_or_else(|_| self.from.get_type().name()), + self.to + ) } } diff --git a/src/python.rs b/src/python.rs index 82283dae94d..dbbd5ed43d7 100644 --- a/src/python.rs +++ b/src/python.rs @@ -375,7 +375,7 @@ impl<'p> Python<'p> { impl<'p> Python<'p> { /// Registers the object in the release pool, and tries to downcast to specific type. - pub fn checked_cast_as(self, obj: PyObject) -> Result<&'p T, PyDowncastError> + pub fn checked_cast_as(self, obj: PyObject) -> Result<&'p T, PyDowncastError<'p>> where T: PyTryFrom<'p>, { diff --git a/src/types/sequence.rs b/src/types/sequence.rs index dc02da80d29..cc555471371 100644 --- a/src/types/sequence.rs +++ b/src/types/sequence.rs @@ -372,18 +372,18 @@ where } impl<'v> PyTryFrom<'v> for PySequence { - fn try_from>(value: V) -> Result<&'v PySequence, PyDowncastError> { + fn try_from>(value: V) -> Result<&'v PySequence, PyDowncastError<'v>> { let value = value.into(); unsafe { if ffi::PySequence_Check(value.as_ptr()) != 0 { Ok(::try_from_unchecked(value)) } else { - Err(PyDowncastError) + Err(PyDowncastError::new(value, "Sequence")) } } } - fn try_from_exact>(value: V) -> Result<&'v PySequence, PyDowncastError> { + fn try_from_exact>(value: V) -> Result<&'v PySequence, PyDowncastError<'v>> { ::try_from(value) } From a058eb52018b5276831c4b339e73d8cb75c980cd Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Sun, 19 Jul 2020 11:23:07 +0100 Subject: [PATCH 2/2] Remove redundant lifetimes --- src/conversion.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/conversion.rs b/src/conversion.rs index 2bf98dbec34..e3629754e6d 100644 --- a/src/conversion.rs +++ b/src/conversion.rs @@ -321,10 +321,10 @@ where /// This trait is similar to `std::convert::TryFrom` pub trait PyTryFrom<'v>: Sized + PyNativeType { /// Cast from a concrete Python object type to PyObject. - fn try_from<'b, V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError<'v>>; + fn try_from>(value: V) -> Result<&'v Self, PyDowncastError<'v>>; /// Cast from a concrete Python object type to PyObject. With exact type check. - fn try_from_exact<'b, V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError<'v>>; + fn try_from_exact>(value: V) -> Result<&'v Self, PyDowncastError<'v>>; /// Cast a PyAny to a specific type of PyObject. The caller must /// have already verified the reference is for this type. @@ -358,7 +358,7 @@ impl<'v, T> PyTryFrom<'v> for T where T: PyTypeInfo + PyNativeType, { - fn try_from<'b, V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError<'v>> { + fn try_from>(value: V) -> Result<&'v Self, PyDowncastError<'v>> { let value = value.into(); unsafe { if T::is_instance(value) { @@ -369,7 +369,7 @@ where } } - fn try_from_exact<'b, V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError<'v>> { + fn try_from_exact>(value: V) -> Result<&'v Self, PyDowncastError<'v>> { let value = value.into(); unsafe { if T::is_exact_instance(value) { @@ -390,7 +390,7 @@ impl<'v, T> PyTryFrom<'v> for PyCell where T: 'v + PyClass, { - fn try_from<'b, V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError<'v>> { + fn try_from>(value: V) -> Result<&'v Self, PyDowncastError<'v>> { let value = value.into(); unsafe { if T::is_instance(value) { @@ -400,7 +400,7 @@ where } } } - fn try_from_exact<'b, V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError<'v>> { + fn try_from_exact>(value: V) -> Result<&'v Self, PyDowncastError<'v>> { let value = value.into(); unsafe { if T::is_exact_instance(value) {