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

Unclear diagnostics for async callback that takes a reference #76609

Closed
panstromek opened this issue Sep 11, 2020 · 4 comments
Closed

Unclear diagnostics for async callback that takes a reference #76609

panstromek opened this issue Sep 11, 2020 · 4 comments
Labels
A-async-await Area: Async & Await A-diagnostics Area: Messages for errors, warnings, and lints A-lifetimes Area: lifetime related AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. D-confusing Confusing diagnostic error that should be reworked T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@panstromek
Copy link
Contributor

panstromek commented Sep 11, 2020

This code..

pub async fn foo() {
    wrapper(callback).await
}

pub async fn callback(buf: &mut [u8]) {}

pub async fn wrapper< Fun, Fut>(cb: Fun)
    where
        Fun: Fn(& mut [u8]) -> Fut ,
        Fut: Future<Output=()>
{
    let mut buf = [8; 8];
    cb(&mut buf).await
}

...triggers this diagnostic:

error: implementation of `std::ops::FnOnce` is not general enough
   --> tests/src/main.rs:8:5
    |
8   |       wrapper(callback).await
    |       ^^^^^^^ implementation of `std::ops::FnOnce` is not general enough
    | 
   ::: /home/matyas/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:219:1
    |
219 | / pub trait FnOnce<Args> {
220 | |     /// The returned type after the call operator is used.
221 | |     #[lang = "fn_once_output"]
222 | |     #[stable(feature = "fn_once_output", since = "1.12.0")]
...   |
227 | |     extern "rust-call" fn call_once(self, args: Args) -> Self::Output;
228 | | }
    | |_- trait `std::ops::FnOnce` defined here
    |
    = note: `std::ops::FnOnce<(&'0 mut [u8],)>` would have to be implemented for the type `for<'_> fn(&mut [u8]) -> impl std::future::Future {callback}`, for some specific lifetime `'0`...
    = note: ...but `std::ops::FnOnce<(&mut [u8],)>` is actually implemented for the type `for<'_> fn(&mut [u8]) -> impl std::future::Future {callback}`

and it's completely unclear to me how to fix it. Well, obviously the problem is that the callback takes a reference, but even that took me some time to realize (longer than I'd like to admit). In the end I just removed it and did it differently.

This code came from refactoring of some non-async code, so I for some time I didn't realize that I was doing something fishy. I don't have much experience with async rust, so I don't know if this is even possible to fix while still keeping the reference there (maybe with some clever bounds?). If not, it should probably suggest you to rewrite it to not use that reference.

@panstromek
Copy link
Contributor Author

panstromek commented Sep 11, 2020

@rustbot modify labels +A-diagnostics +A-async-await +A-lifetimes

@rustbot rustbot added A-async-await Area: Async & Await A-diagnostics Area: Messages for errors, warnings, and lints A-lifetimes Area: lifetime related labels Sep 11, 2020
@estebank estebank added D-confusing Confusing diagnostic error that should be reworked T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 12, 2020
@tmandry
Copy link
Contributor

tmandry commented Sep 15, 2020

For the original error, the function signature needs a lifetime parameter like:

pub async fn wrapper<'a, Fun, Fut>(cb: Fun)
    where
        Fun: Fn(&'a mut [u8]) -> Fut ,
        Fut: Future<Output=()> + 'a

Adding this leads to an error caused by returning a future with a reference to a local (buf in this case), which also needs to be fixed. Thankfully this error message is a bit clearer.

@tmandry
Copy link
Contributor

tmandry commented Sep 15, 2020

Possible duplicate of #71671. EDIT: or #64650. Related to #68521.

@tmandry tmandry added the AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. label Sep 15, 2020
@tmandry
Copy link
Contributor

tmandry commented Sep 16, 2020

On second glance this is the same issue as #68521, closing in favor of it.

@tmandry tmandry closed this as completed Sep 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-async-await Area: Async & Await A-diagnostics Area: Messages for errors, warnings, and lints A-lifetimes Area: lifetime related AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. D-confusing Confusing diagnostic error that should be reworked T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants