Expand unmatched mbe fragments to reasonable default token trees

Currently we expand unmatched fragments by not replacing them at all, leaving us with `$ident`. This trips up the parser or subsequent macro calls. Instead it makes more sense to replace these with some reasonable default depending on the fragment kind which should make more recursive macro calls work better for completions.
2022-10-10 14:25:14 +02:00 · 2022-10-10 14:25:14 +02:00 · 78f33c0e96
commit 78f33c0e96
parent 476d043874
6 changed files with 146 additions and 45 deletions
--- a/crates/mbe/src/benchmark.rs
+++ b/crates/mbe/src/benchmark.rs
@ -8,7 +8,7 @@ use syntax::{
 use test_utils::{bench, bench_fixture, skip_slow_tests};

 use crate::{
-    parser::{Op, RepeatKind, Separator},
+    parser::{MetaVarKind, Op, RepeatKind, Separator},
    syntax_node_to_token_tree, DeclarativeMacro,
 };

@ -111,35 +111,35 @@ fn invocation_fixtures(rules: &FxHashMap<String, DeclarativeMacro>) -> Vec<(Stri

    fn collect_from_op(op: &Op, parent: &mut tt::Subtree, seed: &mut usize) {
        return match op {
-            Op::Var { kind, .. } => match kind.as_ref().map(|it| it.as_str()) {
-                Some("ident") => parent.token_trees.push(make_ident("foo")),
-                Some("ty") => parent.token_trees.push(make_ident("Foo")),
-                Some("tt") => parent.token_trees.push(make_ident("foo")),
-                Some("vis") => parent.token_trees.push(make_ident("pub")),
-                Some("pat") => parent.token_trees.push(make_ident("foo")),
-                Some("path") => parent.token_trees.push(make_ident("foo")),
-                Some("literal") => parent.token_trees.push(make_literal("1")),
-                Some("expr") => parent.token_trees.push(make_ident("foo")),
-                Some("lifetime") => {
+            Op::Var { kind, .. } => match kind.as_ref() {
+                Some(MetaVarKind::Ident) => parent.token_trees.push(make_ident("foo")),
+                Some(MetaVarKind::Ty) => parent.token_trees.push(make_ident("Foo")),
+                Some(MetaVarKind::Tt) => parent.token_trees.push(make_ident("foo")),
+                Some(MetaVarKind::Vis) => parent.token_trees.push(make_ident("pub")),
+                Some(MetaVarKind::Pat) => parent.token_trees.push(make_ident("foo")),
+                Some(MetaVarKind::Path) => parent.token_trees.push(make_ident("foo")),
+                Some(MetaVarKind::Literal) => parent.token_trees.push(make_literal("1")),
+                Some(MetaVarKind::Expr) => parent.token_trees.push(make_ident("foo")),
+                Some(MetaVarKind::Lifetime) => {
                    parent.token_trees.push(make_punct('\''));
                    parent.token_trees.push(make_ident("a"));
                }
-                Some("block") => {
+                Some(MetaVarKind::Block) => {
                    parent.token_trees.push(make_subtree(tt::DelimiterKind::Brace, None))
                }
-                Some("item") => {
+                Some(MetaVarKind::Item) => {
                    parent.token_trees.push(make_ident("fn"));
                    parent.token_trees.push(make_ident("foo"));
                    parent.token_trees.push(make_subtree(tt::DelimiterKind::Parenthesis, None));
                    parent.token_trees.push(make_subtree(tt::DelimiterKind::Brace, None));
                }
-                Some("meta") => {
+                Some(MetaVarKind::Meta) => {
                    parent.token_trees.push(make_ident("foo"));
                    parent.token_trees.push(make_subtree(tt::DelimiterKind::Parenthesis, None));
                }

                None => (),
-                Some(kind) => panic!("Unhandled kind {}", kind),
+                Some(kind) => panic!("Unhandled kind {:?}", kind),
            },
            Op::Leaf(leaf) => parent.token_trees.push(leaf.clone().into()),
            Op::Repeat { tokens, kind, separator } => {
--- a/crates/mbe/src/expander.rs
+++ b/crates/mbe/src/expander.rs
@ -8,7 +8,7 @@ mod transcriber;
 use rustc_hash::FxHashMap;
 use syntax::SmolStr;

-use crate::{ExpandError, ExpandResult};
+use crate::{parser::MetaVarKind, ExpandError, ExpandResult};

 pub(crate) fn expand_rules(
    rules: &[crate::Rule],
@ -104,6 +104,7 @@ enum Binding {
    Fragment(Fragment),
    Nested(Vec<Binding>),
    Empty,
+    Missing(MetaVarKind),
 }

 #[derive(Debug, Clone, PartialEq, Eq)]
--- a/crates/mbe/src/expander/matcher.rs
+++ b/crates/mbe/src/expander/matcher.rs
@ -66,7 +66,7 @@ use syntax::SmolStr;

 use crate::{
    expander::{Binding, Bindings, ExpandResult, Fragment},
-    parser::{Op, RepeatKind, Separator},
+    parser::{MetaVarKind, Op, RepeatKind, Separator},
    tt_iter::TtIter,
    ExpandError, MetaTemplate,
 };
@ -119,6 +119,7 @@ pub(super) fn match_(pattern: &MetaTemplate, input: &tt::Subtree) -> Match {
            .map(|it| match it {
                Binding::Fragment(_) => 1,
                Binding::Empty => 1,
+                Binding::Missing(_) => 1,
                Binding::Nested(it) => count(it.iter()),
            })
            .sum()
@ -130,6 +131,7 @@ enum BindingKind {
    Empty(SmolStr),
    Optional(SmolStr),
    Fragment(SmolStr, Fragment),
+    Missing(SmolStr, MetaVarKind),
    Nested(usize, usize),
 }

@ -190,6 +192,10 @@ impl BindingsBuilder {
            .push(LinkNode::Node(Rc::new(BindingKind::Fragment(var.clone(), fragment))));
    }

+    fn push_missing(&mut self, idx: &mut BindingsIdx, var: &SmolStr, kind: MetaVarKind) {
+        self.nodes[idx.0].push(LinkNode::Node(Rc::new(BindingKind::Missing(var.clone(), kind))));
+    }
+
    fn push_nested(&mut self, parent: &mut BindingsIdx, child: &BindingsIdx) {
        let BindingsIdx(idx, nidx) = self.copy(child);
        self.nodes[parent.0].push(LinkNode::Node(Rc::new(BindingKind::Nested(idx, nidx))));
@ -222,6 +228,9 @@ impl BindingsBuilder {
                BindingKind::Fragment(name, fragment) => {
                    bindings.inner.insert(name.clone(), Binding::Fragment(fragment.clone()));
                }
+                BindingKind::Missing(name, kind) => {
+                    bindings.inner.insert(name.clone(), Binding::Missing(*kind));
+                }
                BindingKind::Nested(idx, nested_idx) => {
                    let mut nested_nodes = Vec::new();
                    self.collect_nested(*idx, *nested_idx, &mut nested_nodes);
@ -458,9 +467,9 @@ fn match_loop_inner<'t>(
                }
            }
            OpDelimited::Op(Op::Var { kind, name, .. }) => {
-                if let Some(kind) = kind {
+                if let &Some(kind) = kind {
                    let mut fork = src.clone();
-                    let match_res = match_meta_var(kind.as_str(), &mut fork);
+                    let match_res = match_meta_var(kind, &mut fork);
                    match match_res.err {
                        None => {
                            // Some meta variables are optional (e.g. vis)
@ -475,8 +484,15 @@ fn match_loop_inner<'t>(
                        }
                        Some(err) => {
                            res.add_err(err);
-                            if let Some(fragment) = match_res.value {
-                                bindings_builder.push_fragment(&mut item.bindings, name, fragment);
+                            match match_res.value {
+                                Some(fragment) => bindings_builder.push_fragment(
+                                    &mut item.bindings,
+                                    name,
+                                    fragment,
+                                ),
+                                None => {
+                                    bindings_builder.push_missing(&mut item.bindings, name, kind)
+                                }
                            }
                            item.is_error = true;
                            error_items.push(item);
@ -668,20 +684,20 @@ fn match_leaf(lhs: &tt::Leaf, src: &mut TtIter<'_>) -> Result<(), ExpandError> {
    }
 }

-fn match_meta_var(kind: &str, input: &mut TtIter<'_>) -> ExpandResult<Option<Fragment>> {
+fn match_meta_var(kind: MetaVarKind, input: &mut TtIter<'_>) -> ExpandResult<Option<Fragment>> {
    let fragment = match kind {
-        "path" => parser::PrefixEntryPoint::Path,
-        "ty" => parser::PrefixEntryPoint::Ty,
+        MetaVarKind::Path => parser::PrefixEntryPoint::Path,
+        MetaVarKind::Ty => parser::PrefixEntryPoint::Ty,
        // FIXME: These two should actually behave differently depending on the edition.
        //
        // https://doc.rust-lang.org/edition-guide/rust-2021/or-patterns-macro-rules.html
-        "pat" | "pat_param" => parser::PrefixEntryPoint::Pat,
-        "stmt" => parser::PrefixEntryPoint::Stmt,
-        "block" => parser::PrefixEntryPoint::Block,
-        "meta" => parser::PrefixEntryPoint::MetaItem,
-        "item" => parser::PrefixEntryPoint::Item,
-        "vis" => parser::PrefixEntryPoint::Vis,
-        "expr" => {
+        MetaVarKind::Pat | MetaVarKind::PatParam => parser::PrefixEntryPoint::Pat,
+        MetaVarKind::Stmt => parser::PrefixEntryPoint::Stmt,
+        MetaVarKind::Block => parser::PrefixEntryPoint::Block,
+        MetaVarKind::Meta => parser::PrefixEntryPoint::MetaItem,
+        MetaVarKind::Item => parser::PrefixEntryPoint::Item,
+        MetaVarKind::Vis => parser::PrefixEntryPoint::Vis,
+        MetaVarKind::Expr => {
            // `expr` should not match underscores.
            // HACK: Macro expansion should not be done using "rollback and try another alternative".
            // rustc [explicitly checks the next token][0].
@ -698,17 +714,17 @@ fn match_meta_var(kind: &str, input: &mut TtIter<'_>) -> ExpandResult<Option<Fra
        }
        _ => {
            let tt_result = match kind {
-                "ident" => input
+                MetaVarKind::Ident => input
                    .expect_ident()
                    .map(|ident| tt::Leaf::from(ident.clone()).into())
                    .map_err(|()| ExpandError::binding_error("expected ident")),
-                "tt" => input
+                MetaVarKind::Tt => input
                    .expect_tt()
                    .map_err(|()| ExpandError::binding_error("expected token tree")),
-                "lifetime" => input
+                MetaVarKind::Lifetime => input
                    .expect_lifetime()
                    .map_err(|()| ExpandError::binding_error("expected lifetime")),
-                "literal" => {
+                MetaVarKind::Literal => {
                    let neg = input.eat_char('-');
                    input
                        .expect_literal()
--- a/crates/mbe/src/expander/transcriber.rs
+++ b/crates/mbe/src/expander/transcriber.rs
@ -6,7 +6,7 @@ use tt::{Delimiter, Subtree};

 use crate::{
    expander::{Binding, Bindings, Fragment},
-    parser::{Op, RepeatKind, Separator},
+    parser::{MetaVarKind, Op, RepeatKind, Separator},
    ExpandError, ExpandResult, MetaTemplate,
 };

@ -15,7 +15,7 @@ impl Bindings {
        self.inner.contains_key(name)
    }

-    fn get(&self, name: &str, nesting: &mut [NestingState]) -> Result<&Fragment, ExpandError> {
+    fn get(&self, name: &str, nesting: &mut [NestingState]) -> Result<Fragment, ExpandError> {
        macro_rules! binding_err {
            ($($arg:tt)*) => { ExpandError::binding_error(format!($($arg)*)) };
        }
@ -26,6 +26,7 @@ impl Bindings {
            nesting_state.hit = true;
            b = match b {
                Binding::Fragment(_) => break,
+                Binding::Missing(_) => break,
                Binding::Nested(bs) => bs.get(nesting_state.idx).ok_or_else(|| {
                    nesting_state.at_end = true;
                    binding_err!("could not find nested binding `{name}`")
@ -37,7 +38,55 @@ impl Bindings {
            };
        }
        match b {
-            Binding::Fragment(it) => Ok(it),
+            Binding::Fragment(it) => Ok(it.clone()),
+            // emit some reasonable default expansion for missing bindings,
+            // this gives better recovery than emitting the `$fragment-name` verbatim
+            Binding::Missing(it) => Ok(match it {
+                MetaVarKind::Stmt => {
+                    Fragment::Tokens(tt::TokenTree::Leaf(tt::Leaf::Punct(tt::Punct {
+                        id: tt::TokenId::unspecified(),
+                        char: ';',
+                        spacing: tt::Spacing::Alone,
+                    })))
+                }
+                MetaVarKind::Block => Fragment::Tokens(tt::TokenTree::Subtree(tt::Subtree {
+                    delimiter: Some(tt::Delimiter {
+                        id: tt::TokenId::unspecified(),
+                        kind: tt::DelimiterKind::Brace,
+                    }),
+                    token_trees: vec![],
+                })),
+                // FIXME: Meta and Item should get proper defaults
+                MetaVarKind::Meta | MetaVarKind::Item | MetaVarKind::Tt | MetaVarKind::Vis => {
+                    Fragment::Tokens(tt::TokenTree::Subtree(tt::Subtree {
+                        delimiter: None,
+                        token_trees: vec![],
+                    }))
+                }
+                MetaVarKind::Path
+                | MetaVarKind::Ty
+                | MetaVarKind::Pat
+                | MetaVarKind::PatParam
+                | MetaVarKind::Expr
+                | MetaVarKind::Ident => {
+                    Fragment::Tokens(tt::TokenTree::Leaf(tt::Leaf::Ident(tt::Ident {
+                        text: SmolStr::new_inline("missing"),
+                        id: tt::TokenId::unspecified(),
+                    })))
+                }
+                MetaVarKind::Lifetime => {
+                    Fragment::Tokens(tt::TokenTree::Leaf(tt::Leaf::Ident(tt::Ident {
+                        text: SmolStr::new_inline("'missing"),
+                        id: tt::TokenId::unspecified(),
+                    })))
+                }
+                MetaVarKind::Literal => {
+                    Fragment::Tokens(tt::TokenTree::Leaf(tt::Leaf::Ident(tt::Ident {
+                        text: SmolStr::new_inline("\"missing\""),
+                        id: tt::TokenId::unspecified(),
+                    })))
+                }
+            }),
            Binding::Nested(_) => {
                Err(binding_err!("expected simple binding, found nested binding `{name}`"))
            }
@ -157,7 +206,7 @@ fn expand_var(ctx: &mut ExpandCtx<'_>, v: &SmolStr, id: tt::TokenId) -> ExpandRe
    } else {
        ctx.bindings.get(v, &mut ctx.nesting).map_or_else(
            |e| ExpandResult { value: Fragment::Tokens(tt::TokenTree::empty()), err: Some(e) },
-            |b| ExpandResult::ok(b.clone()),
+            |it| ExpandResult::ok(it),
        )
    }
 }
--- a/crates/mbe/src/lib.rs
+++ b/crates/mbe/src/lib.rs
@ -21,7 +21,7 @@ mod token_map;
 use std::fmt;

 use crate::{
-    parser::{MetaTemplate, Op},
+    parser::{MetaTemplate, MetaVarKind, Op},
    tt_iter::TtIter,
 };

@ -291,9 +291,9 @@ fn validate(pattern: &MetaTemplate) -> Result<(), ParseError> {
                // 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
                let lsh_is_empty_seq = separator.is_none() && subtree.iter().all(|child_op| {
-                    match child_op {
+                    match *child_op {
                        // vis is optional
-                        Op::Var { kind: Some(kind), .. } => kind == "vis",
+                        Op::Var { kind: Some(kind), .. } => kind == MetaVarKind::Vis,
                        Op::Repeat {
                            kind: parser::RepeatKind::ZeroOrMore | parser::RepeatKind::ZeroOrOne,
                            ..
--- a/crates/mbe/src/parser.rs
+++ b/crates/mbe/src/parser.rs
@ -50,7 +50,7 @@ impl MetaTemplate {

 #[derive(Clone, Debug, PartialEq, Eq)]
 pub(crate) enum Op {
-    Var { name: SmolStr, kind: Option<SmolStr>, id: tt::TokenId },
+    Var { name: SmolStr, kind: Option<MetaVarKind>, id: tt::TokenId },
    Ignore { name: SmolStr, id: tt::TokenId },
    Index { depth: u32 },
    Repeat { tokens: MetaTemplate, kind: RepeatKind, separator: Option<Separator> },
@ -65,6 +65,24 @@ pub(crate) enum RepeatKind {
    ZeroOrOne,
 }

+#[derive(Copy, Clone, Debug, PartialEq, Eq)]
+pub(crate) enum MetaVarKind {
+    Path,
+    Ty,
+    Pat,
+    PatParam,
+    Stmt,
+    Block,
+    Meta,
+    Item,
+    Vis,
+    Expr,
+    Ident,
+    Tt,
+    Lifetime,
+    Literal,
+}
+
 #[derive(Clone, Debug, Eq)]
 pub(crate) enum Separator {
    Literal(tt::Literal),
@ -179,13 +197,30 @@ fn next_op<'a>(first: &tt::TokenTree, src: &mut TtIter<'a>, mode: Mode) -> Resul
    Ok(res)
 }

-fn eat_fragment_kind(src: &mut TtIter<'_>, mode: Mode) -> Result<Option<SmolStr>, ParseError> {
+fn eat_fragment_kind(src: &mut TtIter<'_>, mode: Mode) -> Result<Option<MetaVarKind>, ParseError> {
    if let Mode::Pattern = mode {
        src.expect_char(':').map_err(|()| ParseError::unexpected("missing fragment specifier"))?;
        let ident = src
            .expect_ident()
            .map_err(|()| ParseError::unexpected("missing fragment specifier"))?;
-        return Ok(Some(ident.text.clone()));
+        let kind = match ident.text.as_str() {
+            "path" => MetaVarKind::Path,
+            "ty" => MetaVarKind::Ty,
+            "pat" => MetaVarKind::Pat,
+            "pat_param" => MetaVarKind::PatParam,
+            "stmt" => MetaVarKind::Stmt,
+            "block" => MetaVarKind::Block,
+            "meta" => MetaVarKind::Meta,
+            "item" => MetaVarKind::Item,
+            "vis" => MetaVarKind::Vis,
+            "expr" => MetaVarKind::Expr,
+            "ident" => MetaVarKind::Ident,
+            "tt" => MetaVarKind::Tt,
+            "lifetime" => MetaVarKind::Lifetime,
+            "literal" => MetaVarKind::Literal,
+            _ => return Ok(None),
+        };
+        return Ok(Some(kind));
    };
    Ok(None)
 }