From 371481765cec20db5bb61c3ae8d2c94e31cc89ad Mon Sep 17 00:00:00 2001 From: EliseZeroTwo Date: Wed, 20 Dec 2023 07:42:29 -0700 Subject: [PATCH] fix: better error cases with bad/missing identifiers in MBEs --- compiler/rustc_parse/messages.ftl | 4 ++ compiler/rustc_parse/src/errors.rs | 7 +++ compiler/rustc_parse/src/parser/item.rs | 58 ++++++++++++++----- compiler/rustc_resolve/messages.ftl | 2 - compiler/rustc_resolve/src/diagnostics.rs | 7 +-- compiler/rustc_resolve/src/errors.rs | 7 --- tests/ui/macros/issue-118786.rs | 4 +- tests/ui/macros/issue-118786.stderr | 46 ++------------- tests/ui/macros/mbe-missing-ident-error.rs | 7 +++ .../ui/macros/mbe-missing-ident-error.stderr | 10 ++++ .../ui/macros/mbe-parenthesis-ident-error.rs | 7 +++ .../macros/mbe-parenthesis-ident-error.stderr | 13 +++++ tests/ui/macros/user-defined-macro-rules.rs | 13 ++--- .../ui/macros/user-defined-macro-rules.stderr | 16 +++++ tests/ui/resolve/issue-118295.rs | 5 -- tests/ui/resolve/issue-118295.stderr | 14 ----- 16 files changed, 122 insertions(+), 98 deletions(-) create mode 100644 tests/ui/macros/mbe-missing-ident-error.rs create mode 100644 tests/ui/macros/mbe-missing-ident-error.stderr create mode 100644 tests/ui/macros/mbe-parenthesis-ident-error.rs create mode 100644 tests/ui/macros/mbe-parenthesis-ident-error.stderr create mode 100644 tests/ui/macros/user-defined-macro-rules.stderr delete mode 100644 tests/ui/resolve/issue-118295.rs delete mode 100644 tests/ui/resolve/issue-118295.stderr diff --git a/compiler/rustc_parse/messages.ftl b/compiler/rustc_parse/messages.ftl index 363b8f4bfb9cc..5b0c5ec1eeb43 100644 --- a/compiler/rustc_parse/messages.ftl +++ b/compiler/rustc_parse/messages.ftl @@ -468,6 +468,8 @@ parse_macro_name_remove_bang = macro names aren't followed by a `!` parse_macro_rules_missing_bang = expected `!` after `macro_rules` .suggestion = add a `!` +parse_macro_rules_named_macro_rules = user-defined macros may not be named `macro_rules` + parse_macro_rules_visibility = can't qualify macro_rules invocation with `{$vis}` .suggestion = try exporting the macro @@ -497,6 +499,8 @@ parse_maybe_fn_typo_with_impl = you might have meant to write `impl` instead of parse_maybe_missing_let = you might have meant to continue the let-chain +parse_maybe_missing_macro_rules_name = maybe you have forgotten to define a name for this `macro_rules!` + parse_maybe_recover_from_bad_qpath_stage_2 = missing angle brackets in associated item path .suggestion = types that don't start with an identifier need to be surrounded with angle brackets in qualified paths diff --git a/compiler/rustc_parse/src/errors.rs b/compiler/rustc_parse/src/errors.rs index 008adcc83d0ea..a55ee17373676 100644 --- a/compiler/rustc_parse/src/errors.rs +++ b/compiler/rustc_parse/src/errors.rs @@ -2664,6 +2664,13 @@ pub(crate) struct MacroRulesMissingBang { pub hi: Span, } +#[derive(Diagnostic)] +#[diag(parse_macro_rules_named_macro_rules)] +pub(crate) struct MacroRulesNamedMacroRules { + #[primary_span] + pub span: Span, +} + #[derive(Diagnostic)] #[diag(parse_macro_name_remove_bang)] pub(crate) struct MacroNameRemoveBang { diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index bf619daba5013..f8abf0133178b 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -2036,23 +2036,26 @@ impl<'a> Parser<'a> { /// Is this a possibly malformed start of a `macro_rules! foo` item definition? fn is_macro_rules_item(&mut self) -> IsMacroRulesItem { - if self.check_keyword(kw::MacroRules) { - let macro_rules_span = self.token.span; - - if self.look_ahead(1, |t| *t == token::Not) && self.look_ahead(2, |t| t.is_ident()) { - return IsMacroRulesItem::Yes { has_bang: true }; - } else if self.look_ahead(1, |t| (t.is_ident())) { - // macro_rules foo - self.sess.emit_err(errors::MacroRulesMissingBang { - span: macro_rules_span, - hi: macro_rules_span.shrink_to_hi(), - }); + if !self.check_keyword(kw::MacroRules) { + return IsMacroRulesItem::No; + } + + let macro_rules_span = self.token.span; + let has_bang = self.look_ahead(1, |t| *t == token::Not); - return IsMacroRulesItem::Yes { has_bang: false }; + // macro_rules foo + if !has_bang { + if !self.look_ahead(1, |t| (t.is_ident())) { + return IsMacroRulesItem::No; } + + self.sess.emit_err(errors::MacroRulesMissingBang { + span: macro_rules_span, + hi: macro_rules_span.shrink_to_hi(), + }); } - IsMacroRulesItem::No + IsMacroRulesItem::Yes { has_bang } } /// Parses a `macro_rules! foo { ... }` declarative macro. @@ -2066,7 +2069,34 @@ impl<'a> Parser<'a> { if has_bang { self.expect(&token::Not)?; // `!` } - let ident = self.parse_ident()?; + + let ident = match self.parse_ident() { + Ok(ident) => ident, + Err(mut err) => { + if let TokenKind::OpenDelim(Delimiter::Parenthesis) = self.token.kind + && let Some((Ident { name, .. }, _)) = self.look_ahead(1, |token| token.ident()) + && let Some(closing_span) = self.look_ahead(2, |token| { + (token.kind == TokenKind::CloseDelim(Delimiter::Parenthesis)) + .then_some(token.span) + }) + { + err.span_suggestion( + self.token.span.with_hi(closing_span.hi()), + "try removing the parenthesis around the name for this `macro_rules!`", + name, + Applicability::MachineApplicable, + ); + } else { + err.help(fluent::parse_maybe_missing_macro_rules_name); + } + + return Err(err); + } + }; + + if ident.name == sym::macro_rules { + self.sess.emit_err(errors::MacroRulesNamedMacroRules { span: ident.span }); + } if self.eat(&token::Not) { // Handle macro_rules! foo! diff --git a/compiler/rustc_resolve/messages.ftl b/compiler/rustc_resolve/messages.ftl index 3f8df16e03f77..a5faaaab639a3 100644 --- a/compiler/rustc_resolve/messages.ftl +++ b/compiler/rustc_resolve/messages.ftl @@ -181,8 +181,6 @@ resolve_method_not_member_of_trait = method `{$method}` is not a member of trait `{$trait_}` .label = not a member of trait `{$trait_}` -resolve_missing_macro_rules_name = maybe you have forgotten to define a name for this `macro_rules!` - resolve_module_only = visibility must resolve to a module diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index 542aff69e3459..4b7aface28f4c 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -28,7 +28,7 @@ use rustc_span::{BytePos, Span, SyntaxContext}; use thin_vec::{thin_vec, ThinVec}; use crate::errors::{AddedMacroUse, ChangeImportBinding, ChangeImportBindingSuggestion}; -use crate::errors::{ConsiderAddingADerive, ExplicitUnsafeTraits, MaybeMissingMacroRulesName}; +use crate::errors::{ConsiderAddingADerive, ExplicitUnsafeTraits}; use crate::imports::{Import, ImportKind}; use crate::late::{PatternSource, Rib}; use crate::path_names_to_string; @@ -1419,11 +1419,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { "", ); - if macro_kind == MacroKind::Bang && ident.name == sym::macro_rules { - err.subdiagnostic(MaybeMissingMacroRulesName { span: ident.span }); - return; - } - if macro_kind == MacroKind::Derive && (ident.name == sym::Send || ident.name == sym::Sync) { err.subdiagnostic(ExplicitUnsafeTraits { span: ident.span, ident }); return; diff --git a/compiler/rustc_resolve/src/errors.rs b/compiler/rustc_resolve/src/errors.rs index 1fdb193e571f1..72ff959bbd632 100644 --- a/compiler/rustc_resolve/src/errors.rs +++ b/compiler/rustc_resolve/src/errors.rs @@ -665,13 +665,6 @@ pub(crate) struct ExplicitUnsafeTraits { pub(crate) ident: Ident, } -#[derive(Subdiagnostic)] -#[note(resolve_missing_macro_rules_name)] -pub(crate) struct MaybeMissingMacroRulesName { - #[primary_span] - pub(crate) span: Span, -} - #[derive(Subdiagnostic)] #[help(resolve_added_macro_use)] pub(crate) struct AddedMacroUse; diff --git a/tests/ui/macros/issue-118786.rs b/tests/ui/macros/issue-118786.rs index 84af3a651137e..88642600892ee 100644 --- a/tests/ui/macros/issue-118786.rs +++ b/tests/ui/macros/issue-118786.rs @@ -5,9 +5,7 @@ macro_rules! make_macro { ($macro_name:tt) => { macro_rules! $macro_name { - //~^ ERROR macros that expand to items must be delimited with braces or followed by a semicolon - //~| ERROR macro expansion ignores token `{` and any following - //~| ERROR cannot find macro `macro_rules` in this scope + //~^ ERROR: expected identifier, found `(` () => {} } } diff --git a/tests/ui/macros/issue-118786.stderr b/tests/ui/macros/issue-118786.stderr index ca3a40f31c1f5..9f35cc893e59b 100644 --- a/tests/ui/macros/issue-118786.stderr +++ b/tests/ui/macros/issue-118786.stderr @@ -1,47 +1,13 @@ -error: macros that expand to items must be delimited with braces or followed by a semicolon +error: expected identifier, found `(` --> $DIR/issue-118786.rs:7:22 | LL | macro_rules! $macro_name { - | ^^^^^^^^^^^ + | ^^^^^^^^^^^ expected identifier | -help: change the delimiters to curly braces +help: try removing the parenthesis around the name for this `macro_rules!` | -LL | macro_rules! {} { - | ~ + -help: add a semicolon - | -LL | macro_rules! $macro_name; { - | + - -error: macro expansion ignores token `{` and any following - --> $DIR/issue-118786.rs:7:34 - | -LL | macro_rules! $macro_name { - | ^ -... -LL | make_macro!((meow)); - | ------------------- caused by the macro expansion here - | - = note: the usage of `make_macro!` is likely invalid in item context - -error: cannot find macro `macro_rules` in this scope - --> $DIR/issue-118786.rs:7:9 - | -LL | macro_rules! $macro_name { - | ^^^^^^^^^^^ -... -LL | make_macro!((meow)); - | ------------------- in this macro invocation - | -note: maybe you have forgotten to define a name for this `macro_rules!` - --> $DIR/issue-118786.rs:7:9 - | -LL | macro_rules! $macro_name { - | ^^^^^^^^^^^ -... -LL | make_macro!((meow)); - | ------------------- in this macro invocation - = note: this error originates in the macro `make_macro` (in Nightly builds, run with -Z macro-backtrace for more info) +LL | macro_rules! meow { + | ~~~~ -error: aborting due to 3 previous errors +error: aborting due to 1 previous error diff --git a/tests/ui/macros/mbe-missing-ident-error.rs b/tests/ui/macros/mbe-missing-ident-error.rs new file mode 100644 index 0000000000000..9e9efca63ab00 --- /dev/null +++ b/tests/ui/macros/mbe-missing-ident-error.rs @@ -0,0 +1,7 @@ +// Ensures MBEs with a missing ident produce a readable error + +macro_rules! { + //~^ ERROR: expected identifier, found `{` + //~| HELP: maybe you have forgotten to define a name for this `macro_rules!` + () => {} +} diff --git a/tests/ui/macros/mbe-missing-ident-error.stderr b/tests/ui/macros/mbe-missing-ident-error.stderr new file mode 100644 index 0000000000000..b494c0ff57147 --- /dev/null +++ b/tests/ui/macros/mbe-missing-ident-error.stderr @@ -0,0 +1,10 @@ +error: expected identifier, found `{` + --> $DIR/mbe-missing-ident-error.rs:3:14 + | +LL | macro_rules! { + | ^ expected identifier + | + = help: maybe you have forgotten to define a name for this `macro_rules!` + +error: aborting due to 1 previous error + diff --git a/tests/ui/macros/mbe-parenthesis-ident-error.rs b/tests/ui/macros/mbe-parenthesis-ident-error.rs new file mode 100644 index 0000000000000..786f97cad70e2 --- /dev/null +++ b/tests/ui/macros/mbe-parenthesis-ident-error.rs @@ -0,0 +1,7 @@ +// Ensures MBEs with a invalid ident produce a readable error + +macro_rules! (meepmeep) { + //~^ ERROR: expected identifier, found `(` + //~| HELP: try removing the parenthesis around the name for this `macro_rules!` + () => {} +} diff --git a/tests/ui/macros/mbe-parenthesis-ident-error.stderr b/tests/ui/macros/mbe-parenthesis-ident-error.stderr new file mode 100644 index 0000000000000..4596b37b7ad73 --- /dev/null +++ b/tests/ui/macros/mbe-parenthesis-ident-error.stderr @@ -0,0 +1,13 @@ +error: expected identifier, found `(` + --> $DIR/mbe-parenthesis-ident-error.rs:3:14 + | +LL | macro_rules! (meepmeep) { + | ^ expected identifier + | +help: try removing the parenthesis around the name for this `macro_rules!` + | +LL | macro_rules! meepmeep { + | ~~~~~~~~ + +error: aborting due to 1 previous error + diff --git a/tests/ui/macros/user-defined-macro-rules.rs b/tests/ui/macros/user-defined-macro-rules.rs index 09e071ec45420..5d1eb826e8839 100644 --- a/tests/ui/macros/user-defined-macro-rules.rs +++ b/tests/ui/macros/user-defined-macro-rules.rs @@ -1,9 +1,8 @@ -// check-pass +// check-fail -macro_rules! macro_rules { () => { struct S; } } // OK +macro_rules! macro_rules { () => {} } +//~^ ERROR: user-defined macros may not be named `macro_rules` -macro_rules! {} // OK, calls the macro defined above - -fn main() { - let s = S; -} +macro_rules! {} +//~^ ERROR: expected identifier, found `{` +//~| HELP: maybe you have forgotten to define a name for this `macro_rules!` diff --git a/tests/ui/macros/user-defined-macro-rules.stderr b/tests/ui/macros/user-defined-macro-rules.stderr new file mode 100644 index 0000000000000..8fc8eda018194 --- /dev/null +++ b/tests/ui/macros/user-defined-macro-rules.stderr @@ -0,0 +1,16 @@ +error: user-defined macros may not be named `macro_rules` + --> $DIR/user-defined-macro-rules.rs:3:14 + | +LL | macro_rules! macro_rules { () => {} } + | ^^^^^^^^^^^ + +error: expected identifier, found `{` + --> $DIR/user-defined-macro-rules.rs:6:14 + | +LL | macro_rules! {} + | ^ expected identifier + | + = help: maybe you have forgotten to define a name for this `macro_rules!` + +error: aborting due to 2 previous errors + diff --git a/tests/ui/resolve/issue-118295.rs b/tests/ui/resolve/issue-118295.rs deleted file mode 100644 index b97681d956341..0000000000000 --- a/tests/ui/resolve/issue-118295.rs +++ /dev/null @@ -1,5 +0,0 @@ -macro_rules! {} -//~^ ERROR cannot find macro `macro_rules` in this scope -//~| NOTE maybe you have forgotten to define a name for this `macro_rules!` - -fn main() {} diff --git a/tests/ui/resolve/issue-118295.stderr b/tests/ui/resolve/issue-118295.stderr deleted file mode 100644 index d60d7d9185db4..0000000000000 --- a/tests/ui/resolve/issue-118295.stderr +++ /dev/null @@ -1,14 +0,0 @@ -error: cannot find macro `macro_rules` in this scope - --> $DIR/issue-118295.rs:1:1 - | -LL | macro_rules! {} - | ^^^^^^^^^^^ - | -note: maybe you have forgotten to define a name for this `macro_rules!` - --> $DIR/issue-118295.rs:1:1 - | -LL | macro_rules! {} - | ^^^^^^^^^^^ - -error: aborting due to 1 previous error -