Merge pull request #1050 from sebpuetz/conversion-errors

Add type info to conversion errors.
PyO3 · Jul 19, 2020 · 53a248c · 53a248c
2 parents a1dc970 + a058eb5
commit 53a248c
Show file tree

Hide file tree

Showing 5 changed files with 45 additions and 20 deletions.
diff --git 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<T>` 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
@@ -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<V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError>;
+    fn try_from<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<V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError>;
+    fn try_from_exact<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<V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError> {
+    fn try_from<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<V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError> {
+    fn try_from_exact<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<T>
 where
     T: 'v + PyClass,
 {
-    fn try_from<V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError> {
+    fn try_from<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<V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError> {
+    fn try_from_exact<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
@@ -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<T> = Result<T, PyErr>;
 
 /// 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<Cow<'static, str>>) -> 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<PyObject> for &'a PyErr {
 }
 
 /// Convert `PyDowncastError` to Python `TypeError`.
-impl std::convert::From<PyDowncastError> for PyErr {
-    fn from(_err: PyDowncastError) -> PyErr {
-        exceptions::PyTypeError::py_err(())
+impl<'a> std::convert::From<PyDowncastError<'a>> 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
@@ -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<T>(self, obj: PyObject) -> Result<&'p T, PyDowncastError>
+    pub fn checked_cast_as<T>(self, obj: PyObject) -> Result<&'p T, PyDowncastError<'p>>
     where
         T: PyTryFrom<'p>,
     {

diff --git a/src/types/sequence.rs b/src/types/sequence.rs
@@ -372,18 +372,18 @@ where
 }
 
 impl<'v> PyTryFrom<'v> for PySequence {
-    fn try_from<V: Into<&'v PyAny>>(value: V) -> Result<&'v PySequence, PyDowncastError> {
+    fn try_from<V: Into<&'v PyAny>>(value: V) -> Result<&'v PySequence, PyDowncastError<'v>> {
         let value = value.into();
         unsafe {
             if ffi::PySequence_Check(value.as_ptr()) != 0 {
                 Ok(<PySequence as PyTryFrom>::try_from_unchecked(value))
             } else {
-                Err(PyDowncastError)
+                Err(PyDowncastError::new(value, "Sequence"))
             }
         }
     }
 
-    fn try_from_exact<V: Into<&'v PyAny>>(value: V) -> Result<&'v PySequence, PyDowncastError> {
+    fn try_from_exact<V: Into<&'v PyAny>>(value: V) -> Result<&'v PySequence, PyDowncastError<'v>> {
         <PySequence as PyTryFrom>::try_from(value)
     }