From 9ebaa85d374aa7e8cb07534235134b0074260f5b Mon Sep 17 00:00:00 2001 From: Ryo Yoshida Date: Tue, 23 May 2023 17:55:40 +0900 Subject: [PATCH 1/2] Split test module for metavariable expressions --- .../hir-def/src/macro_expansion_tests/mbe.rs | 87 +----------------- .../macro_expansion_tests/mbe/metavar_expr.rs | 91 +++++++++++++++++++ 2 files changed, 92 insertions(+), 86 deletions(-) create mode 100644 crates/hir-def/src/macro_expansion_tests/mbe/metavar_expr.rs diff --git a/crates/hir-def/src/macro_expansion_tests/mbe.rs b/crates/hir-def/src/macro_expansion_tests/mbe.rs index c056c077a5683..553ffe3d0b88b 100644 --- a/crates/hir-def/src/macro_expansion_tests/mbe.rs +++ b/crates/hir-def/src/macro_expansion_tests/mbe.rs @@ -4,6 +4,7 @@ mod tt_conversion; mod matching; mod meta_syntax; +mod metavar_expr; mod regression; use expect_test::expect; @@ -1614,92 +1615,6 @@ struct Foo; ) } -#[test] -fn test_dollar_dollar() { - check( - r#" -macro_rules! register_struct { ($Struct:ident) => { - macro_rules! register_methods { ($$($method:ident),*) => { - macro_rules! implement_methods { ($$$$($$val:expr),*) => { - struct $Struct; - impl $Struct { $$(fn $method() -> &'static [u32] { &[$$$$($$$$val),*] })*} - }} - }} -}} - -register_struct!(Foo); -register_methods!(alpha, beta); -implement_methods!(1, 2, 3); -"#, - expect![[r#" -macro_rules! register_struct { ($Struct:ident) => { - macro_rules! register_methods { ($$($method:ident),*) => { - macro_rules! implement_methods { ($$$$($$val:expr),*) => { - struct $Struct; - impl $Struct { $$(fn $method() -> &'static [u32] { &[$$$$($$$$val),*] })*} - }} - }} -}} - -macro_rules !register_methods { - ($($method: ident), *) = > { - macro_rules!implement_methods { - ($$($val: expr), *) = > { - struct Foo; - impl Foo { - $(fn $method()-> &'static[u32] { - &[$$($$val), *] - } - )* - } - } - } - } -} -macro_rules !implement_methods { - ($($val: expr), *) = > { - struct Foo; - impl Foo { - fn alpha()-> &'static[u32] { - &[$($val), *] - } - fn beta()-> &'static[u32] { - &[$($val), *] - } - } - } -} -struct Foo; -impl Foo { - fn alpha() -> &'static[u32] { - &[1, 2, 3] - } - fn beta() -> &'static[u32] { - &[1, 2, 3] - } -} -"#]], - ) -} - -#[test] -fn test_metavar_exprs() { - check( - r#" -macro_rules! m { - ( $( $t:tt )* ) => ( $( ${ignore(t)} -${index()} )-* ); -} -const _: i32 = m!(a b c); - "#, - expect![[r#" -macro_rules! m { - ( $( $t:tt )* ) => ( $( ${ignore(t)} -${index()} )-* ); -} -const _: i32 = -0--1--2; - "#]], - ); -} - #[test] fn test_punct_without_space() { // Puncts are "glued" greedily. diff --git a/crates/hir-def/src/macro_expansion_tests/mbe/metavar_expr.rs b/crates/hir-def/src/macro_expansion_tests/mbe/metavar_expr.rs new file mode 100644 index 0000000000000..ae138529ea647 --- /dev/null +++ b/crates/hir-def/src/macro_expansion_tests/mbe/metavar_expr.rs @@ -0,0 +1,91 @@ +//! Tests for RFC 3086 metavariable expressions. + +use expect_test::expect; + +use crate::macro_expansion_tests::check; + +#[test] +fn test_dollar_dollar() { + check( + r#" +macro_rules! register_struct { ($Struct:ident) => { + macro_rules! register_methods { ($$($method:ident),*) => { + macro_rules! implement_methods { ($$$$($$val:expr),*) => { + struct $Struct; + impl $Struct { $$(fn $method() -> &'static [u32] { &[$$$$($$$$val),*] })*} + }} + }} +}} + +register_struct!(Foo); +register_methods!(alpha, beta); +implement_methods!(1, 2, 3); +"#, + expect![[r#" +macro_rules! register_struct { ($Struct:ident) => { + macro_rules! register_methods { ($$($method:ident),*) => { + macro_rules! implement_methods { ($$$$($$val:expr),*) => { + struct $Struct; + impl $Struct { $$(fn $method() -> &'static [u32] { &[$$$$($$$$val),*] })*} + }} + }} +}} + +macro_rules !register_methods { + ($($method: ident), *) = > { + macro_rules!implement_methods { + ($$($val: expr), *) = > { + struct Foo; + impl Foo { + $(fn $method()-> &'static[u32] { + &[$$($$val), *] + } + )* + } + } + } + } +} +macro_rules !implement_methods { + ($($val: expr), *) = > { + struct Foo; + impl Foo { + fn alpha()-> &'static[u32] { + &[$($val), *] + } + fn beta()-> &'static[u32] { + &[$($val), *] + } + } + } +} +struct Foo; +impl Foo { + fn alpha() -> &'static[u32] { + &[1, 2, 3] + } + fn beta() -> &'static[u32] { + &[1, 2, 3] + } +} +"#]], + ) +} + +#[test] +fn test_metavar_exprs() { + check( + r#" +macro_rules! m { + ( $( $t:tt )* ) => ( $( ${ignore(t)} -${index()} )-* ); +} +const _: i32 = m!(a b c); + "#, + expect![[r#" +macro_rules! m { + ( $( $t:tt )* ) => ( $( ${ignore(t)} -${index()} )-* ); +} +const _: i32 = -0--1--2; + "#]], + ); +} From 0d4d1d7e3b820c241e8197926b62830b9d2d3b24 Mon Sep 17 00:00:00 2001 From: Ryo Yoshida Date: Sun, 28 May 2023 19:54:36 +0900 Subject: [PATCH 2/2] Implement `${count()}` metavariable expression --- .../macro_expansion_tests/mbe/metavar_expr.rs | 220 ++++++++++++++++++ crates/mbe/src/benchmark.rs | 2 +- crates/mbe/src/expander/matcher.rs | 8 +- crates/mbe/src/expander/transcriber.rs | 128 ++++++++-- crates/mbe/src/lib.rs | 21 ++ crates/mbe/src/parser.rs | 32 ++- crates/mbe/src/tt_iter.rs | 7 - 7 files changed, 389 insertions(+), 29 deletions(-) diff --git a/crates/hir-def/src/macro_expansion_tests/mbe/metavar_expr.rs b/crates/hir-def/src/macro_expansion_tests/mbe/metavar_expr.rs index ae138529ea647..967b5ad36babf 100644 --- a/crates/hir-def/src/macro_expansion_tests/mbe/metavar_expr.rs +++ b/crates/hir-def/src/macro_expansion_tests/mbe/metavar_expr.rs @@ -89,3 +89,223 @@ const _: i32 = -0--1--2; "#]], ); } + +#[test] +fn count_basic() { + check( + r#" +macro_rules! m { + ($($t:ident),*) => { + ${count(t)} + } +} + +fn test() { + m!(); + m!(a); + m!(a, a); +} +"#, + expect![[r#" +macro_rules! m { + ($($t:ident),*) => { + ${count(t)} + } +} + +fn test() { + 0; + 1; + 2; +} +"#]], + ); +} + +#[test] +fn count_with_depth() { + check( + r#" +macro_rules! foo { + ($( $( $($t:ident)* ),* );*) => { + $( + { + let depth_none = ${count(t)}; + let depth_zero = ${count(t, 0)}; + let depth_one = ${count(t, 1)}; + } + )* + } +} + +fn bar() { + foo!( + a a a, a, a a; + a a a + ) +} +"#, + expect![[r#" +macro_rules! foo { + ($( $( $($t:ident)* ),* );*) => { + $( + { + let depth_none = ${count(t)}; + let depth_zero = ${count(t, 0)}; + let depth_one = ${count(t, 1)}; + } + )* + } +} + +fn bar() { + { + let depth_none = 6; + let depth_zero = 3; + let depth_one = 6; + } { + let depth_none = 3; + let depth_zero = 1; + let depth_one = 3; + } +} +"#]], + ); +} + +#[test] +fn count_depth_out_of_bounds() { + check( + r#" +macro_rules! foo { + ($($t:ident)*) => { ${count(t, 1)} }; + ($( $( $l:literal )* );*) => { $(${count(l, 1)};)* } +} +macro_rules! bar { + ($($t:ident)*) => { ${count(t, 1024)} }; + ($( $( $l:literal )* );*) => { $(${count(l, 8192)};)* } +} + +fn test() { + foo!(a b); + foo!(1 2; 3); + bar!(a b); + bar!(1 2; 3); +} +"#, + expect![[r#" +macro_rules! foo { + ($($t:ident)*) => { ${count(t, 1)} }; + ($( $( $l:literal )* );*) => { $(${count(l, 1)};)* } +} +macro_rules! bar { + ($($t:ident)*) => { ${count(t, 1024)} }; + ($( $( $l:literal )* );*) => { $(${count(l, 8192)};)* } +} + +fn test() { + /* error: ${count} out of bounds */; + /* error: ${count} out of bounds */; + /* error: ${count} out of bounds */; + /* error: ${count} out of bounds */; +} +"#]], + ); +} + +#[test] +fn misplaced_count() { + check( + r#" +macro_rules! foo { + ($($t:ident)*) => { $(${count(t)})* }; + ($l:literal) => { ${count(l)} } +} + +fn test() { + foo!(a b c); + foo!(1); +} +"#, + expect![[r#" +macro_rules! foo { + ($($t:ident)*) => { $(${count(t)})* }; + ($l:literal) => { ${count(l)} } +} + +fn test() { + /* error: ${count} misplaced */; + /* error: ${count} misplaced */; +} +"#]], + ); +} + +#[test] +fn malformed_count() { + check( + r#" +macro_rules! too_many_args { + ($($t:ident)*) => { ${count(t, 1, leftover)} } +} +macro_rules! depth_suffixed { + ($($t:ident)*) => { ${count(t, 0usize)} } +} +macro_rules! depth_too_large { + ($($t:ident)*) => { ${count(t, 18446744073709551616)} } +} + +fn test() { + too_many_args!(); + depth_suffixed!(); + depth_too_large!(); +} +"#, + expect![[r#" +macro_rules! too_many_args { + ($($t:ident)*) => { ${count(t, 1, leftover)} } +} +macro_rules! depth_suffixed { + ($($t:ident)*) => { ${count(t, 0usize)} } +} +macro_rules! depth_too_large { + ($($t:ident)*) => { ${count(t, 18446744073709551616)} } +} + +fn test() { + /* error: invalid macro definition: invalid metavariable expression */; + /* error: invalid macro definition: invalid metavariable expression */; + /* error: invalid macro definition: invalid metavariable expression */; +} +"#]], + ); +} + +#[test] +fn count_interaction_with_empty_binding() { + // FIXME: Should this error? rustc currently accepts it. + check( + r#" +macro_rules! m { + ($($t:ident),*) => { + ${count(t, 100)} + } +} + +fn test() { + m!(); +} +"#, + expect![[r#" +macro_rules! m { + ($($t:ident),*) => { + ${count(t, 100)} + } +} + +fn test() { + 0; +} +"#]], + ); +} diff --git a/crates/mbe/src/benchmark.rs b/crates/mbe/src/benchmark.rs index d640eea19a2e7..d28dd17def378 100644 --- a/crates/mbe/src/benchmark.rs +++ b/crates/mbe/src/benchmark.rs @@ -195,7 +195,7 @@ fn invocation_fixtures(rules: &FxHashMap) -> Vec<(Stri }); parent.token_trees.push(subtree.into()); } - Op::Ignore { .. } | Op::Index { .. } => {} + Op::Ignore { .. } | Op::Index { .. } | Op::Count { .. } => {} }; // Simple linear congruential generator for deterministic result diff --git a/crates/mbe/src/expander/matcher.rs b/crates/mbe/src/expander/matcher.rs index 0ab5c8c3c9e73..474826079d733 100644 --- a/crates/mbe/src/expander/matcher.rs +++ b/crates/mbe/src/expander/matcher.rs @@ -567,7 +567,9 @@ fn match_loop_inner<'t>( item.is_error = true; error_items.push(item); } - OpDelimited::Op(Op::Ignore { .. } | Op::Index { .. }) => {} + OpDelimited::Op(Op::Ignore { .. } | Op::Index { .. } | Op::Count { .. }) => { + stdx::never!("metavariable expression in lhs found"); + } OpDelimited::Open => { if matches!(src.peek_n(0), Some(tt::TokenTree::Subtree(..))) { item.dot.next(); @@ -811,7 +813,9 @@ fn collect_vars(collector_fun: &mut impl FnMut(SmolStr), pattern: &MetaTemplate) Op::Var { name, .. } => collector_fun(name.clone()), Op::Subtree { tokens, .. } => collect_vars(collector_fun, tokens), Op::Repeat { tokens, .. } => collect_vars(collector_fun, tokens), - Op::Ignore { .. } | Op::Index { .. } | Op::Literal(_) | Op::Ident(_) | Op::Punct(_) => { + Op::Literal(_) | Op::Ident(_) | Op::Punct(_) => {} + Op::Ignore { .. } | Op::Index { .. } | Op::Count { .. } => { + stdx::never!("metavariable expression in lhs found"); } } } diff --git a/crates/mbe/src/expander/transcriber.rs b/crates/mbe/src/expander/transcriber.rs index dffb40d4bc886..6161af185871d 100644 --- a/crates/mbe/src/expander/transcriber.rs +++ b/crates/mbe/src/expander/transcriber.rs @@ -7,7 +7,7 @@ use crate::{ expander::{Binding, Bindings, Fragment}, parser::{MetaVarKind, Op, RepeatKind, Separator}, tt::{self, Delimiter}, - ExpandError, ExpandResult, MetaTemplate, + CountError, ExpandError, ExpandResult, MetaTemplate, }; impl Bindings { @@ -15,13 +15,23 @@ impl Bindings { self.inner.contains_key(name) } - fn get(&self, name: &str, nesting: &mut [NestingState]) -> Result { + fn get(&self, name: &str) -> Result<&Binding, ExpandError> { + match self.inner.get(name) { + Some(binding) => Ok(binding), + None => Err(ExpandError::binding_error(format!("could not find binding `{name}`"))), + } + } + + fn get_fragment( + &self, + name: &str, + nesting: &mut [NestingState], + ) -> Result { macro_rules! binding_err { ($($arg:tt)*) => { ExpandError::binding_error(format!($($arg)*)) }; } - let mut b: &Binding = - self.inner.get(name).ok_or_else(|| binding_err!("could not find binding `{name}`"))?; + let mut b = self.get(name)?; for nesting_state in nesting.iter_mut() { nesting_state.hit = true; b = match b { @@ -133,7 +143,7 @@ fn expand_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() { + 'ops: for op in template.iter() { match op { Op::Literal(it) => arena.push(tt::Leaf::from(it.clone()).into()), Op::Ident(it) => arena.push(tt::Leaf::from(it.clone()).into()), @@ -161,13 +171,12 @@ fn expand_subtree( } Op::Ignore { name, id } => { // Expand the variable, but ignore the result. This registers the repetition count. + // FIXME: Any emitted errors are dropped. expand_var(ctx, name, *id); } Op::Index { depth } => { - let index = ctx - .nesting - .get(ctx.nesting.len() - 1 - (*depth as usize)) - .map_or(0, |nest| nest.idx); + let index = + ctx.nesting.get(ctx.nesting.len() - 1 - depth).map_or(0, |nest| nest.idx); arena.push( tt::Leaf::Literal(tt::Literal { text: index.to_string().into(), @@ -176,6 +185,65 @@ fn expand_subtree( .into(), ); } + Op::Count { name, depth } => { + let mut binding = match ctx.bindings.get(name.as_str()) { + Ok(b) => b, + Err(e) => { + if err.is_none() { + err = Some(e); + } + continue; + } + }; + for state in ctx.nesting.iter_mut() { + state.hit = true; + match binding { + Binding::Fragment(_) | Binding::Missing(_) => { + // `count()` will report an error. + break; + } + Binding::Nested(bs) => { + if let Some(b) = bs.get(state.idx) { + binding = b; + } else { + state.at_end = true; + continue 'ops; + } + } + Binding::Empty => { + state.at_end = true; + // FIXME: Breaking here and proceeding to `count()` isn't the most + // correct thing to do here. This could be a binding of some named + // fragment which we don't know the depth of, so `count()` will just + // return 0 for this no matter what `depth` is. See test + // `count_interaction_with_empty_binding` for example. + break; + } + } + } + + let c = match count(ctx, binding, 0, *depth) { + Ok(c) => c, + Err(e) => { + // XXX: It *might* make sense to emit a dummy integer value like `0` here. + // That would type inference a bit more robust in cases like + // `v[${count(t)}]` where index doesn't matter, but also coult also lead to + // wrong infefrence for cases like `tup.${count(t)}` where index itself + // does matter. + if err.is_none() { + err = Some(e.into()); + } + continue; + } + }; + arena.push( + tt::Leaf::Literal(tt::Literal { + text: c.to_string().into(), + span: tt::TokenId::unspecified(), + }) + .into(), + ); + } } } // drain the elements added in this instance of expand_subtree @@ -218,12 +286,9 @@ fn expand_var(ctx: &mut ExpandCtx<'_>, v: &SmolStr, id: tt::TokenId) -> ExpandRe .into(); ExpandResult::ok(Fragment::Tokens(tt)) } else { - ctx.bindings.get(v, &mut ctx.nesting).map_or_else( + ctx.bindings.get_fragment(v, &mut ctx.nesting).map_or_else( |e| ExpandResult { - value: Fragment::Tokens(tt::TokenTree::Subtree(tt::Subtree { - delimiter: tt::Delimiter::unspecified(), - token_trees: vec![], - })), + value: Fragment::Tokens(tt::TokenTree::Subtree(tt::Subtree::empty())), err: Some(e), }, ExpandResult::ok, @@ -245,6 +310,7 @@ fn expand_repeat( let limit = 65536; let mut has_seps = 0; let mut counter = 0; + let mut err = None; loop { let ExpandResult { value: mut t, err: e } = expand_subtree(ctx, template, None, arena); @@ -272,6 +338,7 @@ fn expand_repeat( } if e.is_some() { + err = err.or(e); continue; } @@ -317,7 +384,7 @@ fn expand_repeat( err: Some(ExpandError::UnexpectedToken), }; } - ExpandResult::ok(Fragment::Tokens(tt)) + ExpandResult { value: Fragment::Tokens(tt), err } } fn push_fragment(buf: &mut Vec, fragment: Fragment) { @@ -343,3 +410,34 @@ fn push_subtree(buf: &mut Vec, tt: tt::Subtree) { _ => buf.push(tt.into()), } } + +/// Handles `${count(t, depth)}`. `our_depth` is the recursion depth and `count_depth` is the depth +/// defined by the metavar expression. +fn count( + ctx: &ExpandCtx<'_>, + binding: &Binding, + our_depth: usize, + count_depth: Option, +) -> Result { + match binding { + Binding::Nested(bs) => match count_depth { + None => bs.iter().map(|b| count(ctx, b, our_depth + 1, None)).sum(), + Some(0) => Ok(bs.len()), + Some(d) => bs.iter().map(|b| count(ctx, b, our_depth + 1, Some(d - 1))).sum(), + }, + Binding::Empty => Ok(0), + Binding::Fragment(_) | Binding::Missing(_) => { + if our_depth == 0 { + // `${count(t)}` is placed inside the innermost repetition. This includes cases + // where `t` is not a repeated fragment. + Err(CountError::Misplaced) + } else if count_depth.is_none() { + Ok(1) + } else { + // We've reached at the innermost repeated fragment, but the user wants us to go + // further! + Err(CountError::OutOfBounds) + } + } + } +} diff --git a/crates/mbe/src/lib.rs b/crates/mbe/src/lib.rs index 80352aa4adc1e..5ef20ff8a9b05 100644 --- a/crates/mbe/src/lib.rs +++ b/crates/mbe/src/lib.rs @@ -19,6 +19,7 @@ mod benchmark; mod token_map; use ::tt::token_id as tt; +use stdx::impl_from; use std::fmt; @@ -77,8 +78,11 @@ pub enum ExpandError { LimitExceeded, NoMatchingRule, UnexpectedToken, + CountError(CountError), } +impl_from!(CountError for ExpandError); + impl ExpandError { fn binding_error(e: impl Into>) -> ExpandError { ExpandError::BindingError(Box::new(e.into())) @@ -94,6 +98,23 @@ impl fmt::Display for ExpandError { ExpandError::ConversionError => f.write_str("could not convert tokens"), ExpandError::LimitExceeded => f.write_str("Expand exceed limit"), ExpandError::LeftoverTokens => f.write_str("leftover tokens"), + ExpandError::CountError(e) => e.fmt(f), + } + } +} + +// FIXME: Showing these errors could be nicer. +#[derive(Debug, PartialEq, Eq, Clone, Hash)] +pub enum CountError { + OutOfBounds, + Misplaced, +} + +impl fmt::Display for CountError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + CountError::OutOfBounds => f.write_str("${count} out of bounds"), + CountError::Misplaced => f.write_str("${count} misplaced"), } } } diff --git a/crates/mbe/src/parser.rs b/crates/mbe/src/parser.rs index 0fbf832b06df2..7a143e7466a93 100644 --- a/crates/mbe/src/parser.rs +++ b/crates/mbe/src/parser.rs @@ -52,7 +52,8 @@ impl MetaTemplate { pub(crate) enum Op { Var { name: SmolStr, kind: Option, id: tt::TokenId }, Ignore { name: SmolStr, id: tt::TokenId }, - Index { depth: u32 }, + Index { depth: usize }, + Count { name: SmolStr, depth: Option }, Repeat { tokens: MetaTemplate, kind: RepeatKind, separator: Option }, Subtree { tokens: MetaTemplate, delimiter: tt::Delimiter }, Literal(tt::Literal), @@ -295,9 +296,13 @@ fn parse_metavar_expr(src: &mut TtIter<'_>) -> Result { let ident = args.expect_ident()?; Op::Ignore { name: ident.text.clone(), id: ident.span } } - "index" => { - let depth = if args.len() == 0 { 0 } else { args.expect_u32_literal()? }; - Op::Index { depth } + "index" => Op::Index { depth: parse_depth(&mut args)? }, + "count" => { + let ident = args.expect_ident()?; + // `${count(t)}` and `${count(t,)}` have different meanings. Not sure if this is a bug + // but that's how it's implemented in rustc as of this writing. See rust-lang/rust#111904. + let depth = if try_eat_comma(&mut args) { Some(parse_depth(&mut args)?) } else { None }; + Op::Count { name: ident.text.clone(), depth } } _ => return Err(()), }; @@ -308,3 +313,22 @@ fn parse_metavar_expr(src: &mut TtIter<'_>) -> Result { Ok(op) } + +fn parse_depth(src: &mut TtIter<'_>) -> Result { + if src.len() == 0 { + Ok(0) + } else if let tt::Leaf::Literal(lit) = src.expect_literal()? { + // Suffixes are not allowed. + lit.text.parse().map_err(|_| ()) + } else { + Err(()) + } +} + +fn try_eat_comma(src: &mut TtIter<'_>) -> bool { + if let Some(tt::TokenTree::Leaf(tt::Leaf::Punct(tt::Punct { char: ',', .. }))) = src.peek_n(0) { + let _ = src.next(); + return true; + } + false +} diff --git a/crates/mbe/src/tt_iter.rs b/crates/mbe/src/tt_iter.rs index f744481f3aecb..59dbf156800d4 100644 --- a/crates/mbe/src/tt_iter.rs +++ b/crates/mbe/src/tt_iter.rs @@ -73,13 +73,6 @@ impl<'a> TtIter<'a> { } } - pub(crate) fn expect_u32_literal(&mut self) -> Result { - match self.expect_literal()? { - tt::Leaf::Literal(lit) => lit.text.parse().map_err(drop), - _ => Err(()), - } - } - pub(crate) fn expect_single_punct(&mut self) -> Result<&'a tt::Punct, ()> { match self.expect_leaf()? { tt::Leaf::Punct(it) => Ok(it),