From 4ac01f806f3fad93b75d0c1dc525ff3563458d03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= <39484203+jieyouxu@users.noreply.github.com> Date: Fri, 29 Nov 2024 16:36:20 +0800 Subject: [PATCH] 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. --- compiler/rustc_parse/src/parser/item.rs | 21 ++++++++++--- ...ble-safe-unsafe-keywords-extern-block-1.rs | 31 +++++++++++++++++++ ...safe-unsafe-keywords-extern-block-1.stderr | 14 +++++++++ ...ble-safe-unsafe-keywords-extern-block-2.rs | 15 +++++++++ ...safe-unsafe-keywords-extern-block-2.stderr | 14 +++++++++ ...safe-keywords-extern-fn-in-extern-block.rs | 11 +++++++ ...-keywords-extern-fn-in-extern-block.stderr | 20 ++++++++++++ ...mpatible-safe-unsafe-keywords-extern-fn.rs | 8 +++++ ...ible-safe-unsafe-keywords-extern-fn.stderr | 14 +++++++++ 9 files changed, 144 insertions(+), 4 deletions(-) create mode 100644 tests/ui/parser/incompatible-safe-unsafe-keywords-extern-block-1.rs create mode 100644 tests/ui/parser/incompatible-safe-unsafe-keywords-extern-block-1.stderr create mode 100644 tests/ui/parser/incompatible-safe-unsafe-keywords-extern-block-2.rs create mode 100644 tests/ui/parser/incompatible-safe-unsafe-keywords-extern-block-2.stderr create mode 100644 tests/ui/parser/incompatible-safe-unsafe-keywords-extern-fn-in-extern-block.rs create mode 100644 tests/ui/parser/incompatible-safe-unsafe-keywords-extern-fn-in-extern-block.stderr create mode 100644 tests/ui/parser/incompatible-safe-unsafe-keywords-extern-fn.rs create mode 100644 tests/ui/parser/incompatible-safe-unsafe-keywords-extern-fn.stderr diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index 26e81b7676bb1..3a6a4a0d0e1c4 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -2620,6 +2620,8 @@ impl<'a> Parser<'a> { enum WrongKw { Duplicated(Span), Misplaced(Span), + // E.g. `safe` *and* `unsafe` + Incompatible { first: Span, second: Span }, } // We may be able to recover @@ -2666,8 +2668,8 @@ impl<'a> Parser<'a> { match safety { Safety::Unsafe(sp) => Some(WrongKw::Duplicated(sp)), Safety::Safe(sp) => { - recover_safety = Safety::Unsafe(self.token.span); - Some(WrongKw::Misplaced(sp)) + // `safe unsafe` -- well which one is it? + Some(WrongKw::Incompatible { first: sp, second: self.token.span }) } Safety::Default => { recover_safety = Safety::Unsafe(self.token.span); @@ -2678,8 +2680,8 @@ impl<'a> Parser<'a> { match safety { Safety::Safe(sp) => Some(WrongKw::Duplicated(sp)), Safety::Unsafe(sp) => { - recover_safety = Safety::Safe(self.token.span); - Some(WrongKw::Misplaced(sp)) + // `unsafe safe` -- well which one is it? + Some(WrongKw::Incompatible { first: sp, second: self.token.span }) } Safety::Default => { recover_safety = Safety::Safe(self.token.span); @@ -2719,6 +2721,17 @@ impl<'a> Parser<'a> { ).note("keyword order for functions declaration is `pub`, `default`, `const`, `async`, `unsafe`, `extern`"); } } + // The user wrote incompatible keywords like `safe unsafe` or `unsafe safe` + else if let Some(WrongKw::Incompatible { first, second }) = wrong_kw { + if let Ok(first_kw) = self.span_to_snippet(first) + && let Ok(second_kw) = self.span_to_snippet(second) + { + err.span_help( + first.to(second), + format!("`{first_kw}` and `{second_kw}` are incompatible, use only one of the keywords"), + ); + } + } // Recover incorrect visibility order such as `async pub` else if self.check_keyword(kw::Pub) { let sp = sp_start.to(self.prev_token.span); diff --git a/tests/ui/parser/incompatible-safe-unsafe-keywords-extern-block-1.rs b/tests/ui/parser/incompatible-safe-unsafe-keywords-extern-block-1.rs new file mode 100644 index 0000000000000..d8e2b6b731ad4 --- /dev/null +++ b/tests/ui/parser/incompatible-safe-unsafe-keywords-extern-block-1.rs @@ -0,0 +1,31 @@ +//! Check that we don't try to suggest reordering incompatible keywords `safe` and `unsafe` when +//! parsing things that looks like fn frontmatter/extern blocks. +//! +//! # Context +//! +//! Previously, there was some recovery logic related to misplaced keywords (e.g. `safe` and +//! `unsafe`) when we tried to parse fn frontmatter (this is what happens when trying to parse +//! something malformed like `unsafe safe extern {}` or `safe unsafe extern {}`). Unfortunately, the +//! recovery logic only really handled duplicate keywords or misplaced keywords. This meant that +//! incompatible keywords like {`unsafe`, `safe`} when used together produces some funny suggestion +//! e.g. +//! +//! ```text +//! help: `unsafe` must come before `safe`: `unsafe safe` +//! ``` +//! +//! and then if you applied that suggestion, another suggestion in the recovery logic will tell you +//! to flip it back, ad infinitum. +//! +//! # References +//! +//! See . +//! +//! See `incompatible-safe-unsafe-keywords-extern-block-2.rs` for the `safe unsafe extern {}` +//! version. +#![crate_type = "lib"] + +safe unsafe extern {} +//~^ ERROR expected one of `extern` or `fn`, found keyword `unsafe` +//~| NOTE expected one of `extern` or `fn` +//~| HELP `safe` and `unsafe` are incompatible, use only one of the keywords diff --git a/tests/ui/parser/incompatible-safe-unsafe-keywords-extern-block-1.stderr b/tests/ui/parser/incompatible-safe-unsafe-keywords-extern-block-1.stderr new file mode 100644 index 0000000000000..fda06d60fdc70 --- /dev/null +++ b/tests/ui/parser/incompatible-safe-unsafe-keywords-extern-block-1.stderr @@ -0,0 +1,14 @@ +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 + diff --git a/tests/ui/parser/incompatible-safe-unsafe-keywords-extern-block-2.rs b/tests/ui/parser/incompatible-safe-unsafe-keywords-extern-block-2.rs new file mode 100644 index 0000000000000..c603b70f0634d --- /dev/null +++ b/tests/ui/parser/incompatible-safe-unsafe-keywords-extern-block-2.rs @@ -0,0 +1,15 @@ +//! Check that we don't try to suggest reordering incompatible keywords `safe` and `unsafe` when +//! parsing things that looks like fn frontmatter/extern blocks. +//! +//! # References +//! +//! See . +//! +//! See `incompatible-safe-unsafe-keywords-extern-block-1.rs` for the `unsafe safqe extern {}` +//! version. +#![crate_type = "lib"] + +unsafe safe extern {} +//~^ ERROR expected one of `extern` or `fn`, found `safe` +//~| NOTE expected one of `extern` or `fn` +//~| HELP `unsafe` and `safe` are incompatible, use only one of the keywords diff --git a/tests/ui/parser/incompatible-safe-unsafe-keywords-extern-block-2.stderr b/tests/ui/parser/incompatible-safe-unsafe-keywords-extern-block-2.stderr new file mode 100644 index 0000000000000..70f7b524d5846 --- /dev/null +++ b/tests/ui/parser/incompatible-safe-unsafe-keywords-extern-block-2.stderr @@ -0,0 +1,14 @@ +error: expected one of `extern` or `fn`, found `safe` + --> $DIR/incompatible-safe-unsafe-keywords-extern-block-2.rs:12:8 + | +LL | unsafe safe extern {} + | ^^^^ expected one of `extern` or `fn` + | +help: `unsafe` and `safe` are incompatible, use only one of the keywords + --> $DIR/incompatible-safe-unsafe-keywords-extern-block-2.rs:12:1 + | +LL | unsafe safe extern {} + | ^^^^^^^^^^^ + +error: aborting due to 1 previous error + diff --git a/tests/ui/parser/incompatible-safe-unsafe-keywords-extern-fn-in-extern-block.rs b/tests/ui/parser/incompatible-safe-unsafe-keywords-extern-fn-in-extern-block.rs new file mode 100644 index 0000000000000..cfb0f9cd941b1 --- /dev/null +++ b/tests/ui/parser/incompatible-safe-unsafe-keywords-extern-fn-in-extern-block.rs @@ -0,0 +1,11 @@ +//! Check that we emit a targeted suggestion for an extern fn that uses incompatible `safe` and +//! `unsafe` keywords. +#![crate_type = "lib"] + +unsafe extern { +//~^ NOTE while parsing this item list starting here + pub safe unsafe extern fn foo() {} + //~^ ERROR expected one of `extern` or `fn`, found keyword `unsafe` + //~| NOTE expected one of `extern` or `fn` + //~| `safe` and `unsafe` are incompatible, use only one of the keywords +} //~ NOTE the item list ends here diff --git a/tests/ui/parser/incompatible-safe-unsafe-keywords-extern-fn-in-extern-block.stderr b/tests/ui/parser/incompatible-safe-unsafe-keywords-extern-fn-in-extern-block.stderr new file mode 100644 index 0000000000000..8dbb97d4b76f9 --- /dev/null +++ b/tests/ui/parser/incompatible-safe-unsafe-keywords-extern-fn-in-extern-block.stderr @@ -0,0 +1,20 @@ +error: expected one of `extern` or `fn`, found keyword `unsafe` + --> $DIR/incompatible-safe-unsafe-keywords-extern-fn-in-extern-block.rs:7:14 + | +LL | unsafe extern { + | - while parsing this item list starting here +LL | +LL | pub safe unsafe extern fn foo() {} + | ^^^^^^ expected one of `extern` or `fn` +... +LL | } + | - the item list ends here + | +help: `safe` and `unsafe` are incompatible, use only one of the keywords + --> $DIR/incompatible-safe-unsafe-keywords-extern-fn-in-extern-block.rs:7:9 + | +LL | pub safe unsafe extern fn foo() {} + | ^^^^^^^^^^^ + +error: aborting due to 1 previous error + diff --git a/tests/ui/parser/incompatible-safe-unsafe-keywords-extern-fn.rs b/tests/ui/parser/incompatible-safe-unsafe-keywords-extern-fn.rs new file mode 100644 index 0000000000000..315303e9bfd01 --- /dev/null +++ b/tests/ui/parser/incompatible-safe-unsafe-keywords-extern-fn.rs @@ -0,0 +1,8 @@ +//! Check that we emit a targeted suggestion for an extern fn that uses incompatible `safe` and +//! `unsafe` keywords. +#![crate_type = "lib"] + +pub safe unsafe extern fn foo() {} +//~^ ERROR expected one of `extern` or `fn`, found keyword `unsafe` +//~| NOTE expected one of `extern` or `fn` +//~| `safe` and `unsafe` are incompatible, use only one of the keywords diff --git a/tests/ui/parser/incompatible-safe-unsafe-keywords-extern-fn.stderr b/tests/ui/parser/incompatible-safe-unsafe-keywords-extern-fn.stderr new file mode 100644 index 0000000000000..1598d374d5897 --- /dev/null +++ b/tests/ui/parser/incompatible-safe-unsafe-keywords-extern-fn.stderr @@ -0,0 +1,14 @@ +error: expected one of `extern` or `fn`, found keyword `unsafe` + --> $DIR/incompatible-safe-unsafe-keywords-extern-fn.rs:5:10 + | +LL | pub safe unsafe extern fn foo() {} + | ^^^^^^ expected one of `extern` or `fn` + | +help: `safe` and `unsafe` are incompatible, use only one of the keywords + --> $DIR/incompatible-safe-unsafe-keywords-extern-fn.rs:5:5 + | +LL | pub safe unsafe extern fn foo() {} + | ^^^^^^^^^^^ + +error: aborting due to 1 previous error +