Skip to content

Commit

Permalink
Add type info to conversion errors.
Browse files Browse the repository at this point in the history
  • Loading branch information
sebpuetz committed Jul 18, 2020
1 parent 41b35b8 commit e8596dd
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 42 deletions.
4 changes: 4 additions & 0 deletions examples/rustapi_module/src/pyclass_iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ impl PyClassIter {
pub fn new() -> Self {
PyClassIter { count: 0 }
}

pub fn foo(&self, not_a_string: Option<String>) -> pyo3::PyResult<()> {
Ok(())
}
}

#[pyproto]
Expand Down
27 changes: 17 additions & 10 deletions src/conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use crate::object::PyObject;
use crate::type_object::PyTypeInfo;
use crate::types::PyTuple;
use crate::{ffi, gil, Py, PyAny, PyCell, PyClass, PyNativeType, PyRef, PyRefMut, Python};
use std::borrow::Cow;
use std::ptr::NonNull;

/// This trait represents that **we can do zero-cost conversion from the object
Expand Down Expand Up @@ -321,10 +322,12 @@ 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<'b, V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError<'v, 'b>>;

/// 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<'b, V: Into<&'v PyAny>>(
value: V,
) -> Result<&'v Self, PyDowncastError<'v, 'b>>;

/// Cast a PyAny to a specific type of PyObject. The caller must
/// have already verified the reference is for this type.
Expand Down Expand Up @@ -358,24 +361,26 @@ 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<'b, V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError<'v, 'b>> {
let value = value.into();
unsafe {
if T::is_instance(value) {
Ok(Self::try_from_unchecked(value))
} else {
Err(PyDowncastError)
Err(PyDowncastError::new(value, Cow::Borrowed(T::NAME)))
}
}
}

fn try_from_exact<V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError> {
fn try_from_exact<'b, V: Into<&'v PyAny>>(
value: V,
) -> Result<&'v Self, PyDowncastError<'v, 'b>> {
let value = value.into();
unsafe {
if T::is_exact_instance(value) {
Ok(Self::try_from_unchecked(value))
} else {
Err(PyDowncastError)
Err(PyDowncastError::new(value, Cow::Borrowed(T::NAME)))
}
}
}
Expand All @@ -390,23 +395,25 @@ 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<'b, V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError<'v, 'b>> {
let value = value.into();
unsafe {
if T::is_instance(value) {
Ok(Self::try_from_unchecked(value))
} else {
Err(PyDowncastError)
Err(PyDowncastError::new(value, Cow::Borrowed(T::NAME)))
}
}
}
fn try_from_exact<V: Into<&'v PyAny>>(value: V) -> Result<&'v Self, PyDowncastError> {
fn try_from_exact<'b, V: Into<&'v PyAny>>(
value: V,
) -> Result<&'v Self, PyDowncastError<'v, 'b>> {
let value = value.into();
unsafe {
if T::is_exact_instance(value) {
Ok(Self::try_from_unchecked(value))
} else {
Err(PyDowncastError)
Err(PyDowncastError::new(value, Cow::Borrowed(T::NAME)))
}
}
}
Expand Down
34 changes: 28 additions & 6 deletions src/err.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -56,7 +57,21 @@ 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, 'b> {
from: &'a PyAny,
to: Cow<'b, str>,
}

impl<'a, 'b> PyDowncastError<'a, 'b> {
pub fn new(from: &'a PyAny, to: Cow<'b, str>) -> Self {
PyDowncastError { from, to }
}

pub fn set_to(&mut self, to: Cow<'b, str>) {
self.to = to;
}
}

/// Helper conversion trait that allows to use custom arguments for exception constructor.
pub trait PyErrArguments {
Expand Down Expand Up @@ -460,15 +475,22 @@ 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, 'b> std::convert::From<PyDowncastError<'a, 'b>> for PyErr {
fn from(err: PyDowncastError) -> PyErr {
exceptions::PyTypeError::py_err(err.to_string())
}
}

impl<'p> std::fmt::Debug for PyDowncastError {
impl<'a, 'b> std::error::Error for PyDowncastError<'a, 'b> {}

impl<'a, 'b> std::fmt::Display for PyDowncastError<'a, 'b> {
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_err(|_| std::fmt::Error)?,
self.to
)
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/python.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<'b, T>(self, obj: PyObject) -> Result<&'p T, PyDowncastError<'p, 'b>>
where
T: PyTryFrom<'p>,
{
Expand Down
35 changes: 13 additions & 22 deletions src/types/iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
//
// based on Daniel Grunwald's https://github.com/dgrunwald/rust-cpython

use crate::{ffi, AsPyPointer, PyAny, PyDowncastError, PyErr, PyNativeType, PyResult, Python};
use crate::{ffi, AsPyPointer, PyAny, PyErr, PyNativeType, PyResult, Python};

/// A Python iterator object.
///
Expand All @@ -29,30 +29,21 @@ pub struct PyIterator<'p>(&'p PyAny);

impl<'p> PyIterator<'p> {
/// Constructs a `PyIterator` from a Python iterator object.
pub fn from_object<T>(py: Python<'p>, obj: &T) -> Result<PyIterator<'p>, PyDowncastError>
pub fn from_object<T>(py: Python<'p>, obj: &T) -> PyResult<PyIterator<'p>>
where
T: AsPyPointer,
{
unsafe {
let ptr = ffi::PyObject_GetIter(obj.as_ptr());
// Returns NULL if an object cannot be iterated.
if ptr.is_null() {
PyErr::fetch(py);
return Err(PyDowncastError);
}

if ffi::PyIter_Check(ptr) != 0 {
// This looks suspicious, but is actually correct. Even though ptr is an owned
// reference, PyIterator takes ownership of the reference and decreases the count
// in its Drop implementation.
//
// Therefore we must use from_borrowed_ptr instead of from_owned_ptr so that the
// GILPool does not take ownership of the reference.
Ok(PyIterator(py.from_borrowed_ptr(ptr)))
} else {
Err(PyDowncastError)
}
}
let iter = unsafe {
// This looks suspicious, but is actually correct. Even though ptr is an owned
// reference, PyIterator takes ownership of the reference and decreases the count
// in its Drop implementation.
//
// Therefore we must use from_borrowed_ptr_or_err instead of from_owned_ptr_or_err so
// that the GILPool does not take ownership of the reference.
py.from_borrowed_ptr_or_err(ffi::PyObject_GetIter(obj.as_ptr()))?
};

Ok(PyIterator(iter))
}
}

Expand Down
11 changes: 8 additions & 3 deletions src/types/sequence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use crate::instance::PyNativeType;
use crate::types::{PyAny, PyList, PyTuple};
use crate::AsPyPointer;
use crate::{FromPyObject, PyTryFrom, ToBorrowedObject};
use std::borrow::Cow;

/// Represents a reference to a Python object supporting the sequence protocol.
#[repr(transparent)]
Expand Down Expand Up @@ -372,18 +373,22 @@ where
}

impl<'v> PyTryFrom<'v> for PySequence {
fn try_from<V: Into<&'v PyAny>>(value: V) -> Result<&'v PySequence, PyDowncastError> {
fn try_from<'b, V: Into<&'v PyAny>>(
value: V,
) -> Result<&'v PySequence, PyDowncastError<'v, 'b>> {
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, Cow::Borrowed("Sequence")))
}
}
}

fn try_from_exact<V: Into<&'v PyAny>>(value: V) -> Result<&'v PySequence, PyDowncastError> {
fn try_from_exact<'b, V: Into<&'v PyAny>>(
value: V,
) -> Result<&'v PySequence, PyDowncastError<'v, 'b>> {
<PySequence as PyTryFrom>::try_from(value)
}

Expand Down

0 comments on commit e8596dd

Please sign in to comment.