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

Forgetting "dyn" before Fn() + Trait causes panic #89

Closed
thanatos opened this issue Apr 7, 2020 · 3 comments · Fixed by #90
Closed

Forgetting "dyn" before Fn() + Trait causes panic #89

thanatos opened this issue Apr 7, 2020 · 3 comments · Fixed by #90
Labels
bug Something isn't working

Comments

@thanatos
Copy link

thanatos commented Apr 7, 2020

In non-async-trait code, this is currently legal, though it emits a warning that it should have the dyn keyword:

impl<F> Foo for Fn(i8) -> F + Send + Sync
  where F: ... {}

In async-trait code, this causes a panic:

#[async_trait::async_trait]
impl<F> RequestHandler for Fn(&Request) -> F + Send + Sync
  where F: ... {}
error: custom attribute panicked
  --> src/lib.rs:15:1
   |
15 | #[async_trait::async_trait]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = help: message: unexpected token

A full example in the Playground

I'm not sure whether the code should be accepted, given that the current compiler warns that the impl w/o the dyn is deprecated, but I figure async_trait might want to either accept w/ a similar warning, or at the very least reject with a clearer error message.

(I mostly stumbled into this; I do not think the impl is anything close to what I actually want in my real-world use case. The compiler's insistence on dyn is nice though, since it clued me in that I'm barking up the wrong syntactical tree.)

@dtolnay dtolnay added the bug Something isn't working label Apr 7, 2020
@dtolnay
Copy link
Owner

dtolnay commented Apr 7, 2020

Minimized:

use async_trait::async_trait;

#[async_trait]
trait Trait {
    async fn f(&self);
}

#[async_trait]
impl Trait for Send + Sync {
    async fn f(&self) {}
}

@dtolnay
Copy link
Owner

dtolnay commented Apr 7, 2020

Fixed in 0.1.30.

@thanatos
Copy link
Author

thanatos commented Apr 7, 2020

Wow that was fast! Also, thank you for an excellent library!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants