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

async function with default implementation won't compile #17

Open
rodrigorc opened this issue Dec 23, 2023 · 3 comments · May be fixed by #20 or #28
Open

async function with default implementation won't compile #17

rodrigorc opened this issue Dec 23, 2023 · 3 comments · May be fixed by #20 or #28

Comments

@rodrigorc
Copy link

I have a trait such as this:

#[trait_variant::make(Test: Send)]
trait LocalTest {
    async fn foo() -> i32 { 0 }
}

but if fails with an error:

  |
3 | #[trait_variant::make(Test: Send)]
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `{integer}` is not a future
  |

It looks like the body of the default impl is translated to the Test trait, unchanged.

If I instead I try:

    async fn foo() -> i32 { std::future::ready(0) }

Then it looks like the Test works, but LocalTest fails with a somewhat opposite error:

5 |     async fn foo() -> i32 { std::future::ready(0) }
  |                             ^^^^^^^^^^^^^^^^^^^^^ expected `i32`, found `Ready<{integer}>`

I'm not sure if this is a limitation of this crate, if so I think it should be documented somewhere; or if it is an oversight in the implementation. Or maybe I'm missing something?

Then, just to see what would happen, I tried:

async fn foo() -> i32 { async move { 0 } }

And I got an ICE!

5 |     async fn foo() -> i32 { async move { 0 } }
  |                             ^^^^^^^^^^^^^^^^ expected `i32`, found `async` block
  |
  = note:       expected type `i32`
          found `async` block `{async block@src/main.rs:5:29: 5:45}`

error: internal compiler error: compiler/rustc_mir_build/src/build/mod.rs:655:72: impossible case reached
...
@milesj
Copy link

milesj commented Dec 29, 2023

Also ran into this. I have a trait with this function and a default body:

async fn is_version_supported(&self, req: &str) -> miette::Result<bool> {
    let version = self.get_version().await?;

    Ok(VersionReq::parse(req).into_diagnostic()?.matches(&version))
}

This fails to compile with:

error[E0728]: `await` is only allowed inside `async` functions and blocks
  --> nextgen/vcs/src/vcs.rs:75:42
   |
75 |         let version = self.get_version().await?;
   |                                          ^^^^^ only allowed inside `async` functions and blocks

    Checking moon_deno_tool v0.1.0 (/crates/deno/tool)
    Checking moon_system_platform v0.1.0 (/crates/system/platform)
    Checking moon_app v0.1.0 (/nextgen/app)
error[E0277]: `Result<bool, _>` is not a future
 --> nextgen/vcs/src/vcs.rs:9:1
  |
9 | #[trait_variant::make(Vcs: Send)]
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Result<bool, _>` is not a future
  |
  = help: the trait `Future` is not implemented for `Result<bool, _>`
  = note: Result<bool, _> must be a future or must implement `IntoFuture` to be awaited

When looking at the trait_variant compiled output, it makes sense that it would fail. I'm surprised this wasn't tested at all?

    #[doc = r" Return true if the current binary version matches the provided requirement."]
    fn is_version_supported(
        &self,
        req: &str,
    ) -> impl ::core::future::Future<Output = miette::Result<bool>> + Send {
        let version = self.get_version().await?;
        Ok(VersionReq::parse(req).into_diagnostic()?.matches(&version))
    }

@ryo33
Copy link

ryo33 commented Jan 15, 2024

I almost had opened an issue with the same problem with title, "making a variant with panic!() in async fn's body causes a compile error"

==== The issue body ====

Adding #[trait_variant::make(Test: Send)] to the following no-error code causes an error

pub trait LocalTest {
    async fn test(&self) -> String {
        panic!()
    }
}
rustc: `()` is not a future
the trait `std::future::Future` is not implemented for `()`
() must be a future or must implement `IntoFuture` to be awaited [E0277]

Macros that desugar async fn have to emit async { panic!() } as the body instead of just panic!() to avoid the error, though there are complex cases, like:

  1. todo! or unimplemented! is used instead
  2. the method has more statements above the panic!()
  3. (trivial) a function that returns ! is used instead.

As the user side, I cannot find a way to work around this, especially when using the trait_variant::make variant along with other macros that emit default implementation containing panic!().

Related:
rust-lang/rust#35121
rust-lang/rfcs#1637
rust-lang/rust#20021

@Sherlock-Holo Sherlock-Holo linked a pull request Feb 4, 2024 that will close this issue
@kingwingfly
Copy link

It’s not silky smooth to use for me also. However, I could give a relatively silky example.

#[trait_variant::make(Test: Send)]
trait LocalTest {
    // Below can not work (`Result<(), _>` is not a future)
    // async fn foo() -> anyhow::Result<()> {
    //     Ok(())
    // }

    // Below can work
    fn foo() -> impl core::future::Future<Output = anyhow::Result<()>> {
        async { Ok(()) }
    }

    async fn bar() -> anyhow::Result<()>;

    async fn baz() -> anyhow::Result<()>;
}

struct Foo;

impl LocalTest for Foo {
    async fn foo() -> anyhow::Result<()> {
        Ok(())
    }

    async fn bar() -> anyhow::Result<()> {
        Ok(())
    }

    async fn baz() -> anyhow::Result<()> {
        todo!()
    }
}

struct Bar;

impl Test for Bar {
    async fn foo() -> anyhow::Result<()> {
        Ok(())
    }

    async fn bar() -> anyhow::Result<()> {
        Ok(())
    }

    async fn baz() -> anyhow::Result<()> {
        todo!()
    }
}

To conclude,

  1. Cannot impl LocalTest and Test for the same struct.
  2. When define the trait, only default impl shouldn't use async fn, use fn foo() -> impl core::future::Future<Output = ... > with inner async block instead.
  3. When impl trait, the LocalTest and Test are completely the same.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants