From 3452e015864a52804dc26c32f8fc51c6e0b755da Mon Sep 17 00:00:00 2001 From: EliseZeroTwo Date: Fri, 15 Dec 2023 08:43:01 -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 | 63 ++++++++++++++----- compiler/rustc_resolve/messages.ftl | 2 - compiler/rustc_resolve/src/diagnostics.rs | 7 +-- compiler/rustc_resolve/src/errors.rs | 7 --- tests/ui/macros/mbe-missing-ident-error.rs | 8 +++ .../ui/macros/mbe-missing-ident-error.stderr | 10 +++ .../ui/macros/mbe-parenthesis-ident-error.rs | 8 +++ .../macros/mbe-parenthesis-ident-error.stderr | 14 +++++ tests/ui/macros/user-defined-macro-rules.rs | 10 +-- .../ui/macros/user-defined-macro-rules.stderr | 16 +++++ tests/ui/resolve/issue-118295.rs | 5 -- tests/ui/resolve/issue-118295.stderr | 14 ----- 14 files changed, 120 insertions(+), 55 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 da51d9dbe9f84..266defbacabcc 100644 --- a/compiler/rustc_parse/messages.ftl +++ b/compiler/rustc_parse/messages.ftl @@ -472,6 +472,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 @@ -501,6 +503,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 72e5ca41c7856..a1abfa2638005 100644 --- a/compiler/rustc_parse/src/errors.rs +++ b/compiler/rustc_parse/src/errors.rs @@ -2671,6 +2671,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 8a987767dc4cb..2273fecb72f68 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -2040,23 +2040,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. @@ -2070,7 +2073,39 @@ 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) => { + match ( + &self.token.kind, + self.look_ahead(1, |token| token.ident()), + self.look_ahead(2, |token| { + token.kind == TokenKind::CloseDelim(Delimiter::Parenthesis) + }), + ) { + ( + TokenKind::OpenDelim(Delimiter::Parenthesis), + Some((Ident { span, .. }, _)), + true, + ) => { + err.span_note( + span, + "try removing the parenthesis around the name for this `macro_rules!`", + ); + } + _ => { + err.note(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/mbe-missing-ident-error.rs b/tests/ui/macros/mbe-missing-ident-error.rs new file mode 100644 index 0000000000000..f154f25aefedc --- /dev/null +++ b/tests/ui/macros/mbe-missing-ident-error.rs @@ -0,0 +1,8 @@ +// Ensures MBEs with a missing ident produce a readable error + +macro_rules! { + //~^ ERROR: expected identifier, found `{` + //~| NOTE: expected identifier + //~| NOTE: 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..bf56b2c6d5581 --- /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 + | + = note: 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..8d6445f0c0129 --- /dev/null +++ b/tests/ui/macros/mbe-parenthesis-ident-error.rs @@ -0,0 +1,8 @@ +// Ensures MBEs with a invalid ident produce a readable error + +macro_rules! (meepmeep) { + //~^ ERROR: expected identifier, found `(` + //~| NOTE: expected identifier + //~| NOTE: 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..43e9d041ac57b --- /dev/null +++ b/tests/ui/macros/mbe-parenthesis-ident-error.stderr @@ -0,0 +1,14 @@ +error: expected identifier, found `(` + --> $DIR/mbe-parenthesis-ident-error.rs:3:14 + | +LL | macro_rules! (meepmeep) { + | ^ expected identifier + | +note: try removing the parenthesis around the name for this `macro_rules!` + --> $DIR/mbe-parenthesis-ident-error.rs:3:15 + | +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..d1afa6bcda8a6 100644 --- a/tests/ui/macros/user-defined-macro-rules.rs +++ b/tests/ui/macros/user-defined-macro-rules.rs @@ -1,9 +1,5 @@ -// 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 `{` 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..30b0b156a5de8 --- /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:5:14 + | +LL | macro_rules! {} + | ^ expected identifier + | + = note: 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 -