Merge #11166

11166: minor: Simplify r=Veykril a=Veykril

bors r+

Co-authored-by: Lukas Wirth <lukastw97@gmail.com>

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 */
 "#]],
     )
 }

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() {
+        } else if let [state, ..] = &*eof_items {
-            error_recover_item = Some(eof_items[0].bindings.clone());
+            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())
+        let has_leftover_tokens = (bb_items.is_empty() && next_items.is_empty())
-            || (!bb_items.is_empty() && !next_items.is_empty())
+            || !(bb_items.is_empty() || next_items.is_empty())
-            || bb_items.len() > 1
+            || 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(()) => {
+        Err(()) => return Err(err!("expected leaf: `{}`", lhs)),
-            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 => (),
+        ) if lhs == rhs => Ok(()),
-        _ => {
+        _ => Err(ExpandError::UnexpectedToken),
-            return Err(ExpandError::UnexpectedToken);
-        }
     }
-
-    Ok(())
 }
 
 fn match_meta_var(kind: &str, input: &mut TtIter) -> ExpandResult<Option<Fragment>> {

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(|| {
+        macro_rules! binding_err {
-            ExpandError::BindingError(format!("could not find binding `{}`", name))
+            ($($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!(
+                    return Err(binding_err!("could not find empty binding `{}`", name));
-                        "could not find empty binding `{}`",
-                        name
-                    )));
                 }
             };
         }
         match b {
             Binding::Fragment(it) => Ok(it),
-            Binding::Nested(_) => Err(ExpandError::BindingError(format!(
+            Binding::Nested(_) => {
-                "expected simple binding, found nested binding `{}`",
+                Err(binding_err!("expected simple binding, found nested binding `{}`", name))
-                name
+            }
-            ))),
+            Binding::Empty => {
-            Binding::Empty => Err(ExpandError::BindingError(format!(
+                Err(binding_err!("expected simple binding, found empty binding `{}`", name))
-                "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 {
-                    for punct in puncts {
+                        buf.push(tt::Leaf::from(punct).into());
-                        buf.push(tt::Leaf::from(*punct).into());
                     }
+                    puncts.len()
                 }
-            }
+            };
         }
 
         if RepeatKind::ZeroOrOne == kind {

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<Op, ParseError> {
     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) => {
+                    tt::Leaf::Literal(lit) if is_boolean_literal(lit) => {
-                        if is_boolean_literal(lit) {
+                        let kind = eat_fragment_kind(src, mode)?;
-                            let name = lit.text.clone();
+                        let name = lit.text.clone();
-                            let kind = eat_fragment_kind(src, mode)?;
+                        let id = lit.id;
-                            let id = lit.id;
+                        Op::Var { name, kind, id }
-                            Op::Var { name, kind, id }
+                    }
-                        } else {
+                    tt::Leaf::Punct(_) | tt::Leaf::Literal(_) => {
-                            bail!("bad var 2");
+                        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<Option<SmolStr>, ParseError> {
     if let Mode::Pattern = mode {
-        src.expect_char(':').map_err(|()| err!("bad fragment specifier 1"))?;
+        src.expect_char(':').map_err(|()| err!("missing fragment specifier"))?;
-        let ident = src.expect_ident().map_err(|()| err!("bad fragment specifier 1"))?;
+        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<Separator>, RepeatKind), Par
                     '*' => RepeatKind::ZeroOrMore,
                     '+' => RepeatKind::OneOrMore,
                     '?' => RepeatKind::ZeroOrOne,
-                    _ => {
+                    _ => match &mut separator {
-                        match &mut separator {
+                        Separator::Puncts(puncts) if puncts.len() != 3 => {
-                            Separator::Puncts(puncts) => {
+                            puncts.push(*punct);
-                                if puncts.len() == 3 {
+                            continue;
-                                    return Err(ParseError::InvalidRepeat);
-                                }
-                                puncts.push(*punct)
-                            }
-                            _ => return Err(ParseError::InvalidRepeat),
                         }
-                        continue;
+                        _ => return Err(ParseError::InvalidRepeat),
-                    }
+                    },
                 };
-                let separator = if has_sep { Some(separator) } else { None };
+                return Ok((has_sep.then(|| separator), repeat_kind));
-                return Ok((separator, repeat_kind));
             }
         }
     }