Async pymethods returning result breaks "new" initializer method

[chamoretto]

Bug Description

If you create some async function returning Result<T, E> and then create new method, you will experience strange things. Look at first code snipet - it won't compile, then look at backtrace 1. Same with snippet 3.

use pyo3::exceptions::PyTypeError;
    use pyo3::{pyclass, pymethods, PyResult};

    #[pyclass(name = "SomeStruct", module = "rust_extensions")] // noinspection RsSortImplTraitMembers
    pub struct SomeStruct {
        pub smth: String,
    }

    #[pymethods]
    impl SomeStruct {
        pub async fn return_string(&mut self) -> Result<(), String> {
            Ok(())
        }

        #[new]
        pub fn new() -> PyResult<()> {
            Err(PyTypeError::new_err("Some error of fallible __init__"))
        }
    }

Removing "async" from function fixes the issue. According to macro expansions, something goes wrong at async function expansion.

Expanded code:

impl SomeStruct {
        pub async fn return_string(&mut self) -> Result<(), String> {
            Ok(())
        }

        pub fn new() -> PyResult<()> {
            Err(PyTypeError::new_err("Some error of fallible __init__"))
        }
    }
    #[allow(unknown_lints, non_local_definitions)]
    impl ::pyo3::impl_::pyclass::PyMethods<SomeStruct>
        for ::pyo3::impl_::pyclass::PyClassImplCollector<SomeStruct>
    {
        fn py_methods(self) -> &'static ::pyo3::impl_::pyclass::PyClassItems {
            static ITEMS: ::pyo3::impl_::pyclass::PyClassItems =
                ::pyo3::impl_::pyclass::PyClassItems {
                    methods: &[::pyo3::impl_::pyclass::MaybeRuntimePyMethodDef::Static(
                        ::pyo3::class::PyMethodDefType::Method(
                            ::pyo3::impl_::pymethods::PyMethodDef::noargs(
                                c"return_string",
                                {
                                    unsafe extern "C" fn trampoline(
                                        _slf: *mut ::pyo3::ffi::PyObject,
                                        _args: *mut ::pyo3::ffi::PyObject,
                                    ) -> *mut ::pyo3::ffi::PyObject
                                    {
                                        ::pyo3::impl_::trampoline::noargs(
                                            _slf,
                                            _args,
                                            SomeStruct::__pymethod_return_string__,
                                        )
                                    }
                                    trampoline
                                },
                                c"return_string($self)\n--\n\n",
                            ),
                        ),
                    )],
                    slots: &[::pyo3::ffi::PyType_Slot {
                        slot: ::pyo3::ffi::Py_tp_new,
                        pfunc: {
                            unsafe extern "C" fn trampoline(
                                subtype: *mut ::pyo3::ffi::PyTypeObject,
                                args: *mut ::pyo3::ffi::PyObject,
                                kwargs: *mut ::pyo3::ffi::PyObject,
                            ) -> *mut ::pyo3::ffi::PyObject {
                                use ::pyo3::impl_::pyclass::*;
                                #[allow(unknown_lints, non_local_definitions)]
                                impl PyClassNewTextSignature<SomeStruct> for PyClassImplCollector<SomeStruct> {
                                    #[inline]
                                    fn new_text_signature(
                                        self,
                                    ) -> ::std::option::Option<&'static str>
                                    {
                                        ::std::option::Option::Some("()")
                                    }
                                }
                                ::pyo3::impl_::trampoline::newfunc(
                                    subtype,
                                    args,
                                    kwargs,
                                    SomeStruct::__pymethod___new____,
                                )
                            }
                            trampoline
                        } as ::pyo3::ffi::newfunc as _,
                    }],
                };
            &ITEMS
        }
    }
    #[doc(hidden)]
    #[allow(non_snake_case)]
    impl SomeStruct {
        unsafe fn __pymethod_return_string__<'py>(
            py: ::pyo3::Python<'py>,
            _slf: *mut ::pyo3::ffi::PyObject,
        ) -> ::pyo3::PyResult<*mut ::pyo3::ffi::PyObject> {
            let _slf_ref = &_slf;
            let function = SomeStruct::return_string;
            #[allow(clippy::let_unit_value)]
            let result = {
                let ret = {
                    let future = {
                        let mut __guard = ::pyo3::impl_::coroutine::RefMutGuard::<SomeStruct>::new(
                            &::pyo3::impl_::pymethods::BoundRef::ref_from_ptr(py, &_slf),
                        )?;
                        async move { function(&mut __guard).await }
                    };
                    ::pyo3::impl_::coroutine::new_coroutine( // First error points here
                        {
                            static INTERNED: ::pyo3::sync::Interned =
                                ::pyo3::sync::Interned::new("return_string");
                            INTERNED.get(py)
                        },
                        Some(<SomeStruct as ::pyo3::PyTypeInfo>::NAME),
                        None,
                        async move { ::pyo3::impl_::wrap::OkWrap::wrap(future.await) },
                    )
                };
                ::pyo3::impl_::wrap::map_result_into_ptr(
                    py,
                    ::pyo3::impl_::wrap::OkWrap::wrap(ret)
                        .map_err(::core::convert::Into::<::pyo3::PyErr>::into),
                )
            };
            result
        }
        unsafe fn __pymethod___new____(
            py: ::pyo3::Python<'_>,
            _slf: *mut ::pyo3::ffi::PyTypeObject,
            _args: *mut ::pyo3::ffi::PyObject,
            _kwargs: *mut ::pyo3::ffi::PyObject,
        ) -> ::pyo3::PyResult<*mut ::pyo3::ffi::PyObject> {
            use ::pyo3::callback::IntoPyCallbackOutput;
            let _slf_ref = &_slf;
            let function = SomeStruct::new;
            const DESCRIPTION: ::pyo3::impl_::extract_argument::FunctionDescription =
                ::pyo3::impl_::extract_argument::FunctionDescription {
                    cls_name: ::std::option::Option::Some(
                        <SomeStruct as ::pyo3::type_object::PyTypeInfo>::NAME,
                    ),
                    func_name: "__new__",
                    positional_parameter_names: &[],
                    positional_only_parameters: 0usize,
                    required_positional_parameters: 0usize,
                    keyword_only_parameters: &[],
                };
            let mut output = [::std::option::Option::None; 0usize];
            let (_args, _kwargs) = DESCRIPTION.extract_arguments_tuple_dict::<::pyo3::impl_::extract_argument::NoVarargs, ::pyo3::impl_::extract_argument::NoVarkeywords>(py, _args, _kwargs, &mut output)?;
            #[allow(clippy::let_unit_value)]
            let result = SomeStruct::new();
            let initializer: ::pyo3::PyClassInitializer<SomeStruct> = result.convert(py)?;  // Second error points here
            ::pyo3::impl_::pymethods::tp_new_impl(py, initializer, _slf)
        }
    }

Even such variant fails:

#[pymethods]
    impl SomeStruct {
        pub async fn return_string(&mut self) -> Result<(), i64> {
            Ok(())
        }

        #[new]
        pub fn new() -> Self {
            SomeStruct {
                smth: "Sas".to_string(),
            }
        }
    }

Leads to

Trait `From<i64>` is not implemented for `PyErr`

So there is no problem returning PyErr from new, there is a connection between async function's Result second argument which must be PyErr or must be convertable into it. I think this is a real issue and error at new is just a side effect.
If this is intended (i can't find anything about return types of async functions at Using async and await ), i think more understandable error message should be provided instead of two misleading errors, because it is not easy to match error pointing to new method and return type of your async function.

Steps to Reproduce

1. Write some class using #[pyclass]
2. Implements any "new" constructor for this class using #[pymethods] and #[new]
3. Add any async function, returning Result<Anything, Something>.

And that's it. After adding this, you always will see compilation error with such message:

Trait `From<Something>` is not implemented for `PyErr`

Backtrace

error[E0277]: the trait bound `PyErr: From<std::string::String>` is not satisfied                                                                                                                                                                                                                                                          
   --> src\example.rs:226:5
    |
226 |     #[pymethods]
    |     ^^^^^^^^^^^^ the trait `From<std::string::String>` is not implemented for `PyErr`, which is required by `std::string::String: Into<PyErr>`
    |
    = help: the following other types implement trait `From<T>`:
              <PyErr as From<AddrParseError>>
              <PyErr as From<DecodeUtf16Error>>
              <PyErr as From<DowncastError<'_, '_>>>
              <PyErr as From<DowncastIntoError<'_>>>
              <PyErr as From<FromUtf16Error>>
              <PyErr as From<FromUtf8Error>>
              <PyErr as From<Infallible>>
              <PyErr as From<IntoInnerError<W>>>
            and 12 others
    = note: required for `std::string::String` to implement `Into<PyErr>`
note: required by a bound in `new_coroutine`
   --> C:\Users\g\.cargo\registry\src\index.crates.io-6f17d22bba15001f\pyo3-0.22.2\src\impl_\coroutine.rs:24:8
    |
15  | pub fn new_coroutine<F, T, E>(
    |        ------------- required by a bound in this function
...
24  |     E: Into<PyErr>,
    |        ^^^^^^^^^^^ required by this bound in `new_coroutine`
    = note: this error originates in the attribute macro `pymethods` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `PyClassInitializer<SomeStruct>: From<()>` is not satisfied
   --> src\example.rs:226:5
    |
226 |     #[pymethods]
    |     ^^^^^^^^^^^^ the trait `From<()>` is not implemented for `PyClassInitializer<SomeStruct>`, which is required by `std::result::Result<(), PyErr>: IntoPyCallbackOutput<_>`
    |
    = help: the following other types implement trait `From<T>`:
              <PyClassInitializer<S> as From<(S, B)>>
              <PyClassInitializer<T> as From<Py<T>>>
              <PyClassInitializer<T> as From<T>>
              <PyClassInitializer<T> as From<pyo3::Bound<'py, T>>>
    = note: required for `()` to implement `Into<PyClassInitializer<SomeStruct>>`
    = note: required for `()` to implement `IntoPyCallbackOutput<PyClassInitializer<SomeStruct>>`
    = note: 1 redundant requirement hidden
    = note: required for `std::result::Result<(), PyErr>` to implement `IntoPyCallbackOutput<PyClassInitializer<SomeStruct>>`
    = note: this error originates in the attribute macro `pymethods` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0277`.                                                                                                                                                                                                                                                                        
warning: `rust-extensions-bin` (lib) generated 8 warnings
error: could not compile `rust-extensions-bin` (lib) due to 2 previous errors; 8 warnings emitted

Backtrace of expanded version:

error[E0277]: the trait bound `PyErr: From<std::string::String>` is not satisfied                                                                                                                                                                                                                                                          
   --> src\example.rs:315:21
    |
315 | /                     ::pyo3::impl_::coroutine::new_coroutine(
316 | |                         {
317 | |                             static INTERNED: ::pyo3::sync::Interned =
318 | |                                 ::pyo3::sync::Interned::new("return_string");
...   |
323 | |                         async move { ::pyo3::impl_::wrap::OkWrap::wrap(future.await) },
324 | |                     )
    | |_____________________^ the trait `From<std::string::String>` is not implemented for `PyErr`, which is required by `std::string::String: Into<PyErr>`
    |
    = help: the following other types implement trait `From<T>`:
              <PyErr as From<AddrParseError>>
              <PyErr as From<DecodeUtf16Error>>
              <PyErr as From<DowncastError<'_, '_>>>
              <PyErr as From<DowncastIntoError<'_>>>
              <PyErr as From<FromUtf16Error>>
              <PyErr as From<FromUtf8Error>>
              <PyErr as From<Infallible>>
              <PyErr as From<IntoInnerError<W>>>
            and 12 others
    = note: required for `std::string::String` to implement `Into<PyErr>`
note: required by a bound in `new_coroutine`
   --> C:\Users\g\.cargo\registry\src\index.crates.io-6f17d22bba15001f\pyo3-0.22.2\src\impl_\coroutine.rs:24:8
    |
15  | pub fn new_coroutine<F, T, E>(
    |        ------------- required by a bound in this function
...
24  |     E: Into<PyErr>,
    |        ^^^^^^^^^^^ required by this bound in `new_coroutine`

error[E0277]: the trait bound `PyClassInitializer<SomeStruct>: From<()>` is not satisfied
   --> src\example.rs:358:78
    |
358 |             let initializer: ::pyo3::PyClassInitializer<SomeStruct> = result.convert(py)?;
    |                                                                              ^^^^^^^ the trait `From<()>` is not implemented for `PyClassInitializer<SomeStruct>`, which is required by `std::result::Result<(), PyErr>: IntoPyCallbackOutput<_>`
    |
    = help: the following other types implement trait `From<T>`:
              <PyClassInitializer<S> as From<(S, B)>>
              <PyClassInitializer<T> as From<Py<T>>>
              <PyClassInitializer<T> as From<T>>
              <PyClassInitializer<T> as From<pyo3::Bound<'py, T>>>
    = note: required for `()` to implement `Into<PyClassInitializer<SomeStruct>>`
    = note: required for `()` to implement `IntoPyCallbackOutput<PyClassInitializer<SomeStruct>>`
    = note: 1 redundant requirement hidden
    = note: required for `std::result::Result<(), PyErr>` to implement `IntoPyCallbackOutput<PyClassInitializer<SomeStruct>>`

For more information about this error, try `rustc --explain E0277`.                                                                                                                                                                                                                                                                        
warning: `rust-extensions-bin` (lib) generated 9 warnings
error: could not compile `rust-extensions-bin` (lib) due to 2 previous errors; 9 warnings emitted

### Your operating system and version

Windows 11, 10.0.22631 Build 22631

### Your Python version (`python --version`)

3.10.11

### Your Rust version (`rustc --version`)

rustc 1.79.0 (129f3b996 2024-06-10)

### Your PyO3 version

0.22.2

### How did you install python? Did you use a virtualenv?

venv

### Additional Info

_No response_

[Icxolu]

So for your first snippet news signature is wrong. new has to return something that can be tuned into a PyClassInitializer, for example Self, Bound<'_, Self> or Py<Self> or a PyResult of them.

#[new]
-pub fn new() -> PyResult<()> {
+pub fn new() -> PyResult<Self> {
    Err(PyTypeError::new_err("Some error of fallible __init__"))
}

As for this compile errror:

Trait `From<Something>` is not implemented for `PyErr`

Your error type has to be convertable into PyErr. This is required for both sync and async, so both versions fail. To fix it you have to implement From<Something> for PyErr. (It seems like the Span in the async case is wrong, so it does not mark the correct location, the sync version does, I would consider that a bug.)

I think these are two independent problems, I currently don't seen any interaction between them.

[davidhewitt]

Agreed, and in both of these cases we can probably improve the span and/or emit a better diagnostic.

[chamoretto]

So for your first snippet news signature is wrong. new has to return something that can be tuned into a PyClassInitializer, for example Self, Bound<'_, Self> or Py<Self> or a PyResult of them.

Yes, here you are right, my bad. I was experimenting with !/Never at first position at the main code.

As for this compile errror:

Trait `From<Something>` is not implemented for `PyErr`

Your error type has to be convertable into PyErr. This is required for both sync and async, so both versions fail. To fix it you have to implement From<Something> for PyErr. (It seems like the Span in the async case is wrong, so it does not mark the correct location, the sync version does, I would consider that a bug.)

I think these are two independent problems, I currently don't seen any interaction between them.

Hmmm, that's issue with my IDE (RustRover), because it does not shows errors about sync version, but issue still persists and causes compilation error. My fault, does not double check the compilation on both variants.
So i think to be clear there wold be just better errors in macros. Can you transform this from "bug" into "feature request" or smth?