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

drop-tracking: type parameters are incorrectly required to be Send to make futures Send #98477

Closed
Tracked by #97331
jyn514 opened this issue Jun 25, 2022 · 2 comments · Fixed by #98754
Closed
Tracked by #97331
Labels
A-async-await Area: Async & Await AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jyn514
Copy link
Member

jyn514 commented Jun 25, 2022

cc #97331, @eholk. Minimized from https://crater-reports.s3.amazonaws.com/pr-97334/try%23615edd3ad1cf6871c977dc900317cb6c2070fd6b/gh/AtsushiMiyazaki.rust-cmd/log.txt in the crater run for #97334.

I tried this code:

fn assert_send<F: Send>(_: F) {}

async fn __post<T>() -> T {
    if false {
        todo!()
    } else {
        async {}.await;
        todo!()
    }
}

fn foo<T>() {
    assert_send(__post::<T>());
}

I expected to see this happen: The code compiles fine, like it does without -Zdrop-tracking. The code doesn't actually use T anywhere so it shouldn't require it to be Send.

Instead, this happened:

error: future cannot be sent between threads safely
  --> rust-iost/rpc/src/lib.rs:13:17
   |
13 |     assert_send(__post::<T>());
   |                 ^^^^^^^^^^^^^ future returned by `__post` is not `Send`
   |
note: future is not `Send` as this value is used across an await
  --> rust-iost/rpc/src/lib.rs:7:17
   |
4  |       if false {
   |  ______________-
5  | |         todo!()
6  | |     } else {
   | |_____- has type `T` which is not `Send`
7  |           async {}.await;
   |                   ^^^^^^ await occurs here, with the value maybe used later
8  |           todo!()
9  |       }
   |       - the value is later dropped here
note: required by a bound in `assert_send`
  --> rust-iost/rpc/src/lib.rs:1:19
   |
1  | fn assert_send<F: Send>(_: F) {}
   |                   ^^^^ required by this bound in `assert_send`
help: consider restricting type parameter `T`
   |
12 | fn foo<T: std::marker::Send>() {
   |         +++++++++++++++++++

Meta

rustc --version --verbose: 615edd3ad1cf6871c977dc900317cb6c2070fd6b

@jyn514 jyn514 added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. A-async-await Area: Async & Await labels Jun 25, 2022
@jyn514
Copy link
Member Author

jyn514 commented Jun 25, 2022

https://crater-reports.s3.amazonaws.com/pr-97334/try%23615edd3ad1cf6871c977dc900317cb6c2070fd6b/reg/cloudflare-0.9.1/log.txt looks like the same issue, it's requiring a type parameter to be Send.

@eholk
Copy link
Contributor

eholk commented Jul 4, 2022

@rustbot label +AsyncAwait-Triaged

@rustbot rustbot added the AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. label Jul 4, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Jul 12, 2022
…-errors

Fix drop-tracking ICE when a struct containing a field with a significant drop is used across an await

Previously, drop-tracking would incorrectly assume the struct would be dropped immediately, which was not true.

Fixes rust-lang#98476. Also fixes rust-lang#98477, I think because the parent HIR node for type variables is the whole function instead of the expression where the variable is used.

r? `@eholk`
@bors bors closed this as completed in 8a392a5 Jul 14, 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 AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
3 participants