diff --git a/Cargo.toml b/Cargo.toml index f77de952453..a8c89c45121 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -44,7 +44,8 @@ serde = { version = "1.0", optional = true } assert_approx_eq = "1.1.0" chrono = { version = "0.4" } criterion = "0.3.5" -trybuild = "1.0.49" +# Required for "and $N others" normalization +trybuild = ">=1.0.70" rustversion = "1.0" # 1.0.0 requires Rust 1.50 proptest = { version = "0.10.1", default-features = false, features = ["std"] } diff --git a/newsfragments/2664.changed.md b/newsfragments/2664.changed.md new file mode 100644 index 00000000000..7dd461d7fb5 --- /dev/null +++ b/newsfragments/2664.changed.md @@ -0,0 +1 @@ +PyO3's macros now emit a much nicer error message if function return values don't implement the required trait(s). \ No newline at end of file diff --git a/pyo3-macros-backend/src/method.rs b/pyo3-macros-backend/src/method.rs index dcb0d91b05c..c120b2026f9 100644 --- a/pyo3-macros-backend/src/method.rs +++ b/pyo3-macros-backend/src/method.rs @@ -468,17 +468,12 @@ impl<'a> FnSpec<'a> { quote!(#func_name) }; - // The method call is necessary to generate a decent error message. let rust_call = |args: Vec| { quote! { let mut ret = #rust_name(#self_arg #(#args),*); - - if false { - use _pyo3::impl_::ghost::IntoPyResult; - ret.assert_into_py_result(); - } - - _pyo3::callback::convert(#py, ret) + let owned = _pyo3::impl_::pymethods::OkWrap::wrap(ret, #py); + owned.map(|obj| _pyo3::conversion::IntoPyPointer::into_ptr(obj)) + .map_err(::core::convert::Into::into) } }; diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs index 3bf3e6f4471..ba9b66c2b1b 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -389,11 +389,8 @@ fn impl_py_class_attribute(cls: &syn::Type, spec: &FnSpec<'_>) -> syn::Result) -> _pyo3::PyResult<_pyo3::PyObject> { #deprecations let mut ret = #fncall; - if false { - use _pyo3::impl_::ghost::IntoPyResult; - ret.assert_into_py_result(); - } - _pyo3::callback::convert(py, ret) + let owned = _pyo3::impl_::pymethods::OkWrap::wrap(ret, py); + owned.map_err(::core::convert::Into::into) } }; diff --git a/src/impl_.rs b/src/impl_.rs index 08ef053b072..6b4db2b8634 100644 --- a/src/impl_.rs +++ b/src/impl_.rs @@ -10,7 +10,6 @@ pub mod deprecations; pub mod extract_argument; pub mod freelist; pub mod frompyobject; -pub mod ghost; pub(crate) mod not_send; pub mod panic; pub mod pycell; diff --git a/src/impl_/ghost.rs b/src/impl_/ghost.rs deleted file mode 100644 index 667607dd001..00000000000 --- a/src/impl_/ghost.rs +++ /dev/null @@ -1,18 +0,0 @@ -/// If it does nothing, was it ever really there? 👻 -/// -/// This is code that is just type checked to e.g. create better compile errors, -/// but that never affects anything at runtime, -use crate::{IntoPy, PyErr, PyObject}; - -pub trait IntoPyResult { - fn assert_into_py_result(&mut self) {} -} - -impl IntoPyResult for T where T: IntoPy {} - -impl IntoPyResult for Result -where - T: IntoPy, - E: Into, -{ -} diff --git a/src/impl_/pymethods.rs b/src/impl_/pymethods.rs index 6234d297000..50634323b11 100644 --- a/src/impl_/pymethods.rs +++ b/src/impl_/pymethods.rs @@ -1,5 +1,5 @@ use crate::internal_tricks::{extract_cstr_or_leak_cstring, NulByteInString}; -use crate::{ffi, PyAny, PyObject, PyResult, PyTraverseError, Python}; +use crate::{ffi, IntoPy, Py, PyAny, PyErr, PyObject, PyResult, PyTraverseError, Python}; use std::ffi::CStr; use std::fmt; use std::os::raw::c_int; @@ -262,3 +262,29 @@ pub fn unwrap_traverse_result(result: Result<(), PyTraverseError>) -> c_int { Err(PyTraverseError(value)) => value, } } + +// The macros need to Ok-wrap the output of user defined functions; i.e. if they're not a result, make them into one. +pub trait OkWrap { + type Error; + fn wrap(self, py: Python<'_>) -> Result, Self::Error>; +} + +impl OkWrap for T +where + T: IntoPy, +{ + type Error = PyErr; + fn wrap(self, py: Python<'_>) -> PyResult> { + Ok(self.into_py(py)) + } +} + +impl OkWrap for Result +where + T: IntoPy, +{ + type Error = E; + fn wrap(self, py: Python<'_>) -> Result, Self::Error> { + self.map(|o| o.into_py(py)) + } +} diff --git a/src/pyclass.rs b/src/pyclass.rs index ffec5c9a350..d82cdd7348e 100644 --- a/src/pyclass.rs +++ b/src/pyclass.rs @@ -341,6 +341,10 @@ impl PyTypeBuilder { module_name: Option<&'static str>, basicsize: usize, ) -> PyResult<*mut ffi::PyTypeObject> { + // `c_ulong` and `c_uint` have the same size + // on some platforms (like windows) + #![allow(clippy::useless_conversion)] + self.finalize_methods_and_properties(); if !self.has_new { @@ -376,9 +380,7 @@ impl PyTypeBuilder { name: py_class_qualified_name(module_name, name)?, basicsize: basicsize as c_int, itemsize: 0, - // `c_ulong` and `c_uint` have the same size - // on some platforms (like windows) - #[allow(clippy::useless_conversion)] + flags: (ffi::Py_TPFLAGS_DEFAULT | self.class_flags) .try_into() .unwrap(), diff --git a/tests/ui/invalid_result_conversion.stderr b/tests/ui/invalid_result_conversion.stderr index a92e8ccc91a..30dce88c106 100644 --- a/tests/ui/invalid_result_conversion.stderr +++ b/tests/ui/invalid_result_conversion.stderr @@ -1,30 +1,18 @@ -error[E0599]: no method named `assert_into_py_result` found for enum `Result` in the current scope +error[E0277]: the trait bound `PyErr: From` is not satisfied --> tests/ui/invalid_result_conversion.rs:21:1 | 21 | #[pyfunction] - | ^^^^^^^^^^^^^ method not found in `Result<(), MyError>` + | ^^^^^^^^^^^^^ the trait `From` is not implemented for `PyErr` | -note: the method `assert_into_py_result` exists on the type `()` - --> src/impl_/ghost.rs - | - | fn assert_into_py_result(&mut self) {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = help: the following other types implement trait `From`: + > + > + > + > + > + > + > + > + and $N others + = note: required because of the requirements on the impl of `Into` for `MyError` = note: this error originates in the attribute macro `pyfunction` (in Nightly builds, run with -Z macro-backtrace for more info) -help: use the `?` operator to extract the `()` value, propagating a `Result::Err` value to the caller - | -21 | #[pyfunction]? - | + - -error[E0277]: the trait bound `Result<(), MyError>: IntoPyCallbackOutput<_>` is not satisfied - --> tests/ui/invalid_result_conversion.rs:21:1 - | -21 | #[pyfunction] - | ^^^^^^^^^^^^^ the trait `IntoPyCallbackOutput<_>` is not implemented for `Result<(), MyError>` - | - = help: the trait `IntoPyCallbackOutput` is implemented for `Result` -note: required by a bound in `pyo3::callback::convert` - --> src/callback.rs - | - | T: IntoPyCallbackOutput, - | ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `pyo3::callback::convert` - = note: this error originates in the attribute macro `pyfunction` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/tests/ui/missing_intopy.stderr b/tests/ui/missing_intopy.stderr index f9a90788117..4f2fe3db8b0 100644 --- a/tests/ui/missing_intopy.stderr +++ b/tests/ui/missing_intopy.stderr @@ -1,45 +1,18 @@ -error[E0599]: the method `assert_into_py_result` exists for struct `Blah`, but its trait bounds were not satisfied - --> tests/ui/missing_intopy.rs:3:1 - | -1 | struct Blah; - | ----------- - | | - | method `assert_into_py_result` not found for this struct - | doesn't satisfy `Blah: IntoPy>` - | doesn't satisfy `Blah: IntoPyResult` -2 | -3 | #[pyo3::pyfunction] - | ^^^^^^^^^^^^^^^^^^^ method cannot be called on `Blah` due to unsatisfied trait bounds - | - = note: the following trait bounds were not satisfied: - `Blah: IntoPy>` - which is required by `Blah: IntoPyResult` -note: the following trait must be implemented - --> src/conversion.rs - | - | pub trait IntoPy: Sized { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ - = note: this error originates in the attribute macro `pyo3::pyfunction` (in Nightly builds, run with -Z macro-backtrace for more info) - -error[E0277]: the trait bound `Blah: IntoPyCallbackOutput<_>` is not satisfied - --> tests/ui/missing_intopy.rs:3:1 - | -3 | #[pyo3::pyfunction] - | ^^^^^^^^^^^^^^^^^^^ the trait `IntoPyCallbackOutput<_>` is not implemented for `Blah` - | - = help: the following other types implement trait `IntoPyCallbackOutput`: - <() as IntoPyCallbackOutput<()>> - <() as IntoPyCallbackOutput> - <*mut PyObject as IntoPyCallbackOutput<*mut PyObject>> - > - , Py> as IntoPyCallbackOutput<*mut PyObject>> - as IntoPyCallbackOutput, Py>>> - , Py> as IntoPyCallbackOutput<*mut PyObject>> - as IntoPyCallbackOutput, Py>>> - and 7 others -note: required by a bound in `pyo3::callback::convert` - --> src/callback.rs - | - | T: IntoPyCallbackOutput, - | ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `pyo3::callback::convert` - = note: this error originates in the attribute macro `pyo3::pyfunction` (in Nightly builds, run with -Z macro-backtrace for more info) +error[E0277]: the trait bound `Blah: IntoPy>` is not satisfied + --> tests/ui/missing_intopy.rs:3:1 + | +3 | #[pyo3::pyfunction] + | ^^^^^^^^^^^^^^^^^^^ the trait `IntoPy>` is not implemented for `Blah` + | + = help: the following other types implement trait `IntoPy`: + <&'a OsString as IntoPy>> + <&'a Path as IntoPy>> + <&'a PathBuf as IntoPy>> + <&'a PyErr as IntoPy>> + <&'a String as IntoPy>> + <&'a [u8] as IntoPy>> + <&'a str as IntoPy>> + <&'a str as IntoPy>> + and $N others + = note: required because of the requirements on the impl of `OkWrap` for `Blah` + = note: this error originates in the attribute macro `pyo3::pyfunction` (in Nightly builds, run with -Z macro-backtrace for more info)