From 32dce75747271236be0ca8762b338c317bd48204 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Mon, 16 Mar 2020 18:38:10 +0100 Subject: Some more refactoring --- crates/ra_mbe/src/mbe_expander.rs | 22 +++--- crates/ra_mbe/src/mbe_expander/matcher.rs | 112 +++++++++++++++++------------- 2 files changed, 76 insertions(+), 58 deletions(-) diff --git a/crates/ra_mbe/src/mbe_expander.rs b/crates/ra_mbe/src/mbe_expander.rs index 3c00e3b64..7adb70d45 100644 --- a/crates/ra_mbe/src/mbe_expander.rs +++ b/crates/ra_mbe/src/mbe_expander.rs @@ -16,10 +16,15 @@ pub(crate) fn expand(rules: &crate::MacroRules, input: &tt::Subtree) -> ExpandRe fn expand_rules(rules: &[crate::Rule], input: &tt::Subtree) -> ExpandResult { let mut match_: Option<(matcher::Match, &crate::Rule)> = None; - let mut err = Some(ExpandError::NoMatchingRule); for rule in rules { - let ExpandResult(new_match, bindings_err) = matcher::match_(&rule.lhs, input); - if bindings_err.is_none() { + let new_match = match matcher::match_(&rule.lhs, input) { + Ok(m) => m, + Err(_e) => { + // error in pattern parsing + continue; + } + }; + 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 // `test_repeat_bad_var` test fail. @@ -32,25 +37,22 @@ fn expand_rules(rules: &[crate::Rule], input: &tt::Subtree) -> ExpandResult, + pub err_count: usize, + /// How many top-level token trees were left to match. + pub unmatched_tts: usize, } -pub(super) fn match_(pattern: &tt::Subtree, src: &tt::Subtree) -> ExpandResult { +impl Match { + pub fn add_err(&mut self, err: ExpandError) { + let prev_err = self.err.take(); + self.err = prev_err.or(Some(err)); + self.err_count += 1; + } +} + +// 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: &tt::Subtree, src: &tt::Subtree) -> Result { assert!(pattern.delimiter == None); let mut res = Match::default(); let mut src = TtIter::new(src); - let mut err = match_subtree(&mut res, pattern, &mut src).err(); + match_subtree(&mut res, pattern, &mut src)?; - res.unmatched_tokens += src.len(); - if src.len() > 0 && err.is_none() { - err = Some(err!("leftover tokens")); + if src.len() > 0 { + res.unmatched_tts += src.len(); + res.add_err(err!("leftover tokens")); } - ExpandResult(res, err) + Ok(res) } fn match_subtree( @@ -87,19 +104,13 @@ fn match_subtree( pattern: &tt::Subtree, src: &mut TtIter, ) -> Result<(), ExpandError> { - let mut result = Ok(()); for op in parse_pattern(pattern) { - if result.is_err() { - // We're just going through the patterns to count how many we missed - res.unmatched_patterns += 1; - continue; - } match op? { Op::TokenTree(tt::TokenTree::Leaf(lhs)) => { let rhs = match src.expect_leaf() { Ok(l) => l, Err(()) => { - result = Err(err!("expected leaf: `{}`", lhs)); + res.add_err(err!("expected leaf: `{}`", lhs)); continue; } }; @@ -117,7 +128,7 @@ fn match_subtree( tt::Leaf::Literal(tt::Literal { text: rhs, .. }), ) if lhs == rhs => (), _ => { - result = Err(ExpandError::UnexpectedToken); + res.add_err(ExpandError::UnexpectedToken); } } } @@ -125,26 +136,25 @@ fn match_subtree( let rhs = match src.expect_subtree() { Ok(s) => s, Err(()) => { - result = Err(err!("expected subtree")); + res.add_err(err!("expected subtree")); continue; } }; if lhs.delimiter_kind() != rhs.delimiter_kind() { - result = Err(err!("mismatched delimiter")); + res.add_err(err!("mismatched delimiter")); continue; } let mut src = TtIter::new(rhs); - result = match_subtree(res, lhs, &mut src); - res.unmatched_tokens += src.len(); - if src.len() > 0 && result.is_ok() { - result = Err(err!("leftover tokens")); + match_subtree(res, lhs, &mut src)?; + if src.len() > 0 { + res.add_err(err!("leftover tokens")); } } Op::Var { name, kind } => { let kind = match kind { Some(k) => k, None => { - result = Err(ExpandError::UnexpectedToken); + res.add_err(ExpandError::UnexpectedToken); continue; } }; @@ -156,14 +166,16 @@ fn match_subtree( None if match_err.is_none() => res.bindings.push_optional(name), _ => {} } - result = match_err.map_or(Ok(()), Err); + if let Some(err) = match_err { + res.add_err(err); + } } Op::Repeat { subtree, kind, separator } => { - result = match_repeat(res, subtree, kind, separator, src); + match_repeat(res, subtree, kind, separator, src)?; } } } - result + Ok(()) } impl<'a> TtIter<'a> { @@ -345,35 +357,39 @@ pub(super) fn match_repeat( } let mut nested = Match::default(); - match match_subtree(&mut nested, pattern, &mut fork) { - Ok(()) => { - limit -= 1; - if limit == 0 { - log::warn!( - "match_lhs exceeded repeat pattern limit => {:#?}\n{:#?}\n{:#?}\n{:#?}", - pattern, - src, - kind, - separator - ); - break; - } - *src = fork; + 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; - res.bindings.push_nested(counter, nested.bindings)?; - counter += 1; - if counter == 1 { - if let RepeatKind::ZeroOrOne = kind { - break; - } + 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; } } - Err(_) => break, + } else { + break; } } match (kind, counter) { - (RepeatKind::OneOrMore, 0) => return Err(ExpandError::UnexpectedToken), + (RepeatKind::OneOrMore, 0) => { + res.add_err(ExpandError::UnexpectedToken); + } (_, 0) => { // Collect all empty variables in subtrees let mut vars = Vec::new(); -- cgit v1.2.3