From 706ac8256d878626126756969b48b262d2e187b5 Mon Sep 17 00:00:00 2001 From: Edwin Cheng Date: Sat, 30 Jan 2021 00:21:43 +0800 Subject: [PATCH] Simplify mbe match error. Handle parse error in rule parsing instead of match in mbe --- crates/mbe/src/expander.rs | 9 +- crates/mbe/src/expander/matcher.rs | 286 ++++++++++++------------- crates/mbe/src/expander/transcriber.rs | 7 - crates/mbe/src/lib.rs | 31 +-- crates/mbe/src/parser.rs | 40 ++-- crates/mbe/src/tests.rs | 17 +- 6 files changed, 180 insertions(+), 210 deletions(-) diff --git a/crates/mbe/src/expander.rs b/crates/mbe/src/expander.rs index 802c8fb0ff9..e7e14b3ccee 100644 --- a/crates/mbe/src/expander.rs +++ b/crates/mbe/src/expander.rs @@ -16,13 +16,8 @@ pub(crate) fn expand_rules( ) -> ExpandResult { let mut match_: Option<(matcher::Match, &crate::Rule)> = None; for rule in rules { - let new_match = match matcher::match_(&rule.lhs, input) { - Ok(m) => m, - Err(_e) => { - // error in pattern parsing - continue; - } - }; + let new_match = matcher::match_(&rule.lhs, input); + if new_match.err.is_none() { // If we find a rule that applies without errors, we're done. // Unconditionally returning the transcription here makes the diff --git a/crates/mbe/src/expander/matcher.rs b/crates/mbe/src/expander/matcher.rs index 5b58458509f..987a4f676d3 100644 --- a/crates/mbe/src/expander/matcher.rs +++ b/crates/mbe/src/expander/matcher.rs @@ -77,35 +77,26 @@ pub(super) fn add_err(&mut self, err: ExpandError) { } } -// General note: These functions have two channels to return errors, a `Result` -// return value and the `&mut Match`. The returned Result is for pattern parsing -// errors; if a branch of the macro definition doesn't parse, it doesn't make -// sense to try using it. Matching errors are added to the `Match`. It might -// make sense to make pattern parsing a separate step? - -pub(super) fn match_(pattern: &MetaTemplate, src: &tt::Subtree) -> Result { +/// Matching errors are added to the `Match`. +pub(super) fn match_(pattern: &MetaTemplate, src: &tt::Subtree) -> Match { assert!(pattern.delimiter == None); let mut res = Match::default(); let mut src = TtIter::new(src); - match_subtree(&mut res, pattern, &mut src)?; + match_subtree(&mut res, pattern, &mut src); if src.len() > 0 { res.unmatched_tts += src.len(); res.add_err(err!("leftover tokens")); } - Ok(res) + res } -fn match_subtree( - res: &mut Match, - pattern: &MetaTemplate, - src: &mut TtIter, -) -> Result<(), ExpandError> { +fn match_subtree(res: &mut Match, pattern: &MetaTemplate, src: &mut TtIter) { for op in pattern.iter() { - match op.as_ref().map_err(|err| err.clone())? { + match op { Op::Leaf(lhs) => { let rhs = match src.expect_leaf() { Ok(l) => l, @@ -145,7 +136,7 @@ fn match_subtree( continue; } let mut src = TtIter::new(rhs); - match_subtree(res, lhs, &mut src)?; + match_subtree(res, lhs, &mut src); if src.len() > 0 { res.add_err(err!("leftover tokens")); } @@ -172,11 +163,139 @@ fn match_subtree( } } Op::Repeat { subtree, kind, separator } => { - match_repeat(res, subtree, *kind, separator, src)?; + match_repeat(res, subtree, *kind, separator, src); } } } - Ok(()) +} + +pub(super) fn match_repeat( + res: &mut Match, + pattern: &MetaTemplate, + kind: RepeatKind, + separator: &Option, + src: &mut TtIter, +) { + // Dirty hack to make macro-expansion terminate. + // This should be replaced by a proper macro-by-example implementation + let mut limit = 65536; + let mut counter = 0; + + for i in 0.. { + let mut fork = src.clone(); + + if let Some(separator) = &separator { + if i != 0 && !fork.eat_separator(separator) { + break; + } + } + + let mut nested = Match::default(); + match_subtree(&mut nested, pattern, &mut fork); + if nested.err.is_none() { + limit -= 1; + if limit == 0 { + log::warn!( + "match_lhs exceeded repeat pattern limit => {:#?}\n{:#?}\n{:#?}\n{:#?}", + pattern, + src, + kind, + separator + ); + break; + } + *src = fork; + + if let Err(err) = res.bindings.push_nested(counter, nested.bindings) { + res.add_err(err); + } + counter += 1; + if counter == 1 { + if let RepeatKind::ZeroOrOne = kind { + break; + } + } + } else { + break; + } + } + + match (kind, counter) { + (RepeatKind::OneOrMore, 0) => { + res.add_err(ExpandError::UnexpectedToken); + } + (_, 0) => { + // Collect all empty variables in subtrees + let mut vars = Vec::new(); + collect_vars(&mut vars, pattern); + for var in vars { + res.bindings.push_empty(&var) + } + } + _ => (), + } +} + +fn match_meta_var(kind: &str, input: &mut TtIter) -> ExpandResult> { + let fragment = match kind { + "path" => Path, + "expr" => Expr, + "ty" => Type, + "pat" => Pattern, + "stmt" => Statement, + "block" => Block, + "meta" => MetaItem, + "item" => Item, + _ => { + let tt_result = match kind { + "ident" => input + .expect_ident() + .map(|ident| Some(tt::Leaf::from(ident.clone()).into())) + .map_err(|()| err!("expected ident")), + "tt" => input.expect_tt().map(Some).map_err(|()| err!()), + "lifetime" => input + .expect_lifetime() + .map(|tt| Some(tt)) + .map_err(|()| err!("expected lifetime")), + "literal" => { + let neg = input.eat_char('-'); + input + .expect_literal() + .map(|literal| { + let lit = tt::Leaf::from(literal.clone()); + match neg { + None => Some(lit.into()), + Some(neg) => Some(tt::TokenTree::Subtree(tt::Subtree { + delimiter: None, + token_trees: vec![neg, lit.into()], + })), + } + }) + .map_err(|()| err!()) + } + // `vis` is optional + "vis" => match input.eat_vis() { + Some(vis) => Ok(Some(vis)), + None => Ok(None), + }, + _ => Err(ExpandError::UnexpectedToken), + }; + return tt_result.map(|it| it.map(Fragment::Tokens)).into(); + } + }; + let result = input.expect_fragment(fragment); + result.map(|tt| if kind == "expr" { tt.map(Fragment::Ast) } else { tt.map(Fragment::Tokens) }) +} + +fn collect_vars(buf: &mut Vec, pattern: &MetaTemplate) { + for op in pattern.iter() { + match op { + Op::Var { name, .. } => buf.push(name.clone()), + Op::Leaf(_) => (), + Op::Subtree(subtree) => collect_vars(buf, subtree), + Op::Repeat { subtree, .. } => collect_vars(buf, subtree), + } + } } impl<'a> TtIter<'a> { @@ -369,134 +488,3 @@ pub(crate) fn eat_char(&mut self, c: char) -> Option { } } } - -pub(super) fn match_repeat( - res: &mut Match, - pattern: &MetaTemplate, - kind: RepeatKind, - separator: &Option, - src: &mut TtIter, -) -> Result<(), ExpandError> { - // Dirty hack to make macro-expansion terminate. - // This should be replaced by a proper macro-by-example implementation - let mut limit = 65536; - let mut counter = 0; - - for i in 0.. { - let mut fork = src.clone(); - - if let Some(separator) = &separator { - if i != 0 && !fork.eat_separator(separator) { - break; - } - } - - let mut nested = Match::default(); - match_subtree(&mut nested, pattern, &mut fork)?; - if nested.err.is_none() { - limit -= 1; - if limit == 0 { - log::warn!( - "match_lhs exceeded repeat pattern limit => {:#?}\n{:#?}\n{:#?}\n{:#?}", - pattern, - src, - kind, - separator - ); - break; - } - *src = fork; - - if let Err(err) = res.bindings.push_nested(counter, nested.bindings) { - res.add_err(err); - } - counter += 1; - if counter == 1 { - if let RepeatKind::ZeroOrOne = kind { - break; - } - } - } else { - break; - } - } - - match (kind, counter) { - (RepeatKind::OneOrMore, 0) => { - res.add_err(ExpandError::UnexpectedToken); - } - (_, 0) => { - // Collect all empty variables in subtrees - let mut vars = Vec::new(); - collect_vars(&mut vars, pattern)?; - for var in vars { - res.bindings.push_empty(&var) - } - } - _ => (), - } - Ok(()) -} - -fn match_meta_var(kind: &str, input: &mut TtIter) -> ExpandResult> { - let fragment = match kind { - "path" => Path, - "expr" => Expr, - "ty" => Type, - "pat" => Pattern, - "stmt" => Statement, - "block" => Block, - "meta" => MetaItem, - "item" => Item, - _ => { - let tt_result = match kind { - "ident" => input - .expect_ident() - .map(|ident| Some(tt::Leaf::from(ident.clone()).into())) - .map_err(|()| err!("expected ident")), - "tt" => input.expect_tt().map(Some).map_err(|()| err!()), - "lifetime" => input - .expect_lifetime() - .map(|tt| Some(tt)) - .map_err(|()| err!("expected lifetime")), - "literal" => { - let neg = input.eat_char('-'); - input - .expect_literal() - .map(|literal| { - let lit = tt::Leaf::from(literal.clone()); - match neg { - None => Some(lit.into()), - Some(neg) => Some(tt::TokenTree::Subtree(tt::Subtree { - delimiter: None, - token_trees: vec![neg, lit.into()], - })), - } - }) - .map_err(|()| err!()) - } - // `vis` is optional - "vis" => match input.eat_vis() { - Some(vis) => Ok(Some(vis)), - None => Ok(None), - }, - _ => Err(ExpandError::UnexpectedToken), - }; - return tt_result.map(|it| it.map(Fragment::Tokens)).into(); - } - }; - let result = input.expect_fragment(fragment); - result.map(|tt| if kind == "expr" { tt.map(Fragment::Ast) } else { tt.map(Fragment::Tokens) }) -} - -fn collect_vars(buf: &mut Vec, pattern: &MetaTemplate) -> Result<(), ExpandError> { - for op in pattern.iter() { - match op.as_ref().map_err(|e| e.clone())? { - Op::Var { name, .. } => buf.push(name.clone()), - Op::Leaf(_) => (), - Op::Subtree(subtree) => collect_vars(buf, subtree)?, - Op::Repeat { subtree, .. } => collect_vars(buf, subtree)?, - } - } - Ok(()) -} diff --git a/crates/mbe/src/expander/transcriber.rs b/crates/mbe/src/expander/transcriber.rs index 82bace110f4..30c090f32bd 100644 --- a/crates/mbe/src/expander/transcriber.rs +++ b/crates/mbe/src/expander/transcriber.rs @@ -86,13 +86,6 @@ fn expand_subtree( let start_elements = arena.len(); let mut err = None; for op in template.iter() { - let op = match op { - Ok(op) => op, - Err(e) => { - err = Some(e.clone()); - break; - } - }; match op { Op::Leaf(tt) => arena.push(tt.clone().into()), Op::Subtree(tt) => { diff --git a/crates/mbe/src/lib.rs b/crates/mbe/src/lib.rs index bbe71ce3e68..56c632665ea 100644 --- a/crates/mbe/src/lib.rs +++ b/crates/mbe/src/lib.rs @@ -24,7 +24,9 @@ #[derive(Debug, PartialEq, Eq)] pub enum ParseError { + UnexpectedToken(String), Expected(String), + InvalidRepeat, RepetitionEmptyTokenTree, } @@ -34,7 +36,6 @@ pub enum ExpandError { UnexpectedToken, BindingError(String), ConversionError, - InvalidRepeat, ProcMacroError(tt::ExpansionError), UnresolvedProcMacro, Other(String), @@ -53,7 +54,6 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { ExpandError::UnexpectedToken => f.write_str("unexpected token in input"), ExpandError::BindingError(e) => f.write_str(e), ExpandError::ConversionError => f.write_str("could not convert tokens"), - ExpandError::InvalidRepeat => f.write_str("invalid repeat expression"), ExpandError::ProcMacroError(e) => e.fmt(f), ExpandError::UnresolvedProcMacro => f.write_str("unresolved proc macro"), ExpandError::Other(e) => f.write_str(e), @@ -94,11 +94,11 @@ struct Rule { #[derive(Clone, Debug, PartialEq, Eq)] struct MetaTemplate { delimiter: Option, - tokens: Vec>, + tokens: Vec, } impl<'a> MetaTemplate { - fn iter(&self) -> impl Iterator> { + fn iter(&self) -> impl Iterator { self.tokens.iter() } @@ -288,25 +288,15 @@ fn parse(src: &mut TtIter, expect_arrow: bool) -> Result { .expect_subtree() .map_err(|()| ParseError::Expected("expected subtree".to_string()))?; - let lhs = MetaTemplate { tokens: parse_pattern(&lhs), delimiter: None }; - let rhs = MetaTemplate { tokens: parse_template(&rhs), delimiter: None }; + let lhs = MetaTemplate { tokens: parse_pattern(&lhs)?, delimiter: None }; + let rhs = MetaTemplate { tokens: parse_template(&rhs)?, delimiter: None }; Ok(crate::Rule { lhs, rhs }) } } -fn to_parse_error(e: &ExpandError) -> ParseError { - let msg = match e { - ExpandError::InvalidRepeat => "invalid repeat".to_string(), - _ => "invalid macro definition".to_string(), - }; - ParseError::Expected(msg) -} - fn validate(pattern: &MetaTemplate) -> Result<(), ParseError> { for op in pattern.iter() { - let op = op.as_ref().map_err(|e| to_parse_error(&e))?; - match op { Op::Subtree(subtree) => validate(&subtree)?, Op::Repeat { subtree, separator, .. } => { @@ -315,20 +305,21 @@ fn validate(pattern: &MetaTemplate) -> Result<(), ParseError> { if separator.is_none() { if subtree.iter().all(|child_op| { - match child_op.as_ref().map_err(to_parse_error) { - Ok(Op::Var { kind, .. }) => { + match child_op { + Op::Var { kind, .. } => { // vis is optional if kind.as_ref().map_or(false, |it| it == "vis") { return true; } } - Ok(Op::Repeat { kind, .. }) => { + Op::Repeat { kind, .. } => { return matches!( kind, parser::RepeatKind::ZeroOrMore | parser::RepeatKind::ZeroOrOne ) } - _ => {} + Op::Leaf(_) => {} + Op::Subtree(_) => {} } false }) { diff --git a/crates/mbe/src/parser.rs b/crates/mbe/src/parser.rs index f3047972dd2..b90ae7015db 100644 --- a/crates/mbe/src/parser.rs +++ b/crates/mbe/src/parser.rs @@ -4,7 +4,7 @@ use smallvec::SmallVec; use syntax::SmolStr; -use crate::{tt_iter::TtIter, ExpandError, MetaTemplate}; +use crate::{tt_iter::TtIter, MetaTemplate, ParseError}; #[derive(Clone, Debug, PartialEq, Eq)] pub(crate) enum Op { @@ -46,12 +46,12 @@ fn eq(&self, other: &Separator) -> bool { } } -pub(crate) fn parse_template(template: &tt::Subtree) -> Vec> { - parse_inner(&template, Mode::Template) +pub(crate) fn parse_template(template: &tt::Subtree) -> Result, ParseError> { + parse_inner(&template, Mode::Template).into_iter().collect() } -pub(crate) fn parse_pattern(pattern: &tt::Subtree) -> Vec> { - parse_inner(&pattern, Mode::Pattern) +pub(crate) fn parse_pattern(pattern: &tt::Subtree) -> Result, ParseError> { + parse_inner(&pattern, Mode::Pattern).into_iter().collect() } #[derive(Clone, Copy)] @@ -60,7 +60,7 @@ enum Mode { Template, } -fn parse_inner(tt: &tt::Subtree, mode: Mode) -> Vec> { +fn parse_inner(tt: &tt::Subtree, mode: Mode) -> Vec> { let mut src = TtIter::new(&tt); std::iter::from_fn(move || { let first = src.next()?; @@ -71,7 +71,7 @@ fn parse_inner(tt: &tt::Subtree, mode: Mode) -> Vec> { macro_rules! err { ($($tt:tt)*) => { - ExpandError::UnexpectedToken + ParseError::UnexpectedToken(($($tt)*).to_string()) }; } @@ -81,7 +81,7 @@ macro_rules! bail { }; } -fn next_op<'a>(first: &tt::TokenTree, src: &mut TtIter<'a>, mode: Mode) -> Result { +fn next_op<'a>(first: &tt::TokenTree, src: &mut TtIter<'a>, mode: Mode) -> Result { let res = match first { tt::TokenTree::Leaf(leaf @ tt::Leaf::Punct(tt::Punct { char: '$', .. })) => { // Note that the '$' itself is a valid token inside macro_rules. @@ -93,7 +93,9 @@ fn next_op<'a>(first: &tt::TokenTree, src: &mut TtIter<'a>, mode: Mode) -> Resul tt::TokenTree::Subtree(subtree) => { let (separator, kind) = parse_repeat(src)?; let delimiter = subtree.delimiter; - let tokens = parse_inner(&subtree, mode); + let tokens = parse_inner(&subtree, mode) + .into_iter() + .collect::, ParseError>>()?; let subtree = MetaTemplate { tokens, delimiter }; Op::Repeat { subtree, separator, kind } } @@ -102,7 +104,7 @@ fn next_op<'a>(first: &tt::TokenTree, src: &mut TtIter<'a>, mode: Mode) -> Resul static UNDERSCORE: SmolStr = SmolStr::new_inline("_"); if punct.char != '_' { - return Err(ExpandError::UnexpectedToken); + return Err(ParseError::Expected("_".to_string())); } let name = UNDERSCORE.clone(); let kind = eat_fragment_kind(src, mode)?; @@ -135,7 +137,9 @@ fn next_op<'a>(first: &tt::TokenTree, src: &mut TtIter<'a>, mode: Mode) -> Resul tt::TokenTree::Leaf(tt) => Op::Leaf(tt.clone()), tt::TokenTree::Subtree(subtree) => { let delimiter = subtree.delimiter; - let tokens = parse_inner(&subtree, mode); + let tokens = + parse_inner(&subtree, mode).into_iter().collect::, ParseError>>()?; + let subtree = MetaTemplate { tokens, delimiter }; Op::Subtree(subtree) } @@ -143,7 +147,7 @@ fn next_op<'a>(first: &tt::TokenTree, src: &mut TtIter<'a>, mode: Mode) -> Resul Ok(res) } -fn eat_fragment_kind<'a>(src: &mut TtIter<'a>, mode: Mode) -> Result, ExpandError> { +fn eat_fragment_kind<'a>(src: &mut TtIter<'a>, mode: Mode) -> Result, ParseError> { if let Mode::Pattern = mode { src.expect_char(':').map_err(|()| err!("bad fragment specifier 1"))?; let ident = src.expect_ident().map_err(|()| err!("bad fragment specifier 1"))?; @@ -156,12 +160,12 @@ fn is_boolean_literal(lit: &tt::Literal) -> bool { matches!(lit.text.as_str(), "true" | "false") } -fn parse_repeat(src: &mut TtIter) -> Result<(Option, RepeatKind), ExpandError> { +fn parse_repeat(src: &mut TtIter) -> Result<(Option, RepeatKind), ParseError> { let mut separator = Separator::Puncts(SmallVec::new()); for tt in src { let tt = match tt { tt::TokenTree::Leaf(leaf) => leaf, - tt::TokenTree::Subtree(_) => return Err(ExpandError::InvalidRepeat), + tt::TokenTree::Subtree(_) => return Err(ParseError::InvalidRepeat), }; let has_sep = match &separator { Separator::Puncts(puncts) => !puncts.is_empty(), @@ -169,7 +173,7 @@ fn parse_repeat(src: &mut TtIter) -> Result<(Option, RepeatKind), Exp }; match tt { tt::Leaf::Ident(_) | tt::Leaf::Literal(_) if has_sep => { - return Err(ExpandError::InvalidRepeat) + return Err(ParseError::InvalidRepeat) } tt::Leaf::Ident(ident) => separator = Separator::Ident(ident.clone()), tt::Leaf::Literal(lit) => separator = Separator::Literal(lit.clone()), @@ -182,11 +186,11 @@ fn parse_repeat(src: &mut TtIter) -> Result<(Option, RepeatKind), Exp match &mut separator { Separator::Puncts(puncts) => { if puncts.len() == 3 { - return Err(ExpandError::InvalidRepeat); + return Err(ParseError::InvalidRepeat); } puncts.push(punct.clone()) } - _ => return Err(ExpandError::InvalidRepeat), + _ => return Err(ParseError::InvalidRepeat), } continue; } @@ -196,5 +200,5 @@ fn parse_repeat(src: &mut TtIter) -> Result<(Option, RepeatKind), Exp } } } - Err(ExpandError::InvalidRepeat) + Err(ParseError::InvalidRepeat) } diff --git a/crates/mbe/src/tests.rs b/crates/mbe/src/tests.rs index 8d978163d6f..1c467facdc3 100644 --- a/crates/mbe/src/tests.rs +++ b/crates/mbe/src/tests.rs @@ -33,19 +33,18 @@ fn check(macro_body: &str) { #[test] fn test_invalid_arms() { - fn check(macro_body: &str, err: &str) { + fn check(macro_body: &str, err: ParseError) { let m = parse_macro_arm(macro_body); - assert_eq!(m, Err(ParseError::Expected(String::from(err)))); + assert_eq!(m, Err(err.into())); } + check("invalid", ParseError::Expected("expected subtree".into())); - check("invalid", "expected subtree"); + check("$i:ident => ()", ParseError::Expected("expected subtree".into())); + check("($i:ident) ()", ParseError::Expected("expected `=`".into())); + check("($($i:ident)_) => ()", ParseError::InvalidRepeat); - check("$i:ident => ()", "expected subtree"); - check("($i:ident) ()", "expected `=`"); - check("($($i:ident)_) => ()", "invalid repeat"); - - check("($i) => ($i)", "invalid macro definition"); - check("($i:) => ($i)", "invalid macro definition"); + check("($i) => ($i)", ParseError::UnexpectedToken("bad fragment specifier 1".into())); + check("($i:) => ($i)", ParseError::UnexpectedToken("bad fragment specifier 1".into())); } fn parse_macro_arm(arm_definition: &str) -> Result {