Proto PyO3#1: Exception instances as Py<BaseException>

This prototype implements use of Py<BaseException> as the instance to
use for exception instances. These instances integrate reasonably well
with the Rust’s standard error mechanisms by implementing the `Display`
and `Error` traits. These error types can also be stored into e.g.
`failure::Fail`s or other error types as a cause of some greater error.

src/err.rs

@@ -299,12 +299,13 @@ impl PyErr {
     /// 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.
-    fn instance(mut self, py: Python) -> PyObject {
+    fn instance(mut self, py: Python) -> Py<exceptions::BaseException> {
         self.normalize(py);
-        match self.pvalue {
-            PyErrValue::Value(ref instance) => instance.clone_ref(py),
-            _ => py.None(),
-        }
+        let r = match self.pvalue {
+            PyErrValue::Value(ref instance) => instance.clone_ref(py).extract(py),
+            _ => Err(PyDowncastError.into()),
+        };
+        r.expect("Normalized error instance should be a BaseException")
     }
 
     /// Writes the error back to the Python interpreter's global state.
@@ -370,6 +371,12 @@ impl std::fmt::Debug for PyErr {
 }
 
 impl FromPy<PyErr> for PyObject {
+    fn from_py(other: PyErr, py: Python) -> Self {
+        other.instance(py).into()
+    }
+}
+
+impl FromPy<PyErr> for Py<exceptions::BaseException> {
     fn from_py(other: PyErr, py: Python) -> Self {
         other.instance(py)
     }
@@ -378,14 +385,14 @@ impl FromPy<PyErr> for PyObject {
 impl ToPyObject for PyErr {
     fn to_object(&self, py: Python) -> PyObject {
         let err = self.clone_ref(py);
-        err.instance(py)
+        err.instance(py).into()
     }
 }
 
 impl<'a> IntoPy<PyObject> for &'a PyErr {
     fn into_py(self, py: Python) -> PyObject {
         let err = self.clone_ref(py);
-        err.instance(py)
+        err.instance(py).into()
     }
 }
 

src/exceptions.rs

@@ -8,6 +8,7 @@ use crate::type_object::PyTypeObject;
 use crate::types::{PyAny, PyTuple};
 use crate::Python;
 use crate::{AsPyPointer, ToPyObject};
+use crate::{AsPyRef, Py, PyDowncastError, PyTryFrom};
 use std::ffi::CStr;
 use std::os::raw::c_char;
 use std::{self, ops};
@@ -208,11 +209,13 @@ macro_rules! impl_native_exception (
                 PyErr::new::<$name, _>(())
             }
         }
+
         impl<T> std::convert::Into<$crate::PyResult<T>> for $name {
             fn into(self) -> $crate::PyResult<T> {
                 PyErr::new::<$name, _>(()).into()
             }
         }
+
         impl $name {
             pub fn py_err<V: ToPyObject + 'static>(args: V) -> PyErr {
                 PyErr::new::<$name, V>(args)
@@ -221,11 +224,71 @@ macro_rules! impl_native_exception (
                 PyErr::new::<$name, V>(args).into()
             }
         }
+
         unsafe impl PyTypeObject for $name {
             fn init_type() -> std::ptr::NonNull<$crate::ffi::PyTypeObject> {
                 unsafe { std::ptr::NonNull::new_unchecked(ffi::$exc_name as *mut _) }
             }
         }
+
+        impl<'v> PyTryFrom<'v> for $name {
+            fn try_from<V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError> {
+                unsafe {
+                    let value = value.into();
+                    if ffi::PyObject_TypeCheck(value.as_ptr(), ffi::$exc_name as *mut _) != 0 {
+                        Ok(PyTryFrom::try_from_unchecked(value))
+                    } else {
+                        Err(PyDowncastError)
+                    }
+                }
+            }
+
+            fn try_from_exact<V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError> {
+                unsafe {
+                    let value = value.into();
+                    if (*value.as_ptr()).ob_type == ffi::$exc_name as *mut _ {
+                        Ok(PyTryFrom::try_from_unchecked(value))
+                    } else {
+                        Err(PyDowncastError)
+                    }
+                }
+            }
+
+            fn try_from_mut<V: Into<&'v PyAny>>(value: V) -> Result<&'v mut Self, PyDowncastError> {
+                unsafe {
+                    let value = value.into();
+                    if ffi::PyObject_TypeCheck(value.as_ptr(), ffi::$exc_name as *mut _) != 0 {
+                        Ok(PyTryFrom::try_from_mut_unchecked(value))
+                    } else {
+                        Err(PyDowncastError)
+                    }
+                }
+            }
+            fn try_from_mut_exact<V: Into<&'v PyAny>>(value: V) -> Result<&'v mut Self, PyDowncastError> {
+                unsafe {
+                    let value = value.into();
+                    if (*value.as_ptr()).ob_type == ffi::$exc_name as *mut _ {
+                        Ok(PyTryFrom::try_from_mut_unchecked(value))
+                    } else {
+                        Err(PyDowncastError)
+                    }
+                }
+            }
+
+            unsafe fn try_from_unchecked<V: Into<&'v PyAny>>(value: V) -> &'v Self {
+                &*(value.into().as_ptr() as *const _)
+            }
+
+            unsafe fn try_from_mut_unchecked<V: Into<&'v PyAny>>(value: V) -> &'v mut Self {
+                &mut *(value.into().as_ptr() as *mut _)
+            }
+        }
+
+        impl AsPyPointer for $name {
+            fn as_ptr(&self) -> *mut ffi::PyObject {
+                return self as *const _ as *const _ as *mut ffi::PyObject;
+            }
+        }
     );
 );
 
@@ -340,6 +403,72 @@ impl StopIteration {
     }
 }
 
+impl std::fmt::Debug for BaseException {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        // Sneaky: we don’t really need a GIL lock here as nothing should be able to just mutate
+        // the "type" of an object, right? RIGHT???
+        //
+        // let gil = Python::acquire_gil();
+        // let _py = gil.python();
+        let py_type_name = unsafe { CStr::from_ptr((*(*self.as_ptr()).ob_type).tp_name) };
+        let type_name = py_type_name.to_string_lossy();
+        f.debug_struct(&*type_name)
+            // TODO: print out actual fields!
+            .finish()
+    }
+}
+
+impl std::fmt::Display for BaseException {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        let py_type_name = unsafe { CStr::from_ptr((*(*self.as_ptr()).ob_type).tp_name) };
+        let type_name = py_type_name.to_string_lossy();
+        write!(f, "{}", type_name)?;
+        let py_self: Py<PyAny> = unsafe { Py::from_borrowed_ptr(self.as_ptr()) };
+
+        let gil = Python::acquire_gil();
+        let py = gil.python();
+        if let Ok(s) = crate::ObjectProtocol::str(&*py_self.as_ref(py)) {
+            write!(f, ": {}", &s.to_string_lossy())
+        } else {
+            write!(f, ": <exception str() failed>")
+        }
+    }
+}
+
+impl std::error::Error for BaseException {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        unsafe {
+            // Returns either `None` or an instance of an exception.
+            let cause_object = ffi::PyException_GetCause(self.as_ptr());
+            if cause_object == ffi::Py_None() {
+                None
+            } else {
+                // FIXME: PyException_GetCause returns a new reference to the cause object.
+                //
+                // While we know that `self` is "immutable" (`&self`!) it is also true that between
+                // now and when the return value is actually read the GIL could be unlocked and
+                // then concurrent threads could modify `self` to change its `__cause__`.
+                //
+                // If this was not a possibility, we could just `DECREF` here, instead, now, we
+                // must return a `&Py<BaseException>` instead… but we cannot do that because
+                // nothing is storing such a thing anywhere and thus we cannot take a reference to
+                // that…
+                //
+                // The only way to make this function to work sanely, without leaking, is to ensure
+                // that between a call to `Error::source` and drop of the reference there’s no
+                // possible way for for the object to be modified. Even if we had a way to prevent
+                // GIL from unlocking, people could modify the object through a different
+                // reference to the Exception.
+                //
+                // Sounds unsound, doesn’t it?
+                //
+                // ffi::Py_DECREF(cause_object);
+                Some(&*(cause_object as *const _ as *const BaseException))
+            }
+        }
+    }
+}
+
 /// Exceptions defined in `asyncio` module
 pub mod asyncio {
     import_exception!(asyncio, CancelledError);
@@ -363,7 +492,9 @@ mod test {
     use crate::exceptions::Exception;
     use crate::objectprotocol::ObjectProtocol;
     use crate::types::{IntoPyDict, PyDict};
-    use crate::{PyErr, Python};
+    use crate::{AsPyPointer, FromPy, Py, PyErr, Python};
+    use std::error::Error;
+    use std::fmt::Write;
 
     import_exception!(socket, gaierror);
     import_exception!(email.errors, MessageError);
@@ -441,4 +572,42 @@ mod test {
         )
         .unwrap();
     }
+
+    #[test]
+    fn native_exception_display() {
+        let mut out = String::new();
+        let gil = Python::acquire_gil();
+        let py = gil.python();
+        let result = py
+            .run("raise Exception('banana')", None, None)
+            .expect_err("raising should have given us an error");
+        let convert = Py::<super::BaseException>::from_py(result, py);
+        write!(&mut out, "{}", convert).expect("successful format");
+        assert_eq!(out, "Exception: banana");
+    }
+
+    #[test]
+    fn native_exception_chain() {
+        let mut out = String::new();
+        let gil = Python::acquire_gil();
+        let py = gil.python();
+        let result = py
+            .run(
+                "raise Exception('banana') from TypeError('peach')",
+                None,
+                None,
+            )
+            .expect_err("raising should have given us an error");
+        let convert = Py::<super::BaseException>::from_py(result, py);
+        write!(&mut out, "{}", convert).expect("successful format");
+        assert_eq!(out, "Exception: banana");
+        out.clear();
+        let convert_ref: &super::BaseException =
+            unsafe { &*(convert.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");
+        let source_source = source.source();
+        assert!(source_source.is_none(), "source_source should be None");
+    }
 }

src/instance.rs

@@ -8,7 +8,7 @@ use crate::objectprotocol::ObjectProtocol;
 use crate::type_object::PyTypeCreate;
 use crate::type_object::{PyTypeInfo, PyTypeObject};
 use crate::types::PyAny;
-use crate::{ffi, IntoPy};
+use crate::{ffi, IntoPy, PyTryFrom};
 use crate::{AsPyPointer, FromPyObject, FromPyPointer, IntoPyPointer, Python, ToPyObject};
 use std::marker::PhantomData;
 use std::mem;
@@ -54,7 +54,6 @@ pub unsafe trait PyNativeType: Sized {
 /// let d = [("p", obj)].into_py_dict(py);
 /// py.run("assert p.length() == 12", None, Some(d)).unwrap();
 /// ```
-#[derive(Debug)]
 pub struct PyRef<'a, T: PyTypeInfo>(&'a T, Unsendable);
 
 #[allow(clippy::cast_ptr_alignment)]
@@ -82,6 +81,24 @@ where
     }
 }
 
+impl<'a, T: std::fmt::Display + PyTypeInfo> std::fmt::Display for PyRef<'a, T> {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        unsafe {
+            let gil = Python::acquire_gil();
+            let py = gil.python();
+            let object = self.to_object(py);
+            let obj_ref: &T = PyTryFrom::try_from_unchecked(object.as_ref(py));
+            std::fmt::Display::fmt(&obj_ref, f)
+        }
+    }
+}
+
+impl<'a, T: PyTypeInfo> std::fmt::Debug for PyRef<'a, T> {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        f.debug_tuple("PyRef").field(&(self.0 as *const _)).finish()
+    }
+}
+
 impl<'a, T: PyTypeInfo> AsPyPointer for PyRef<'a, T> {
     fn as_ptr(&self) -> *mut ffi::PyObject {
         ref_to_ptr(self.0)
@@ -142,7 +159,6 @@ where
 /// obj.x = 5; obj.y = 20;
 /// py.run("assert p.length() == 100", None, Some(d)).unwrap();
 /// ```
-#[derive(Debug)]
 pub struct PyRefMut<'a, T: PyTypeInfo>(&'a mut T, Unsendable);
 
 impl<'a, T: PyTypeInfo> PyRefMut<'a, T> {
@@ -162,6 +178,26 @@ where
     }
 }
 
+impl<'a, T: std::fmt::Display + PyTypeInfo> std::fmt::Display for PyRefMut<'a, T> {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        unsafe {
+            let gil = Python::acquire_gil();
+            let py = gil.python();
+            let object = self.to_object(py);
+            let obj_ref: &T = PyTryFrom::try_from_unchecked(object.as_ref(py));
+            std::fmt::Display::fmt(&obj_ref, f)
+        }
+    }
+}
+
+impl<'a, T: PyTypeInfo> std::fmt::Debug for PyRefMut<'a, T> {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        f.debug_tuple("PyRefMut")
+            .field(&(self.0 as *const _))
+            .finish()
+    }
+}
+
 impl<'a, T: PyTypeInfo> AsPyPointer for PyRefMut<'a, T> {
     fn as_ptr(&self) -> *mut ffi::PyObject {
         ref_to_ptr(self.0)
@@ -265,7 +301,6 @@ pub trait AsPyRef<T: PyTypeInfo>: Sized {
 /// Safe wrapper around unsafe `*mut ffi::PyObject` pointer with specified type information.
 ///
 /// `Py<T>` is thread-safe, because any python related operations require a Python<'p> token.
-#[derive(Debug)]
 #[repr(transparent)]
 pub struct Py<T>(NonNull<ffi::PyObject>, PhantomData<T>);
 
@@ -357,6 +392,24 @@ impl<T> Py<T> {
     }
 }
 
+impl<T: std::fmt::Display + for<'a> PyTryFrom<'a>> std::fmt::Display for Py<T> {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        unsafe {
+            let gil = Python::acquire_gil();
+            let py = gil.python();
+            let object = self.to_object(py);
+            let obj_ref: &T = PyTryFrom::try_from_unchecked(object.as_ref(py));
+            std::fmt::Display::fmt(&obj_ref, f)
+        }
+    }
+}
+
+impl<T> std::fmt::Debug for Py<T> {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        f.debug_tuple("Py").field(&self.0.as_ptr()).finish()
+    }
+}
+
 /// Specialization workaround
 trait AsPyRefDispatch<T: PyTypeInfo>: AsPyPointer {
     fn as_ref_dispatch(&self, _py: Python) -> &T;