Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add type info to conversion errors. #1050

Merged
merged 2 commits into from
Jul 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
20 changes: 10 additions & 10 deletions src/conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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))
}
}
}
Expand All @@ -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))
}
}
}
Expand Down
36 changes: 30 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,20 @@ pub struct PyErr {
pub type PyResult<T> = Result<T, PyErr>;

/// Marker type that indicates an error while downcasting
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

technically we should stop calling it a marker type now.

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 {
Expand Down Expand Up @@ -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
)
}
}

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<T>(self, obj: PyObject) -> Result<&'p T, PyDowncastError<'p>>
where
T: PyTryFrom<'p>,
{
Expand Down
6 changes: 3 additions & 3 deletions src/types/sequence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down