From bcb1762b0f8ce46299fce9c76519316213ea511c Mon Sep 17 00:00:00 2001 From: mejrs Date: Sun, 1 May 2022 11:22:51 +0200 Subject: [PATCH 1/8] Implement IntoPy for arrays of IntoPy --- src/conversions/array.rs | 89 +++++++++++++++++++++++++++++++++++++--- src/types/list.rs | 5 ++- 2 files changed, 86 insertions(+), 8 deletions(-) diff --git a/src/conversions/array.rs b/src/conversions/array.rs index 614b56c46a3..cf385f365d7 100644 --- a/src/conversions/array.rs +++ b/src/conversions/array.rs @@ -3,14 +3,39 @@ use crate::{exceptions, PyErr}; #[cfg(min_const_generics)] mod min_const_generics { use super::invalid_sequence_length; - use crate::{FromPyObject, IntoPy, PyAny, PyObject, PyResult, PyTryFrom, Python, ToPyObject}; + use crate::conversion::IntoPyPointer; + use crate::{ + ffi, FromPyObject, IntoPy, Py, PyAny, PyObject, PyResult, PyTryFrom, Python, ToPyObject, + }; impl IntoPy for [T; N] where - T: ToPyObject, + T: IntoPy, { fn into_py(self, py: Python<'_>) -> PyObject { - self.as_ref().to_object(py) + unsafe { + #[allow(deprecated)] // we're not on edition 2021 yet + let elements = std::array::IntoIter::new(self); + let len = N as ffi::Py_ssize_t; + + let ptr = ffi::PyList_New(len); + + // We create the `Py` pointer here for two reasons: + // - panics if the ptr is null + // - its Drop cleans up the list if user code panics. + let list: Py = Py::from_owned_ptr(py, ptr); + + for (i, obj) in (0..len).zip(elements) { + let obj = obj.into_py(py).into_ptr(); + + #[cfg(not(Py_LIMITED_API))] + ffi::PyList_SET_ITEM(ptr, i, obj); + #[cfg(Py_LIMITED_API)] + ffi::PyList_SetItem(ptr, i, obj); + } + + list + } } } @@ -149,17 +174,69 @@ mod min_const_generics { #[cfg(not(min_const_generics))] mod array_impls { use super::invalid_sequence_length; - use crate::{FromPyObject, IntoPy, PyAny, PyObject, PyResult, PyTryFrom, Python, ToPyObject}; + use crate::conversion::IntoPyPointer; + use crate::{ + ffi, FromPyObject, IntoPy, Py, PyAny, PyObject, PyResult, PyTryFrom, Python, ToPyObject, + }; + use std::mem::{transmute_copy, ManuallyDrop}; macro_rules! array_impls { ($($N:expr),+) => { $( impl IntoPy for [T; $N] where - T: ToPyObject + T: IntoPy { fn into_py(self, py: Python<'_>) -> PyObject { - self.as_ref().to_object(py) + + struct ArrayGuard { + elements: [ManuallyDrop; $N], + start: usize, + } + + impl Drop for ArrayGuard { + fn drop(&mut self) { + unsafe { + let needs_drop = self.elements.get_mut(self.start..).unwrap(); + for item in needs_drop{ + ManuallyDrop::drop(item); + } + } + } + } + + unsafe { + let ptr = ffi::PyList_New($N as ffi::Py_ssize_t); + + // We create the `Py` pointer here for two reasons: + // - panics if the ptr is null + // - its Drop cleans up the list if user code panics. + let list: Py = Py::from_owned_ptr(py, ptr); + + let slf = ManuallyDrop::new(self); + + let mut guard = ArrayGuard{ + // the transmute size check is _very_ dumb around generics + elements: transmute_copy(&slf), + start: 0 + }; + + for i in 0..$N { + let obj: T = ManuallyDrop::take(&mut guard.elements[i]); + guard.start += 1; + + let obj = obj.into_py(py).into_ptr(); + + #[cfg(not(Py_LIMITED_API))] + ffi::PyList_SET_ITEM(ptr, i as ffi::Py_ssize_t, obj); + #[cfg(Py_LIMITED_API)] + ffi::PyList_SetItem(ptr, i as ffi::Py_ssize_t, obj); + } + + std::mem::forget(guard); + + list + } } } diff --git a/src/types/list.rs b/src/types/list.rs index ab69859e9fd..f8da1119f8b 100644 --- a/src/types/list.rs +++ b/src/types/list.rs @@ -34,8 +34,9 @@ fn new_from_iter( let ptr = ffi::PyList_New(len); - // - Panics if the ptr is null - // - Cleans up the list if `convert` or the asserts panic + // We create the `Py` pointer here for two reasons: + // - panics if the ptr is null + // - its Drop cleans up the list if user code or the asserts panic. let list: Py = Py::from_owned_ptr(py, ptr); let mut counter: Py_ssize_t = 0; From 139cae166aa09cfba1d510accde202ae6f0acd81 Mon Sep 17 00:00:00 2001 From: mejrs Date: Sun, 1 May 2022 11:23:09 +0200 Subject: [PATCH 2/8] Improve `IntoPyCallbackOutput` compile error --- pyo3-macros-backend/src/method.rs | 12 +++++++++-- pyo3-macros-backend/src/pymethod.rs | 19 ++++++++++++++++- src/callback.rs | 24 +++++++++++++++++++++ tests/ui/invalid_result_conversion.stderr | 26 +++++++++++------------ 4 files changed, 64 insertions(+), 17 deletions(-) diff --git a/pyo3-macros-backend/src/method.rs b/pyo3-macros-backend/src/method.rs index ddf2244f199..d0209c67f5e 100644 --- a/pyo3-macros-backend/src/method.rs +++ b/pyo3-macros-backend/src/method.rs @@ -478,8 +478,16 @@ impl<'a> FnSpec<'a> { } else { quote!(#func_name) }; - let rust_call = - quote! { _pyo3::callback::convert(#py, #rust_name(#self_arg #(#arg_names),*)) }; + + // The method call is necessary to generate a decent error message. + let rust_call = quote! { + let ret = #rust_name(#self_arg #(#arg_names),*); + + use _pyo3::callback::IntoResult; + let ret: _pyo3::PyResult<_> = ret.into_result(); + + _pyo3::callback::convert(#py, ret) + }; let krate = &self.krate; Ok(match self.convention { CallingConvention::Noargs => { diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs index 9ed9b4300b8..9d5a1bbf20f 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -495,6 +495,22 @@ pub fn impl_py_getter_def(cls: &syn::Type, property_type: PropertyType<'_>) -> R self_type.receiver(cls, ExtractErrorMode::Raise) } }; + + let conversion = match property_type { + PropertyType::Descriptor { .. } => { + quote! { + let item: _pyo3::Py<_pyo3::PyAny> = _pyo3::IntoPy::into_py(item, _py); + ::std::result::Result::Ok(_pyo3::conversion::IntoPyPointer::into_ptr(item)) + } + } + // Forward to `IntoPyCallbackOutput`, to handle `#[getter]`s returning results. + PropertyType::Function { .. } => { + quote! { + _pyo3::callback::convert(_py, item) + } + } + }; + Ok(quote! { _pyo3::class::PyMethodDefType::Getter({ #deprecations @@ -509,7 +525,8 @@ pub fn impl_py_getter_def(cls: &syn::Type, property_type: PropertyType<'_>) -> R let _py = gil.python(); _pyo3::callback::panic_result_into_callback_output(_py, ::std::panic::catch_unwind(move || -> _pyo3::PyResult<_> { #slf - _pyo3::callback::convert(_py, #getter_impl) + let item = #getter_impl; + #conversion })) } __wrap diff --git a/src/callback.rs b/src/callback.rs index 3794b9e5906..81fb4d1d5b1 100644 --- a/src/callback.rs +++ b/src/callback.rs @@ -35,6 +35,30 @@ impl PyCallbackOutput for () { const ERR_VALUE: Self = (); } +#[doc(hidden)] +pub trait IntoResult { + fn into_result(self) -> R; +} + +impl IntoResult> for T +where + T: IntoPy, +{ + fn into_result(self) -> PyResult { + Ok(self) + } +} + +impl IntoResult> for Result +where + T: IntoPy, + E: Into, +{ + fn into_result(self) -> PyResult { + self.map_err(Into::into) + } +} + /// Convert the result of callback function into the appropriate return value. pub trait IntoPyCallbackOutput { fn convert(self, py: Python<'_>) -> PyResult; diff --git a/tests/ui/invalid_result_conversion.stderr b/tests/ui/invalid_result_conversion.stderr index a606f605fcf..2e3e2be3c74 100644 --- a/tests/ui/invalid_result_conversion.stderr +++ b/tests/ui/invalid_result_conversion.stderr @@ -1,14 +1,12 @@ -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 following implementations were found: - as IntoPyCallbackOutput> -note: required by a bound in `pyo3::callback::convert` - --> src/callback.rs:182:8 - | -182 | 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) +error[E0599]: the method `into_result` exists for enum `Result<(), MyError>`, but its trait bounds were not satisfied + --> tests/ui/invalid_result_conversion.rs:21:1 + | +21 | #[pyfunction] + | ^^^^^^^^^^^^^ method cannot be called on `Result<(), MyError>` due to unsatisfied trait bounds + | + = note: the following trait bounds were not satisfied: + `&Result<(), MyError>: IntoPy>` + which is required by `&Result<(), MyError>: IntoResult, PyErr>>` + `&mut Result<(), MyError>: IntoPy>` + which is required by `&mut Result<(), MyError>: IntoResult, PyErr>>` + = note: this error originates in the attribute macro `pyfunction` (in Nightly builds, run with -Z macro-backtrace for more info) From 924cb28fbdb79729090bd9f3f8f47a425fc3f0ce Mon Sep 17 00:00:00 2001 From: mejrs Date: Sun, 1 May 2022 16:39:45 +0200 Subject: [PATCH 3/8] Add tests --- src/conversions/array.rs | 30 ++++++++++++++++++++++++++++-- tests/test_compile_error.rs | 1 + tests/ui/missing_intopy.rs | 8 ++++++++ tests/ui/missing_intopy.stderr | 29 +++++++++++++++++++++++++++++ 4 files changed, 66 insertions(+), 2 deletions(-) create mode 100644 tests/ui/missing_intopy.rs create mode 100644 tests/ui/missing_intopy.stderr diff --git a/src/conversions/array.rs b/src/conversions/array.rs index cf385f365d7..c6e9e70c15c 100644 --- a/src/conversions/array.rs +++ b/src/conversions/array.rs @@ -295,7 +295,7 @@ fn invalid_sequence_length(expected: usize, actual: usize) -> PyErr { #[cfg(test)] mod tests { - use crate::{types::PyList, PyResult, Python}; + use crate::{types::PyList, IntoPy, PyResult, Python, ToPyObject}; #[test] fn test_extract_small_bytearray_to_array() { @@ -310,7 +310,6 @@ mod tests { } #[test] fn test_topyobject_array_conversion() { - use crate::ToPyObject; Python::with_gil(|py| { let array: [f32; 4] = [0.0, -16.0, 16.0, 42.0]; let pyobject = array.to_object(py); @@ -335,4 +334,31 @@ mod tests { ); }) } + + #[test] + fn test_intopy_array_conversion() { + Python::with_gil(|py| { + let array: [f32; 4] = [0.0, -16.0, 16.0, 42.0]; + let pyobject = array.into_py(py); + let pylist: &PyList = pyobject.extract(py).unwrap(); + assert_eq!(pylist[0].extract::().unwrap(), 0.0); + assert_eq!(pylist[1].extract::().unwrap(), -16.0); + assert_eq!(pylist[2].extract::().unwrap(), 16.0); + assert_eq!(pylist[3].extract::().unwrap(), 42.0); + }); + } + + #[cfg(feature = "macros")] + #[test] + fn test_pyclass_intopy_array_conversion() { + #[crate::pyclass(crate = "crate")] + struct Foo; + + Python::with_gil(|py| { + let array: [Foo; 8] = [Foo, Foo, Foo, Foo, Foo, Foo, Foo, Foo]; + let pyobject = array.into_py(py); + let list: &PyList = pyobject.cast_as(py).unwrap(); + let _cell: &crate::PyCell = list.get_item(4).unwrap().extract().unwrap(); + }); + } } diff --git a/tests/test_compile_error.rs b/tests/test_compile_error.rs index 713c50925c8..989928c2775 100644 --- a/tests/test_compile_error.rs +++ b/tests/test_compile_error.rs @@ -96,6 +96,7 @@ fn _test_compile_errors() { fn tests_rust_1_60(t: &trybuild::TestCases) { t.compile_fail("tests/ui/invalid_immutable_pyclass_borrow.rs"); t.compile_fail("tests/ui/invalid_pymethod_receiver.rs"); + t.compile_fail("tests/ui/missing_intopy.rs"); } #[rustversion::before(1.60)] diff --git a/tests/ui/missing_intopy.rs b/tests/ui/missing_intopy.rs new file mode 100644 index 00000000000..7a465b14221 --- /dev/null +++ b/tests/ui/missing_intopy.rs @@ -0,0 +1,8 @@ +struct Blah; + +#[pyo3::pyfunction] +fn blah() -> Blah{ + Blah +} + +fn main(){} \ No newline at end of file diff --git a/tests/ui/missing_intopy.stderr b/tests/ui/missing_intopy.stderr new file mode 100644 index 00000000000..b4cf47e1b29 --- /dev/null +++ b/tests/ui/missing_intopy.stderr @@ -0,0 +1,29 @@ +error[E0599]: the method `into_result` exists for struct `Blah`, but its trait bounds were not satisfied + --> tests/ui/missing_intopy.rs:3:1 + | +1 | struct Blah; + | ------------ + | | + | method `into_result` not found for this + | doesn't satisfy `Blah: IntoPy>` + | doesn't satisfy `Blah: IntoResult>` +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: IntoResult>` + `&Blah: IntoPy>` + which is required by `&Blah: IntoResult>` + `&mut Blah: IntoPy>` + which is required by `&mut Blah: IntoResult>` +note: the following trait must be implemented + --> src/conversion.rs + | + | / pub trait IntoPy: Sized { + | | /// Performs the conversion. + | | fn into_py(self, py: Python<'_>) -> T; + | | } + | |_^ + = note: this error originates in the attribute macro `pyo3::pyfunction` (in Nightly builds, run with -Z macro-backtrace for more info) From f348c179400561051cc4bdbe6f61cd6d0f343004 Mon Sep 17 00:00:00 2001 From: mejrs Date: Sun, 1 May 2022 21:53:43 +0200 Subject: [PATCH 4/8] Rename helper trait --- pyo3-macros-backend/src/method.rs | 4 ++-- src/callback.rs | 13 +++++++------ tests/ui/invalid_result_conversion.stderr | 6 +++--- tests/ui/missing_intopy.stderr | 12 ++++++------ 4 files changed, 18 insertions(+), 17 deletions(-) diff --git a/pyo3-macros-backend/src/method.rs b/pyo3-macros-backend/src/method.rs index d0209c67f5e..62e5079eba4 100644 --- a/pyo3-macros-backend/src/method.rs +++ b/pyo3-macros-backend/src/method.rs @@ -483,8 +483,8 @@ impl<'a> FnSpec<'a> { let rust_call = quote! { let ret = #rust_name(#self_arg #(#arg_names),*); - use _pyo3::callback::IntoResult; - let ret: _pyo3::PyResult<_> = ret.into_result(); + use _pyo3::callback::IntoPyResult; + let ret: _pyo3::PyResult<_> = ret.into_py_result(); _pyo3::callback::convert(#py, ret) }; diff --git a/src/callback.rs b/src/callback.rs index 8d3614cf9d3..9f912a84c67 100644 --- a/src/callback.rs +++ b/src/callback.rs @@ -35,26 +35,27 @@ impl PyCallbackOutput for () { const ERR_VALUE: Self = (); } +/// A helper trait to result-wrap the return values of functions and methods. #[doc(hidden)] -pub trait IntoResult { - fn into_result(self) -> R; +pub trait IntoPyResult { + fn into_py_result(self) -> R; } -impl IntoResult> for T +impl IntoPyResult> for T where T: IntoPy, { - fn into_result(self) -> PyResult { + fn into_py_result(self) -> PyResult { Ok(self) } } -impl IntoResult> for Result +impl IntoPyResult> for Result where T: IntoPy, E: Into, { - fn into_result(self) -> PyResult { + fn into_py_result(self) -> PyResult { self.map_err(Into::into) } } diff --git a/tests/ui/invalid_result_conversion.stderr b/tests/ui/invalid_result_conversion.stderr index 2e3e2be3c74..88bff60ef45 100644 --- a/tests/ui/invalid_result_conversion.stderr +++ b/tests/ui/invalid_result_conversion.stderr @@ -1,4 +1,4 @@ -error[E0599]: the method `into_result` exists for enum `Result<(), MyError>`, but its trait bounds were not satisfied +error[E0599]: the method `into_py_result` exists for enum `Result<(), MyError>`, but its trait bounds were not satisfied --> tests/ui/invalid_result_conversion.rs:21:1 | 21 | #[pyfunction] @@ -6,7 +6,7 @@ error[E0599]: the method `into_result` exists for enum `Result<(), MyError>`, bu | = note: the following trait bounds were not satisfied: `&Result<(), MyError>: IntoPy>` - which is required by `&Result<(), MyError>: IntoResult, PyErr>>` + which is required by `&Result<(), MyError>: IntoPyResult, PyErr>>` `&mut Result<(), MyError>: IntoPy>` - which is required by `&mut Result<(), MyError>: IntoResult, PyErr>>` + which is required by `&mut Result<(), MyError>: IntoPyResult, PyErr>>` = 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 b4cf47e1b29..69da4c3a8e6 100644 --- a/tests/ui/missing_intopy.stderr +++ b/tests/ui/missing_intopy.stderr @@ -1,23 +1,23 @@ -error[E0599]: the method `into_result` exists for struct `Blah`, but its trait bounds were not satisfied +error[E0599]: the method `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 `into_result` not found for this + | method `into_py_result` not found for this | doesn't satisfy `Blah: IntoPy>` - | doesn't satisfy `Blah: IntoResult>` + | 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: IntoResult>` + which is required by `Blah: IntoPyResult>` `&Blah: IntoPy>` - which is required by `&Blah: IntoResult>` + which is required by `&Blah: IntoPyResult>` `&mut Blah: IntoPy>` - which is required by `&mut Blah: IntoResult>` + which is required by `&mut Blah: IntoPyResult>` note: the following trait must be implemented --> src/conversion.rs | From acae5558e01024e4f6267b40d319d0f9714edab9 Mon Sep 17 00:00:00 2001 From: mejrs Date: Sun, 1 May 2022 22:41:12 +0200 Subject: [PATCH 5/8] Add to changelog and migration --- CHANGELOG.md | 2 ++ guide/src/migration.md | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e368a1f6f7..50de5db8c0c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,12 +11,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Implement `ToPyObject` for `[T; N]`. [#2313](https://github.com/PyO3/pyo3/pull/2313) +- Added the internal `IntoPyResult` trait to give better error messages when function return types do not implement `IntoPy`. [#2326](https://github.com/PyO3/pyo3/pull/2326) ### Changed - Move `PyTypeObject::type_object` method to `PyTypeInfo` trait, and deprecate `PyTypeObject` trait. [#2287](https://github.com/PyO3/pyo3/pull/2287) - The deprecated `pyproto` feature is now disabled by default. [#2322](https://github.com/PyO3/pyo3/pull/2322) - Deprecate `ToBorrowedObject` trait (it is only used as a wrapper for `ToPyObject`). [#2333](https://github.com/PyO3/pyo3/pull/2333) +- `impl IntoPy for [T; N]` now requires `T: IntoPy` rather than `T: ToPyObject`. [#2326](https://github.com/PyO3/pyo3/pull/2326) ## [0.16.4] - 2022-04-14 diff --git a/guide/src/migration.md b/guide/src/migration.md index 3aa50f7f218..66172fd1640 100644 --- a/guide/src/migration.md +++ b/guide/src/migration.md @@ -40,6 +40,10 @@ fn get_type_object(py: Python<'_>) -> &PyType { # Python::with_gil(|py| { get_type_object::(py); }); ``` +### `impl IntoPy for [T; N]` now requires `T: IntoPy` rather than `T: ToPyObject` + +If this leads to errors, simply implement `IntoPy`. Because pyclasses already implement `IntoPy`, you probably don't need to worry about this. + ## from 0.15.* to 0.16 ### Drop support for older technologies From 63f67f1dff37ba4b458a7dc85a63498093b34841 Mon Sep 17 00:00:00 2001 From: mejrs Date: Fri, 6 May 2022 23:33:42 +0200 Subject: [PATCH 6/8] A different approach --- pyo3-macros-backend/src/method.rs | 9 +++++--- src/callback.rs | 25 ----------------------- src/impl_.rs | 1 + src/impl_/ghost.rs | 19 +++++++++++++++++ tests/ui/invalid_result_conversion.stderr | 24 +++++++++++++++------- tests/ui/missing_intopy.stderr | 21 +++++++++++++------ 6 files changed, 58 insertions(+), 41 deletions(-) create mode 100644 src/impl_/ghost.rs diff --git a/pyo3-macros-backend/src/method.rs b/pyo3-macros-backend/src/method.rs index 62e5079eba4..02bb5d3254b 100644 --- a/pyo3-macros-backend/src/method.rs +++ b/pyo3-macros-backend/src/method.rs @@ -481,13 +481,16 @@ impl<'a> FnSpec<'a> { // The method call is necessary to generate a decent error message. let rust_call = quote! { - let ret = #rust_name(#self_arg #(#arg_names),*); + let mut ret = #rust_name(#self_arg #(#arg_names),*); - use _pyo3::callback::IntoPyResult; - let ret: _pyo3::PyResult<_> = ret.into_py_result(); + if false { + use _pyo3::impl_::ghost::IntoPyResult; + ret.into_py_result(); + } _pyo3::callback::convert(#py, ret) }; + let krate = &self.krate; Ok(match self.convention { CallingConvention::Noargs => { diff --git a/src/callback.rs b/src/callback.rs index 9f912a84c67..7b487c48424 100644 --- a/src/callback.rs +++ b/src/callback.rs @@ -35,31 +35,6 @@ impl PyCallbackOutput for () { const ERR_VALUE: Self = (); } -/// A helper trait to result-wrap the return values of functions and methods. -#[doc(hidden)] -pub trait IntoPyResult { - fn into_py_result(self) -> R; -} - -impl IntoPyResult> for T -where - T: IntoPy, -{ - fn into_py_result(self) -> PyResult { - Ok(self) - } -} - -impl IntoPyResult> for Result -where - T: IntoPy, - E: Into, -{ - fn into_py_result(self) -> PyResult { - self.map_err(Into::into) - } -} - /// Convert the result of callback function into the appropriate return value. pub trait IntoPyCallbackOutput { fn convert(self, py: Python<'_>) -> PyResult; diff --git a/src/impl_.rs b/src/impl_.rs index 5f27a1fc20f..64ca136062c 100644 --- a/src/impl_.rs +++ b/src/impl_.rs @@ -9,6 +9,7 @@ pub mod extract_argument; pub mod freelist; #[doc(hidden)] pub mod frompyobject; +pub mod ghost; pub(crate) mod not_send; #[doc(hidden)] pub mod pyclass; diff --git a/src/impl_/ghost.rs b/src/impl_/ghost.rs new file mode 100644 index 00000000000..68fd3408112 --- /dev/null +++ b/src/impl_/ghost.rs @@ -0,0 +1,19 @@ +/// 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}; + +#[allow(clippy::wrong_self_convention)] +pub trait IntoPyResult { + fn into_py_result(&mut self) {} +} + +impl IntoPyResult for T where T: IntoPy {} + +impl IntoPyResult for Result +where + T: IntoPy, + E: Into, +{ +} diff --git a/tests/ui/invalid_result_conversion.stderr b/tests/ui/invalid_result_conversion.stderr index 88bff60ef45..decaa8c72ae 100644 --- a/tests/ui/invalid_result_conversion.stderr +++ b/tests/ui/invalid_result_conversion.stderr @@ -1,12 +1,22 @@ -error[E0599]: the method `into_py_result` exists for enum `Result<(), MyError>`, but its trait bounds were not satisfied +error[E0599]: no method named `into_py_result` found for enum `Result` in the current scope --> tests/ui/invalid_result_conversion.rs:21:1 | 21 | #[pyfunction] - | ^^^^^^^^^^^^^ method cannot be called on `Result<(), MyError>` due to unsatisfied trait bounds + | ^^^^^^^^^^^^^ method not found in `Result<(), MyError>` | - = note: the following trait bounds were not satisfied: - `&Result<(), MyError>: IntoPy>` - which is required by `&Result<(), MyError>: IntoPyResult, PyErr>>` - `&mut Result<(), MyError>: IntoPy>` - which is required by `&mut Result<(), MyError>: IntoPyResult, PyErr>>` = note: this error originates in the attribute macro `pyfunction` (in Nightly builds, run with -Z macro-backtrace for more info) + +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 following implementations were found: + as IntoPyCallbackOutput> +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 69da4c3a8e6..f3b71704a18 100644 --- a/tests/ui/missing_intopy.stderr +++ b/tests/ui/missing_intopy.stderr @@ -6,18 +6,14 @@ error[E0599]: the method `into_py_result` exists for struct `Blah`, but its trai | | | method `into_py_result` not found for this | doesn't satisfy `Blah: IntoPy>` - | doesn't satisfy `Blah: IntoPyResult>` + | 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>` - `&Blah: IntoPy>` - which is required by `&Blah: IntoPyResult>` - `&mut Blah: IntoPy>` - which is required by `&mut Blah: IntoPyResult>` + which is required by `Blah: IntoPyResult` note: the following trait must be implemented --> src/conversion.rs | @@ -27,3 +23,16 @@ note: the following trait must be implemented | | } | |_^ = 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` + | +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) From 44f3bc76a953811cd09d1b8c3aa214db02aa7f99 Mon Sep 17 00:00:00 2001 From: mejrs Date: Sat, 7 May 2022 00:40:58 +0200 Subject: [PATCH 7/8] Ignore ghost in coverage --- codecov.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/codecov.yml b/codecov.yml index 84c56f0dd18..cfef461049a 100644 --- a/codecov.yml +++ b/codecov.yml @@ -11,3 +11,4 @@ coverage: ignore: - tests/*.rs - src/test_hygiene/*.rs + - src/impl_/ghost.rs From ab0bf9e4b8ab00d73a33c6189ef601cd34a402b9 Mon Sep 17 00:00:00 2001 From: mejrs Date: Mon, 9 May 2022 16:33:05 +0200 Subject: [PATCH 8/8] Change method name --- pyo3-macros-backend/src/method.rs | 2 +- src/impl_/ghost.rs | 3 +-- tests/ui/invalid_result_conversion.stderr | 2 +- tests/ui/missing_intopy.stderr | 4 ++-- 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/pyo3-macros-backend/src/method.rs b/pyo3-macros-backend/src/method.rs index 02bb5d3254b..e25575c1f41 100644 --- a/pyo3-macros-backend/src/method.rs +++ b/pyo3-macros-backend/src/method.rs @@ -485,7 +485,7 @@ impl<'a> FnSpec<'a> { if false { use _pyo3::impl_::ghost::IntoPyResult; - ret.into_py_result(); + ret.assert_into_py_result(); } _pyo3::callback::convert(#py, ret) diff --git a/src/impl_/ghost.rs b/src/impl_/ghost.rs index 68fd3408112..667607dd001 100644 --- a/src/impl_/ghost.rs +++ b/src/impl_/ghost.rs @@ -4,9 +4,8 @@ /// but that never affects anything at runtime, use crate::{IntoPy, PyErr, PyObject}; -#[allow(clippy::wrong_self_convention)] pub trait IntoPyResult { - fn into_py_result(&mut self) {} + fn assert_into_py_result(&mut self) {} } impl IntoPyResult for T where T: IntoPy {} diff --git a/tests/ui/invalid_result_conversion.stderr b/tests/ui/invalid_result_conversion.stderr index decaa8c72ae..f2ca0f5cc03 100644 --- a/tests/ui/invalid_result_conversion.stderr +++ b/tests/ui/invalid_result_conversion.stderr @@ -1,4 +1,4 @@ -error[E0599]: no method named `into_py_result` found for enum `Result` in the current scope +error[E0599]: no method named `assert_into_py_result` found for enum `Result` in the current scope --> tests/ui/invalid_result_conversion.rs:21:1 | 21 | #[pyfunction] diff --git a/tests/ui/missing_intopy.stderr b/tests/ui/missing_intopy.stderr index f3b71704a18..b714d798dd2 100644 --- a/tests/ui/missing_intopy.stderr +++ b/tests/ui/missing_intopy.stderr @@ -1,10 +1,10 @@ -error[E0599]: the method `into_py_result` exists for struct `Blah`, but its trait bounds were not satisfied +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 `into_py_result` not found for this + | method `assert_into_py_result` not found for this | doesn't satisfy `Blah: IntoPy>` | doesn't satisfy `Blah: IntoPyResult` 2 |