Skip to content

Commit

Permalink
fix: better error cases with bad/missing identifiers in MBEs
Browse files Browse the repository at this point in the history
  • Loading branch information
EliseZeroTwo committed Dec 24, 2023
1 parent f9d52dc commit 3714817
Show file tree
Hide file tree
Showing 16 changed files with 122 additions and 98 deletions.
4 changes: 4 additions & 0 deletions compiler/rustc_parse/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_parse/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
58 changes: 44 additions & 14 deletions compiler/rustc_parse/src/parser/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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!
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_resolve/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 1 addition & 6 deletions compiler/rustc_resolve/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
7 changes: 0 additions & 7 deletions compiler/rustc_resolve/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 1 addition & 3 deletions tests/ui/macros/issue-118786.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 `(`
() => {}
}
}
Expand Down
46 changes: 6 additions & 40 deletions tests/ui/macros/issue-118786.stderr
Original file line number Diff line number Diff line change
@@ -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

7 changes: 7 additions & 0 deletions tests/ui/macros/mbe-missing-ident-error.rs
Original file line number Diff line number Diff line change
@@ -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!`
() => {}
}
10 changes: 10 additions & 0 deletions tests/ui/macros/mbe-missing-ident-error.stderr
Original file line number Diff line number Diff line change
@@ -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

7 changes: 7 additions & 0 deletions tests/ui/macros/mbe-parenthesis-ident-error.rs
Original file line number Diff line number Diff line change
@@ -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!`
() => {}
}
13 changes: 13 additions & 0 deletions tests/ui/macros/mbe-parenthesis-ident-error.stderr
Original file line number Diff line number Diff line change
@@ -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

13 changes: 6 additions & 7 deletions tests/ui/macros/user-defined-macro-rules.rs
Original file line number Diff line number Diff line change
@@ -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!`
16 changes: 16 additions & 0 deletions tests/ui/macros/user-defined-macro-rules.stderr
Original file line number Diff line number Diff line change
@@ -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

5 changes: 0 additions & 5 deletions tests/ui/resolve/issue-118295.rs

This file was deleted.

14 changes: 0 additions & 14 deletions tests/ui/resolve/issue-118295.stderr

This file was deleted.

0 comments on commit 3714817

Please sign in to comment.