diff --git a/CHANGELOG.md b/CHANGELOG.md index 7fa99062..e8e2dd1a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Add `PyBytes::new_with` and `PyByteArray::new_with` for initialising Python-allocated bytes and bytearrays using a closure. [#1074](https://github.com/PyO3/pyo3/pull/1074) - Add `Py::as_ref` and `Py::into_ref`. [#1098](https://github.com/PyO3/pyo3/pull/1098) - Add optional implementations of `ToPyObject`, `IntoPy`, and `FromPyObject` for [hashbrown](https://crates.io/crates/hashbrown)'s `HashMap` and `HashSet` types. The `hashbrown` feature must be enabled for these implementations to be built. [#1114](https://github.com/PyO3/pyo3/pull/1114/) +- Allow other `Result` types when using `#[pyfunction]`. [#1106](https://github.com/PyO3/pyo3/issues/1106). ### 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/guide/src/conversions.md b/guide/src/conversions.md index df6d4065..41ddb645 100644 --- a/guide/src/conversions.md +++ b/guide/src/conversions.md @@ -72,7 +72,7 @@ When returning values from functions callable from Python, Python-native types ( Because these types are references, in some situations the Rust compiler may ask for lifetime annotations. If this is the case, you should use `Py`, `Py` etc. instead - which are also zero-cost. For all of these Python-native types `T`, `Py` can be created from `T` with an `.into()` conversion. -If your function is fallible, it should return `PyResult`, which will raise a `Python` exception if the `Err` variant is returned. +If your function is fallible, it should return `PyResult` or `Result` where `E` implements `From for PyErr`. This will raise a `Python` exception if the `Err` variant is returned. Finally, the following Rust types are also able to convert to Python as return values: diff --git a/src/callback.rs b/src/callback.rs index 7dbd1aff..5f83cfaa 100644 --- a/src/callback.rs +++ b/src/callback.rs @@ -2,7 +2,7 @@ //! Utilities for a Python callable object that invokes a Rust function. -use crate::err::PyResult; +use crate::err::{PyErr, PyResult}; use crate::exceptions::PyOverflowError; use crate::ffi::{self, Py_hash_t}; use crate::IntoPyPointer; @@ -37,12 +37,13 @@ pub trait IntoPyCallbackOutput { fn convert(self, py: Python) -> PyResult; } -impl IntoPyCallbackOutput for PyResult +impl IntoPyCallbackOutput for Result where T: IntoPyCallbackOutput, + E: Into, { fn convert(self, py: Python) -> PyResult { - self.and_then(|t| t.convert(py)) + self.map_err(Into::into).and_then(|t| t.convert(py)) } } diff --git a/src/class/macros.rs b/src/class/macros.rs index b72efdbd..2ad6a974 100644 --- a/src/class/macros.rs +++ b/src/class/macros.rs @@ -152,7 +152,7 @@ macro_rules! py_binary_self_func { let arg = py.from_borrowed_ptr::<$crate::PyAny>(arg); call_operator_mut!(py, slf_, $f, arg).convert(py)?; ffi::Py_INCREF(slf); - Ok(slf) + Ok::<_, $crate::err::PyErr>(slf) }) } Some(wrap::<$class>) diff --git a/src/class/number.rs b/src/class/number.rs index 149a8b98..568ed38b 100644 --- a/src/class/number.rs +++ b/src/class/number.rs @@ -4,6 +4,7 @@ //! Trait and support implementation for implementing number protocol use crate::callback::IntoPyCallbackOutput; +use crate::err::PyErr; use crate::{ffi, FromPyObject, PyClass, PyObject}; /// Number interface @@ -941,7 +942,7 @@ impl ffi::PyNumberMethods { let other = py.from_borrowed_ptr::(other); call_operator_mut!(py, slf_cell, __ipow__, other).convert(py)?; ffi::Py_INCREF(slf); - Ok(slf) + Ok::<_, PyErr>(slf) }) } self.nb_inplace_power = Some(wrap_ipow::); diff --git a/tests/test_compile_error.rs b/tests/test_compile_error.rs index 5d839fa5..142750d8 100644 --- a/tests/test_compile_error.rs +++ b/tests/test_compile_error.rs @@ -10,6 +10,7 @@ fn test_compile_errors() { t.compile_fail("tests/ui/missing_clone.rs"); t.compile_fail("tests/ui/reject_generics.rs"); t.compile_fail("tests/ui/wrong_aspyref_lifetimes.rs"); + t.compile_fail("tests/ui/invalid_result_conversion.rs"); skip_min_stable(&t); diff --git a/tests/test_various.rs b/tests/test_various.rs index a3b82a84..87270c39 100644 --- a/tests/test_various.rs +++ b/tests/test_various.rs @@ -3,6 +3,8 @@ use pyo3::types::IntoPyDict; use pyo3::types::{PyDict, PyTuple}; use pyo3::{py_run, wrap_pyfunction, PyCell}; +use std::fmt; + mod common; #[pyclass] @@ -168,3 +170,41 @@ fn test_pickle() { "# ); } + +/// Testing https://github.com/PyO3/pyo3/issues/1106. A result type that +/// implements `From for PyErr` should be automatically converted +/// when using `#[pyfunction]`. +/// +/// This only makes sure that valid `Result` types do work. For an invalid +/// enum type, see `ui/invalid_result_conversion.py`. +#[derive(Debug)] +struct MyError { + pub descr: &'static str, +} + +impl fmt::Display for MyError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "My error message: {}", self.descr) + } +} + +/// Important for the automatic conversion to `PyErr`. +impl From for PyErr { + fn from(err: MyError) -> pyo3::PyErr { + pyo3::exceptions::PyOSError::py_err(err.to_string()) + } +} + +#[pyfunction] +fn result_conversion_function() -> Result<(), MyError> { + Err(MyError { + descr: "something went wrong", + }) +} + +#[test] +fn test_result_conversion() { + let gil = Python::acquire_gil(); + let py = gil.python(); + wrap_pyfunction!(result_conversion_function)(py); +} diff --git a/tests/ui/invalid_result_conversion.rs b/tests/ui/invalid_result_conversion.rs new file mode 100644 index 00000000..3da4d217 --- /dev/null +++ b/tests/ui/invalid_result_conversion.rs @@ -0,0 +1,31 @@ +//! Testing https://github.com/PyO3/pyo3/issues/1106. A result type that +//! *doesn't* implement `From for PyErr` won't be automatically +//! converted when using `#[pyfunction]`. +use pyo3::prelude::*; +use pyo3::wrap_pyfunction; + +use std::fmt; + +/// A basic error type for the tests. It's missing `From for PyErr`, +/// though, so it shouldn't work. +#[derive(Debug)] +struct MyError { + pub descr: &'static str, +} + +impl fmt::Display for MyError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "My error message: {}", self.descr) + } +} + +#[pyfunction] +fn should_not_work() -> Result<(), MyError> { + Err(MyError { descr: "something went wrong" }) +} + +fn main() { + let gil = Python::acquire_gil(); + let py = gil.python(); + wrap_pyfunction!(should_not_work)(py); +} diff --git a/tests/ui/invalid_result_conversion.stderr b/tests/ui/invalid_result_conversion.stderr new file mode 100644 index 00000000..fc92ebf6 --- /dev/null +++ b/tests/ui/invalid_result_conversion.stderr @@ -0,0 +1,14 @@ +error[E0277]: the trait bound `std::result::Result<(), MyError>: pyo3::callback::IntoPyCallbackOutput<_>` is not satisfied + --> $DIR/invalid_result_conversion.rs:22:1 + | +22 | #[pyfunction] + | ^^^^^^^^^^^^^ the trait `pyo3::callback::IntoPyCallbackOutput<_>` is not implemented for `std::result::Result<(), MyError>` + | + ::: $WORKSPACE/src/callback.rs + | + | T: IntoPyCallbackOutput, + | ----------------------- required by this bound in `pyo3::callback::convert` + | + = help: the following implementations were found: + as pyo3::callback::IntoPyCallbackOutput> + = note: this error originates in an attribute macro (in Nightly builds, run with -Z macro-backtrace for more info)