Skip to content
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.

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

Fixup path fragments upon MBE transcription #15360

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 31 additions & 0 deletions crates/hir-def/src/macro_expansion_tests/mbe.rs
Expand Up @@ -848,6 +848,37 @@ fn foo() {
);
}

#[test]
fn test_type_path_is_transcribed_as_expr_path() {
check(
r#"
macro_rules! m {
($p:path) => { let $p; }
}
fn test() {
m!(S)
m!(S<i32>)
m!(S<S<i32>>)
m!(S<{ module::CONST < 42 }>)
}
"#,
expect![[r#"
macro_rules! m {
($p:path) => { let $p; }
}
fn test() {
let S;
let S:: <i32> ;
let S:: <S:: <i32>> ;
let S:: < {
module::CONST<42
}
> ;
}
"#]],
);
}

#[test]
fn test_expr() {
check(
Expand Down
10 changes: 10 additions & 0 deletions crates/mbe/src/expander.rs
Expand Up @@ -123,4 +123,14 @@ enum Fragment {
/// proc-macro delimiter=none. As we later discovered, "none" delimiters are
/// tricky to handle in the parser, and rustc doesn't handle those either.
Expr(tt::TokenTree),
/// There are roughly two types of paths: paths in expression context, where a
/// separator `::` between an identifier and its following generic argument list
/// is mandatory, and paths in type context, where `::` can be omitted.
///
/// Unlike rustc, we need to transform the parsed fragments back into tokens
/// during transcription. When the matched path fragment is a type-context path
/// and is trasncribed as an expression-context path, verbatim transcription
/// would cause a syntax error. We need to fix it up just before transcribing;
/// see `transcriber::fix_up_and_push_path_tt()`.
Path(tt::TokenTree),
}
10 changes: 7 additions & 3 deletions crates/mbe/src/expander/matcher.rs
Expand Up @@ -742,7 +742,11 @@ fn match_meta_var(
is_2021: bool,
) -> ExpandResult<Option<Fragment>> {
let fragment = match kind {
MetaVarKind::Path => parser::PrefixEntryPoint::Path,
MetaVarKind::Path => {
return input
.expect_fragment(parser::PrefixEntryPoint::Path)
.map(|it| it.map(Fragment::Path));
}
MetaVarKind::Ty => parser::PrefixEntryPoint::Ty,
MetaVarKind::Pat if is_2021 => parser::PrefixEntryPoint::PatTop,
MetaVarKind::Pat => parser::PrefixEntryPoint::Pat,
Expand Down Expand Up @@ -771,7 +775,7 @@ fn match_meta_var(
.expect_fragment(parser::PrefixEntryPoint::Expr)
.map(|tt| tt.map(Fragment::Expr));
}
_ => {
MetaVarKind::Ident | MetaVarKind::Tt | MetaVarKind::Lifetime | MetaVarKind::Literal => {
let tt_result = match kind {
MetaVarKind::Ident => input
.expect_ident()
Expand Down Expand Up @@ -799,7 +803,7 @@ fn match_meta_var(
})
.map_err(|()| ExpandError::binding_error("expected literal"))
}
_ => Err(ExpandError::UnexpectedToken),
_ => unreachable!(),
};
return tt_result.map(|it| Some(Fragment::Tokens(it))).into();
}
Expand Down
42 changes: 41 additions & 1 deletion crates/mbe/src/expander/transcriber.rs
Expand Up @@ -400,7 +400,8 @@ fn push_fragment(buf: &mut Vec<tt::TokenTree>, fragment: Fragment) {
}
buf.push(tt.into())
}
Fragment::Tokens(tt) | Fragment::Expr(tt) => buf.push(tt),
Fragment::Path(tt::TokenTree::Subtree(tt)) => fix_up_and_push_path_tt(buf, tt),
Fragment::Tokens(tt) | Fragment::Expr(tt) | Fragment::Path(tt) => buf.push(tt),
}
}

Expand All @@ -411,6 +412,45 @@ fn push_subtree(buf: &mut Vec<tt::TokenTree>, tt: tt::Subtree) {
}
}

/// Inserts the path separator `::` between an identifier and its following generic
/// argument list, and then pushes into the buffer. See [`Fragment::Path`] for why
/// we need this fixup.
fn fix_up_and_push_path_tt(buf: &mut Vec<tt::TokenTree>, subtree: tt::Subtree) {
stdx::always!(matches!(subtree.delimiter.kind, tt::DelimiterKind::Invisible));
let mut prev_was_ident = false;
// Note that we only need to fix up the top-level `TokenTree`s because the
// context of the paths in the descendant `Subtree`s won't be changed by the
// mbe transcription.
for tt in subtree.token_trees {
if prev_was_ident {
// Pedantically, `(T) -> U` in `FnOnce(T) -> U` is treated as a generic
// argument list and thus needs `::` between it and `FnOnce`. However in
// today's Rust this type of path *semantically* cannot appear as a
// top-level expression-context path, so we can safely ignore it.
if let tt::TokenTree::Leaf(tt::Leaf::Punct(tt::Punct { char: '<', .. })) = tt {
buf.push(
tt::Leaf::Punct(tt::Punct {
char: ':',
spacing: tt::Spacing::Joint,
span: tt::Span::unspecified(),
})
.into(),
);
buf.push(
tt::Leaf::Punct(tt::Punct {
char: ':',
spacing: tt::Spacing::Alone,
span: tt::Span::unspecified(),
})
.into(),
);
}
}
prev_was_ident = matches!(tt, tt::TokenTree::Leaf(tt::Leaf::Ident(_)));
buf.push(tt);
}
}

/// Handles `${count(t, depth)}`. `our_depth` is the recursion depth and `count_depth` is the depth
/// defined by the metavar expression.
fn count(
Expand Down
1 change: 0 additions & 1 deletion crates/tt/src/lib.rs
Expand Up @@ -122,7 +122,6 @@ impl_from!(Literal<Span>, Punct<Span>, Ident<Span> for Leaf);

#[derive(Clone, PartialEq, Eq, Hash)]
pub struct Subtree<Span> {
// FIXME, this should not be Option
pub delimiter: Delimiter<Span>,
pub token_trees: Vec<TokenTree<Span>>,
}
Expand Down