Skip to content

Conversation

davidtwco
Copy link
Member

@davidtwco davidtwco commented Sep 29, 2019

cc #64130.

note: future does not implement `std::marker::Send` because this value is used across an await
  --> $DIR/issue-64130-non-send-future-diags.rs:15:5
   |
LL |     let g = x.lock().unwrap();
   |         - has type `std::sync::MutexGuard<'_, u32>`
LL |     baz().await;
   |     ^^^^^^^^^^^ await occurs here, with `g` maybe used later
LL | }
   | - `g` is later dropped here

r? @nikomatsakis

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 29, 2019
@rust-highfive

This comment has been minimized.

@davidtwco davidtwco force-pushed the issue-64130-async-error-definition branch from 42ce06e to f7cb10d Compare September 29, 2019 17:36
Copy link
Contributor

@Centril Centril left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some nits.

@rust-highfive

This comment has been minimized.

@davidtwco davidtwco force-pushed the issue-64130-async-error-definition branch from f7cb10d to bdaec6a Compare September 29, 2019 19:18
@davidtwco
Copy link
Member Author

I'm not sure where the duplicate diagnostic item error is coming from, a quick glance only found the rustc_diagnostic_item attribute that I added.

@Centril
Copy link
Contributor

Centril commented Sep 29, 2019

cc @oli-obk ^---

@rust-highfive

This comment has been minimized.

@davidtwco davidtwco force-pushed the issue-64130-async-error-definition branch from bdaec6a to e0f08b5 Compare September 30, 2019 09:13
@davidtwco davidtwco force-pushed the issue-64130-async-error-definition branch from e0f08b5 to 28be5ae Compare September 30, 2019 18:29
@oli-obk
Copy link
Contributor

oli-obk commented Sep 30, 2019

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 30, 2019

📌 Commit 28be5ae8c558613e59a56eb83f069da47eda56ed has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 30, 2019
Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome. Beautiful work. My one concern is that I think we can do better still with the error, but I'm sort of inclined to land this PR and continue that work afterwards.

@davidtwco davidtwco force-pushed the issue-64130-async-error-definition branch from 28be5ae to 9076646 Compare September 30, 2019 22:15
This commit improves obligation errors for async/await:

```
note: future does not implement `std::marker::Send` because this value is used across an
      await
  --> $DIR/issue-64130-non-send-future-diags.rs:15:5
   |
LL |     let g = x.lock().unwrap();
   |         - has type `std::sync::MutexGuard<'_, u32>`
LL |     baz().await;
   |     ^^^^^^^^^^^ await occurs here, with `g` maybe used later
LL | }
   | - `g` is later dropped here
```

Signed-off-by: David Wood <[email protected]>
@davidtwco davidtwco force-pushed the issue-64130-async-error-definition branch from 9076646 to 04fa9b1 Compare September 30, 2019 22:41
@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 30, 2019

📌 Commit 04fa9b1 has been approved by nikomatsakis

Centril added a commit to Centril/rust that referenced this pull request Oct 1, 2019
…efinition, r=nikomatsakis

async/await: improve not-send errors

cc rust-lang#64130.

```
note: future does not implement `std::marker::Send` because this value is used across an await
  --> $DIR/issue-64130-non-send-future-diags.rs:15:5
   |
LL |     let g = x.lock().unwrap();
   |         - has type `std::sync::MutexGuard<'_, u32>`
LL |     baz().await;
   |     ^^^^^^^^^^^ await occurs here, with `g` maybe used later
LL | }
   | - `g` is later dropped here
```

r? @nikomatsakis
bors added a commit that referenced this pull request Oct 1, 2019
Rollup of 10 pull requests

Successful merges:

 - #63674 (syntax: Support modern attribute syntax in the `meta` matcher)
 - #63931 (Stabilize macros in some more positions)
 - #64887 (syntax: recover trailing `|` in or-patterns)
 - #64895 (async/await: improve not-send errors)
 - #64896 (Remove legacy grammar)
 - #64907 (A small amount of tidying-up factored out from PR #64648)
 - #64928 (Add tests for some issues)
 - #64930 (Silence unreachable code lint from await desugaring)
 - #64935 (Improve code clarity)
 - #64937 (Deduplicate closure type errors)

Failed merges:

r? @ghost
@bors bors merged commit 04fa9b1 into rust-lang:master Oct 1, 2019
@bors
Copy link
Collaborator

bors commented Oct 1, 2019

☔ The latest upstream changes (presumably #64946) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 1, 2019
@davidtwco davidtwco deleted the issue-64130-async-error-definition branch October 1, 2019 15:32
@ghost
Copy link

ghost commented Oct 2, 2019

It appears that this causes ICE #64964 (comment)

I'm currently retesting that by applying this PR on top of the rust compilation that succeeded without it.confirmed

@nikomatsakis nikomatsakis added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Oct 3, 2019
@nikomatsakis
Copy link
Contributor

I'd like to nominate this PR (and the follow-up #64989) for inclusion in beta. It's not the sort of thing I would normally include, but this error is (a) one of the most common problems I hear people complaining about with async-await and (b) particularly opaque without this fix. Moreover, it is early in the cycle, so it will presumably get a reasonable amount of testing before landing. The change is relatively low risk, in that it is confined to error reporting.

@nikomatsakis nikomatsakis mentioned this pull request Oct 3, 2019
@Centril Centril added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 3, 2019
@pnkfelix
Copy link
Contributor

pnkfelix commented Oct 3, 2019

discussed at rustc meeting. accepted for beta backport in tandem with PR #64989 (since that fixes an ICE this injected).

@pnkfelix pnkfelix added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Oct 3, 2019
@Mark-Simulacrum Mark-Simulacrum removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Oct 22, 2019
bors added a commit that referenced this pull request Oct 24, 2019
[beta] backport rollup

This includes a bunch of PRs:
 *  Fix redundant semicolon lint interaction with proc macro attributes #64387
 *  Upgrade async/await to "used" keywords. #64875
 *  syntax: fix dropping of attribute on first param of non-method assocated fn #64894
 *  async/await: improve not-send errors #64895
 *  Silence unreachable code lint from await desugaring #64930
 *  Always mark rust and rust-call abi's as unwind #65020
 *  Account for macro invocation in `let mut $pat` diagnostic. #65123
 *  Ensure that associated `async fn`s have unique fresh param names #65142
 *  Add troubleshooting section to PGO chapter in rustc book. #65402
 *  Upgrade GCC to 8.3.0, glibc to 1.17.0 and crosstool-ng to 1.24.0 for dist-armv7-linux #65302
 *  Optimize `try_expand_impl_trait_type` #65293
 *  use precalculated dominators in explain_borrow #65172
 *  Fix ICE #64964 #64989
bors added a commit that referenced this pull request Oct 26, 2019
[beta] backport rollup

This includes a bunch of PRs:
 *  Fix redundant semicolon lint interaction with proc macro attributes #64387
 *  Upgrade async/await to "used" keywords. #64875
 *  syntax: fix dropping of attribute on first param of non-method assocated fn #64894
 *  async/await: improve not-send errors #64895
 *  Silence unreachable code lint from await desugaring #64930
 *  Always mark rust and rust-call abi's as unwind #65020
 *  Account for macro invocation in `let mut $pat` diagnostic. #65123
 *  Ensure that associated `async fn`s have unique fresh param names #65142
 *  Add troubleshooting section to PGO chapter in rustc book. #65402
 *  Upgrade GCC to 8.3.0, glibc to 1.17.0 and crosstool-ng to 1.24.0 for dist-armv7-linux #65302
 *  Optimize `try_expand_impl_trait_type` #65293
 *  use precalculated dominators in explain_borrow #65172
 *  Fix ICE #64964 #64989
 *  [beta] Revert "Auto merge of #62948 - matklad:failable-file-loading, r=petro… #65273
 *  save-analysis: Don't ICE when resolving qualified type paths in struct members #65353
 *  save-analysis: Nest tables when processing impl block definitions #65511
 *  Avoid ICE when checking `Destination` of `break` inside a closure #65518
 *  Avoid ICE when adjusting bad self ty #65755
 *  workaround msys2 bug #65762
bors added a commit that referenced this pull request Dec 11, 2019
…provements, r=nikomatsakis

async/await: improve not-send errors, part 2

Part of #64130. Fixes #65667.

This PR improves the errors introduced in #64895 so that they have specialized messages for `Send` and `Sync`.

r? @nikomatsakis
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants