Skip to content

Commit c7819c5

Browse files
borsbors
committed
Auto merge of #154014 - Unique-Usman:ua/decmacrounrepeatable2, r=<try>
rustc_expand: improve diagnostics for non-repeatable metavars
2 parents 13e2aba + 35d11f3 commit c7819c5

File tree

10 files changed

+255
-21
lines changed

10 files changed

+255
-21
lines changed

‎compiler/rustc_expand/src/base.rs‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,8 @@ impl<'cx> MacroExpanderResult<'cx> {
277277
// Emit the SEMICOLON_IN_EXPRESSIONS_FROM_MACROS deprecation lint.
278278
let is_local = true;
279279

280-
let parser = ParserAnyMacro::from_tts(cx, tts, site_span, arm_span, is_local, macro_ident);
280+
let parser =
281+
ParserAnyMacro::from_tts(cx, tts, site_span, arm_span, is_local, macro_ident, &[], &[]);
281282
ExpandResult::Ready(Box::new(parser))
282283
}
283284
}
@@ -377,8 +378,8 @@ where
377378

378379
/// Represents a thing that maps token trees to Macro Results
379380
pub trait TTMacroExpander: Any {
380-
fn expand<'cx>(
381-
&self,
381+
fn expand<'cx, 'a: 'cx>(
382+
&'a self,
382383
ecx: &'cx mut ExtCtxt<'_>,
383384
span: Span,
384385
input: TokenStream,
@@ -394,8 +395,8 @@ impl<F: 'static> TTMacroExpander for F
394395
where
395396
F: for<'cx> Fn(&'cx mut ExtCtxt<'_>, Span, TokenStream) -> MacroExpanderResult<'cx>,
396397
{
397-
fn expand<'cx>(
398-
&self,
398+
fn expand<'cx, 'a: 'cx>(
399+
&'a self,
399400
ecx: &'cx mut ExtCtxt<'_>,
400401
span: Span,
401402
input: TokenStream,

‎compiler/rustc_expand/src/expand.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,8 +183,8 @@ macro_rules! ast_fragments {
183183
}
184184
}
185185

186-
impl<'a> MacResult for crate::mbe::macro_rules::ParserAnyMacro<'a> {
187-
$(fn $make_ast(self: Box<crate::mbe::macro_rules::ParserAnyMacro<'a>>)
186+
impl<'a, 'b> MacResult for crate::mbe::macro_rules::ParserAnyMacro<'a, 'b> {
187+
$(fn $make_ast(self: Box<crate::mbe::macro_rules::ParserAnyMacro<'a, 'b>>)
188188
-> Option<$AstTy> {
189189
Some(self.make(AstFragmentKind::$Kind).$make_ast())
190190
})*

‎compiler/rustc_expand/src/mbe/diagnostics.rs‎

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,11 +222,13 @@ impl<'dcx> CollectTrackerAndEmitter<'dcx, '_> {
222222

223223
pub(super) fn emit_frag_parse_err(
224224
mut e: Diag<'_>,
225-
parser: &Parser<'_>,
225+
parser: &mut Parser<'_>,
226226
orig_parser: &mut Parser<'_>,
227227
site_span: Span,
228228
arm_span: Span,
229229
kind: AstFragmentKind,
230+
bindings: &[MacroRule],
231+
matched_rule_bindings: &[MatcherLoc],
230232
) -> ErrorGuaranteed {
231233
// FIXME(davidtwco): avoid depending on the error message text
232234
if parser.token == token::Eof
@@ -285,6 +287,69 @@ pub(super) fn emit_frag_parse_err(
285287
},
286288
_ => annotate_err_with_kind(&mut e, kind, site_span),
287289
};
290+
291+
let mut bindings_rules = vec![];
292+
for rule in bindings {
293+
let MacroRule::Func { lhs, .. } = rule else { continue };
294+
for param in lhs {
295+
let MatcherLoc::MetaVarDecl { bind, .. } = param else { continue };
296+
bindings_rules.push(*bind);
297+
}
298+
}
299+
300+
let mut matched_rule_bindings_rules = vec![];
301+
for param in matched_rule_bindings {
302+
let MatcherLoc::MetaVarDecl { bind, .. } = param else { continue };
303+
matched_rule_bindings_rules.push(*bind);
304+
}
305+
306+
let matched_rule_bindings_names: Vec<_> =
307+
matched_rule_bindings_rules.iter().map(|bind| bind.name).collect();
308+
let bindings_name: Vec<_> = bindings_rules.iter().map(|bind| bind.name).collect();
309+
if parser.token.kind == token::Dollar {
310+
parser.bump();
311+
if let token::Ident(name, _) = parser.token.kind {
312+
if let Some(matched_name) = rustc_span::edit_distance::find_best_match_for_name(
313+
&matched_rule_bindings_names[..],
314+
name,
315+
None,
316+
) {
317+
e.span_suggestion_verbose(
318+
parser.token.span,
319+
"there is a macro metavariable with similar name",
320+
format!("{matched_name}"),
321+
Applicability::MaybeIncorrect,
322+
);
323+
} else if bindings_name.contains(&name) {
324+
e.span_label(
325+
parser.token.span,
326+
format!(
327+
"there is an macro metavariable with this name in another macro matcher"
328+
),
329+
);
330+
} else if let Some(matched_name) =
331+
rustc_span::edit_distance::find_best_match_for_name(&bindings_name[..], name, None)
332+
{
333+
e.span_suggestion_verbose(
334+
parser.token.span,
335+
"there is a macro metavariable with a similar name in another macro matcher",
336+
format!("{matched_name}"),
337+
Applicability::MaybeIncorrect,
338+
);
339+
} else {
340+
let msg = matched_rule_bindings_names
341+
.iter()
342+
.map(|sym| format!("${}", sym))
343+
.collect::<Vec<_>>()
344+
.join(", ");
345+
346+
e.span_label(parser.token.span, format!("macro metavariable not found"));
347+
if !matched_rule_bindings_names.is_empty() {
348+
e.note(format!("available metavariable names are: {msg}"));
349+
}
350+
}
351+
}
352+
}
288353
e.emit()
289354
}
290355

‎compiler/rustc_expand/src/mbe/macro_rules.rs‎

Lines changed: 46 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ use crate::mbe::quoted::{RulePart, parse_one_tt};
4343
use crate::mbe::transcribe::transcribe;
4444
use crate::mbe::{self, KleeneOp};
4545

46-
pub(crate) struct ParserAnyMacro<'a> {
46+
pub(crate) struct ParserAnyMacro<'a, 'b> {
4747
parser: Parser<'a>,
4848

4949
/// Span of the expansion site of the macro this parser is for
@@ -55,10 +55,15 @@ pub(crate) struct ParserAnyMacro<'a> {
5555
arm_span: Span,
5656
/// Whether or not this macro is defined in the current crate
5757
is_local: bool,
58+
bindings: &'b [MacroRule],
59+
matched_rule_bindings: &'b [MatcherLoc],
5860
}
5961

60-
impl<'a> ParserAnyMacro<'a> {
61-
pub(crate) fn make(mut self: Box<ParserAnyMacro<'a>>, kind: AstFragmentKind) -> AstFragment {
62+
impl<'a, 'b> ParserAnyMacro<'a, 'b> {
63+
pub(crate) fn make(
64+
mut self: Box<ParserAnyMacro<'a, 'b>>,
65+
kind: AstFragmentKind,
66+
) -> AstFragment {
6267
let ParserAnyMacro {
6368
site_span,
6469
macro_ident,
@@ -67,13 +72,22 @@ impl<'a> ParserAnyMacro<'a> {
6772
arm_span,
6873
is_trailing_mac,
6974
is_local,
75+
bindings,
76+
matched_rule_bindings,
7077
} = *self;
7178
let snapshot = &mut parser.create_snapshot_for_diagnostic();
7279
let fragment = match parse_ast_fragment(parser, kind) {
7380
Ok(f) => f,
7481
Err(err) => {
7582
let guar = diagnostics::emit_frag_parse_err(
76-
err, parser, snapshot, site_span, arm_span, kind,
83+
err,
84+
parser,
85+
snapshot,
86+
site_span,
87+
arm_span,
88+
kind,
89+
bindings,
90+
matched_rule_bindings,
7791
);
7892
return kind.dummy(site_span, guar);
7993
}
@@ -100,14 +114,17 @@ impl<'a> ParserAnyMacro<'a> {
100114
fragment
101115
}
102116

103-
#[instrument(skip(cx, tts))]
117+
#[instrument(skip(cx, tts, bindings, matched_rule_bindings))]
104118
pub(crate) fn from_tts<'cx>(
105119
cx: &'cx mut ExtCtxt<'a>,
106120
tts: TokenStream,
107121
site_span: Span,
108122
arm_span: Span,
109123
is_local: bool,
110124
macro_ident: Ident,
125+
// bindings and lhs is for diagnostics
126+
bindings: &'b [MacroRule],
127+
matched_rule_bindings: &'b [MatcherLoc],
111128
) -> Self {
112129
Self {
113130
parser: Parser::new(&cx.sess.psess, tts, None),
@@ -121,11 +138,13 @@ impl<'a> ParserAnyMacro<'a> {
121138
is_trailing_mac: cx.current_expansion.is_trailing_mac,
122139
arm_span,
123140
is_local,
141+
bindings,
142+
matched_rule_bindings,
124143
}
125144
}
126145
}
127146

128-
pub(super) enum MacroRule {
147+
pub(crate) enum MacroRule {
129148
/// A function-style rule, for use with `m!()`
130149
Func { lhs: Vec<MatcherLoc>, lhs_span: Span, rhs: mbe::TokenTree },
131150
/// An attr rule, for use with `#[m]`
@@ -226,8 +245,8 @@ impl MacroRulesMacroExpander {
226245
}
227246

228247
impl TTMacroExpander for MacroRulesMacroExpander {
229-
fn expand<'cx>(
230-
&self,
248+
fn expand<'cx, 'a: 'cx>(
249+
&'a self,
231250
cx: &'cx mut ExtCtxt<'_>,
232251
sp: Span,
233252
input: TokenStream,
@@ -337,15 +356,15 @@ impl<'matcher> Tracker<'matcher> for NoopTracker {
337356

338357
/// Expands the rules based macro defined by `rules` for a given input `arg`.
339358
#[instrument(skip(cx, transparency, arg, rules))]
340-
fn expand_macro<'cx>(
359+
fn expand_macro<'cx, 'a: 'cx>(
341360
cx: &'cx mut ExtCtxt<'_>,
342361
sp: Span,
343362
def_span: Span,
344363
node_id: NodeId,
345364
name: Ident,
346365
transparency: Transparency,
347366
arg: TokenStream,
348-
rules: &[MacroRule],
367+
rules: &'a [MacroRule],
349368
) -> Box<dyn MacResult + 'cx> {
350369
let psess = &cx.sess.psess;
351370

@@ -359,7 +378,7 @@ fn expand_macro<'cx>(
359378

360379
match try_success_result {
361380
Ok((rule_index, rule, named_matches)) => {
362-
let MacroRule::Func { rhs, .. } = rule else {
381+
let MacroRule::Func { lhs, rhs, .. } = rule else {
363382
panic!("try_match_macro returned non-func rule");
364383
};
365384
let mbe::TokenTree::Delimited(rhs_span, _, rhs) = rhs else {
@@ -387,8 +406,23 @@ fn expand_macro<'cx>(
387406
cx.resolver.record_macro_rule_usage(node_id, rule_index);
388407
}
389408

409+
// let mut bindings = vec![];
410+
// for rule in rules {
411+
// let MacroRule::Func { lhs, .. } = rule else { continue };
412+
// for param in lhs {
413+
// let MatcherLoc::MetaVarDecl { bind, .. } = param else { continue };
414+
// bindings.push(*bind);
415+
// }
416+
// }
417+
//
418+
// let mut matched_rule_bindings = vec![];
419+
// for param in lhs {
420+
// let MatcherLoc::MetaVarDecl { bind, .. } = param else { continue };
421+
// matched_rule_bindings.push(*bind);
422+
// }
423+
390424
// Let the context choose how to interpret the result. Weird, but useful for X-macros.
391-
Box::new(ParserAnyMacro::from_tts(cx, tts, sp, arm_span, is_local, name))
425+
Box::new(ParserAnyMacro::from_tts(cx, tts, sp, arm_span, is_local, name, rules, lhs))
392426
}
393427
Err(CanRetry::No(guar)) => {
394428
debug!("Will not retry matching as an error was emitted already");

‎tests/ui/macros/issue-6596-1.stderr‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,15 @@ error: expected expression, found `$`
22
--> $DIR/issue-6596-1.rs:3:9
33
|
44
LL | $nonexistent
5-
| ^^^^^^^^^^^^ expected expression
5+
| ^-----------
6+
| ||
7+
| |macro metavariable not found
8+
| expected expression
69
...
710
LL | e!(foo);
811
| ------- in this macro invocation
912
|
13+
= note: available metavariable names are: $inp
1014
= note: this error originates in the macro `e` (in Nightly builds, run with -Z macro-backtrace for more info)
1115

1216
error: aborting due to 1 previous error

‎tests/ui/macros/typo-in-norepeat-expr-2.rs‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
macro_rules! err {
2+
(begin $follow:ident end $arg:expr) => {
3+
[$arg]
4+
};
5+
(begin1 $arg1:ident end $agr2:expr) => {
6+
[$follow] //~ ERROR: expected expression, found `$`
7+
//~^ NOTE: there is an macro metavariable with this name in another macro matcher
8+
//~| NOTE: expected expression
9+
};
10+
}
11+
12+
macro_rules! err1 {
13+
(begin $follow:ident end $arg:expr) => {
14+
[$arg]
15+
};
16+
(begin1 $arg1:ident end) => {
17+
[$follo] //~ ERROR: expected expression, found `$`
18+
//~| NOTE: expected expression
19+
//~| HELP: there is a macro metavariable with a similar name in another macro matcher
20+
};
21+
}
22+
23+
macro_rules! err2 {
24+
(begin $follow:ident end $arg:expr) => {
25+
[$arg]
26+
};
27+
(begin1 $arg1:ident end) => {
28+
[$xyz] //~ ERROR: expected expression, found `$`
29+
//~^ NOTE: expected expression
30+
//~| NOTE available metavariable names are: $arg1
31+
//~| NOTE: macro metavariable not found
32+
};
33+
}
34+
35+
fn main () {
36+
let _ = err![begin1 x end ig]; //~ NOTE: in this expansion of err!
37+
let _ = err1![begin1 x end]; //~ NOTE: in this expansion of err1!
38+
//~| NOTE: in this expansion of err1!
39+
40+
let _ = err2![begin1 x end]; //~ NOTE: in this expansion of err2!
41+
//~| NOTE in this expansion of err2!
42+
}

‎tests/ui/macros/typo-in-norepeat-expr-2.stderr‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
error: expected expression, found `$`
2+
--> $DIR/typo-in-norepeat-expr-2.rs:6:10
3+
|
4+
LL | [$follow]
5+
| ^------
6+
| ||
7+
| |there is an macro metavariable with this name in another macro matcher
8+
| expected expression
9+
...
10+
LL | let _ = err![begin1 x end ig];
11+
| ---------------------- in this macro invocation
12+
|
13+
= note: this error originates in the macro `err` (in Nightly builds, run with -Z macro-backtrace for more info)
14+
15+
error: expected expression, found `$`
16+
--> $DIR/typo-in-norepeat-expr-2.rs:17:10
17+
|
18+
LL | [$follo]
19+
| ^^^^^^ expected expression
20+
...
21+
LL | let _ = err1![begin1 x end];
22+
| -------------------- in this macro invocation
23+
|
24+
= note: this error originates in the macro `err1` (in Nightly builds, run with -Z macro-backtrace for more info)
25+
help: there is a macro metavariable with a similar name in another macro matcher
26+
|
27+
LL | [$follow]
28+
| +
29+
30+
error: expected expression, found `$`
31+
--> $DIR/typo-in-norepeat-expr-2.rs:28:10
32+
|
33+
LL | [$xyz]
34+
| ^---
35+
| ||
36+
| |macro metavariable not found
37+
| expected expression
38+
...
39+
LL | let _ = err2![begin1 x end];
40+
| -------------------- in this macro invocation
41+
|
42+
= note: available metavariable names are: $arg1
43+
= note: this error originates in the macro `err2` (in Nightly builds, run with -Z macro-backtrace for more info)
44+
45+
error: aborting due to 3 previous errors
46+

‎tests/ui/macros/typo-in-norepeat-expr.fixed‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
//@ run-rustfix
2+
macro_rules! m {
3+
(begin $ard:ident end) => {
4+
[$ard] //~ ERROR: expected expression, found `$`
5+
//~^ HELP: there is a macro metavariable with similar name
6+
};
7+
}
8+
9+
fn main() {
10+
let x = 1;
11+
let _ = m![begin x end];
12+
}

0 commit comments

Comments
 (0)