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

E0373 help suggests move async but the correct syntax is async move #61920

Closed
sinkuu opened this issue Jun 18, 2019 · 7 comments
Closed

E0373 help suggests move async but the correct syntax is async move #61920

sinkuu opened this issue Jun 18, 2019 · 7 comments

Comments

@sinkuu
Copy link
Contributor

@sinkuu sinkuu commented Jun 18, 2019

E0373 help suggests appending move in front of async closures:

error[E0373]: closure may outlive the current function, but it borrows `n`, which is owned by the current function
  --> src/main.rs:10:19
   |
10 |         Box::pin((async || {
   |                   ^^^^^^^^ may outlive borrowed value `n`
11 |             Some((n, n + 1))
   |                   - `n` is borrowed here
   |
note: closure is returned here
  --> src/main.rs:10:9
   |
10 | /         Box::pin((async || {
11 | |             Some((n, n + 1))
12 | |         })())
   | |_____________^
help: to force the closure to take ownership of `n` (and any other referenced variables), use the `move` keyword
   |
10 |         Box::pin((move async || {
   |                   ^^^^^^^^^^^^^

but move async is not in the correct order:

error: expected one of `|` or `||`, found `async`
  --> src/main.rs:10:24
   |
10 |         Box::pin((move async || {
   |                        ^^^^^ expected one of `|` or `||` here

The suggestion should be async move || { ... instead.

Meta: rustc 1.37.0-nightly (4edff843d 2019-06-16)

@cramertj
Copy link
Member

@cramertj cramertj commented Jun 18, 2019

Marking as "deferred" because async closures are not a candidate for stabilization. async blocks do not give any similar suggestion, though it would be nice if they did:

error[E0597]: `a` does not live long enough
 --> src/main.rs:7:30
  |
4 |       let x = {
  |           - borrow later stored here
5 |           let a = 5;
6 |           async {
  |  _______________-
7 | |             println!("{:?}", a);
  | |                              ^ borrowed value does not live long enough
8 | |         }
  | |_________- value captured here by generator
9 |       };
  |       - `a` dropped here while still borrowed

@estebank
Copy link
Contributor

@estebank estebank commented Jun 18, 2019

Marking as "deferred" because async closures are not a candidate for stabilization

@cramertj if we're making the suggestion now then it is wrong and we should at least hide it. Users will get confused if they are offered a feature that is not yet stable, and even worse offered it with the wrong syntax.

cc @rust-lang/wg-diagnostics

@cramertj
Copy link
Member

@cramertj cramertj commented Jun 18, 2019

@estebank we're not making the suggestion for any stable code, or any code we intend to stabilize.

@estebank
Copy link
Contributor

@estebank estebank commented Jun 18, 2019

Oh! That's a different case entirely :)

@gilescope
Copy link
Contributor

@gilescope gilescope commented Jul 31, 2019

RipGrepped for "move async " in the current codebase and not found so I think someone's already fixed this? - move to close.

@sinkuu
Copy link
Contributor Author

@sinkuu sinkuu commented Jul 31, 2019

This is not fixed (playground - I should have provided one). IIRC E0373's suggestion just adds "move " in front of the closure span.

Centril added a commit to Centril/rust that referenced this issue Aug 19, 2019
…estebank

Fix suggestion from incorrect `move async` to `async move`.

PR for rust-lang#61920. Happy with the test. There must be a better implementation though - possibly a MIR visitor to estabilsh a span that doesn't include the `async` keyword?
@gilescope
Copy link
Contributor

@gilescope gilescope commented Aug 20, 2019

@estebank this issue is now fixed. Can you close it for me please?

@sinkuu sinkuu closed this Aug 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants