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

improve diagnostics for non-send Futures that might be solved by introducing temporary variables #67376

Closed
yoshuawuyts opened this issue Dec 17, 2019 · 2 comments · Fixed by #68212
Assignees
Labels
A-async-await Area: Async & Await A-diagnostics Area: Messages for errors, warnings, and lints AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-enhancement Category: An issue proposing an enhancement or a PR with one. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. D-papercut Diagnostics: An error or lint that needs small tweaks. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@yoshuawuyts
Copy link
Member

In http-rs/tide#370 we had a user report that they encountered a Send error similar to the ones discussed in #64130. Thanks to the diagnostics introduced #65345 on nightly we were able to correctly diagnose the root cause of the error.

But for many users this might still not be enough to figure out how to solve this, and we could improve by providing a hint.

Current Diagnostics

2019-12-17-175740_1920x1080

   Compiling tide-demo v0.1.0 (/home/anon/Code/importcjj/tide-demo)
error: future cannot be sent between threads safely
  --> src/main.rs:47:27
   |
47 |         app.at("/submit").post(|req: tide::Request<State>| {
   |                           ^^^^ future is not `Send`
   |
   = help: the trait `std::marker::Sync` is not implemented for `(dyn futures_io::if_std::AsyncBufRead + std::marker::Send + 'static)`
note: future is not `Send` as this value is used across an await
  --> src/main.rs:49:29
   |
49 |                 let conn = &req.state().pool.get().await;
   |                             ---^^^^^^^^^^^^^^^^^^^^^^^^^- `req` is later dropped here
   |                             |
   |                             await occurs here, with `req` maybe used later
   |                             has type `&tide::request::Request<State>`

Suggested Diagnostics

@estebank suggested we could add a suggestion here:

help: consider assigning `&req.state().pool` to a temporary variable

Repro

Refs

@jonas-schievink jonas-schievink added A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-async-await Area: Async & Await labels Dec 17, 2019
@estebank estebank added D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. D-papercut Diagnostics: An error or lint that needs small tweaks. labels Dec 17, 2019
@tmandry tmandry added AsyncAwait-OnDeck AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. labels Jan 7, 2020
@tmandry
Copy link
Member

tmandry commented Jan 7, 2020

Visiting from triage.. assigning @csmoe for implementation and @nikomatsakis to leave some tips.

@csmoe
Copy link
Member

csmoe commented Jan 8, 2020

reproduced with exsited ui testcase:

use std::any::Any;
use std::future::Future;

struct Client(Box<dyn Any + Send>);

impl Client {
    fn status(&self) -> u16 {
        200
    }
}

async fn get() { }

pub fn foo() -> impl Future + Send {
    //~^ ERROR future cannot be sent between threads safely
    let client = Client(Box::new(true));
    async move {
        match client.status() {
            200 => {
                let _x = get().await;
            },
            _ => (),
        }
    }
}

fn main() {}
  --> $DIR/issue-64130-4-async-move.rs:21:26
   |
LL |         match client.status() {
   |               ------ has type `&Client`
LL |             200 => {
LL |                 let _x = get().await;
   |                          ^^^^^^^^^^^ await occurs here, with `client` maybe used later
...
LL |     }
   |     - `client` is later dropped here
   = note: the return type of a function must have a statically known size

rewrite the match expr as:

let status = client.status();
match status { ... }

will fix this.
The problem is that client does NOT has type &Client actually, but .status() creates a temporary borrow as &Client.

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 AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-enhancement Category: An issue proposing an enhancement or a PR with one. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. D-papercut Diagnostics: An error or lint that needs small tweaks. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants