Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion compiler/rustc_expand/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,16 @@ impl<'cx> MacroExpanderResult<'cx> {
// Emit the SEMICOLON_IN_EXPRESSIONS_FROM_MACROS deprecation lint.
let is_local = true;

let parser = ParserAnyMacro::from_tts(cx, tts, site_span, arm_span, is_local, macro_ident);
let parser = ParserAnyMacro::from_tts(
cx,
tts,
site_span,
arm_span,
is_local,
macro_ident,
vec![],
vec![],
);
ExpandResult::Ready(Box::new(parser))
}
}
Expand Down
52 changes: 51 additions & 1 deletion compiler/rustc_expand/src/mbe/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,11 +222,13 @@ impl<'dcx> CollectTrackerAndEmitter<'dcx, '_> {

pub(super) fn emit_frag_parse_err(
mut e: Diag<'_>,
parser: &Parser<'_>,
parser: &mut Parser<'_>,
orig_parser: &mut Parser<'_>,
site_span: Span,
arm_span: Span,
kind: AstFragmentKind,
bindings: Vec<Ident>,
matched_rule_bindings: Vec<Ident>,
) -> ErrorGuaranteed {
// FIXME(davidtwco): avoid depending on the error message text
if parser.token == token::Eof
Expand Down Expand Up @@ -285,6 +287,54 @@ pub(super) fn emit_frag_parse_err(
},
_ => annotate_err_with_kind(&mut e, kind, site_span),
};

let matched_rule_bindings_names: Vec<_> =
matched_rule_bindings.iter().map(|bind| bind.name).collect();
let bindings_name: Vec<_> = bindings.iter().map(|bind| bind.name).collect();
if parser.token.kind == token::Dollar {
parser.bump();
if let token::Ident(name, _) = parser.token.kind {
if let Some(matched_name) = rustc_span::edit_distance::find_best_match_for_name(
&matched_rule_bindings_names[..],
name,
None,
) {
e.span_suggestion_verbose(
parser.token.span,
"there is a macro metavariable with similar name",
format!("{matched_name}"),
Applicability::MaybeIncorrect,
);
} else if bindings_name.contains(&name) {
e.span_label(
parser.token.span,
format!(
"there is an macro metavariable with this name in another macro matcher"
),
);
} else if let Some(matched_name) =
rustc_span::edit_distance::find_best_match_for_name(&bindings_name[..], name, None)
{
e.span_suggestion_verbose(
parser.token.span,
"there is a macro metavariable with a similar name in another macro matcher",
format!("{matched_name}"),
Applicability::MaybeIncorrect,
);
} else {
let msg = matched_rule_bindings_names
.iter()
.map(|sym| format!("${}", sym))
.collect::<Vec<_>>()
.join(", ");

e.span_label(parser.token.span, format!("macro metavariable not found"));
if !matched_rule_bindings_names.is_empty() {
e.note(format!("available metavariable names are: {msg}"));
}
}
}
}
e.emit()
}

Expand Down
46 changes: 43 additions & 3 deletions compiler/rustc_expand/src/mbe/macro_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ pub(crate) struct ParserAnyMacro<'a> {
arm_span: Span,
/// Whether or not this macro is defined in the current crate
is_local: bool,
bindings: Vec<Ident>,
matched_rule_bindings: Vec<Ident>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
matched_rule_bindings: Vec<Ident>,
bindings: &'a [MacroRule],
matched_rule_bindings: &'a [MatcherLoc],

}

impl<'a> ParserAnyMacro<'a> {
Expand All @@ -68,13 +70,22 @@ impl<'a> ParserAnyMacro<'a> {
arm_span,
is_trailing_mac,
is_local,
bindings,
matched_rule_bindings,
} = *self;
let snapshot = &mut parser.create_snapshot_for_diagnostic();
let fragment = match parse_ast_fragment(parser, kind) {
Ok(f) => f,
Err(err) => {
let guar = diagnostics::emit_frag_parse_err(
err, parser, snapshot, site_span, arm_span, kind,
err,
parser,
snapshot,
site_span,
arm_span,
kind,
bindings,
matched_rule_bindings,
);
return kind.dummy(site_span, guar);
}
Expand Down Expand Up @@ -109,6 +120,9 @@ impl<'a> ParserAnyMacro<'a> {
arm_span: Span,
is_local: bool,
macro_ident: Ident,
// bindings and lhs is for diagnostics
bindings: Vec<Ident>,
matched_rule_bindings: Vec<Ident>,
) -> Self {
Self {
parser: Parser::new(&cx.sess.psess, tts, None),
Expand All @@ -122,6 +136,8 @@ impl<'a> ParserAnyMacro<'a> {
is_trailing_mac: cx.current_expansion.is_trailing_mac,
arm_span,
is_local,
bindings,
matched_rule_bindings,
}
}
}
Expand Down Expand Up @@ -360,7 +376,7 @@ fn expand_macro<'cx>(

match try_success_result {
Ok((rule_index, rule, named_matches)) => {
let MacroRule::Func { rhs, .. } = rule else {
let MacroRule::Func { lhs, rhs, .. } = rule else {
panic!("try_match_macro returned non-func rule");
};
let mbe::TokenTree::Delimited(rhs_span, _, rhs) = rhs else {
Expand Down Expand Up @@ -388,8 +404,32 @@ fn expand_macro<'cx>(
cx.resolver.record_macro_rule_usage(node_id, rule_index);
}

let mut bindings = vec![];
for rule in rules {
let MacroRule::Func { lhs, .. } = rule else { continue };
for param in lhs {
let MatcherLoc::MetaVarDecl { bind, .. } = param else { continue };
bindings.push(*bind);
}
}

let mut matched_rule_bindings = vec![];
for param in lhs {
let MatcherLoc::MetaVarDecl { bind, .. } = param else { continue };
matched_rule_bindings.push(*bind);
}
Comment on lines +407 to +420
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without closely looking at the lifetimes, it might be that we can pass lhs and rules to from_tts and move this logic to the error code path.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I will take a look. Is there any local way to test for the regression though ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can try x perf (https://rustc-dev-guide.rust-lang.org/profiling/with-rustc-perf.html), though it's hard to tell whether it will find this regression. You can try it with the serde/clap derive crates.

Comment on lines +407 to +420
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be moved to where diagnostics::emit_frag_parse_err is called


// Let the context choose how to interpret the result. Weird, but useful for X-macros.
Box::new(ParserAnyMacro::from_tts(cx, tts, sp, arm_span, is_local, name))
Box::new(ParserAnyMacro::from_tts(
cx,
tts,
sp,
arm_span,
is_local,
name,
bindings,
matched_rule_bindings,
))
}
Err(CanRetry::No(guar)) => {
debug!("Will not retry matching as an error was emitted already");
Expand Down
6 changes: 5 additions & 1 deletion tests/ui/macros/issue-6596-1.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,15 @@ error: expected expression, found `$`
--> $DIR/issue-6596-1.rs:3:9
|
LL | $nonexistent
| ^^^^^^^^^^^^ expected expression
| ^-----------
| ||
| |macro metavariable not found
| expected expression
...
LL | e!(foo);
| ------- in this macro invocation
|
= note: available metavariable names are: $inp
= note: this error originates in the macro `e` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 1 previous error
Expand Down
42 changes: 42 additions & 0 deletions tests/ui/macros/typo-in-norepeat-expr-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
macro_rules! err {
(begin $follow:ident end $arg:expr) => {
[$arg]
};
(begin1 $arg1:ident end $agr2:expr) => {
[$follow] //~ ERROR: expected expression, found `$`
//~^ NOTE: there is an macro metavariable with this name in another macro matcher
//~| NOTE: expected expression
};
}

macro_rules! err1 {
(begin $follow:ident end $arg:expr) => {
[$arg]
};
(begin1 $arg1:ident end) => {
[$follo] //~ ERROR: expected expression, found `$`
//~| NOTE: expected expression
//~| HELP: there is a macro metavariable with a similar name in another macro matcher
};
}

macro_rules! err2 {
(begin $follow:ident end $arg:expr) => {
[$arg]
};
(begin1 $arg1:ident end) => {
[$xyz] //~ ERROR: expected expression, found `$`
//~^ NOTE: expected expression
//~| NOTE available metavariable names are: $arg1
//~| NOTE: macro metavariable not found
};
}

fn main () {
let _ = err![begin1 x end ig]; //~ NOTE: in this expansion of err!
let _ = err1![begin1 x end]; //~ NOTE: in this expansion of err1!
//~| NOTE: in this expansion of err1!

let _ = err2![begin1 x end]; //~ NOTE: in this expansion of err2!
//~| NOTE in this expansion of err2!
}
46 changes: 46 additions & 0 deletions tests/ui/macros/typo-in-norepeat-expr-2.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
error: expected expression, found `$`
--> $DIR/typo-in-norepeat-expr-2.rs:6:10
|
LL | [$follow]
| ^------
| ||
| |there is an macro metavariable with this name in another macro matcher
| expected expression
...
LL | let _ = err![begin1 x end ig];
| ---------------------- in this macro invocation
|
= note: this error originates in the macro `err` (in Nightly builds, run with -Z macro-backtrace for more info)

error: expected expression, found `$`
--> $DIR/typo-in-norepeat-expr-2.rs:17:10
|
LL | [$follo]
| ^^^^^^ expected expression
...
LL | let _ = err1![begin1 x end];
| -------------------- in this macro invocation
|
= note: this error originates in the macro `err1` (in Nightly builds, run with -Z macro-backtrace for more info)
help: there is a macro metavariable with a similar name in another macro matcher
|
LL | [$follow]
| +

error: expected expression, found `$`
--> $DIR/typo-in-norepeat-expr-2.rs:28:10
|
LL | [$xyz]
| ^---
| ||
| |macro metavariable not found
| expected expression
...
LL | let _ = err2![begin1 x end];
| -------------------- in this macro invocation
|
= note: available metavariable names are: $arg1
= note: this error originates in the macro `err2` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 3 previous errors

12 changes: 12 additions & 0 deletions tests/ui/macros/typo-in-norepeat-expr.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
//@ run-rustfix
macro_rules! m {
(begin $ard:ident end) => {
[$ard] //~ ERROR: expected expression, found `$`
//~^ HELP: there is a macro metavariable with similar name
};
}

fn main() {
let x = 1;
let _ = m![begin x end];
}
12 changes: 12 additions & 0 deletions tests/ui/macros/typo-in-norepeat-expr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
//@ run-rustfix
macro_rules! m {
(begin $ard:ident end) => {
[$arg] //~ ERROR: expected expression, found `$`
//~^ HELP: there is a macro metavariable with similar name
};
}

fn main() {
let x = 1;
let _ = m![begin x end];
}
18 changes: 18 additions & 0 deletions tests/ui/macros/typo-in-norepeat-expr.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
error: expected expression, found `$`
--> $DIR/typo-in-norepeat-expr.rs:4:10
|
LL | [$arg]
| ^^^^ expected expression
...
LL | let _ = m![begin x end];
| --------------- in this macro invocation
|
= note: this error originates in the macro `m` (in Nightly builds, run with -Z macro-backtrace for more info)
help: there is a macro metavariable with similar name
|
LL - [$arg]
LL + [$ard]
|

error: aborting due to 1 previous error

Loading