From 6d84dee4e7d70f329df71365f98b2421652b2432 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vincent=20Rouill=C3=A9?= Date: Sat, 28 Dec 2019 22:26:24 +0100 Subject: fix #2520: change expand_repeat loop stop condition --- crates/ra_mbe/src/mbe_expander/transcriber.rs | 67 +++++++++++++-------------- crates/ra_mbe/src/tests.rs | 46 ++++++++++++++++++ 2 files changed, 77 insertions(+), 36 deletions(-) diff --git a/crates/ra_mbe/src/mbe_expander/transcriber.rs b/crates/ra_mbe/src/mbe_expander/transcriber.rs index eda66cd50..8bb1a25a3 100644 --- a/crates/ra_mbe/src/mbe_expander/transcriber.rs +++ b/crates/ra_mbe/src/mbe_expander/transcriber.rs @@ -14,21 +14,24 @@ impl Bindings { self.inner.contains_key(name) } - fn get(&self, name: &str, nesting: &[usize]) -> Result<&Fragment, ExpandError> { + fn get(&self, name: &str, nesting: &mut [NestingState]) -> Result<&Fragment, ExpandError> { let mut b = self.inner.get(name).ok_or_else(|| { ExpandError::BindingError(format!("could not find binding `{}`", name)) })?; - for &idx in nesting.iter() { + for s in nesting.iter_mut() { + s.hit = true; b = match b { Binding::Fragment(_) => break, - Binding::Nested(bs) => bs.get(idx).ok_or_else(|| { + Binding::Nested(bs) => bs.get(s.idx).ok_or_else(|| { + s.at_end = true; ExpandError::BindingError(format!("could not find nested binding `{}`", name)) })?, Binding::Empty => { + s.at_end = true; return Err(ExpandError::BindingError(format!( "could not find empty binding `{}`", name - ))) + ))); } }; } @@ -51,15 +54,21 @@ pub(super) fn transcribe( bindings: &Bindings, ) -> Result { assert!(template.delimiter == None); - let mut ctx = ExpandCtx { bindings: &bindings, nesting: Vec::new(), var_expanded: false }; + let mut ctx = ExpandCtx { bindings: &bindings, nesting: Vec::new() }; expand_subtree(&mut ctx, template) } +#[derive(Debug)] +struct NestingState { + idx: usize, + hit: bool, + at_end: bool, +} + #[derive(Debug)] struct ExpandCtx<'a> { bindings: &'a Bindings, - nesting: Vec, - var_expanded: bool, + nesting: Vec, } fn expand_subtree(ctx: &mut ExpandCtx, template: &tt::Subtree) -> Result { @@ -121,8 +130,7 @@ fn expand_var(ctx: &mut ExpandCtx, v: &SmolStr) -> Result .into(); Fragment::Tokens(tt) } else { - let fragment = ctx.bindings.get(&v, &ctx.nesting)?.clone(); - ctx.var_expanded = true; + let fragment = ctx.bindings.get(&v, &mut ctx.nesting)?.clone(); fragment }; Ok(res) @@ -135,37 +143,24 @@ fn expand_repeat( separator: Option, ) -> Result { let mut buf: Vec = Vec::new(); - ctx.nesting.push(0); + ctx.nesting.push(NestingState { idx: 0, at_end: false, hit: false }); // Dirty hack to make macro-expansion terminate. // This should be replaced by a propper macro-by-example implementation - let mut limit = 65536; + let limit = 65536; let mut has_seps = 0; let mut counter = 0; - // We store the old var expanded value, and restore it later - // It is because before this `$repeat`, - // it is possible some variables already expanad in the same subtree - // - // `some_var_expanded` keep check if the deeper subtree has expanded variables - let mut some_var_expanded = false; - let old_var_expanded = ctx.var_expanded; - ctx.var_expanded = false; - - while let Ok(mut t) = expand_subtree(ctx, template) { - t.delimiter = None; - // if no var expanded in the child, we count it as a fail - if !ctx.var_expanded { + loop { + let res = expand_subtree(ctx, template); + let nesting_state = ctx.nesting.last_mut().unwrap(); + if nesting_state.at_end || !nesting_state.hit { break; } - - // Reset `ctx.var_expandeded` to see if there is other expanded variable - // in the next matching - some_var_expanded = true; - ctx.var_expanded = false; + nesting_state.idx += 1; + nesting_state.hit = false; counter += 1; - limit -= 1; - if limit == 0 { + if counter == limit { log::warn!( "expand_tt excced in repeat pattern exceed limit => {:#?}\n{:#?}", template, @@ -174,8 +169,11 @@ fn expand_repeat( break; } - let idx = ctx.nesting.pop().unwrap(); - ctx.nesting.push(idx + 1); + let mut t = match res { + Ok(t) => t, + Err(_) => continue, + }; + t.delimiter = None; push_subtree(&mut buf, t); if let Some(ref sep) = separator { @@ -203,9 +201,6 @@ fn expand_repeat( } } - // Restore the `var_expanded` by combining old one and the new one - ctx.var_expanded = some_var_expanded || old_var_expanded; - ctx.nesting.pop().unwrap(); for _ in 0..has_seps { buf.pop(); diff --git a/crates/ra_mbe/src/tests.rs b/crates/ra_mbe/src/tests.rs index e640d115b..288366cca 100644 --- a/crates/ra_mbe/src/tests.rs +++ b/crates/ra_mbe/src/tests.rs @@ -1491,3 +1491,49 @@ fn debug_dump_ignore_spaces(node: &ra_syntax::SyntaxNode) -> String { buf } + +#[test] +fn test_issue_2520() { + let macro_fixture = parse_macro( + r#" + macro_rules! my_macro { + { + ( $( + $( [] $sname:ident : $stype:ty )? + $( [$expr:expr] $nname:ident : $ntype:ty )? + ),* ) + } => { + Test { + $( + $( $sname, )? + )* + } + }; + } + "#, + ); + + macro_fixture.assert_expand_items( + r#"my_macro ! { + ([] p1 : u32 , [|_| S0K0] s : S0K0 , [] k0 : i32) + }"#, + "Test {p1 , k0 ,}", + ); +} + +#[test] +fn test_repeat_bad_var() { + parse_macro( + r#" + macro_rules! foo { + ($( $b:ident )+) => { + $( $c )+ + }; + ($( $b:ident )+) => { + $( $b )+ + } + } + "#, + ) + .assert_expand_items("foo!(b0 b1);", "b0 b1"); +} -- cgit v1.2.3