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

Explanation for why future is not Send is wrong #68112

Closed
tmandry opened this issue Jan 11, 2020 · 4 comments · Fixed by #70679
Closed

Explanation for why future is not Send is wrong #68112

tmandry opened this issue Jan 11, 2020 · 4 comments · Fixed by #70679
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-bug Category: This is a bug. C-enhancement Category: An issue proposing an enhancement or a PR with one. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@tmandry
Copy link
Member

tmandry commented Jan 11, 2020

The following code (playground):

fn main() {
    let send_fut = async {
        let non_send_fut = make_non_send_future();
        let _ = non_send_fut.await;
        ready(0).await;
    };
    require_send(send_fut);
}

Gives an error message stating that non_send_fut can live until the end of the scope, and that's why send_fut is not Send:

error: future cannot be sent between threads safely
  --> src/main.rs:18:5
   |
6  | fn require_send(_: impl Send) {}
   |    ------------         ---- required by this bound in `require_send`
...
18 |     require_send(send_fut);
   |     ^^^^^^^^^^^^ future returned by `main` is not `Send`
   |
   = help: the trait `std::marker::Sync` is not implemented for `std::cell::RefCell<i32>`
note: future is not `Send` as this value is used across an await
  --> src/main.rs:16:9
   |
14 |         let non_send_fut = make_non_send_future();
   |             ------------ has type `impl core::future::future::Future`
15 |         let _ = non_send_fut.await;
16 |         ready(0).await;
   |         ^^^^^^^^^^^^^^ await occurs here, with `non_send_fut` maybe used later
17 |     };
   |     - `non_send_fut` is later dropped here

but it doesn't; it's consumed by value when it gets awaited. The problem is we're awaiting a non-Send future inside of send_fut, which has to be Send.

Also, the text

future returned by `main` is not `Send`

is wrong; main doesn't return anything.

Thanks to @kellerb for the reproducer and @JakeEhrlich for originally reporting this.

@varkor varkor added A-async-await Area: Async & Await A-diagnostics Area: Messages for errors, warnings, and lints D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. labels Jan 11, 2020
@tmandry
Copy link
Member Author

tmandry commented Jan 11, 2020

The problem is we're awaiting a non-Send future inside of send_fut, which has to be Send.

Some version of this is roughly the error message I'd want.

@JohnTitor JohnTitor added 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. labels Jan 12, 2020
@tmandry
Copy link
Member Author

tmandry commented Jan 14, 2020

@rustbot modify labels to +AsyncAwait-OnDeck +AsyncAwait-Triaged

@rustbot rustbot added AsyncAwait-OnDeck AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. labels Jan 14, 2020
@nikomatsakis
Copy link
Contributor

nikomatsakis commented Jan 14, 2020

A few thoughts:

  • I suspect that the code might be incorrectly identifying the await that should be highlighted

If you remove the second await, you get this error (playground):

error: future cannot be sent between threads safely
  --> src/main.rs:17:5
   |
6  | fn require_send(_: impl Send) {}
   |    ------------         ---- required by this bound in `require_send`
...
17 |     require_send(send_fut);
   |     ^^^^^^^^^^^^ future returned by `main` is not `Send`
   |
   = help: the trait `std::marker::Sync` is not implemented for `std::cell::RefCell<i32>`
note: future is not `Send` as this value is used across an await
  --> src/main.rs:15:17
   |
14 |         let non_send_fut = make_non_send_future();
   |             ------------ has type `impl core::future::future::Future`
15 |         let _ = non_send_fut.await;
   |                 ^^^^^^^^^^^^^^^^^^ await occurs here, with `non_send_fut` maybe used later
16 |         //ready(0).await;
17 |     };
   |     - `non_send_fut` is later dropped here

Granted, also wrong, though it might be "correct" in the sense that the compiler thinks that non_send_fut may be used later (because our analysis is not precise).

@csmoe
Copy link
Member

csmoe commented Jan 15, 2020

yes, live-across-yield analysis inside generator should be more precise, a test case from src/test/ui:

#![feature(optin_builtin_traits)]
// edition:2018
struct Foo;

impl !Send for Foo {}

fn is_send<T: Send>(t: T) { }

async fn bar() {
    let x = Foo;
    baz().await;
}

async fn baz() { }

fn main() {
    is_send(bar());
}

this snippet will compile if the non-Send Foo's scope is shorten within a block

@tmandry tmandry added C-bug Category: This is a bug. P-high High priority labels Mar 3, 2020
@tmandry tmandry self-assigned this Mar 24, 2020
tmandry added a commit to tmandry/rust that referenced this issue Apr 2, 2020
Centril added a commit to Centril/rust that referenced this issue Apr 11, 2020
Improve async-await/generator obligation errors in some cases

Fixes rust-lang#68112.

This change is best read one commit at a time (I add a test at the beginning and update it in each change after).

The `test2` function is a case I found while writing the test that we don't handle with this code yet. I don't attempt to fix it in this PR, but it's a good candidate for future work.

r? @davidtwco, @nikomatsakis
tmandry added a commit to tmandry/rust that referenced this issue Apr 14, 2020
@bors bors closed this as completed in a3ef360 Apr 14, 2020
@tmandry tmandry moved this to Done in wg-async work Dec 8, 2022
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-bug Category: This is a bug. C-enhancement Category: An issue proposing an enhancement or a PR with one. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

6 participants