Skip to content

fix: Expand unmatched mbe fragments to reasonable default token trees #13384

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 10, 2022
Merged

fix: Expand unmatched mbe fragments to reasonable default token trees #13384

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 15 additions & 15 deletions crates/mbe/src/benchmark.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down Expand Up @@ -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 } => {
Expand Down
3 changes: 2 additions & 1 deletion crates/mbe/src/expander.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down Expand Up @@ -104,6 +104,7 @@ enum Binding {
Fragment(Fragment),
Nested(Vec<Binding>),
Empty,
Missing(MetaVarKind),
}

#[derive(Debug, Clone, PartialEq, Eq)]
Expand Down
54 changes: 35 additions & 19 deletions crates/mbe/src/expander/matcher.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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()
Expand All @@ -130,6 +131,7 @@ enum BindingKind {
Empty(SmolStr),
Optional(SmolStr),
Fragment(SmolStr, Fragment),
Missing(SmolStr, MetaVarKind),
Nested(usize, usize),
}

Expand Down Expand Up @@ -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))));
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)
Expand All @@ -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);
Expand Down Expand Up @@ -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].
Expand All @@ -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()
Expand Down
57 changes: 53 additions & 4 deletions crates/mbe/src/expander/transcriber.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand All @@ -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)*)) };
}
Expand All @@ -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}`")
Expand All @@ -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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should \' need to include here ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

' is already part of the identifier here "'lifetime"

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}`"))
}
Expand Down Expand Up @@ -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),
)
}
}
Expand Down
6 changes: 3 additions & 3 deletions crates/mbe/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ mod token_map;
use std::fmt;

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

Expand Down Expand Up @@ -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,
..
Expand Down
41 changes: 38 additions & 3 deletions crates/mbe/src/parser.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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> },
Expand All @@ -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),
Expand Down Expand Up @@ -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)
}
Expand Down