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

Implemented a compiler diagnostic for move async mistake #80160

Merged
merged 1 commit into from
Dec 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions compiler/rustc_parse/src/parser/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1912,4 +1912,22 @@ impl<'a> Parser<'a> {
*self = snapshot;
Err(err)
}

/// Get the diagnostics for the cases where `move async` is found.
///
/// `move_async_span` starts at the 'm' of the move keyword and ends with the 'c' of the async keyword
pub(super) fn incorrect_move_async_order_found(
&self,
move_async_span: Span,
) -> DiagnosticBuilder<'a> {
let mut err =
self.struct_span_err(move_async_span, "the order of `move` and `async` is incorrect");
err.span_suggestion_verbose(
move_async_span,
"try switching the order",
"async move".to_owned(),
Applicability::MaybeIncorrect,
);
err
}
}
18 changes: 14 additions & 4 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1603,7 +1603,7 @@ impl<'a> Parser<'a> {
self.sess.gated_spans.gate(sym::async_closure, span);
}

let capture_clause = self.parse_capture_clause();
let capture_clause = self.parse_capture_clause()?;
let decl = self.parse_fn_block_decl()?;
let decl_hi = self.prev_token.span;
let body = match decl.output {
Expand All @@ -1626,8 +1626,18 @@ impl<'a> Parser<'a> {
}

/// Parses an optional `move` prefix to a closure-like construct.
fn parse_capture_clause(&mut self) -> CaptureBy {
if self.eat_keyword(kw::Move) { CaptureBy::Value } else { CaptureBy::Ref }
fn parse_capture_clause(&mut self) -> PResult<'a, CaptureBy> {
davidtwco marked this conversation as resolved.
Show resolved Hide resolved
if self.eat_keyword(kw::Move) {
// Check for `move async` and recover
if self.check_keyword(kw::Async) {
let move_async_span = self.token.span.with_lo(self.prev_token.span.data().lo);
diondokter marked this conversation as resolved.
Show resolved Hide resolved
Err(self.incorrect_move_async_order_found(move_async_span))
} else {
Ok(CaptureBy::Value)
}
} else {
Ok(CaptureBy::Ref)
}
}

/// Parses the `|arg, arg|` header of a closure.
Expand Down Expand Up @@ -2018,7 +2028,7 @@ impl<'a> Parser<'a> {
fn parse_async_block(&mut self, mut attrs: AttrVec) -> PResult<'a, P<Expr>> {
let lo = self.token.span;
self.expect_keyword(kw::Async)?;
let capture_clause = self.parse_capture_clause();
let capture_clause = self.parse_capture_clause()?;
let (iattrs, body) = self.parse_inner_attrs_and_block()?;
attrs.extend(iattrs);
let kind = ExprKind::Async(capture_clause, DUMMY_NODE_ID, body);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// run-rustfix
// edition:2018

// Regression test for issue 79694

fn main() {
let _ = async move { }; //~ ERROR 7:13: 7:23: the order of `move` and `async` is incorrect
}
8 changes: 8 additions & 0 deletions src/test/ui/parser/incorrect-move-async-order-issue-79694.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// run-rustfix
// edition:2018

// Regression test for issue 79694

fn main() {
let _ = move async { }; //~ ERROR 7:13: 7:23: the order of `move` and `async` is incorrect
}
13 changes: 13 additions & 0 deletions src/test/ui/parser/incorrect-move-async-order-issue-79694.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
error: the order of `move` and `async` is incorrect
--> $DIR/incorrect-move-async-order-issue-79694.rs:7:13
|
LL | let _ = move async { };
| ^^^^^^^^^^
|
help: try switching the order
Copy link
Contributor

Choose a reason for hiding this comment

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

We should show the correct one rather than try switching the order. I opened a pull request for the order of pub async on function, that shows did you mean xxx yyy, easier to follow compared to 'switching the order'.

Copy link
Member

Choose a reason for hiding this comment

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

I requested this wording to remain consistent with other diagnostics. In the suggestion that follows the message, it does show the correct order. I don't mind seeing our wording here change, if you'd like to submit a PR for that, but I'd like to see all of the relevant diagnostics changed for consistency.

Copy link
Contributor

@pickfire pickfire Jan 9, 2021

Choose a reason for hiding this comment

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

error: the order of mut and ref is incorrect

But what if there are more than 2 items? Say, pub async const fn becomes async const pub fn? So we say "the order of pub and async and const is incorrect"?

|
LL | let _ = async move { };
| ^^^^^^^^^^

error: aborting due to previous error