Skip to content

Commit 01f38b4

Browse files
committed
Rewrite macro_rules! parser to not use the MBE engine itself
The `macro_rules!` parser was written to match the series of rules using the macros-by-example (MBE) engine and a hand-written equivalent of the left-hand side of a MBE macro. This was complex to read, difficult to extend, and produced confusing error messages. Because it was using the MBE engine, any parse failure would be reported as if some macro was being applied to the `macro_rules!` invocation itself; for instance, errors would talk about "macro invocation", "macro arguments", and "macro call", when they were actually about the macro *definition*. And in practice, the `macro_rules!` parser only used the MBE engine to extract the left-hand side and right-hand side of each rule as a token tree, and then parsed the rest using a separate parser. Rewrite it to parse the series of rules using a simple loop, instead. This makes it more extensible in the future, and improves error messages. For instance, omitting a semicolon between rules will result in "expected `;`" and "unexpected token", rather than the confusing "no rules expected this token in macro call". This work was greatly aided by pair programming with Vincenzo Palazzo and Eric Holk.
1 parent df8102f commit 01f38b4

File tree

11 files changed

+65
-198
lines changed

11 files changed

+65
-198
lines changed

compiler/rustc_expand/src/mbe/diagnostics.rs

Lines changed: 2 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -194,38 +194,6 @@ impl<'dcx> CollectTrackerAndEmitter<'dcx, '_> {
194194
}
195195
}
196196

197-
/// Currently used by macro_rules! compilation to extract a little information from the `Failure`
198-
/// case.
199-
pub(crate) struct FailureForwarder<'matcher> {
200-
expected_token: Option<&'matcher Token>,
201-
}
202-
203-
impl<'matcher> FailureForwarder<'matcher> {
204-
pub(crate) fn new() -> Self {
205-
Self { expected_token: None }
206-
}
207-
}
208-
209-
impl<'matcher> Tracker<'matcher> for FailureForwarder<'matcher> {
210-
type Failure = (Token, u32, &'static str);
211-
212-
fn build_failure(tok: Token, position: u32, msg: &'static str) -> Self::Failure {
213-
(tok, position, msg)
214-
}
215-
216-
fn description() -> &'static str {
217-
"failure-forwarder"
218-
}
219-
220-
fn set_expected_token(&mut self, tok: &'matcher Token) {
221-
self.expected_token = Some(tok);
222-
}
223-
224-
fn get_expected_token(&self) -> Option<&'matcher Token> {
225-
self.expected_token
226-
}
227-
}
228-
229197
pub(super) fn emit_frag_parse_err(
230198
mut e: Diag<'_>,
231199
parser: &Parser<'_>,
@@ -320,7 +288,7 @@ enum ExplainDocComment {
320288
},
321289
}
322290

323-
pub(super) fn annotate_doc_comment(err: &mut Diag<'_>, sm: &SourceMap, span: Span) {
291+
fn annotate_doc_comment(err: &mut Diag<'_>, sm: &SourceMap, span: Span) {
324292
if let Ok(src) = sm.span_to_snippet(span) {
325293
if src.starts_with("///") || src.starts_with("/**") {
326294
err.subdiagnostic(ExplainDocComment::Outer { span });
@@ -332,7 +300,7 @@ pub(super) fn annotate_doc_comment(err: &mut Diag<'_>, sm: &SourceMap, span: Spa
332300

333301
/// Generates an appropriate parsing failure message. For EOF, this is "unexpected end...". For
334302
/// other tokens, this is "unexpected token...".
335-
pub(super) fn parse_failure_msg(tok: &Token, expected_token: Option<&Token>) -> Cow<'static, str> {
303+
fn parse_failure_msg(tok: &Token, expected_token: Option<&Token>) -> Cow<'static, str> {
336304
if let Some(expected_token) = expected_token {
337305
Cow::from(format!("expected {}, found {}", token_descr(expected_token), token_descr(tok)))
338306
} else {

compiler/rustc_expand/src/mbe/macro_parser.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -541,8 +541,6 @@ impl TtParser {
541541
// The separator matches the current token. Advance past it.
542542
mp.idx += 1;
543543
self.next_mps.push(mp);
544-
} else {
545-
track.set_expected_token(separator);
546544
}
547545
}
548546
&MatcherLoc::SequenceKleeneOpAfterSep { idx_first } => {

compiler/rustc_expand/src/mbe/macro_rules.rs

Lines changed: 51 additions & 151 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,13 @@ use rustc_lint_defs::BuiltinLintDiag;
1919
use rustc_lint_defs::builtin::{
2020
RUST_2021_INCOMPATIBLE_OR_PATTERNS, SEMICOLON_IN_EXPRESSIONS_FROM_MACROS,
2121
};
22-
use rustc_parse::parser::{ParseNtResult, Parser, Recovery};
22+
use rustc_parse::exp;
23+
use rustc_parse::parser::{Parser, Recovery};
2324
use rustc_session::Session;
2425
use rustc_session::parse::ParseSess;
2526
use rustc_span::edition::Edition;
2627
use rustc_span::hygiene::Transparency;
27-
use rustc_span::{Ident, MacroRulesNormalizedIdent, Span, kw, sym};
28+
use rustc_span::{Ident, Span, kw, sym};
2829
use tracing::{debug, instrument, trace, trace_span};
2930

3031
use super::macro_parser::{NamedMatches, NamedParseResult};
@@ -34,8 +35,6 @@ use crate::base::{
3435
SyntaxExtensionKind, TTMacroExpander,
3536
};
3637
use crate::expand::{AstFragment, AstFragmentKind, ensure_complete_parse, parse_ast_fragment};
37-
use crate::mbe::diagnostics::{annotate_doc_comment, parse_failure_msg};
38-
use crate::mbe::macro_parser::NamedMatch::*;
3938
use crate::mbe::macro_parser::{Error, ErrorReported, Failure, MatcherLoc, Success, TtParser};
4039
use crate::mbe::transcribe::transcribe;
4140
use crate::mbe::{self, KleeneOp, macro_check};
@@ -168,11 +167,6 @@ pub(super) trait Tracker<'matcher> {
168167
fn recovery() -> Recovery {
169168
Recovery::Forbidden
170169
}
171-
172-
fn set_expected_token(&mut self, _tok: &'matcher Token) {}
173-
fn get_expected_token(&self) -> Option<&'matcher Token> {
174-
None
175-
}
176170
}
177171

178172
/// A noop tracker that is used in the hot path of the expansion, has zero overhead thanks to
@@ -360,11 +354,6 @@ pub(super) fn try_match_macro<'matcher, T: Tracker<'matcher>>(
360354
Err(CanRetry::Yes)
361355
}
362356

363-
// Note that macro-by-example's input is also matched against a token tree:
364-
// $( $lhs:tt => $rhs:tt );+
365-
//
366-
// Holy self-referential!
367-
368357
/// Converts a macro item into a syntax extension.
369358
pub fn compile_declarative_macro(
370359
sess: &Session,
@@ -390,152 +379,63 @@ pub fn compile_declarative_macro(
390379
};
391380
let dummy_syn_ext = |guar| (mk_syn_ext(Arc::new(DummyExpander(guar))), Vec::new());
392381

393-
let lhs_nm = Ident::new(sym::lhs, span);
394-
let rhs_nm = Ident::new(sym::rhs, span);
395-
let tt_spec = Some(NonterminalKind::TT);
396382
let macro_rules = macro_def.macro_rules;
383+
let exp_sep = if macro_rules { exp!(Semi) } else { exp!(Comma) };
397384

398-
// Parse the macro_rules! invocation
399-
400-
// The pattern that macro_rules matches.
401-
// The grammar for macro_rules! is:
402-
// $( $lhs:tt => $rhs:tt );+
403-
// ...quasiquoting this would be nice.
404-
// These spans won't matter, anyways
405-
let argument_gram = vec![
406-
mbe::TokenTree::Sequence(
407-
DelimSpan::dummy(),
408-
mbe::SequenceRepetition {
409-
tts: vec![
410-
mbe::TokenTree::MetaVarDecl(span, lhs_nm, tt_spec),
411-
mbe::TokenTree::token(token::FatArrow, span),
412-
mbe::TokenTree::MetaVarDecl(span, rhs_nm, tt_spec),
413-
],
414-
separator: Some(Token::new(
415-
if macro_rules { token::Semi } else { token::Comma },
416-
span,
417-
)),
418-
kleene: mbe::KleeneToken::new(mbe::KleeneOp::OneOrMore, span),
419-
num_captures: 2,
420-
},
421-
),
422-
// to phase into semicolon-termination instead of semicolon-separation
423-
mbe::TokenTree::Sequence(
424-
DelimSpan::dummy(),
425-
mbe::SequenceRepetition {
426-
tts: vec![mbe::TokenTree::token(
427-
if macro_rules { token::Semi } else { token::Comma },
428-
span,
429-
)],
430-
separator: None,
431-
kleene: mbe::KleeneToken::new(mbe::KleeneOp::ZeroOrMore, span),
432-
num_captures: 0,
433-
},
434-
),
435-
];
436-
// Convert it into `MatcherLoc` form.
437-
let argument_gram = mbe::macro_parser::compute_locs(&argument_gram);
438-
439-
let create_parser = || {
440-
let body = macro_def.body.tokens.clone();
441-
Parser::new(&sess.psess, body, rustc_parse::MACRO_ARGUMENTS)
442-
};
443-
444-
let parser = create_parser();
445-
let mut tt_parser =
446-
TtParser::new(Ident::with_dummy_span(if macro_rules { kw::MacroRules } else { kw::Macro }));
447-
let argument_map =
448-
match tt_parser.parse_tt(&mut Cow::Owned(parser), &argument_gram, &mut NoopTracker) {
449-
Success(m) => m,
450-
Failure(()) => {
451-
// The fast `NoopTracker` doesn't have any info on failure, so we need to retry it
452-
// with another one that gives us the information we need.
453-
// For this we need to reclone the macro body as the previous parser consumed it.
454-
let retry_parser = create_parser();
455-
456-
let mut track = diagnostics::FailureForwarder::new();
457-
let parse_result =
458-
tt_parser.parse_tt(&mut Cow::Owned(retry_parser), &argument_gram, &mut track);
459-
let Failure((token, _, msg)) = parse_result else {
460-
unreachable!("matcher returned something other than Failure after retry");
461-
};
462-
463-
let s = parse_failure_msg(&token, track.get_expected_token());
464-
let sp = token.span.substitute_dummy(span);
465-
let mut err = sess.dcx().struct_span_err(sp, s);
466-
err.span_label(sp, msg);
467-
annotate_doc_comment(&mut err, sess.source_map(), sp);
468-
let guar = err.emit();
469-
return dummy_syn_ext(guar);
470-
}
471-
Error(sp, msg) => {
472-
let guar = sess.dcx().span_err(sp.substitute_dummy(span), msg);
473-
return dummy_syn_ext(guar);
474-
}
475-
ErrorReported(guar) => {
476-
return dummy_syn_ext(guar);
477-
}
478-
};
385+
let body = macro_def.body.tokens.clone();
386+
let mut p = Parser::new(&sess.psess, body, rustc_parse::MACRO_ARGUMENTS);
479387

388+
// Don't abort iteration early, so that multiple errors can be reported.
480389
let mut guar = None;
481390
let mut check_emission = |ret: Result<(), ErrorGuaranteed>| guar = guar.or(ret.err());
482391

483-
// Extract the arguments:
484-
let lhses = match &argument_map[&MacroRulesNormalizedIdent::new(lhs_nm)] {
485-
MatchedSeq(s) => s
486-
.iter()
487-
.map(|m| {
488-
if let MatchedSingle(ParseNtResult::Tt(tt)) = m {
489-
let tt = mbe::quoted::parse(
490-
&TokenStream::new(vec![tt.clone()]),
491-
true,
492-
sess,
493-
node_id,
494-
features,
495-
edition,
496-
)
497-
.pop()
498-
.unwrap();
499-
// We don't handle errors here, the driver will abort
500-
// after parsing/expansion. We can report every error in every macro this way.
501-
check_emission(check_lhs_nt_follows(sess, node_id, &tt));
502-
return tt;
503-
}
504-
sess.dcx().span_bug(span, "wrong-structured lhs")
505-
})
506-
.collect::<Vec<mbe::TokenTree>>(),
507-
_ => sess.dcx().span_bug(span, "wrong-structured lhs"),
508-
};
509-
510-
let rhses = match &argument_map[&MacroRulesNormalizedIdent::new(rhs_nm)] {
511-
MatchedSeq(s) => s
512-
.iter()
513-
.map(|m| {
514-
if let MatchedSingle(ParseNtResult::Tt(tt)) = m {
515-
return mbe::quoted::parse(
516-
&TokenStream::new(vec![tt.clone()]),
517-
false,
518-
sess,
519-
node_id,
520-
features,
521-
edition,
522-
)
523-
.pop()
524-
.unwrap();
525-
}
526-
sess.dcx().span_bug(span, "wrong-structured rhs")
527-
})
528-
.collect::<Vec<mbe::TokenTree>>(),
529-
_ => sess.dcx().span_bug(span, "wrong-structured rhs"),
530-
};
392+
let mut lhses = Vec::new();
393+
let mut rhses = Vec::new();
531394

532-
for rhs in &rhses {
533-
check_emission(check_rhs(sess, rhs));
395+
while p.token != token::Eof {
396+
let lhs_tt = p.parse_token_tree();
397+
let lhs_tt = mbe::quoted::parse(
398+
&TokenStream::new(vec![lhs_tt]),
399+
true, // LHS
400+
sess,
401+
node_id,
402+
features,
403+
edition,
404+
)
405+
.pop()
406+
.unwrap();
407+
// We don't handle errors here, the driver will abort after parsing/expansion. We can
408+
// report every error in every macro this way.
409+
check_emission(check_lhs_nt_follows(sess, node_id, &lhs_tt));
410+
check_emission(check_lhs_no_empty_seq(sess, slice::from_ref(&lhs_tt)));
411+
if let Err(e) = p.expect(exp!(FatArrow)) {
412+
return dummy_syn_ext(e.emit());
413+
}
414+
let rhs_tt = p.parse_token_tree();
415+
let rhs_tt = mbe::quoted::parse(
416+
&TokenStream::new(vec![rhs_tt]),
417+
false, // RHS
418+
sess,
419+
node_id,
420+
features,
421+
edition,
422+
)
423+
.pop()
424+
.unwrap();
425+
check_emission(check_rhs(sess, &rhs_tt));
426+
lhses.push(lhs_tt);
427+
rhses.push(rhs_tt);
428+
if p.token == token::Eof {
429+
break;
430+
}
431+
if let Err(e) = p.expect(exp_sep) {
432+
return dummy_syn_ext(e.emit());
433+
}
534434
}
535435

536-
// Don't abort iteration early, so that errors for multiple lhses can be reported.
537-
for lhs in &lhses {
538-
check_emission(check_lhs_no_empty_seq(sess, slice::from_ref(lhs)));
436+
if lhses.is_empty() {
437+
let guar = sess.dcx().span_err(span, "macros must contain at least one rule");
438+
return dummy_syn_ext(guar);
539439
}
540440

541441
check_emission(macro_check::check_meta_variables(&sess.psess, node_id, span, &lhses, &rhses));

compiler/rustc_span/src/symbol.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1748,7 +1748,6 @@ symbols! {
17481748
resume,
17491749
return_position_impl_trait_in_trait,
17501750
return_type_notation,
1751-
rhs,
17521751
riscv_target_feature,
17531752
rlib,
17541753
ropi,

tests/ui/attributes/crate-type-macro-empty.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,6 @@
22
#[crate_type = foo!()]
33
//~^ ERROR cannot find macro `foo` in this scope
44

5-
macro_rules! foo {} //~ ERROR unexpected end of macro invocation
5+
macro_rules! foo {} //~ ERROR macros must contain at least one rule
66

77
fn main() {}

tests/ui/attributes/crate-type-macro-empty.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
error: unexpected end of macro invocation
1+
error: macros must contain at least one rule
22
--> $DIR/crate-type-macro-empty.rs:5:1
33
|
44
LL | macro_rules! foo {}
5-
| ^^^^^^^^^^^^^^^^^^^ missing tokens in macro arguments
5+
| ^^^^^^^^^^^^^^^^^^^
66

77
error: cannot find macro `foo` in this scope
88
--> $DIR/crate-type-macro-empty.rs:2:16

tests/ui/macros/missing-semi.stderr

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
error: expected `;`, found `(`
22
--> $DIR/missing-semi.rs:6:5
33
|
4+
LL | }
5+
| - expected `;`
46
LL | () => {
5-
| ^ no rules expected this token in macro call
7+
| ^ unexpected token
68

79
error: aborting due to 1 previous error
810

tests/ui/parser/issues/issue-7970b.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
fn main() {}
22

33
macro_rules! test {}
4-
//~^ ERROR unexpected end of macro invocation
4+
//~^ ERROR macros must contain at least one rule
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
error: unexpected end of macro invocation
1+
error: macros must contain at least one rule
22
--> $DIR/issue-7970b.rs:3:1
33
|
44
LL | macro_rules! test {}
5-
| ^^^^^^^^^^^^^^^^^^^^ missing tokens in macro arguments
5+
| ^^^^^^^^^^^^^^^^^^^^
66

77
error: aborting due to 1 previous error
88

tests/ui/parser/macros-no-semicolon-items.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
macro_rules! foo() //~ ERROR semicolon
2-
//~| ERROR unexpected end of macro
2+
//~| ERROR macros must contain at least one rule
33

44
macro_rules! bar {
55
($($tokens:tt)*) => {}

tests/ui/parser/macros-no-semicolon-items.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,11 @@ help: add a semicolon
3838
LL | );
3939
| +
4040

41-
error: unexpected end of macro invocation
41+
error: macros must contain at least one rule
4242
--> $DIR/macros-no-semicolon-items.rs:1:1
4343
|
4444
LL | macro_rules! foo()
45-
| ^^^^^^^^^^^^^^^^^^ missing tokens in macro arguments
45+
| ^^^^^^^^^^^^^^^^^^
4646

4747
error: aborting due to 3 previous errors
4848

0 commit comments

Comments
 (0)