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 error message regression in issue-70935-complex-spans.rs #97332

Closed
Tracked by #97331
eholk opened this issue May 23, 2022 · 5 comments
Closed
Tracked by #97331

Drop tracking error message regression in issue-70935-complex-spans.rs #97332

eholk opened this issue May 23, 2022 · 5 comments
Assignees

Comments

@eholk
Copy link
Contributor

eholk commented May 23, 2022

With -Zdrop-tracking enabled, issue-70935-complex-spans.rs gives the following stderr output:

error[E0277]: `Sender<i32>` cannot be shared between threads safely
  --> $DIR/issue-70935-complex-spans.rs:10:45
   |
LL | fn foo(tx: std::sync::mpsc::Sender<i32>) -> impl Future + Send {
   |                                             ^^^^^^^^^^^^^^^^^^ `Sender<i32>` cannot be shared between threads safely
   |
   = help: the trait `Sync` is not implemented for `Sender<i32>`
   = note: required because of the requirements on the impl of `Send` for `&Sender<i32>`
   = note: required because it appears within the type `[closure@$DIR/issue-70935-complex-spans.rs:13:13: 15:10]`
   = note: required because it appears within the type `[static generator@$DIR/issue-70935-complex-spans.rs:7:67: 8:2]`
   = note: required because it appears within the type `from_generator::GenFuture<[static generator@$DIR/issue-70935-complex-spans.rs:7:67: 8:2]>`
   = note: required because it appears within the type `impl Future<Output = ()>`
   = note: required because it appears within the type `impl Future<Output = ()>`
   = note: required because it appears within the type `for<'r, 's, 't0> {ResumeTy, impl Future<Output = ()>, ()}`
   = note: required because it appears within the type `[static generator@$DIR/issue-70935-complex-spans.rs:12:16: 16:6]`
   = note: required because it appears within the type `from_generator::GenFuture<[static generator@$DIR/issue-70935-complex-spans.rs:12:16: 16:6]>`
   = note: required because it appears within the type `impl Future<Output = ()>`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.

Without -Zdrop-tracking, we get the following error message:

error: future cannot be sent between threads safely
  --> src/test/ui/async-await/issue-70935-complex-spans.rs:10:45
   |
10 | fn foo(tx: std::sync::mpsc::Sender<i32>) -> impl Future + Send {
   |                                             ^^^^^^^^^^^^^^^^^^ future created by async block is not `Send`
   |
   = help: the trait `Sync` is not implemented for `Sender<i32>`
note: future is not `Send` as this value is used across an await
  --> src/test/ui/async-await/issue-70935-complex-spans.rs:15:11
   |
13 |           baz(|| async{
   |  _____________-
14 | |             foo(tx.clone());
15 | |         }).await;
   | |         - ^^^^^^ await occurs here, with the value maybe used later
   | |_________|
   |           has type `[closure@src/test/ui/async-await/issue-70935-complex-spans.rs:13:13: 15:10]` which is not `Send`
note: the value is later dropped here
  --> src/test/ui/async-await/issue-70935-complex-spans.rs:15:17
   |
15 |         }).await;
   |                 ^

error: aborting due to previous error

The old message seems much better, so it'd be good to match the behavior with drop tracking.

@eholk
Copy link
Contributor Author

eholk commented May 23, 2022

@jyn514
Copy link
Member

jyn514 commented May 23, 2022

@rustbot claim

@jyn514
Copy link
Member

jyn514 commented Jun 18, 2022

Noticed an existing weirdness with one of the diagnostics - this should point to tx, not the closure itself:

LL | baz(|| async{
| _____________-
LL | | foo(tx.clone());
LL | | }).await;
| | - ^^^^^^ await occurs here, with the value maybe used later
| |_________|
| has type `[closure@$DIR/issue-70935-complex-spans.rs:13:13: 15:10]` which is not `Send`

@jyn514
Copy link
Member

jyn514 commented Jun 19, 2022

#98259 makes this significantly better but not the same as without -Zdrop-tracking. Not sure whether that's sufficient, or whether I should keep working on this. I'm not sure it's possible to make them identical without radical changes to how async-await diagnostics work: https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/josh.20rambles.20about.20drop.20tracking.20.2397332/near/286660618

compiler-errors added a commit to compiler-errors/rust that referenced this issue Jun 23, 2022
…=estebank

Greatly improve error reporting for futures and generators in `note_obligation_cause_code`

Most futures don't go through this code path, because they're caught by
`maybe_note_obligation_cause_for_async_await`. But all generators do,
and `maybe_note` is imperfect and doesn't catch all futures. Improve the error message for those it misses.

At some point, we may want to consider unifying this with the code for `maybe_note_async_await`,
so that `async_await` notes all parent constraints, and `note_obligation` can point to yield points.
But both functions are quite complicated, and it's not clear to me how to combine them;
this seems like a good incremental improvement.

Helps with rust-lang#97332.

r? `@estebank` cc `@eholk` `@compiler-errors`
compiler-errors added a commit to compiler-errors/rust that referenced this issue Jun 23, 2022
…=estebank

Greatly improve error reporting for futures and generators in `note_obligation_cause_code`

Most futures don't go through this code path, because they're caught by
`maybe_note_obligation_cause_for_async_await`. But all generators do,
and `maybe_note` is imperfect and doesn't catch all futures. Improve the error message for those it misses.

At some point, we may want to consider unifying this with the code for `maybe_note_async_await`,
so that `async_await` notes all parent constraints, and `note_obligation` can point to yield points.
But both functions are quite complicated, and it's not clear to me how to combine them;
this seems like a good incremental improvement.

Helps with rust-lang#97332.

r? ``@estebank`` cc ``@eholk`` ``@compiler-errors``
@eholk
Copy link
Contributor Author

eholk commented Jul 29, 2022

I'm happy with how this error message looks after the recent changes, so I'll go ahead and mark it as closed.

@eholk eholk closed this as completed Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants