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

Recover from incorrectly ordered/duplicated function keywords #117282

Merged
merged 1 commit into from
Nov 8, 2023
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
40 changes: 37 additions & 3 deletions compiler/rustc_parse/src/parser/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2382,22 +2382,39 @@ impl<'a> Parser<'a> {
Misplaced(Span),
}

// We may be able to recover
let mut recover_constness = constness;
let mut recover_asyncness = asyncness;
let mut recover_unsafety = unsafety;
// This will allow the machine fix to directly place the keyword in the correct place or to indicate
// that the keyword is already present and the second instance should be removed.
let wrong_kw = if self.check_keyword(kw::Const) {
match constness {
Const::Yes(sp) => Some(WrongKw::Duplicated(sp)),
Const::No => Some(WrongKw::Misplaced(async_start_sp)),
Const::No => {
recover_constness = Const::Yes(self.token.span);
Some(WrongKw::Misplaced(async_start_sp))
}
}
} else if self.check_keyword(kw::Async) {
match asyncness {
Async::Yes { span, .. } => Some(WrongKw::Duplicated(span)),
Async::No => Some(WrongKw::Misplaced(unsafe_start_sp)),
Async::No => {
recover_asyncness = Async::Yes {
span: self.token.span,
closure_id: DUMMY_NODE_ID,
return_impl_trait_id: DUMMY_NODE_ID,
};
Some(WrongKw::Misplaced(unsafe_start_sp))
}
}
} else if self.check_keyword(kw::Unsafe) {
match unsafety {
Unsafe::Yes(sp) => Some(WrongKw::Duplicated(sp)),
Unsafe::No => Some(WrongKw::Misplaced(ext_start_sp)),
Unsafe::No => {
recover_unsafety = Unsafe::Yes(self.token.span);
Some(WrongKw::Misplaced(ext_start_sp))
}
}
} else {
None
Expand Down Expand Up @@ -2467,6 +2484,23 @@ impl<'a> Parser<'a> {
}
}
}

if wrong_kw.is_some()
&& self.may_recover()
&& self.look_ahead(1, |tok| tok.is_keyword_case(kw::Fn, case))
{
// Advance past the misplaced keyword and `fn`
self.bump();
self.bump();
err.emit();
return Ok(FnHeader {
constness: recover_constness,
unsafety: recover_unsafety,
asyncness: recover_asyncness,
ext,
});
}

return Err(err);
}
}
Expand Down
1 change: 1 addition & 0 deletions tests/ui/async-await/no-async-const.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@

pub async const fn x() {}
//~^ ERROR expected one of `extern`, `fn`, or `unsafe`, found keyword `const`
//~| ERROR functions cannot be both `const` and `async`
11 changes: 10 additions & 1 deletion tests/ui/async-await/no-async-const.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,14 @@ LL | pub async const fn x() {}
|
= note: keyword order for functions declaration is `pub`, `default`, `const`, `async`, `unsafe`, `extern`

error: aborting due to previous error
error: functions cannot be both `const` and `async`
--> $DIR/no-async-const.rs:4:5
|
LL | pub async const fn x() {}
| ----^^^^^-^^^^^----------
| | |
| | `const` because of this
| `async` because of this

error: aborting due to 2 previous errors

2 changes: 2 additions & 0 deletions tests/ui/async-await/no-unsafe-async.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,5 @@ impl S {

#[cfg(FALSE)]
unsafe async fn f() {} //~ ERROR expected one of `extern` or `fn`, found keyword `async`

fn main() {}
5 changes: 0 additions & 5 deletions tests/ui/async-await/no-unsafe-async.stderr
Original file line number Diff line number Diff line change
@@ -1,16 +1,11 @@
error: expected one of `extern` or `fn`, found keyword `async`
--> $DIR/no-unsafe-async.rs:7:12
|
LL | impl S {
| - while parsing this item list starting here
LL | #[cfg(FALSE)]
LL | unsafe async fn g() {}
| -------^^^^^
| | |
| | expected one of `extern` or `fn`
| help: `async` must come before `unsafe`: `async unsafe`
LL | }
| - the item list ends here
Comment on lines -4 to -13
Copy link
Member

Choose a reason for hiding this comment

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

Ideally, I think it's best to leave this as it is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, this message indicates that it wasn't able to recover the rest of the item list if I understand right? I think this is an improvement as it's able to continue parsing other items in the list, so the message isn't really necessary

|
= note: keyword order for functions declaration is `pub`, `default`, `const`, `async`, `unsafe`, `extern`

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,8 @@ const async const fn test() {}
//~| NOTE expected one of `extern`, `fn`, or `unsafe`
//~| HELP `const` already used earlier, remove this one
//~| NOTE `const` first seen here
//~| ERROR functions cannot be both `const` and `async`
//~| NOTE `const` because of this
//~| NOTE `async` because of this

fn main() {}
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,14 @@ note: `const` first seen here
LL | const async const fn test() {}
| ^^^^^

error: aborting due to previous error
error: functions cannot be both `const` and `async`
--> $DIR/const-async-const.rs:5:1
|
LL | const async const fn test() {}
| ^^^^^-^^^^^-------------------
| | |
| | `async` because of this
| `const` because of this

error: aborting due to 2 previous errors

22 changes: 22 additions & 0 deletions tests/ui/parser/issues/issue-87217-keyword-order/recovery.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// test for #115714

struct Misplaced;

impl Misplaced {
unsafe const fn from_u32(val: u32) {}
//~^ ERROR expected one of `extern` or `fn`
fn oof(self){}
}

struct Duplicated;

impl Duplicated {
unsafe unsafe fn from_u32(val: u32) {}
//~^ ERROR expected one of `extern` or `fn`
fn oof(self){}
}

fn main() {
Misplaced.oof();
Duplicated.oof();
}
28 changes: 28 additions & 0 deletions tests/ui/parser/issues/issue-87217-keyword-order/recovery.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
error: expected one of `extern` or `fn`, found keyword `const`
--> $DIR/recovery.rs:6:12
|
LL | unsafe const fn from_u32(val: u32) {}
| -------^^^^^
| | |
| | expected one of `extern` or `fn`
| help: `const` must come before `unsafe`: `const unsafe`
|
= note: keyword order for functions declaration is `pub`, `default`, `const`, `async`, `unsafe`, `extern`

error: expected one of `extern` or `fn`, found keyword `unsafe`
--> $DIR/recovery.rs:14:12
|
LL | unsafe unsafe fn from_u32(val: u32) {}
| ^^^^^^
| |
| expected one of `extern` or `fn`
| help: `unsafe` already used earlier, remove this one
|
note: `unsafe` first seen here
--> $DIR/recovery.rs:14:5
|
LL | unsafe unsafe fn from_u32(val: u32) {}
| ^^^^^^

error: aborting due to 2 previous errors

Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,8 @@ async unsafe const fn test() {}
//~| HELP `const` must come before `async unsafe`
//~| SUGGESTION const async unsafe
//~| NOTE keyword order for functions declaration is `pub`, `default`, `const`, `async`, `unsafe`, `extern`
//~| ERROR functions cannot be both `const` and `async`
//~| NOTE `const` because of this
//~| NOTE `async` because of this

fn main() {}
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,14 @@ LL | async unsafe const fn test() {}
|
= note: keyword order for functions declaration is `pub`, `default`, `const`, `async`, `unsafe`, `extern`

error: aborting due to previous error
error: functions cannot be both `const` and `async`
--> $DIR/several-kw-jump.rs:9:1
|
LL | async unsafe const fn test() {}
| ^^^^^--------^^^^^-------------
| | |
| | `const` because of this
| `async` because of this

error: aborting due to 2 previous errors

Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,5 @@ unsafe async fn test() {}
//~| HELP `async` must come before `unsafe`
//~| SUGGESTION async unsafe
//~| NOTE keyword order for functions declaration is `pub`, `default`, `const`, `async`, `unsafe`, `extern`

fn main() {}
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,5 @@ unsafe const fn test() {}
//~| HELP `const` must come before `unsafe`
//~| SUGGESTION const unsafe
//~| NOTE keyword order for functions declaration is `pub`, `default`, `const`, `async`, `unsafe`, `extern`

fn main() {}
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,5 @@ extern unsafe fn test() {}
//~| HELP `unsafe` must come before `extern`
//~| SUGGESTION unsafe extern
//~| NOTE keyword order for functions declaration is `pub`, `default`, `const`, `async`, `unsafe`, `extern`

fn main() {}
Loading