Skip to content

Commit 4ac01f8

Browse files
jieyouxujieyouxu
committed
Add targeted suggestion for incompatible {un,}safe keywords when parsing fn-frontmatter-like fragments
The diagnostics now look like: ``` error: expected one of `extern` or `fn`, found keyword `unsafe` --> $DIR/incompatible-safe-unsafe-keywords-extern-block-1.rs:28:6 | LL | safe unsafe extern {} | ^^^^^^ expected one of `extern` or `fn` | help: `safe` and `unsafe` are incompatible, use only one of the keywords --> $DIR/incompatible-safe-unsafe-keywords-extern-block-1.rs:28:1 | LL | safe unsafe extern {} | ^^^^^^^^^^^ error: aborting due to 1 previous error ``` whereas previously two suggestions created a cycle prompting the user to keep reordering `unsafe` and `safe`. No suggestions are offered here because the user needs to pick one, and user intent is not clear-cut.
1 parent d6f8829 commit 4ac01f8

9 files changed

+144
-4
lines changed

‎compiler/rustc_parse/src/parser/item.rs

+17-4
Original file line numberDiff line numberDiff line change
@@ -2620,6 +2620,8 @@ impl<'a> Parser<'a> {
26202620
enum WrongKw {
26212621
Duplicated(Span),
26222622
Misplaced(Span),
2623+
// E.g. `safe` *and* `unsafe`
2624+
Incompatible { first: Span, second: Span },
26232625
}
26242626

26252627
// We may be able to recover
@@ -2666,8 +2668,8 @@ impl<'a> Parser<'a> {
26662668
match safety {
26672669
Safety::Unsafe(sp) => Some(WrongKw::Duplicated(sp)),
26682670
Safety::Safe(sp) => {
2669-
recover_safety = Safety::Unsafe(self.token.span);
2670-
Some(WrongKw::Misplaced(sp))
2671+
// `safe unsafe` -- well which one is it?
2672+
Some(WrongKw::Incompatible { first: sp, second: self.token.span })
26712673
}
26722674
Safety::Default => {
26732675
recover_safety = Safety::Unsafe(self.token.span);
@@ -2678,8 +2680,8 @@ impl<'a> Parser<'a> {
26782680
match safety {
26792681
Safety::Safe(sp) => Some(WrongKw::Duplicated(sp)),
26802682
Safety::Unsafe(sp) => {
2681-
recover_safety = Safety::Safe(self.token.span);
2682-
Some(WrongKw::Misplaced(sp))
2683+
// `unsafe safe` -- well which one is it?
2684+
Some(WrongKw::Incompatible { first: sp, second: self.token.span })
26832685
}
26842686
Safety::Default => {
26852687
recover_safety = Safety::Safe(self.token.span);
@@ -2719,6 +2721,17 @@ impl<'a> Parser<'a> {
27192721
).note("keyword order for functions declaration is `pub`, `default`, `const`, `async`, `unsafe`, `extern`");
27202722
}
27212723
}
2724+
// The user wrote incompatible keywords like `safe unsafe` or `unsafe safe`
2725+
else if let Some(WrongKw::Incompatible { first, second }) = wrong_kw {
2726+
if let Ok(first_kw) = self.span_to_snippet(first)
2727+
&& let Ok(second_kw) = self.span_to_snippet(second)
2728+
{
2729+
err.span_help(
2730+
first.to(second),
2731+
format!("`{first_kw}` and `{second_kw}` are incompatible, use only one of the keywords"),
2732+
);
2733+
}
2734+
}
27222735
// Recover incorrect visibility order such as `async pub`
27232736
else if self.check_keyword(kw::Pub) {
27242737
let sp = sp_start.to(self.prev_token.span);

‎tests/ui/parser/incompatible-safe-unsafe-keywords-extern-block-1.rs

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
//! Check that we don't try to suggest reordering incompatible keywords `safe` and `unsafe` when
2+
//! parsing things that looks like fn frontmatter/extern blocks.
3+
//!
4+
//! # Context
5+
//!
6+
//! Previously, there was some recovery logic related to misplaced keywords (e.g. `safe` and
7+
//! `unsafe`) when we tried to parse fn frontmatter (this is what happens when trying to parse
8+
//! something malformed like `unsafe safe extern {}` or `safe unsafe extern {}`). Unfortunately, the
9+
//! recovery logic only really handled duplicate keywords or misplaced keywords. This meant that
10+
//! incompatible keywords like {`unsafe`, `safe`} when used together produces some funny suggestion
11+
//! e.g.
12+
//!
13+
//! ```text
14+
//! help: `unsafe` must come before `safe`: `unsafe safe`
15+
//! ```
16+
//!
17+
//! and then if you applied that suggestion, another suggestion in the recovery logic will tell you
18+
//! to flip it back, ad infinitum.
19+
//!
20+
//! # References
21+
//!
22+
//! See <https://github.com/rust-lang/rust/issues/133586>.
23+
//!
24+
//! See `incompatible-safe-unsafe-keywords-extern-block-2.rs` for the `safe unsafe extern {}`
25+
//! version.
26+
#![crate_type = "lib"]
27+
28+
safe unsafe extern {}
29+
//~^ ERROR expected one of `extern` or `fn`, found keyword `unsafe`
30+
//~| NOTE expected one of `extern` or `fn`
31+
//~| HELP `safe` and `unsafe` are incompatible, use only one of the keywords

‎tests/ui/parser/incompatible-safe-unsafe-keywords-extern-block-1.stderr

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
error: expected one of `extern` or `fn`, found keyword `unsafe`
2+
--> $DIR/incompatible-safe-unsafe-keywords-extern-block-1.rs:28:6
3+
|
4+
LL | safe unsafe extern {}
5+
| ^^^^^^ expected one of `extern` or `fn`
6+
|
7+
help: `safe` and `unsafe` are incompatible, use only one of the keywords
8+
--> $DIR/incompatible-safe-unsafe-keywords-extern-block-1.rs:28:1
9+
|
10+
LL | safe unsafe extern {}
11+
| ^^^^^^^^^^^
12+
13+
error: aborting due to 1 previous error
14+

‎tests/ui/parser/incompatible-safe-unsafe-keywords-extern-block-2.rs

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
//! Check that we don't try to suggest reordering incompatible keywords `safe` and `unsafe` when
2+
//! parsing things that looks like fn frontmatter/extern blocks.
3+
//!
4+
//! # References
5+
//!
6+
//! See <https://github.com/rust-lang/rust/issues/133586>.
7+
//!
8+
//! See `incompatible-safe-unsafe-keywords-extern-block-1.rs` for the `unsafe safqe extern {}`
9+
//! version.
10+
#![crate_type = "lib"]
11+
12+
unsafe safe extern {}
13+
//~^ ERROR expected one of `extern` or `fn`, found `safe`
14+
//~| NOTE expected one of `extern` or `fn`
15+
//~| HELP `unsafe` and `safe` are incompatible, use only one of the keywords

‎tests/ui/parser/incompatible-safe-unsafe-keywords-extern-block-2.stderr

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
error: expected one of `extern` or `fn`, found `safe`
2+
--> $DIR/incompatible-safe-unsafe-keywords-extern-block-2.rs:12:8
3+
|
4+
LL | unsafe safe extern {}
5+
| ^^^^ expected one of `extern` or `fn`
6+
|
7+
help: `unsafe` and `safe` are incompatible, use only one of the keywords
8+
--> $DIR/incompatible-safe-unsafe-keywords-extern-block-2.rs:12:1
9+
|
10+
LL | unsafe safe extern {}
11+
| ^^^^^^^^^^^
12+
13+
error: aborting due to 1 previous error
14+

‎tests/ui/parser/incompatible-safe-unsafe-keywords-extern-fn-in-extern-block.rs

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
//! Check that we emit a targeted suggestion for an extern fn that uses incompatible `safe` and
2+
//! `unsafe` keywords.
3+
#![crate_type = "lib"]
4+
5+
unsafe extern {
6+
//~^ NOTE while parsing this item list starting here
7+
pub safe unsafe extern fn foo() {}
8+
//~^ ERROR expected one of `extern` or `fn`, found keyword `unsafe`
9+
//~| NOTE expected one of `extern` or `fn`
10+
//~| `safe` and `unsafe` are incompatible, use only one of the keywords
11+
} //~ NOTE the item list ends here

‎tests/ui/parser/incompatible-safe-unsafe-keywords-extern-fn-in-extern-block.stderr

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
error: expected one of `extern` or `fn`, found keyword `unsafe`
2+
--> $DIR/incompatible-safe-unsafe-keywords-extern-fn-in-extern-block.rs:7:14
3+
|
4+
LL | unsafe extern {
5+
| - while parsing this item list starting here
6+
LL |
7+
LL | pub safe unsafe extern fn foo() {}
8+
| ^^^^^^ expected one of `extern` or `fn`
9+
...
10+
LL | }
11+
| - the item list ends here
12+
|
13+
help: `safe` and `unsafe` are incompatible, use only one of the keywords
14+
--> $DIR/incompatible-safe-unsafe-keywords-extern-fn-in-extern-block.rs:7:9
15+
|
16+
LL | pub safe unsafe extern fn foo() {}
17+
| ^^^^^^^^^^^
18+
19+
error: aborting due to 1 previous error
20+

‎tests/ui/parser/incompatible-safe-unsafe-keywords-extern-fn.rs

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
//! Check that we emit a targeted suggestion for an extern fn that uses incompatible `safe` and
2+
//! `unsafe` keywords.
3+
#![crate_type = "lib"]
4+
5+
pub safe unsafe extern fn foo() {}
6+
//~^ ERROR expected one of `extern` or `fn`, found keyword `unsafe`
7+
//~| NOTE expected one of `extern` or `fn`
8+
//~| `safe` and `unsafe` are incompatible, use only one of the keywords

‎tests/ui/parser/incompatible-safe-unsafe-keywords-extern-fn.stderr

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
error: expected one of `extern` or `fn`, found keyword `unsafe`
2+
--> $DIR/incompatible-safe-unsafe-keywords-extern-fn.rs:5:10
3+
|
4+
LL | pub safe unsafe extern fn foo() {}
5+
| ^^^^^^ expected one of `extern` or `fn`
6+
|
7+
help: `safe` and `unsafe` are incompatible, use only one of the keywords
8+
--> $DIR/incompatible-safe-unsafe-keywords-extern-fn.rs:5:5
9+
|
10+
LL | pub safe unsafe extern fn foo() {}
11+
| ^^^^^^^^^^^
12+
13+
error: aborting due to 1 previous error
14+

0 commit comments

Comments
 (0)