From f31863b189aa438ba8d1d508b38a8dcdcae2fa9e Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 2 Jan 2022 16:35:58 +0100 Subject: [PATCH] minor: Simplify --- .../macro_expansion_tests/mbe/meta_syntax.rs | 6 +-- crates/mbe/src/expander/matcher.rs | 38 +++++++------- crates/mbe/src/expander/transcriber.rs | 52 +++++++++---------- crates/mbe/src/parser.rs | 52 +++++++------------ 4 files changed, 65 insertions(+), 83 deletions(-) diff --git a/crates/hir_def/src/macro_expansion_tests/mbe/meta_syntax.rs b/crates/hir_def/src/macro_expansion_tests/mbe/meta_syntax.rs index dd5effa368..2de10ddbdf 100644 --- a/crates/hir_def/src/macro_expansion_tests/mbe/meta_syntax.rs +++ b/crates/hir_def/src/macro_expansion_tests/mbe/meta_syntax.rs @@ -69,11 +69,11 @@ macro_rules! e3 { ($(i:ident)_) => () } /* error: invalid macro definition: invalid repeat */ macro_rules! f1 { ($i) => ($i) } -/* error: invalid macro definition: bad fragment specifier 1 */ +/* error: invalid macro definition: missing fragment specifier */ macro_rules! f2 { ($i:) => ($i) } -/* error: invalid macro definition: bad fragment specifier 1 */ +/* error: invalid macro definition: missing fragment specifier */ macro_rules! f3 { ($i:_) => () } -/* error: invalid macro definition: bad fragment specifier 1 */ +/* error: invalid macro definition: missing fragment specifier */ "#]], ) } diff --git a/crates/mbe/src/expander/matcher.rs b/crates/mbe/src/expander/matcher.rs index bcda2381a4..910724411d 100644 --- a/crates/mbe/src/expander/matcher.rs +++ b/crates/mbe/src/expander/matcher.rs @@ -571,18 +571,18 @@ fn match_loop(pattern: &MetaTemplate, src: &tt::Subtree) -> Match { if !error_items.is_empty() { error_recover_item = error_items.pop().map(|it| it.bindings); - } else if !eof_items.is_empty() { - error_recover_item = Some(eof_items[0].bindings.clone()); + } else if let [state, ..] = &*eof_items { + error_recover_item = Some(state.bindings.clone()); } // We need to do some post processing after the `match_loop_inner`. // If we reached the EOF, check that there is EXACTLY ONE possible matcher. Otherwise, // either the parse is ambiguous (which should never happen) or there is a syntax error. if src.peek_n(0).is_none() && stack.is_empty() { - if eof_items.len() == 1 { + if let [state] = &*eof_items { // remove all errors, because it is the correct answer ! res = Match::default(); - res.bindings = bindings_builder.build(&eof_items[0].bindings); + res.bindings = bindings_builder.build(&state.bindings); } else { // Error recovery if let Some(item) = error_recover_item { @@ -598,10 +598,10 @@ fn match_loop(pattern: &MetaTemplate, src: &tt::Subtree) -> Match { // // Another possibility is that we need to call out to parse some rust nonterminal // (black-box) parser. However, if there is not EXACTLY ONE of these, something is wrong. - if (bb_items.is_empty() && next_items.is_empty()) - || (!bb_items.is_empty() && !next_items.is_empty()) - || bb_items.len() > 1 - { + let has_leftover_tokens = (bb_items.is_empty() && next_items.is_empty()) + || !(bb_items.is_empty() || next_items.is_empty()) + || bb_items.len() > 1; + if has_leftover_tokens { res.unmatched_tts += src.len(); while let Some(it) = stack.pop() { src = it; @@ -624,7 +624,11 @@ fn match_loop(pattern: &MetaTemplate, src: &tt::Subtree) -> Match { stack.push(src.clone()); src = TtIter::new(subtree); } - None if !stack.is_empty() => src = stack.pop().unwrap(), + None => { + if let Some(iter) = stack.pop() { + src = iter; + } + } _ => (), } } @@ -662,29 +666,23 @@ fn match_loop(pattern: &MetaTemplate, src: &tt::Subtree) -> Match { fn match_leaf(lhs: &tt::Leaf, src: &mut TtIter) -> Result<(), ExpandError> { let rhs = match src.expect_leaf() { Ok(l) => l, - Err(()) => { - return Err(err!("expected leaf: `{}`", lhs)); - } + Err(()) => return Err(err!("expected leaf: `{}`", lhs)), }; match (lhs, rhs) { ( tt::Leaf::Punct(tt::Punct { char: lhs, .. }), tt::Leaf::Punct(tt::Punct { char: rhs, .. }), - ) if lhs == rhs => (), + ) if lhs == rhs => Ok(()), ( tt::Leaf::Ident(tt::Ident { text: lhs, .. }), tt::Leaf::Ident(tt::Ident { text: rhs, .. }), - ) if lhs == rhs => (), + ) if lhs == rhs => Ok(()), ( tt::Leaf::Literal(tt::Literal { text: lhs, .. }), tt::Leaf::Literal(tt::Literal { text: rhs, .. }), - ) if lhs == rhs => (), - _ => { - return Err(ExpandError::UnexpectedToken); - } + ) if lhs == rhs => Ok(()), + _ => Err(ExpandError::UnexpectedToken), } - - Ok(()) } fn match_meta_var(kind: &str, input: &mut TtIter) -> ExpandResult> { diff --git a/crates/mbe/src/expander/transcriber.rs b/crates/mbe/src/expander/transcriber.rs index 98b37dd122..b5ae5b91bb 100644 --- a/crates/mbe/src/expander/transcriber.rs +++ b/crates/mbe/src/expander/transcriber.rs @@ -4,11 +4,10 @@ use syntax::SmolStr; use tt::{Delimiter, Subtree}; -use super::ExpandResult; use crate::{ expander::{Binding, Bindings, Fragment}, parser::{Op, RepeatKind, Separator}, - ExpandError, MetaTemplate, + ExpandError, ExpandResult, MetaTemplate, }; impl Bindings { @@ -17,36 +16,36 @@ impl Bindings { } fn get(&self, name: &str, nesting: &mut [NestingState]) -> Result<&Fragment, ExpandError> { - let mut b: &Binding = self.inner.get(name).ok_or_else(|| { - ExpandError::BindingError(format!("could not find binding `{}`", name)) - })?; + macro_rules! binding_err { + ($($arg:tt)*) => { ExpandError::BindingError(format!($($arg)*)) }; + } + + let mut b: &Binding = self + .inner + .get(name) + .ok_or_else(|| binding_err!("could not find binding `{}`", name))?; for nesting_state in nesting.iter_mut() { nesting_state.hit = true; b = match b { Binding::Fragment(_) => break, Binding::Nested(bs) => bs.get(nesting_state.idx).ok_or_else(|| { nesting_state.at_end = true; - ExpandError::BindingError(format!("could not find nested binding `{}`", name)) + binding_err!("could not find nested binding `{}`", name) })?, Binding::Empty => { nesting_state.at_end = true; - return Err(ExpandError::BindingError(format!( - "could not find empty binding `{}`", - name - ))); + return Err(binding_err!("could not find empty binding `{}`", name)); } }; } match b { Binding::Fragment(it) => Ok(it), - Binding::Nested(_) => Err(ExpandError::BindingError(format!( - "expected simple binding, found nested binding `{}`", - name - ))), - Binding::Empty => Err(ExpandError::BindingError(format!( - "expected simple binding, found empty binding `{}`", - name - ))), + Binding::Nested(_) => { + Err(binding_err!("expected simple binding, found nested binding `{}`", name)) + } + Binding::Empty => { + Err(binding_err!("expected simple binding, found empty binding `{}`", name)) + } } } } @@ -109,7 +108,7 @@ fn expand_subtree( } } // drain the elements added in this instance of expand_subtree - let tts = arena.drain(start_elements..arena.len()).collect(); + let tts = arena.drain(start_elements..).collect(); ExpandResult { value: tt::Subtree { delimiter, token_trees: tts }, err } } @@ -193,23 +192,22 @@ fn expand_repeat( push_subtree(&mut buf, t); if let Some(sep) = separator { - match sep { + has_seps = match sep { Separator::Ident(ident) => { - has_seps = 1; buf.push(tt::Leaf::from(ident.clone()).into()); + 1 } Separator::Literal(lit) => { - has_seps = 1; buf.push(tt::Leaf::from(lit.clone()).into()); + 1 } - Separator::Puncts(puncts) => { - has_seps = puncts.len(); - for punct in puncts { - buf.push(tt::Leaf::from(*punct).into()); + for &punct in puncts { + buf.push(tt::Leaf::from(punct).into()); } + puncts.len() } - } + }; } if RepeatKind::ZeroOrOne == kind { diff --git a/crates/mbe/src/parser.rs b/crates/mbe/src/parser.rs index 7543054494..ef29380557 100644 --- a/crates/mbe/src/parser.rs +++ b/crates/mbe/src/parser.rs @@ -41,7 +41,7 @@ impl MetaTemplate { let mut res = Vec::new(); while let Some(first) = src.next() { let op = next_op(first, &mut src, mode)?; - res.push(op) + res.push(op); } Ok(MetaTemplate(res)) @@ -110,12 +110,6 @@ macro_rules! err { }; } -macro_rules! bail { - ($($tt:tt)*) => { - return Err(err!($($tt)*)) - }; -} - 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: '$', .. })) => { @@ -131,26 +125,24 @@ fn next_op<'a>(first: &tt::TokenTree, src: &mut TtIter<'a>, mode: Mode) -> Resul Op::Repeat { tokens, separator, kind } } tt::TokenTree::Leaf(leaf) => match leaf { - tt::Leaf::Punct(_) => return Err(ParseError::Expected("ident".to_string())), tt::Leaf::Ident(ident) if ident.text == "crate" => { // We simply produce identifier `$crate` here. And it will be resolved when lowering ast to Path. Op::Leaf(tt::Leaf::from(tt::Ident { text: "$crate".into(), id: ident.id })) } tt::Leaf::Ident(ident) => { - let name = ident.text.clone(); let kind = eat_fragment_kind(src, mode)?; + let name = ident.text.clone(); let id = ident.id; Op::Var { name, kind, id } } - tt::Leaf::Literal(lit) => { - if is_boolean_literal(lit) { - let name = lit.text.clone(); - let kind = eat_fragment_kind(src, mode)?; - let id = lit.id; - Op::Var { name, kind, id } - } else { - bail!("bad var 2"); - } + tt::Leaf::Literal(lit) if is_boolean_literal(lit) => { + let kind = eat_fragment_kind(src, mode)?; + let name = lit.text.clone(); + let id = lit.id; + Op::Var { name, kind, id } + } + tt::Leaf::Punct(_) | tt::Leaf::Literal(_) => { + return Err(ParseError::Expected("ident".to_string())) } }, } @@ -166,8 +158,8 @@ fn next_op<'a>(first: &tt::TokenTree, src: &mut TtIter<'a>, mode: Mode) -> Resul fn eat_fragment_kind(src: &mut TtIter<'_>, 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"))?; + src.expect_char(':').map_err(|()| err!("missing fragment specifier"))?; + let ident = src.expect_ident().map_err(|()| err!("missing fragment specifier"))?; return Ok(Some(ident.text.clone())); }; Ok(None) @@ -199,21 +191,15 @@ fn parse_repeat(src: &mut TtIter) -> Result<(Option, RepeatKind), Par '*' => RepeatKind::ZeroOrMore, '+' => RepeatKind::OneOrMore, '?' => RepeatKind::ZeroOrOne, - _ => { - match &mut separator { - Separator::Puncts(puncts) => { - if puncts.len() == 3 { - return Err(ParseError::InvalidRepeat); - } - puncts.push(*punct) - } - _ => return Err(ParseError::InvalidRepeat), + _ => match &mut separator { + Separator::Puncts(puncts) if puncts.len() != 3 => { + puncts.push(*punct); + continue; } - continue; - } + _ => return Err(ParseError::InvalidRepeat), + }, }; - let separator = if has_sep { Some(separator) } else { None }; - return Ok((separator, repeat_kind)); + return Ok((has_sep.then(|| separator), repeat_kind)); } } }