Skip to content

feat: Support the ${concat(...)} metavariable expression #18151

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

Merged
merged 1 commit into from
Sep 20, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

147 changes: 147 additions & 0 deletions crates/hir-def/src/macro_expansion_tests/mbe/metavar_expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -311,3 +311,150 @@ fn test() {
"#]],
);
}

#[test]
fn concat() {
// FIXME: Should this error? rustc currently accepts it.
check(
r#"
macro_rules! m {
( $a:ident, $b:literal ) => {
let ${concat($a, _, "123", _foo, $b, _, 123)};
};
}

fn test() {
m!( abc, 456 );
m!( def, "hello" );
}
"#,
expect![[r#"
macro_rules! m {
( $a:ident, $b:literal ) => {
let ${concat($a, _, "123", _foo, $b, _, 123)};
};
}

fn test() {
let abc_123_foo456_123;;
let def_123_foohello_123;;
}
"#]],
);
}

#[test]
fn concat_less_than_two_elements() {
// FIXME: Should this error? rustc currently accepts it.
check(
r#"
macro_rules! m {
() => {
let ${concat(abc)};
};
}

fn test() {
m!()
}
"#,
expect![[r#"
macro_rules! m {
() => {
let ${concat(abc)};
};
}

fn test() {
/* error: macro definition has parse errors */
}
"#]],
);
}

#[test]
fn concat_invalid_ident() {
// FIXME: Should this error? rustc currently accepts it.
check(
r#"
macro_rules! m {
() => {
let ${concat(abc, '"')};
};
}

fn test() {
m!()
}
"#,
expect![[r#"
macro_rules! m {
() => {
let ${concat(abc, '"')};
};
}

fn test() {
/* error: `${concat(..)}` is not generating a valid identifier */let __ra_concat_dummy;
}
"#]],
);
}

#[test]
fn concat_invalid_fragment() {
// FIXME: Should this error? rustc currently accepts it.
check(
r#"
macro_rules! m {
( $e:expr ) => {
let ${concat(abc, $e)};
};
}

fn test() {
m!(())
}
"#,
expect![[r#"
macro_rules! m {
( $e:expr ) => {
let ${concat(abc, $e)};
};
}

fn test() {
/* error: metavariables of `${concat(..)}` must be of type `ident`, `literal` or `tt` */let abc;
}
"#]],
);
}

#[test]
fn concat_repetition() {
// FIXME: Should this error? rustc currently accepts it.
check(
r#"
macro_rules! m {
( $($i:ident)* ) => {
let ${concat(abc, $i)};
};
}

fn test() {
m!(a b c)
}
"#,
expect![[r#"
macro_rules! m {
( $($i:ident)* ) => {
let ${concat(abc, $i)};
};
}

fn test() {
/* error: expected simple binding, found nested binding `i` */let abc;
}
"#]],
);
}
1 change: 1 addition & 0 deletions crates/mbe/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ rustc-hash.workspace = true
smallvec.workspace = true
tracing.workspace = true
arrayvec.workspace = true
ra-ap-rustc_lexer.workspace = true

# local deps
syntax.workspace = true
Expand Down
6 changes: 5 additions & 1 deletion crates/mbe/src/benchmark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,11 @@ fn invocation_fixtures(

token_trees.push(subtree.into());
}
Op::Ignore { .. } | Op::Index { .. } | Op::Count { .. } | Op::Len { .. } => {}
Op::Ignore { .. }
| Op::Index { .. }
| Op::Count { .. }
| Op::Len { .. }
| Op::Concat { .. } => {}
};

// Simple linear congruential generator for deterministic result
Expand Down
12 changes: 10 additions & 2 deletions crates/mbe/src/expander/matcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,11 @@ fn match_loop_inner<'t>(
error_items.push(item);
}
OpDelimited::Op(
Op::Ignore { .. } | Op::Index { .. } | Op::Count { .. } | Op::Len { .. },
Op::Ignore { .. }
| Op::Index { .. }
| Op::Count { .. }
| Op::Len { .. }
| Op::Concat { .. },
) => {
stdx::never!("metavariable expression in lhs found");
}
Expand Down Expand Up @@ -879,7 +883,11 @@ fn collect_vars(collector_fun: &mut impl FnMut(Symbol), pattern: &MetaTemplate)
Op::Subtree { tokens, .. } => collect_vars(collector_fun, tokens),
Op::Repeat { tokens, .. } => collect_vars(collector_fun, tokens),
Op::Literal(_) | Op::Ident(_) | Op::Punct(_) => {}
Op::Ignore { .. } | Op::Index { .. } | Op::Count { .. } | Op::Len { .. } => {
Op::Ignore { .. }
| Op::Index { .. }
| Op::Count { .. }
| Op::Len { .. }
| Op::Concat { .. } => {
stdx::never!("metavariable expression in lhs found");
}
}
Expand Down
80 changes: 78 additions & 2 deletions crates/mbe/src/expander/transcriber.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
//! `$ident => foo`, interpolates variables in the template, to get `fn foo() {}`

use intern::{sym, Symbol};
use span::Span;
use span::{Edition, Span};
use tt::Delimiter;

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

Expand Down Expand Up @@ -312,6 +312,82 @@ fn expand_subtree(
.into(),
);
}
Op::Concat { elements, span: concat_span } => {
let mut concatenated = String::new();
for element in elements {
match element {
ConcatMetaVarExprElem::Ident(ident) => {
concatenated.push_str(ident.sym.as_str())
}
ConcatMetaVarExprElem::Literal(lit) => {
// FIXME: This isn't really correct wrt. escaping, but that's what rustc does and anyway
// escaping is used most of the times for characters that are invalid in identifiers.
concatenated.push_str(lit.symbol.as_str())
}
ConcatMetaVarExprElem::Var(var) => {
// Handling of repetitions in `${concat}` isn't fleshed out in rustc, so we currently
// err at it.
// FIXME: Do what rustc does for repetitions.
let var_value = match ctx.bindings.get_fragment(
&var.sym,
var.span,
&mut ctx.nesting,
marker,
) {
Ok(var) => var,
Err(e) => {
if err.is_none() {
err = Some(e);
};
continue;
}
};
let value = match &var_value {
Fragment::Tokens(tt::TokenTree::Leaf(tt::Leaf::Ident(ident))) => {
ident.sym.as_str()
}
Fragment::Tokens(tt::TokenTree::Leaf(tt::Leaf::Literal(lit))) => {
lit.symbol.as_str()
}
_ => {
if err.is_none() {
err = Some(ExpandError::binding_error(var.span, "metavariables of `${concat(..)}` must be of type `ident`, `literal` or `tt`"))
}
continue;
}
};
concatenated.push_str(value);
}
}
}

// `${concat}` span comes from the macro (at least for now).
// See https://github.com/rust-lang/rust/blob/b0af276da341/compiler/rustc_expand/src/mbe/transcribe.rs#L724-L726.
let mut result_span = *concat_span;
marker(&mut result_span);

// FIXME: NFC normalize the result.
if !rustc_lexer::is_ident(&concatenated) {
if err.is_none() {
err = Some(ExpandError::binding_error(
*concat_span,
"`${concat(..)}` is not generating a valid identifier",
));
}
// Insert a dummy identifier for better parsing.
concatenated.clear();
concatenated.push_str("__ra_concat_dummy");
}

let needs_raw =
parser::SyntaxKind::from_keyword(&concatenated, Edition::LATEST).is_some();
let is_raw = if needs_raw { tt::IdentIsRaw::Yes } else { tt::IdentIsRaw::No };
arena.push(tt::TokenTree::Leaf(tt::Leaf::Ident(tt::Ident {
is_raw,
span: result_span,
sym: Symbol::intern(&concatenated),
})));
}
}
}
// drain the elements added in this instance of expand_subtree
Expand Down
5 changes: 5 additions & 0 deletions crates/mbe/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@
//! The tests for this functionality live in another crate:
//! `hir_def::macro_expansion_tests::mbe`.

#[cfg(not(feature = "in-rust-tree"))]
extern crate ra_ap_rustc_lexer as rustc_lexer;
#[cfg(feature = "in-rust-tree")]
extern crate rustc_lexer;

mod expander;
mod parser;

Expand Down
50 changes: 50 additions & 0 deletions crates/mbe/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ pub(crate) enum Op {
// FIXME: `usize`` once we drop support for 1.76
depth: Option<usize>,
},
Concat {
elements: Box<[ConcatMetaVarExprElem]>,
span: Span,
},
Repeat {
tokens: MetaTemplate,
kind: RepeatKind,
Expand All @@ -98,6 +102,18 @@ pub(crate) enum Op {
Ident(tt::Ident<Span>),
}

#[derive(Clone, Debug, PartialEq, Eq)]
pub(crate) enum ConcatMetaVarExprElem {
/// There is NO preceding dollar sign, which means that this identifier should be interpreted
/// as a literal.
Ident(tt::Ident<Span>),
/// There is a preceding dollar sign, which means that this identifier should be expanded
/// and interpreted as a variable.
Var(tt::Ident<Span>),
/// For example, a number or a string.
Literal(tt::Literal<Span>),
}

#[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub(crate) enum RepeatKind {
ZeroOrMore,
Expand Down Expand Up @@ -384,6 +400,32 @@ fn parse_metavar_expr(src: &mut TtIter<'_, Span>) -> Result<Op, ()> {
let depth = if try_eat_comma(&mut args) { Some(parse_depth(&mut args)?) } else { None };
Op::Count { name: ident.sym.clone(), depth }
}
s if sym::concat == *s => {
let mut elements = Vec::new();
while let Some(next) = args.peek_n(0) {
let element = if let tt::TokenTree::Leaf(tt::Leaf::Literal(lit)) = next {
args.next().expect("already peeked");
ConcatMetaVarExprElem::Literal(lit.clone())
} else {
let is_var = try_eat_dollar(&mut args);
let ident = args.expect_ident_or_underscore()?.clone();

if is_var {
ConcatMetaVarExprElem::Var(ident)
} else {
ConcatMetaVarExprElem::Ident(ident)
}
};
elements.push(element);
if args.peek_n(0).is_some() {
args.expect_comma()?;
}
}
if elements.len() < 2 {
return Err(());
}
Op::Concat { elements: elements.into_boxed_slice(), span: func.span }
}
_ => return Err(()),
};

Expand Down Expand Up @@ -414,3 +456,11 @@ fn try_eat_comma(src: &mut TtIter<'_, Span>) -> bool {
}
false
}

fn try_eat_dollar(src: &mut TtIter<'_, Span>) -> bool {
if let Some(tt::TokenTree::Leaf(tt::Leaf::Punct(tt::Punct { char: '$', .. }))) = src.peek_n(0) {
let _ = src.next();
return true;
}
false
}
Loading