6 files changed, 243 insertions, 276 deletions

diff --git a/crates/mbe/src/mbe_expander.rs b/crates/mbe/src/expander.rs
index 802c8fb0f..e7e14b3cc 100644
--- a/crates/mbe/src/mbe_expander.rs
+++ b/crates/mbe/src/expander.rs
@@ -16,13 +16,8 @@ pub(crate) fn expand_rules(
 ) -> ExpandResult<tt::Subtree> {
     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/mbe_expander/matcher.rs b/crates/mbe/src/expander/matcher.rs
index d32e60521..800931cd1 100644
--- a/crates/mbe/src/mbe_expander/matcher.rs
+++ b/crates/mbe/src/expander/matcher.rs
@@ -1,7 +1,7 @@
 //! FIXME: write short doc here
 
 use crate::{
-    mbe_expander::{Binding, Bindings, Fragment},
+    expander::{Binding, Bindings, Fragment},
     parser::{Op, RepeatKind, Separator},
     subtree_source::SubtreeTokenSource,
     tt_iter::TtIter,
@@ -70,69 +70,38 @@ pub(super) struct Match {
 }
 
 impl Match {
-    pub(super) fn add_err(&mut self, err: ExpandError) {
+    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: &MetaTemplate, src: &tt::Subtree) -> Result<Match, ExpandError> {
-    assert!(pattern.delimiter == None);
-
+/// Matching errors are added to the `Match`.
+pub(super) fn match_(pattern: &MetaTemplate, src: &tt::Subtree) -> Match {
     let mut res = Match::default();
     let mut src = TtIter::new(src);
 
-    match_subtree(&mut res, pattern, &mut src)?;
+    match_tokens(&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_tokens(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,
-                    Err(()) => {
-                        res.add_err(err!("expected leaf: `{}`", lhs));
-                        continue;
-                    }
-                };
-                match (lhs, rhs) {
-                    (
-                        tt::Leaf::Punct(tt::Punct { char: lhs, .. }),
-                        tt::Leaf::Punct(tt::Punct { char: rhs, .. }),
-                    ) if lhs == rhs => (),
-                    (
-                        tt::Leaf::Ident(tt::Ident { text: lhs, .. }),
-                        tt::Leaf::Ident(tt::Ident { text: rhs, .. }),
-                    ) if lhs == rhs => (),
-                    (
-                        tt::Leaf::Literal(tt::Literal { text: lhs, .. }),
-                        tt::Leaf::Literal(tt::Literal { text: rhs, .. }),
-                    ) if lhs == rhs => (),
-                    _ => {
-                        res.add_err(ExpandError::UnexpectedToken);
-                    }
+                if let Err(err) = match_leaf(lhs, src) {
+                    res.add_err(err);
+                    continue;
                 }
             }
-            Op::Subtree(lhs) => {
+            Op::Subtree { tokens, delimiter: delim } => {
                 let rhs = match src.expect_subtree() {
                     Ok(s) => s,
                     Err(()) => {
@@ -140,12 +109,12 @@ fn match_subtree(
                         continue;
                     }
                 };
-                if lhs.delimiter_kind() != rhs.delimiter_kind() {
+                if delim.map(|it| it.kind) != rhs.delimiter_kind() {
                     res.add_err(err!("mismatched delimiter"));
                     continue;
                 }
                 let mut src = TtIter::new(rhs);
-                match_subtree(res, lhs, &mut src)?;
+                match_tokens(res, tokens, &mut src);
                 if src.len() > 0 {
                     res.add_err(err!("leftover tokens"));
                 }
@@ -171,14 +140,170 @@ fn match_subtree(
                     res.add_err(err);
                 }
             }
-            Op::Repeat { subtree, kind, separator } => {
-                match_repeat(res, subtree, *kind, separator, src)?;
+            Op::Repeat { tokens: subtree, kind, separator } => {
+                match_repeat(res, subtree, *kind, separator, src);
             }
         }
     }
+}
+
+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));
+        }
+    };
+    match (lhs, rhs) {
+        (
+            tt::Leaf::Punct(tt::Punct { char: lhs, .. }),
+            tt::Leaf::Punct(tt::Punct { char: rhs, .. }),
+        ) if lhs == rhs => (),
+        (
+            tt::Leaf::Ident(tt::Ident { text: lhs, .. }),
+            tt::Leaf::Ident(tt::Ident { text: rhs, .. }),
+        ) if lhs == rhs => (),
+        (
+            tt::Leaf::Literal(tt::Literal { text: lhs, .. }),
+            tt::Leaf::Literal(tt::Literal { text: rhs, .. }),
+        ) if lhs == rhs => (),
+        _ => {
+            return Err(ExpandError::UnexpectedToken);
+        }
+    }
+
     Ok(())
 }
 
+fn match_repeat(
+    res: &mut Match,
+    pattern: &MetaTemplate,
+    kind: RepeatKind,
+    separator: &Option<Separator>,
+    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_tokens(&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<Option<Fragment>> {
+    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<SmolStr>, pattern: &MetaTemplate) {
+    for op in pattern.iter() {
+        match op {
+            Op::Var { name, .. } => buf.push(name.clone()),
+            Op::Leaf(_) => (),
+            Op::Subtree { tokens, .. } => collect_vars(buf, tokens),
+            Op::Repeat { tokens, .. } => collect_vars(buf, tokens),
+        }
+    }
+}
+
 impl<'a> TtIter<'a> {
     fn eat_separator(&mut self, separator: &Separator) -> bool {
         let mut fork = self.clone();
@@ -206,7 +331,7 @@ impl<'a> TtIter<'a> {
         ok
     }
 
-    pub(crate) fn expect_tt(&mut self) -> Result<tt::TokenTree, ()> {
+    fn expect_tt(&mut self) -> Result<tt::TokenTree, ()> {
         match self.peek_n(0) {
             Some(tt::TokenTree::Leaf(tt::Leaf::Punct(punct))) if punct.char == '\'' => {
                 return self.expect_lifetime();
@@ -267,7 +392,7 @@ impl<'a> TtIter<'a> {
         }
     }
 
-    pub(crate) fn expect_lifetime(&mut self) -> Result<tt::TokenTree, ()> {
+    fn expect_lifetime(&mut self) -> Result<tt::TokenTree, ()> {
         let punct = self.expect_punct()?;
         if punct.char != '\'' {
             return Err(());
@@ -284,13 +409,13 @@ impl<'a> TtIter<'a> {
         .into())
     }
 
-    pub(crate) fn expect_fragment(
+    fn expect_fragment(
         &mut self,
         fragment_kind: parser::FragmentKind,
     ) -> ExpandResult<Option<tt::TokenTree>> {
-        pub(crate) struct OffsetTokenSink<'a> {
-            pub(crate) cursor: Cursor<'a>,
-            pub(crate) error: bool,
+        struct OffsetTokenSink<'a> {
+            cursor: Cursor<'a>,
+            error: bool,
         }
 
         impl<'a> TreeSink for OffsetTokenSink<'a> {
@@ -346,7 +471,7 @@ impl<'a> TtIter<'a> {
         ExpandResult { value: res, err }
     }
 
-    pub(crate) fn eat_vis(&mut self) -> Option<tt::TokenTree> {
+    fn eat_vis(&mut self) -> Option<tt::TokenTree> {
         let mut fork = self.clone();
         match fork.expect_fragment(Visibility) {
             ExpandResult { value: tt, err: None } => {
@@ -357,7 +482,7 @@ impl<'a> TtIter<'a> {
         }
     }
 
-    pub(crate) fn eat_char(&mut self, c: char) -> Option<tt::TokenTree> {
+    fn eat_char(&mut self, c: char) -> Option<tt::TokenTree> {
         let mut fork = self.clone();
         match fork.expect_char(c) {
             Ok(_) => {
@@ -369,134 +494,3 @@ impl<'a> TtIter<'a> {
         }
     }
 }
-
-pub(super) fn match_repeat(
-    res: &mut Match,
-    pattern: &MetaTemplate,
-    kind: RepeatKind,
-    separator: &Option<Separator>,
-    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<Option<Fragment>> {
-    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<SmolStr>, 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/mbe_expander/transcriber.rs b/crates/mbe/src/expander/transcriber.rs
index 59a3c80a8..78368a33e 100644
--- a/crates/mbe/src/mbe_expander/transcriber.rs
+++ b/crates/mbe/src/expander/transcriber.rs
@@ -2,10 +2,11 @@
 //! `$ident => foo`, interpolates variables in the template, to get `fn foo() {}`
 
 use syntax::SmolStr;
+use tt::Delimiter;
 
 use super::ExpandResult;
 use crate::{
-    mbe_expander::{Binding, Bindings, Fragment},
+    expander::{Binding, Bindings, Fragment},
     parser::{Op, RepeatKind, Separator},
     ExpandError, MetaTemplate,
 };
@@ -54,10 +55,9 @@ pub(super) fn transcribe(
     template: &MetaTemplate,
     bindings: &Bindings,
 ) -> ExpandResult<tt::Subtree> {
-    assert!(template.delimiter == None);
     let mut ctx = ExpandCtx { bindings: &bindings, nesting: Vec::new() };
     let mut arena: Vec<tt::TokenTree> = Vec::new();
-    expand_subtree(&mut ctx, template, &mut arena)
+    expand_subtree(&mut ctx, template, None, &mut arena)
 }
 
 #[derive(Debug)]
@@ -80,23 +80,18 @@ struct ExpandCtx<'a> {
 fn expand_subtree(
     ctx: &mut ExpandCtx,
     template: &MetaTemplate,
+    delimiter: Option<Delimiter>,
     arena: &mut Vec<tt::TokenTree>,
 ) -> ExpandResult<tt::Subtree> {
     // remember how many elements are in the arena now - when returning, we want to drain exactly how many elements we added. This way, the recursive uses of the arena get their own "view" of the arena, but will reuse the allocation
     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) => {
-                let ExpandResult { value: tt, err: e } = expand_subtree(ctx, &tt, arena);
+            Op::Subtree { tokens, delimiter } => {
+                let ExpandResult { value: tt, err: e } =
+                    expand_subtree(ctx, &tokens, *delimiter, arena);
                 err = err.or(e);
                 arena.push(tt.into());
             }
@@ -105,7 +100,7 @@ fn expand_subtree(
                 err = err.or(e);
                 push_fragment(arena, fragment);
             }
-            Op::Repeat { subtree, kind, separator } => {
+            Op::Repeat { tokens: subtree, kind, separator } => {
                 let ExpandResult { value: fragment, err: e } =
                     expand_repeat(ctx, subtree, *kind, separator, arena);
                 err = err.or(e);
@@ -115,7 +110,7 @@ fn expand_subtree(
     }
     // drain the elements added in this instance of expand_subtree
     let tts = arena.drain(start_elements..arena.len()).collect();
-    ExpandResult { value: tt::Subtree { delimiter: template.delimiter, token_trees: tts }, err }
+    ExpandResult { value: tt::Subtree { delimiter, token_trees: tts }, err }
 }
 
 fn expand_var(ctx: &mut ExpandCtx, v: &SmolStr, id: tt::TokenId) -> ExpandResult<Fragment> {
@@ -169,7 +164,7 @@ fn expand_repeat(
     let mut counter = 0;
 
     loop {
-        let ExpandResult { value: mut t, err: e } = expand_subtree(ctx, template, arena);
+        let ExpandResult { value: mut t, err: e } = expand_subtree(ctx, template, None, arena);
         let nesting_state = ctx.nesting.last_mut().unwrap();
         if nesting_state.at_end || !nesting_state.hit {
             break;
diff --git a/crates/mbe/src/lib.rs b/crates/mbe/src/lib.rs
index 35cde5f10..d80bd7a33 100644
--- a/crates/mbe/src/lib.rs
+++ b/crates/mbe/src/lib.rs
@@ -4,7 +4,7 @@
 //! `TokenTree`s as well!
 
 mod parser;
-mod mbe_expander;
+mod expander;
 mod syntax_bridge;
 mod tt_iter;
 mod subtree_source;
@@ -24,7 +24,9 @@ use crate::{
 
 #[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 @@ impl fmt::Display for ExpandError {
             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),
@@ -92,18 +92,11 @@ struct Rule {
 }
 
 #[derive(Clone, Debug, PartialEq, Eq)]
-struct MetaTemplate {
-    delimiter: Option<Delimiter>,
-    tokens: Vec<Result<Op, ExpandError>>,
-}
+struct MetaTemplate(Vec<Op>);
 
 impl<'a> MetaTemplate {
-    fn iter(&self) -> impl Iterator<Item = &Result<Op, ExpandError>> {
-        self.tokens.iter()
-    }
-
-    fn delimiter_kind(&self) -> Option<DelimiterKind> {
-        self.delimiter.map(|it| it.kind)
+    fn iter(&self) -> impl Iterator<Item = &Op> {
+        self.0.iter()
     }
 }
 
@@ -209,7 +202,7 @@ impl MacroRules {
         // apply shift
         let mut tt = tt.clone();
         self.shift.shift_all(&mut tt);
-        mbe_expander::expand_rules(&self.rules, &tt)
+        expander::expand_rules(&self.rules, &tt)
     }
 
     pub fn map_id_down(&self, id: tt::TokenId) -> tt::TokenId {
@@ -260,7 +253,7 @@ impl MacroDef {
         // apply shift
         let mut tt = tt.clone();
         self.shift.shift_all(&mut tt);
-        mbe_expander::expand_rules(&self.rules, &tt)
+        expander::expand_rules(&self.rules, &tt)
     }
 
     pub fn map_id_down(&self, id: tt::TokenId) -> tt::TokenId {
@@ -288,47 +281,38 @@ impl Rule {
             .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(parse_pattern(&lhs)?);
+        let rhs = MetaTemplate(parse_template(&rhs)?);
 
         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, .. } => {
+            Op::Subtree { tokens, .. } => validate(&tokens)?,
+            Op::Repeat { tokens: subtree, separator, .. } => {
                 // Checks that no repetition which could match an empty token
                 // https://github.com/rust-lang/rust/blob/a58b1ed44f5e06976de2bdc4d7dc81c36a96934f/src/librustc_expand/mbe/macro_rules.rs#L558
 
                 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 f3047972d..f891ec29c 100644
--- a/crates/mbe/src/parser.rs
+++ b/crates/mbe/src/parser.rs
@@ -3,15 +3,16 @@
 
 use smallvec::SmallVec;
 use syntax::SmolStr;
+use tt::Delimiter;
 
-use crate::{tt_iter::TtIter, ExpandError, MetaTemplate};
+use crate::{tt_iter::TtIter, MetaTemplate, ParseError};
 
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub(crate) enum Op {
     Var { name: SmolStr, kind: Option<SmolStr>, id: tt::TokenId },
-    Repeat { subtree: MetaTemplate, kind: RepeatKind, separator: Option<Separator> },
+    Repeat { tokens: MetaTemplate, kind: RepeatKind, separator: Option<Separator> },
     Leaf(tt::Leaf),
-    Subtree(MetaTemplate),
+    Subtree { tokens: MetaTemplate, delimiter: Option<Delimiter> },
 }
 
 #[derive(Copy, Clone, Debug, PartialEq, Eq)]
@@ -46,12 +47,12 @@ impl PartialEq for Separator {
     }
 }
 
-pub(crate) fn parse_template(template: &tt::Subtree) -> Vec<Result<Op, ExpandError>> {
-    parse_inner(&template, Mode::Template)
+pub(crate) fn parse_template(template: &tt::Subtree) -> Result<Vec<Op>, ParseError> {
+    parse_inner(&template, Mode::Template).into_iter().collect()
 }
 
-pub(crate) fn parse_pattern(pattern: &tt::Subtree) -> Vec<Result<Op, ExpandError>> {
-    parse_inner(&pattern, Mode::Pattern)
+pub(crate) fn parse_pattern(pattern: &tt::Subtree) -> Result<Vec<Op>, ParseError> {
+    parse_inner(&pattern, Mode::Pattern).into_iter().collect()
 }
 
 #[derive(Clone, Copy)]
@@ -60,7 +61,7 @@ enum Mode {
     Template,
 }
 
-fn parse_inner(tt: &tt::Subtree, mode: Mode) -> Vec<Result<Op, ExpandError>> {
+fn parse_inner(tt: &tt::Subtree, mode: Mode) -> Vec<Result<Op, ParseError>> {
     let mut src = TtIter::new(&tt);
     std::iter::from_fn(move || {
         let first = src.next()?;
@@ -71,7 +72,7 @@ fn parse_inner(tt: &tt::Subtree, mode: Mode) -> Vec<Result<Op, ExpandError>> {
 
 macro_rules! err {
     ($($tt:tt)*) => {
-        ExpandError::UnexpectedToken
+        ParseError::UnexpectedToken(($($tt)*).to_string())
     };
 }
 
@@ -81,7 +82,7 @@ macro_rules! bail {
     };
 }
 
-fn next_op<'a>(first: &tt::TokenTree, src: &mut TtIter<'a>, mode: Mode) -> Result<Op, ExpandError> {
+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: '$', .. })) => {
             // Note that the '$' itself is a valid token inside macro_rules.
@@ -92,17 +93,17 @@ fn next_op<'a>(first: &tt::TokenTree, src: &mut TtIter<'a>, mode: Mode) -> Resul
             match second {
                 tt::TokenTree::Subtree(subtree) => {
                     let (separator, kind) = parse_repeat(src)?;
-                    let delimiter = subtree.delimiter;
-                    let tokens = parse_inner(&subtree, mode);
-                    let subtree = MetaTemplate { tokens, delimiter };
-                    Op::Repeat { subtree, separator, kind }
+                    let tokens = parse_inner(&subtree, mode)
+                        .into_iter()
+                        .collect::<Result<Vec<Op>, ParseError>>()?;
+                    Op::Repeat { tokens: MetaTemplate(tokens), separator, kind }
                 }
                 tt::TokenTree::Leaf(leaf) => match leaf {
                     tt::Leaf::Punct(punct) => {
                         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)?;
@@ -134,16 +135,15 @@ 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 subtree = MetaTemplate { tokens, delimiter };
-            Op::Subtree(subtree)
+            let tokens =
+                parse_inner(&subtree, mode).into_iter().collect::<Result<Vec<Op>, ParseError>>()?;
+            Op::Subtree { tokens: MetaTemplate(tokens), delimiter: subtree.delimiter }
         }
     };
     Ok(res)
 }
 
-fn eat_fragment_kind<'a>(src: &mut TtIter<'a>, mode: Mode) -> Result<Option<SmolStr>, ExpandError> {
+fn eat_fragment_kind<'a>(src: &mut TtIter<'a>, mode: Mode) -> Result<Option<SmolStr>, 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 +156,12 @@ fn is_boolean_literal(lit: &tt::Literal) -> bool {
     matches!(lit.text.as_str(), "true" | "false")
 }
 
-fn parse_repeat(src: &mut TtIter) -> Result<(Option<Separator>, RepeatKind), ExpandError> {
+fn parse_repeat(src: &mut TtIter) -> Result<(Option<Separator>, 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 +169,7 @@ fn parse_repeat(src: &mut TtIter) -> Result<(Option<Separator>, 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 +182,11 @@ fn parse_repeat(src: &mut TtIter) -> Result<(Option<Separator>, 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 +196,5 @@ fn parse_repeat(src: &mut TtIter) -> Result<(Option<Separator>, RepeatKind), Exp
             }
         }
     }
-    Err(ExpandError::InvalidRepeat)
+    Err(ParseError::InvalidRepeat)
 }
diff --git a/crates/mbe/src/tests.rs b/crates/mbe/src/tests.rs
index 8d978163d..1c467facd 100644
--- a/crates/mbe/src/tests.rs
+++ b/crates/mbe/src/tests.rs
@@ -33,19 +33,18 @@ mod rule_parsing {
 
     #[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<crate::MacroRules, ParseError> {