From b973158aeb337041d4e1434cf5d8c609a0b02bef Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Fri, 13 Mar 2020 13:03:31 +0100 Subject: Make MBE expansion more resilient (WIP) --- crates/ra_hir_expand/src/db.rs | 90 +++++++++++++++------------ crates/ra_hir_ty/src/tests/macros.rs | 8 +-- crates/ra_ide/src/completion/complete_dot.rs | 37 +++++++++++ crates/ra_ide/src/expand_macro.rs | 2 +- crates/ra_mbe/src/lib.rs | 4 +- crates/ra_mbe/src/mbe_expander.rs | 29 ++++++--- crates/ra_mbe/src/mbe_expander/matcher.rs | 11 ++-- crates/ra_mbe/src/mbe_expander/transcriber.rs | 69 +++++++++++--------- crates/ra_mbe/src/tests.rs | 3 +- crates/ra_tt/src/lib.rs | 6 ++ 10 files changed, 168 insertions(+), 91 deletions(-) diff --git a/crates/ra_hir_expand/src/db.rs b/crates/ra_hir_expand/src/db.rs index c3e1c68b7..e7b81a1e6 100644 --- a/crates/ra_hir_expand/src/db.rs +++ b/crates/ra_hir_expand/src/db.rs @@ -27,11 +27,12 @@ impl TokenExpander { db: &dyn AstDatabase, id: LazyMacroId, tt: &tt::Subtree, - ) -> Result { + ) -> mbe::ExpandResult { match self { TokenExpander::MacroRules(it) => it.expand(tt), - TokenExpander::Builtin(it) => it.expand(db, id, tt), - TokenExpander::BuiltinDerive(it) => it.expand(db, id, tt), + // FIXME switch these to ExpandResult as well + TokenExpander::Builtin(it) => it.expand(db, id, tt).map_or_else(|e| (tt::Subtree::default(), Some(e)), |r| (r, None)), + TokenExpander::BuiltinDerive(it) => it.expand(db, id, tt).map_or_else(|e| (tt::Subtree::default(), Some(e)), |r| (r, None)), } } @@ -66,7 +67,7 @@ pub trait AstDatabase: SourceDatabase { fn macro_def(&self, id: MacroDefId) -> Option>; fn parse_macro(&self, macro_file: MacroFile) -> Option<(Parse, Arc)>; - fn macro_expand(&self, macro_call: MacroCallId) -> Result, String>; + fn macro_expand(&self, macro_call: MacroCallId) -> (Option>, Option); #[salsa::interned] fn intern_eager_expansion(&self, eager: EagerCallLoc) -> EagerMacroId; @@ -153,7 +154,7 @@ pub(crate) fn macro_arg( pub(crate) fn macro_expand( db: &dyn AstDatabase, id: MacroCallId, -) -> Result, String> { +) -> (Option>, Option) { macro_expand_with_arg(db, id, None) } @@ -174,31 +175,39 @@ fn macro_expand_with_arg( db: &dyn AstDatabase, id: MacroCallId, arg: Option>, -) -> Result, String> { +) -> (Option>, Option) { let lazy_id = match id { MacroCallId::LazyMacro(id) => id, MacroCallId::EagerMacro(id) => { if arg.is_some() { - return Err( - "hypothetical macro expansion not implemented for eager macro".to_owned() + return ( + None, + Some("hypothetical macro expansion not implemented for eager macro".to_owned()) ); } else { - return Ok(db.lookup_intern_eager_expansion(id).subtree); + return (Some(db.lookup_intern_eager_expansion(id).subtree), None); } } }; let loc = db.lookup_intern_macro(lazy_id); - let macro_arg = arg.or_else(|| db.macro_arg(id)).ok_or("Fail to args in to tt::TokenTree")?; + let macro_arg = match arg.or_else(|| db.macro_arg(id)) { + Some(it) => it, + None => return (None, Some("Fail to args in to tt::TokenTree".into())), + }; - let macro_rules = db.macro_def(loc.def).ok_or("Fail to find macro definition")?; - let tt = macro_rules.0.expand(db, lazy_id, ¯o_arg.0).map_err(|err| format!("{:?}", err))?; + let macro_rules = match db.macro_def(loc.def) { + Some(it) => it, + None => return (None, Some("Fail to find macro definition".into())), + }; + let (tt, err) = macro_rules.0.expand(db, lazy_id, ¯o_arg.0); // Set a hard limit for the expanded tt + eprintln!("expansion size: {}", tt.count()); let count = tt.count(); if count > 65536 { - return Err(format!("Total tokens count exceed limit : count = {}", count)); + return (None, Some(format!("Total tokens count exceed limit : count = {}", count))); } - Ok(Arc::new(tt)) + (Some(Arc::new(tt)), err.map(|e| format!("{:?}", e))) } pub(crate) fn parse_or_expand(db: &dyn AstDatabase, file_id: HirFileId) -> Option { @@ -225,42 +234,41 @@ pub fn parse_macro_with_arg( let _p = profile("parse_macro_query"); let macro_call_id = macro_file.macro_call_id; - let expansion = if let Some(arg) = arg { + let (tt, err) = if let Some(arg) = arg { macro_expand_with_arg(db, macro_call_id, Some(arg)) } else { db.macro_expand(macro_call_id) }; - let tt = expansion - .map_err(|err| { - // Note: - // The final goal we would like to make all parse_macro success, - // such that the following log will not call anyway. - match macro_call_id { - MacroCallId::LazyMacro(id) => { - let loc: MacroCallLoc = db.lookup_intern_macro(id); - let node = loc.kind.node(db); - - // collect parent information for warning log - let parents = std::iter::successors(loc.kind.file_id().call_node(db), |it| { - it.file_id.call_node(db) - }) + if let Some(err) = err { + // Note: + // The final goal we would like to make all parse_macro success, + // such that the following log will not call anyway. + match macro_call_id { + MacroCallId::LazyMacro(id) => { + let loc: MacroCallLoc = db.lookup_intern_macro(id); + let node = loc.kind.node(db); + + // collect parent information for warning log + let parents = std::iter::successors(loc.kind.file_id().call_node(db), |it| { + it.file_id.call_node(db) + }) .map(|n| format!("{:#}", n.value)) .collect::>() .join("\n"); - log::warn!( - "fail on macro_parse: (reason: {} macro_call: {:#}) parents: {}", - err, - node.value, - parents - ); - } - _ => { - log::warn!("fail on macro_parse: (reason: {})", err); - } + log::warn!( + "fail on macro_parse: (reason: {} macro_call: {:#}) parents: {}", + err, + node.value, + parents + ); + } + _ => { + log::warn!("fail on macro_parse: (reason: {})", err); } - }) - .ok()?; + } + }; + let tt = tt?; let fragment_kind = to_fragment_kind(db, macro_call_id); diff --git a/crates/ra_hir_ty/src/tests/macros.rs b/crates/ra_hir_ty/src/tests/macros.rs index 3b7022ad5..2e309a379 100644 --- a/crates/ra_hir_ty/src/tests/macros.rs +++ b/crates/ra_hir_ty/src/tests/macros.rs @@ -462,7 +462,7 @@ fn main() { fn infer_builtin_macros_include() { let (db, pos) = TestDB::with_position( r#" -//- /main.rs +//- /main.rs #[rustc_builtin_macro] macro_rules! include {() => {}} @@ -483,7 +483,7 @@ fn bar() -> u32 {0} fn infer_builtin_macros_include_concat() { let (db, pos) = TestDB::with_position( r#" -//- /main.rs +//- /main.rs #[rustc_builtin_macro] macro_rules! include {() => {}} @@ -507,7 +507,7 @@ fn bar() -> u32 {0} fn infer_builtin_macros_include_concat_with_bad_env_should_failed() { let (db, pos) = TestDB::with_position( r#" -//- /main.rs +//- /main.rs #[rustc_builtin_macro] macro_rules! include {() => {}} @@ -534,7 +534,7 @@ fn bar() -> u32 {0} fn infer_builtin_macros_include_itself_should_failed() { let (db, pos) = TestDB::with_position( r#" -//- /main.rs +//- /main.rs #[rustc_builtin_macro] macro_rules! include {() => {}} diff --git a/crates/ra_ide/src/completion/complete_dot.rs b/crates/ra_ide/src/completion/complete_dot.rs index f07611d88..a30d1c2de 100644 --- a/crates/ra_ide/src/completion/complete_dot.rs +++ b/crates/ra_ide/src/completion/complete_dot.rs @@ -751,6 +751,43 @@ mod tests { ); } + #[test] + fn macro_expansion_resilient() { + assert_debug_snapshot!( + do_ref_completion( + r" + macro_rules! dbg { + () => {}; + ($val:expr) => { + match $val { tmp => { tmp } } + }; + // Trailing comma with single argument is ignored + ($val:expr,) => { $crate::dbg!($val) }; + ($($val:expr),+ $(,)?) => { + ($($crate::dbg!($val)),+,) + }; + } + struct A { the_field: u32 } + fn foo(a: A) { + dbg!(a.<|>) + } + ", + ), + @r###" + [ + CompletionItem { + label: "the_field", + source_range: [552; 553), + delete: [552; 553), + insert: "the_field", + kind: Field, + detail: "u32", + }, + ] + "### + ); + } + #[test] fn test_method_completion_3547() { assert_debug_snapshot!( diff --git a/crates/ra_ide/src/expand_macro.rs b/crates/ra_ide/src/expand_macro.rs index f6667cb33..e58526f31 100644 --- a/crates/ra_ide/src/expand_macro.rs +++ b/crates/ra_ide/src/expand_macro.rs @@ -259,7 +259,7 @@ fn some_thing() -> u32 { ); assert_eq!(res.name, "foo"); - assert_snapshot!(res.expansion, @r###"bar!()"###); + assert_snapshot!(res.expansion, @r###""###); } #[test] diff --git a/crates/ra_mbe/src/lib.rs b/crates/ra_mbe/src/lib.rs index 43afe24cc..3adec4978 100644 --- a/crates/ra_mbe/src/lib.rs +++ b/crates/ra_mbe/src/lib.rs @@ -30,6 +30,8 @@ pub enum ExpandError { InvalidRepeat, } +pub type ExpandResult = (T, Option); + pub use crate::syntax_bridge::{ ast_to_token_tree, parse_to_token_tree, syntax_node_to_token_tree, token_tree_to_syntax_node, TokenMap, @@ -150,7 +152,7 @@ impl MacroRules { Ok(MacroRules { rules, shift: Shift::new(tt) }) } - pub fn expand(&self, tt: &tt::Subtree) -> Result { + pub fn expand(&self, tt: &tt::Subtree) -> ExpandResult { // apply shift let mut tt = tt.clone(); self.shift.shift_all(&mut tt); diff --git a/crates/ra_mbe/src/mbe_expander.rs b/crates/ra_mbe/src/mbe_expander.rs index b455b7321..da3952428 100644 --- a/crates/ra_mbe/src/mbe_expander.rs +++ b/crates/ra_mbe/src/mbe_expander.rs @@ -8,19 +8,30 @@ mod transcriber; use ra_syntax::SmolStr; use rustc_hash::FxHashMap; -use crate::ExpandError; +use crate::{ExpandResult, ExpandError}; pub(crate) fn expand( rules: &crate::MacroRules, input: &tt::Subtree, -) -> Result { - rules.rules.iter().find_map(|it| expand_rule(it, input).ok()).ok_or(ExpandError::NoMatchingRule) +) -> ExpandResult { + let (mut result, mut err) = (tt::Subtree::default(), Some(ExpandError::NoMatchingRule)); + for rule in &rules.rules { + let (res, e) = expand_rule(rule, input); + if e.is_none() { + // if we find a rule that applies without errors, we're done + return (res, None); + } + // TODO decide which result is better + result = res; + err = e; + } + (result, err) } -fn expand_rule(rule: &crate::Rule, input: &tt::Subtree) -> Result { - let bindings = matcher::match_(&rule.lhs, input)?; - let res = transcriber::transcribe(&rule.rhs, &bindings)?; - Ok(res) +fn expand_rule(rule: &crate::Rule, input: &tt::Subtree) -> ExpandResult { + let (bindings, bindings_err) = dbg!(matcher::match_(&rule.lhs, input)); + let (res, transcribe_err) = dbg!(transcriber::transcribe(&rule.rhs, &bindings)); + (res, bindings_err.or(transcribe_err)) } /// The actual algorithm for expansion is not too hard, but is pretty tricky. @@ -111,7 +122,7 @@ mod tests { } fn assert_err(macro_body: &str, invocation: &str, err: ExpandError) { - assert_eq!(expand_first(&create_rules(&format_macro(macro_body)), invocation), Err(err)); + assert_eq!(expand_first(&create_rules(&format_macro(macro_body)), invocation).1, Some(err)); } fn format_macro(macro_body: &str) -> String { @@ -138,7 +149,7 @@ mod tests { fn expand_first( rules: &crate::MacroRules, invocation: &str, - ) -> Result { + ) -> ExpandResult { let source_file = ast::SourceFile::parse(invocation).ok().unwrap(); let macro_invocation = source_file.syntax().descendants().find_map(ast::MacroCall::cast).unwrap(); diff --git a/crates/ra_mbe/src/mbe_expander/matcher.rs b/crates/ra_mbe/src/mbe_expander/matcher.rs index 49c53183a..f9d4952c6 100644 --- a/crates/ra_mbe/src/mbe_expander/matcher.rs +++ b/crates/ra_mbe/src/mbe_expander/matcher.rs @@ -11,6 +11,7 @@ use crate::{ use ra_parser::{FragmentKind::*, TreeSink}; use ra_syntax::{SmolStr, SyntaxKind}; use tt::buffer::{Cursor, TokenBuffer}; +use super::ExpandResult; impl Bindings { fn push_optional(&mut self, name: &SmolStr) { @@ -64,19 +65,19 @@ macro_rules! bail { }; } -pub(super) fn match_(pattern: &tt::Subtree, src: &tt::Subtree) -> Result { +pub(super) fn match_(pattern: &tt::Subtree, src: &tt::Subtree) -> ExpandResult { assert!(pattern.delimiter == None); let mut res = Bindings::default(); let mut src = TtIter::new(src); - match_subtree(&mut res, pattern, &mut src)?; + let mut err = match_subtree(&mut res, pattern, &mut src).err(); - if src.len() > 0 { - bail!("leftover tokens"); + if src.len() > 0 && err.is_none() { + err = Some(err!("leftover tokens")); } - Ok(res) + (res, err) } fn match_subtree( diff --git a/crates/ra_mbe/src/mbe_expander/transcriber.rs b/crates/ra_mbe/src/mbe_expander/transcriber.rs index 7662020f3..c53c2d35e 100644 --- a/crates/ra_mbe/src/mbe_expander/transcriber.rs +++ b/crates/ra_mbe/src/mbe_expander/transcriber.rs @@ -3,6 +3,7 @@ use ra_syntax::SmolStr; +use super::ExpandResult; use crate::{ mbe_expander::{Binding, Bindings, Fragment}, parser::{parse_template, Op, RepeatKind, Separator}, @@ -49,10 +50,7 @@ impl Bindings { } } -pub(super) fn transcribe( - template: &tt::Subtree, - bindings: &Bindings, -) -> Result { +pub(super) fn transcribe(template: &tt::Subtree, bindings: &Bindings) -> ExpandResult { assert!(template.delimiter == None); let mut ctx = ExpandCtx { bindings: &bindings, nesting: Vec::new() }; expand_subtree(&mut ctx, template) @@ -75,35 +73,46 @@ struct ExpandCtx<'a> { nesting: Vec, } -fn expand_subtree(ctx: &mut ExpandCtx, template: &tt::Subtree) -> Result { +fn expand_subtree(ctx: &mut ExpandCtx, template: &tt::Subtree) -> ExpandResult { let mut buf: Vec = Vec::new(); + let mut err = None; for op in parse_template(template) { - match op? { + let op = match op { + Ok(op) => op, + Err(e) => { + err = Some(e); + break; + } + }; + match op { Op::TokenTree(tt @ tt::TokenTree::Leaf(..)) => buf.push(tt.clone()), Op::TokenTree(tt::TokenTree::Subtree(tt)) => { - let tt = expand_subtree(ctx, tt)?; + let (tt, e) = expand_subtree(ctx, tt); + err = err.or(e); buf.push(tt.into()); } Op::Var { name, kind: _ } => { - let fragment = expand_var(ctx, name)?; + let (fragment, e) = expand_var(ctx, name); + err = err.or(e); push_fragment(&mut buf, fragment); } Op::Repeat { subtree, kind, separator } => { - let fragment = expand_repeat(ctx, subtree, kind, separator)?; + let (fragment, e) = expand_repeat(ctx, subtree, kind, separator); + err = err.or(e); push_fragment(&mut buf, fragment) } } } - Ok(tt::Subtree { delimiter: template.delimiter, token_trees: buf }) + (tt::Subtree { delimiter: template.delimiter, token_trees: buf }, err) } -fn expand_var(ctx: &mut ExpandCtx, v: &SmolStr) -> Result { - let res = if v == "crate" { +fn expand_var(ctx: &mut ExpandCtx, v: &SmolStr) -> ExpandResult { + if v == "crate" { // We simply produce identifier `$crate` here. And it will be resolved when lowering ast to Path. let tt = tt::Leaf::from(tt::Ident { text: "$crate".into(), id: tt::TokenId::unspecified() }) .into(); - Fragment::Tokens(tt) + (Fragment::Tokens(tt), None) } else if !ctx.bindings.contains(v) { // Note that it is possible to have a `$var` inside a macro which is not bound. // For example: @@ -132,11 +141,13 @@ fn expand_var(ctx: &mut ExpandCtx, v: &SmolStr) -> Result ], } .into(); - Fragment::Tokens(tt) + (Fragment::Tokens(tt), None) } else { - ctx.bindings.get(&v, &mut ctx.nesting)?.clone() - }; - Ok(res) + ctx.bindings.get(&v, &mut ctx.nesting).map_or_else( + |e| (Fragment::Tokens(tt::TokenTree::empty()), Some(e)), + |b| (b.clone(), None), + ) + } } fn expand_repeat( @@ -144,17 +155,17 @@ fn expand_repeat( template: &tt::Subtree, kind: RepeatKind, separator: Option, -) -> Result { +) -> ExpandResult { let mut buf: Vec = Vec::new(); 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 + // This should be replaced by a proper macro-by-example implementation let limit = 65536; let mut has_seps = 0; let mut counter = 0; loop { - let res = expand_subtree(ctx, template); + let (mut t, e) = expand_subtree(ctx, template); let nesting_state = ctx.nesting.last_mut().unwrap(); if nesting_state.at_end || !nesting_state.hit { break; @@ -172,10 +183,10 @@ fn expand_repeat( break; } - let mut t = match res { - Ok(t) => t, - Err(_) => continue, - }; + if e.is_some() { + continue; + } + t.delimiter = None; push_subtree(&mut buf, t); @@ -209,14 +220,14 @@ fn expand_repeat( buf.pop(); } - if RepeatKind::OneOrMore == kind && counter == 0 { - return Err(ExpandError::UnexpectedToken); - } - // Check if it is a single token subtree without any delimiter // e.g {Delimiter:None> ['>'] /Delimiter:None>} let tt = tt::Subtree { delimiter: None, token_trees: buf }.into(); - Ok(Fragment::Tokens(tt)) + + if RepeatKind::OneOrMore == kind && counter == 0 { + return (Fragment::Tokens(tt), Some(ExpandError::UnexpectedToken)); + } + (Fragment::Tokens(tt), None) } fn push_fragment(buf: &mut Vec, fragment: Fragment) { diff --git a/crates/ra_mbe/src/tests.rs b/crates/ra_mbe/src/tests.rs index 6d5d1e9e6..4d3140fa9 100644 --- a/crates/ra_mbe/src/tests.rs +++ b/crates/ra_mbe/src/tests.rs @@ -1430,7 +1430,8 @@ impl MacroFixture { let (invocation_tt, _) = ast_to_token_tree(¯o_invocation.token_tree().unwrap()).unwrap(); - self.rules.expand(&invocation_tt) + let (tt, err) = self.rules.expand(&invocation_tt); + err.map(Err).unwrap_or(Ok(tt)) } fn assert_expand_err(&self, invocation: &str, err: &ExpandError) { diff --git a/crates/ra_tt/src/lib.rs b/crates/ra_tt/src/lib.rs index 10f424aae..1e2fb8b91 100644 --- a/crates/ra_tt/src/lib.rs +++ b/crates/ra_tt/src/lib.rs @@ -40,6 +40,12 @@ pub enum TokenTree { } impl_froms!(TokenTree: Leaf, Subtree); +impl TokenTree { + pub fn empty() -> Self { + TokenTree::Subtree(Subtree::default()) + } +} + #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub enum Leaf { Literal(Literal), -- cgit v1.2.3 From 6305d094ac61ed6e437537b93f4e587b415678c9 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Fri, 13 Mar 2020 15:18:17 +0100 Subject: Attempt to implement ranking of rules when none matches perfectly (wip) --- crates/ra_ide/src/completion/complete_scope.rs | 39 +++++++++++++++++++++++++- crates/ra_mbe/src/mbe_expander.rs | 19 +++++++------ crates/ra_mbe/src/mbe_expander/matcher.rs | 4 +-- 3 files changed, 51 insertions(+), 11 deletions(-) diff --git a/crates/ra_ide/src/completion/complete_scope.rs b/crates/ra_ide/src/completion/complete_scope.rs index 5ffff5a1c..2733922f8 100644 --- a/crates/ra_ide/src/completion/complete_scope.rs +++ b/crates/ra_ide/src/completion/complete_scope.rs @@ -811,7 +811,44 @@ mod tests { } " ), - @"[]" + @r###" + [ + CompletionItem { + label: "m!", + source_range: [145; 145), + delete: [145; 145), + insert: "m!($0)", + kind: Macro, + detail: "macro_rules! m", + }, + CompletionItem { + label: "quux(…)", + source_range: [145; 145), + delete: [145; 145), + insert: "quux(${1:x})$0", + kind: Function, + lookup: "quux", + detail: "fn quux(x: i32)", + trigger_call_info: true, + }, + CompletionItem { + label: "x", + source_range: [145; 145), + delete: [145; 145), + insert: "x", + kind: Binding, + detail: "i32", + }, + CompletionItem { + label: "y", + source_range: [145; 145), + delete: [145; 145), + insert: "y", + kind: Binding, + detail: "i32", + }, + ] + "### ); } diff --git a/crates/ra_mbe/src/mbe_expander.rs b/crates/ra_mbe/src/mbe_expander.rs index da3952428..5083d5410 100644 --- a/crates/ra_mbe/src/mbe_expander.rs +++ b/crates/ra_mbe/src/mbe_expander.rs @@ -14,24 +14,27 @@ pub(crate) fn expand( rules: &crate::MacroRules, input: &tt::Subtree, ) -> ExpandResult { - let (mut result, mut err) = (tt::Subtree::default(), Some(ExpandError::NoMatchingRule)); + let (mut result, mut left_over, mut err) = (tt::Subtree::default(), usize::max_value(), Some(ExpandError::NoMatchingRule)); for rule in &rules.rules { - let (res, e) = expand_rule(rule, input); + let ((res, left), e) = expand_rule(rule, input); if e.is_none() { // if we find a rule that applies without errors, we're done return (res, None); } - // TODO decide which result is better - result = res; - err = e; + // use the rule if we matched more tokens + if left < left_over { + result = res; + err = e; + left_over = left; + } } (result, err) } -fn expand_rule(rule: &crate::Rule, input: &tt::Subtree) -> ExpandResult { - let (bindings, bindings_err) = dbg!(matcher::match_(&rule.lhs, input)); +fn expand_rule(rule: &crate::Rule, input: &tt::Subtree) -> ExpandResult<(tt::Subtree, usize)> { + let ((bindings, left_over), bindings_err) = dbg!(matcher::match_(&rule.lhs, input)); let (res, transcribe_err) = dbg!(transcriber::transcribe(&rule.rhs, &bindings)); - (res, bindings_err.or(transcribe_err)) + ((res, left_over), bindings_err.or(transcribe_err)) } /// The actual algorithm for expansion is not too hard, but is pretty tricky. diff --git a/crates/ra_mbe/src/mbe_expander/matcher.rs b/crates/ra_mbe/src/mbe_expander/matcher.rs index f9d4952c6..52f1ac252 100644 --- a/crates/ra_mbe/src/mbe_expander/matcher.rs +++ b/crates/ra_mbe/src/mbe_expander/matcher.rs @@ -65,7 +65,7 @@ macro_rules! bail { }; } -pub(super) fn match_(pattern: &tt::Subtree, src: &tt::Subtree) -> ExpandResult { +pub(super) fn match_(pattern: &tt::Subtree, src: &tt::Subtree) -> ExpandResult<(Bindings, usize)> { assert!(pattern.delimiter == None); let mut res = Bindings::default(); @@ -77,7 +77,7 @@ pub(super) fn match_(pattern: &tt::Subtree, src: &tt::Subtree) -> ExpandResult Date: Sat, 14 Mar 2020 20:24:18 +0100 Subject: wip --- crates/ra_hir_expand/src/db.rs | 16 ++- crates/ra_ide/src/completion/complete_dot.rs | 4 +- crates/ra_ide/src/completion/complete_pattern.rs | 13 +- crates/ra_mbe/src/mbe_expander.rs | 49 ++++--- crates/ra_mbe/src/mbe_expander/matcher.rs | 172 ++++++++++++++--------- 5 files changed, 160 insertions(+), 94 deletions(-) diff --git a/crates/ra_hir_expand/src/db.rs b/crates/ra_hir_expand/src/db.rs index e7b81a1e6..ad4a0732e 100644 --- a/crates/ra_hir_expand/src/db.rs +++ b/crates/ra_hir_expand/src/db.rs @@ -31,8 +31,12 @@ impl TokenExpander { match self { TokenExpander::MacroRules(it) => it.expand(tt), // FIXME switch these to ExpandResult as well - TokenExpander::Builtin(it) => it.expand(db, id, tt).map_or_else(|e| (tt::Subtree::default(), Some(e)), |r| (r, None)), - TokenExpander::BuiltinDerive(it) => it.expand(db, id, tt).map_or_else(|e| (tt::Subtree::default(), Some(e)), |r| (r, None)), + TokenExpander::Builtin(it) => it + .expand(db, id, tt) + .map_or_else(|e| (tt::Subtree::default(), Some(e)), |r| (r, None)), + TokenExpander::BuiltinDerive(it) => it + .expand(db, id, tt) + .map_or_else(|e| (tt::Subtree::default(), Some(e)), |r| (r, None)), } } @@ -182,7 +186,7 @@ fn macro_expand_with_arg( if arg.is_some() { return ( None, - Some("hypothetical macro expansion not implemented for eager macro".to_owned()) + Some("hypothetical macro expansion not implemented for eager macro".to_owned()), ); } else { return (Some(db.lookup_intern_eager_expansion(id).subtree), None); @@ -252,9 +256,9 @@ pub fn parse_macro_with_arg( let parents = std::iter::successors(loc.kind.file_id().call_node(db), |it| { it.file_id.call_node(db) }) - .map(|n| format!("{:#}", n.value)) - .collect::>() - .join("\n"); + .map(|n| format!("{:#}", n.value)) + .collect::>() + .join("\n"); log::warn!( "fail on macro_parse: (reason: {} macro_call: {:#}) parents: {}", diff --git a/crates/ra_ide/src/completion/complete_dot.rs b/crates/ra_ide/src/completion/complete_dot.rs index a30d1c2de..22f5077f5 100644 --- a/crates/ra_ide/src/completion/complete_dot.rs +++ b/crates/ra_ide/src/completion/complete_dot.rs @@ -777,8 +777,8 @@ mod tests { [ CompletionItem { label: "the_field", - source_range: [552; 553), - delete: [552; 553), + source_range: [552; 552), + delete: [552; 552), insert: "the_field", kind: Field, detail: "u32", diff --git a/crates/ra_ide/src/completion/complete_pattern.rs b/crates/ra_ide/src/completion/complete_pattern.rs index 6a1a66ef1..cb84bb934 100644 --- a/crates/ra_ide/src/completion/complete_pattern.rs +++ b/crates/ra_ide/src/completion/complete_pattern.rs @@ -89,7 +89,6 @@ mod tests { #[test] fn completes_in_simple_macro_call() { - // FIXME: doesn't work yet because of missing error recovery in macro expansion let completions = complete( r" macro_rules! m { ($e:expr) => { $e } } @@ -102,6 +101,16 @@ mod tests { } ", ); - assert_debug_snapshot!(completions, @r###"[]"###); + assert_debug_snapshot!(completions, @r###" + [ + CompletionItem { + label: "E", + source_range: [151; 151), + delete: [151; 151), + insert: "E", + kind: Enum, + }, + ] + "###); } } diff --git a/crates/ra_mbe/src/mbe_expander.rs b/crates/ra_mbe/src/mbe_expander.rs index 5083d5410..b2faa86d2 100644 --- a/crates/ra_mbe/src/mbe_expander.rs +++ b/crates/ra_mbe/src/mbe_expander.rs @@ -8,33 +8,44 @@ mod transcriber; use ra_syntax::SmolStr; use rustc_hash::FxHashMap; -use crate::{ExpandResult, ExpandError}; - -pub(crate) fn expand( - rules: &crate::MacroRules, - input: &tt::Subtree, -) -> ExpandResult { - let (mut result, mut left_over, mut err) = (tt::Subtree::default(), usize::max_value(), Some(ExpandError::NoMatchingRule)); +use crate::{ExpandError, ExpandResult}; + +pub(crate) fn expand(rules: &crate::MacroRules, input: &tt::Subtree) -> ExpandResult { + let (mut result, mut unmatched_tokens, mut unmatched_patterns, mut err) = ( + tt::Subtree::default(), + usize::max_value(), + usize::max_value(), + Some(ExpandError::NoMatchingRule), + ); for rule in &rules.rules { - let ((res, left), e) = expand_rule(rule, input); + let ((res, tokens, patterns), e) = expand_rule(rule, input); if e.is_none() { // if we find a rule that applies without errors, we're done return (res, None); } - // use the rule if we matched more tokens - if left < left_over { + // use the rule if we matched more tokens, or had fewer patterns left + if tokens < unmatched_tokens || tokens == unmatched_tokens && patterns < unmatched_patterns + { result = res; err = e; - left_over = left; + unmatched_tokens = tokens; + unmatched_patterns = patterns; } } (result, err) } -fn expand_rule(rule: &crate::Rule, input: &tt::Subtree) -> ExpandResult<(tt::Subtree, usize)> { - let ((bindings, left_over), bindings_err) = dbg!(matcher::match_(&rule.lhs, input)); - let (res, transcribe_err) = dbg!(transcriber::transcribe(&rule.rhs, &bindings)); - ((res, left_over), bindings_err.or(transcribe_err)) +fn expand_rule( + rule: &crate::Rule, + input: &tt::Subtree, +) -> ExpandResult<(tt::Subtree, usize, usize)> { + dbg!(&rule.lhs); + let (match_result, bindings_err) = dbg!(matcher::match_(&rule.lhs, input)); + let (res, transcribe_err) = dbg!(transcriber::transcribe(&rule.rhs, &match_result.bindings)); + ( + (res, match_result.unmatched_tokens, match_result.unmatched_patterns), + bindings_err.or(transcribe_err), + ) } /// The actual algorithm for expansion is not too hard, but is pretty tricky. @@ -149,10 +160,7 @@ mod tests { crate::MacroRules::parse(&definition_tt).unwrap() } - fn expand_first( - rules: &crate::MacroRules, - invocation: &str, - ) -> ExpandResult { + fn expand_first(rules: &crate::MacroRules, invocation: &str) -> ExpandResult { let source_file = ast::SourceFile::parse(invocation).ok().unwrap(); let macro_invocation = source_file.syntax().descendants().find_map(ast::MacroCall::cast).unwrap(); @@ -160,6 +168,7 @@ mod tests { let (invocation_tt, _) = ast_to_token_tree(¯o_invocation.token_tree().unwrap()).unwrap(); - expand_rule(&rules.rules[0], &invocation_tt) + let expanded = expand_rule(&rules.rules[0], &invocation_tt); + ((expanded.0).0, expanded.1) } } diff --git a/crates/ra_mbe/src/mbe_expander/matcher.rs b/crates/ra_mbe/src/mbe_expander/matcher.rs index 52f1ac252..ae65fb69a 100644 --- a/crates/ra_mbe/src/mbe_expander/matcher.rs +++ b/crates/ra_mbe/src/mbe_expander/matcher.rs @@ -8,10 +8,10 @@ use crate::{ ExpandError, }; +use super::ExpandResult; use ra_parser::{FragmentKind::*, TreeSink}; use ra_syntax::{SmolStr, SyntaxKind}; use tt::buffer::{Cursor, TokenBuffer}; -use super::ExpandResult; impl Bindings { fn push_optional(&mut self, name: &SmolStr) { @@ -59,36 +59,50 @@ macro_rules! err { }; } -macro_rules! bail { - ($($tt:tt)*) => { - return Err(err!($($tt)*)) - }; +#[derive(Debug, Default)] +pub(super) struct Match { + pub bindings: Bindings, + pub unmatched_tokens: usize, + pub unmatched_patterns: usize, } -pub(super) fn match_(pattern: &tt::Subtree, src: &tt::Subtree) -> ExpandResult<(Bindings, usize)> { +pub(super) fn match_(pattern: &tt::Subtree, src: &tt::Subtree) -> ExpandResult { assert!(pattern.delimiter == None); - let mut res = Bindings::default(); + let mut res = Match::default(); let mut src = TtIter::new(src); let mut err = match_subtree(&mut res, pattern, &mut src).err(); + res.unmatched_tokens += src.len(); if src.len() > 0 && err.is_none() { err = Some(err!("leftover tokens")); } - ((res, src.len()), err) + (res, err) } fn match_subtree( - bindings: &mut Bindings, + res: &mut Match, pattern: &tt::Subtree, src: &mut TtIter, ) -> Result<(), ExpandError> { + let mut result = Ok(()); for op in parse_pattern(pattern) { + if result.is_err() { + // We're just going through the patterns to count how many we missed + res.unmatched_patterns += 1; + continue; + } match op? { Op::TokenTree(tt::TokenTree::Leaf(lhs)) => { - let rhs = src.expect_leaf().map_err(|()| err!("expected leaf: `{}`", lhs))?; + let rhs = match src.expect_leaf() { + Ok(l) => l, + Err(()) => { + result = Err(err!("expected leaf: `{}`", lhs)); + continue; + } + }; match (lhs, rhs) { ( tt::Leaf::Punct(tt::Punct { char: lhs, .. }), @@ -102,35 +116,54 @@ fn match_subtree( tt::Leaf::Literal(tt::Literal { text: lhs, .. }), tt::Leaf::Literal(tt::Literal { text: rhs, .. }), ) if lhs == rhs => (), - _ => return Err(ExpandError::UnexpectedToken), + _ => { + result = Err(ExpandError::UnexpectedToken); + } } } Op::TokenTree(tt::TokenTree::Subtree(lhs)) => { - let rhs = src.expect_subtree().map_err(|()| err!("expected subtree"))?; + let rhs = match src.expect_subtree() { + Ok(s) => s, + Err(()) => { + result = Err(err!("expected subtree")); + continue; + } + }; if lhs.delimiter_kind() != rhs.delimiter_kind() { - bail!("mismatched delimiter") + result = Err(err!("mismatched delimiter")); + continue; } let mut src = TtIter::new(rhs); - match_subtree(bindings, lhs, &mut src)?; - if src.len() > 0 { - bail!("leftover tokens"); + result = match_subtree(res, lhs, &mut src); + res.unmatched_tokens += src.len(); + if src.len() > 0 && result.is_ok() { + result = Err(err!("leftover tokens")); } } Op::Var { name, kind } => { - let kind = kind.as_ref().ok_or(ExpandError::UnexpectedToken)?; - match match_meta_var(kind.as_str(), src)? { + let kind = match kind { + Some(k) => k, + None => { + result = Err(ExpandError::UnexpectedToken); + continue; + } + }; + let (matched, match_err) = match_meta_var(kind.as_str(), src); + match matched { Some(fragment) => { - bindings.inner.insert(name.clone(), Binding::Fragment(fragment)); + res.bindings.inner.insert(name.clone(), Binding::Fragment(fragment)); } - None => bindings.push_optional(name), + None if match_err.is_none() => res.bindings.push_optional(name), + _ => {} } + result = match_err.map_or(Ok(()), Err); } Op::Repeat { subtree, kind, separator } => { - match_repeat(bindings, subtree, kind, separator, src)? + result = match_repeat(res, subtree, kind, separator, src); } } } - Ok(()) + result } impl<'a> TtIter<'a> { @@ -222,7 +255,7 @@ impl<'a> TtIter<'a> { pub(crate) fn expect_fragment( &mut self, fragment_kind: ra_parser::FragmentKind, - ) -> Result { + ) -> ExpandResult { pub(crate) struct OffsetTokenSink<'a> { pub(crate) cursor: Cursor<'a>, pub(crate) error: bool, @@ -247,45 +280,47 @@ impl<'a> TtIter<'a> { ra_parser::parse_fragment(&mut src, &mut sink, fragment_kind); + let mut err = None; if !sink.cursor.is_root() || sink.error { - // FIXME better recovery in this case would help completion inside macros immensely - return Err(()); + err = Some(err!("expected {:?}", fragment_kind)); } let mut curr = buffer.begin(); let mut res = vec![]; - while curr != sink.cursor { - if let Some(token) = curr.token_tree() { - res.push(token); + if sink.cursor.is_root() { + while curr != sink.cursor { + if let Some(token) = curr.token_tree() { + res.push(token); + } + curr = curr.bump(); } - curr = curr.bump(); } self.inner = self.inner.as_slice()[res.len()..].iter(); - match res.len() { - 0 => Err(()), - 1 => Ok(res[0].clone()), - _ => Ok(tt::TokenTree::Subtree(tt::Subtree { + let res = match res.len() { + 1 => res[0].clone(), + _ => tt::TokenTree::Subtree(tt::Subtree { delimiter: None, token_trees: res.into_iter().cloned().collect(), - })), - } + }), + }; + (res, err) } pub(crate) fn eat_vis(&mut self) -> Option { let mut fork = self.clone(); match fork.expect_fragment(Visibility) { - Ok(tt) => { + (tt, None) => { *self = fork; Some(tt) } - Err(()) => None, + (_, Some(_)) => None, } } } pub(super) fn match_repeat( - bindings: &mut Bindings, + res: &mut Match, pattern: &tt::Subtree, kind: RepeatKind, separator: Option, @@ -305,17 +340,23 @@ pub(super) fn match_repeat( } } - let mut nested = Bindings::default(); + let mut nested = Match::default(); match match_subtree(&mut nested, pattern, &mut fork) { Ok(()) => { limit -= 1; if limit == 0 { - log::warn!("match_lhs excced in repeat pattern exceed limit => {:#?}\n{:#?}\n{:#?}\n{:#?}", pattern, src, kind, separator); + log::warn!( + "match_lhs exceeded repeat pattern limit => {:#?}\n{:#?}\n{:#?}\n{:#?}", + pattern, + src, + kind, + separator + ); break; } *src = fork; - bindings.push_nested(counter, nested)?; + res.bindings.push_nested(counter, nested.bindings)?; counter += 1; if counter == 1 { if let RepeatKind::ZeroOrOne = kind { @@ -334,7 +375,7 @@ pub(super) fn match_repeat( let mut vars = Vec::new(); collect_vars(&mut vars, pattern)?; for var in vars { - bindings.push_empty(&var) + res.bindings.push_empty(&var) } } _ => (), @@ -342,7 +383,7 @@ pub(super) fn match_repeat( Ok(()) } -fn match_meta_var(kind: &str, input: &mut TtIter) -> Result, ExpandError> { +fn match_meta_var(kind: &str, input: &mut TtIter) -> ExpandResult> { let fragment = match kind { "path" => Path, "expr" => Expr, @@ -353,34 +394,33 @@ fn match_meta_var(kind: &str, input: &mut TtIter) -> Result, Ex "meta" => MetaItem, "item" => Item, _ => { - let tt = match kind { - "ident" => { - let ident = input.expect_ident().map_err(|()| err!("expected ident"))?.clone(); - tt::Leaf::from(ident).into() - } - "tt" => input.expect_tt().map_err(|()| err!())?.clone(), - "lifetime" => { - let ident = input.expect_lifetime().map_err(|()| err!())?; - tt::Leaf::Ident(ident.clone()).into() - } - "literal" => { - let literal = input.expect_literal().map_err(|()| err!())?.clone(); - tt::Leaf::from(literal).into() - } + let tt_result = match kind { + "ident" => input + .expect_ident() + .map(|ident| Some(tt::Leaf::from(ident.clone()).into())) + .map_err(|()| err!("expected ident")), + "tt" => input.expect_tt().map(Some).map_err(|()| err!()), + "lifetime" => input + .expect_lifetime() + .map(|ident| Some(tt::Leaf::Ident(ident.clone()).into())) + .map_err(|()| err!("expected lifetime")), + "literal" => input + .expect_literal() + .map(|literal| Some(tt::Leaf::from(literal.clone()).into())) + .map_err(|()| err!()), // `vis` is optional "vis" => match input.eat_vis() { - Some(vis) => vis, - None => return Ok(None), + Some(vis) => Ok(Some(vis)), + None => Ok(None), }, - _ => return Err(ExpandError::UnexpectedToken), + _ => Err(ExpandError::UnexpectedToken), }; - return Ok(Some(Fragment::Tokens(tt))); + return to_expand_result(tt_result.map(|it| it.map(Fragment::Tokens))); } }; - let tt = - input.expect_fragment(fragment).map_err(|()| err!("fragment did not parse as {}", kind))?; + let (tt, err) = input.expect_fragment(fragment); let fragment = if kind == "expr" { Fragment::Ast(tt) } else { Fragment::Tokens(tt) }; - Ok(Some(fragment)) + (Some(fragment), err) } fn collect_vars(buf: &mut Vec, pattern: &tt::Subtree) -> Result<(), ExpandError> { @@ -394,3 +434,7 @@ fn collect_vars(buf: &mut Vec, pattern: &tt::Subtree) -> Result<(), Exp } Ok(()) } + +fn to_expand_result(result: Result) -> ExpandResult { + result.map_or_else(|e| (Default::default(), Some(e)), |it| (it, None)) +} -- cgit v1.2.3 From c32529ddd0d66a219226dd63da2d4b1825375c0e Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Sun, 15 Mar 2020 11:17:13 +0100 Subject: Get tests working --- crates/ra_ide/src/completion/complete_dot.rs | 13 ++++++++++++- crates/ra_ide/src/completion/completion_context.rs | 2 +- crates/ra_mbe/src/mbe_expander.rs | 6 +++--- crates/ra_mbe/src/syntax_bridge.rs | 1 + crates/ra_parser/src/grammar/expressions/atom.rs | 4 ++-- 5 files changed, 19 insertions(+), 7 deletions(-) diff --git a/crates/ra_ide/src/completion/complete_dot.rs b/crates/ra_ide/src/completion/complete_dot.rs index 22f5077f5..82ec16913 100644 --- a/crates/ra_ide/src/completion/complete_dot.rs +++ b/crates/ra_ide/src/completion/complete_dot.rs @@ -720,7 +720,18 @@ mod tests { } ", ), - @r###"[]"### + @r###" + [ + CompletionItem { + label: "the_field", + source_range: [156; 156), + delete: [156; 156), + insert: "the_field", + kind: Field, + detail: "u32", + }, + ] + "### ); } diff --git a/crates/ra_ide/src/completion/completion_context.rs b/crates/ra_ide/src/completion/completion_context.rs index 3646fb8dc..54589a2a8 100644 --- a/crates/ra_ide/src/completion/completion_context.rs +++ b/crates/ra_ide/src/completion/completion_context.rs @@ -135,7 +135,7 @@ impl<'a> CompletionContext<'a> { ), ) { let new_offset = hypothetical_expansion.1.text_range().start(); - if new_offset >= actual_expansion.text_range().end() { + if new_offset > actual_expansion.text_range().end() { break; } original_file = actual_expansion; diff --git a/crates/ra_mbe/src/mbe_expander.rs b/crates/ra_mbe/src/mbe_expander.rs index b2faa86d2..c2a5702f0 100644 --- a/crates/ra_mbe/src/mbe_expander.rs +++ b/crates/ra_mbe/src/mbe_expander.rs @@ -11,6 +11,7 @@ use rustc_hash::FxHashMap; use crate::{ExpandError, ExpandResult}; pub(crate) fn expand(rules: &crate::MacroRules, input: &tt::Subtree) -> ExpandResult { + eprintln!("expanding input: {:?}", input); let (mut result, mut unmatched_tokens, mut unmatched_patterns, mut err) = ( tt::Subtree::default(), usize::max_value(), @@ -39,9 +40,8 @@ fn expand_rule( rule: &crate::Rule, input: &tt::Subtree, ) -> ExpandResult<(tt::Subtree, usize, usize)> { - dbg!(&rule.lhs); - let (match_result, bindings_err) = dbg!(matcher::match_(&rule.lhs, input)); - let (res, transcribe_err) = dbg!(transcriber::transcribe(&rule.rhs, &match_result.bindings)); + let (match_result, bindings_err) = matcher::match_(&rule.lhs, input); + let (res, transcribe_err) = transcriber::transcribe(&rule.rhs, &match_result.bindings); ( (res, match_result.unmatched_tokens, match_result.unmatched_patterns), bindings_err.or(transcribe_err), diff --git a/crates/ra_mbe/src/syntax_bridge.rs b/crates/ra_mbe/src/syntax_bridge.rs index fcb73fbc7..8aa3b906b 100644 --- a/crates/ra_mbe/src/syntax_bridge.rs +++ b/crates/ra_mbe/src/syntax_bridge.rs @@ -73,6 +73,7 @@ pub fn token_tree_to_syntax_node( tt: &tt::Subtree, fragment_kind: FragmentKind, ) -> Result<(Parse, TokenMap), ExpandError> { + eprintln!("token_tree_to_syntax_node {:?} as {:?}", tt, fragment_kind); let tmp; let tokens = match tt { tt::Subtree { delimiter: None, token_trees } => token_trees.as_slice(), diff --git a/crates/ra_parser/src/grammar/expressions/atom.rs b/crates/ra_parser/src/grammar/expressions/atom.rs index b77b683b5..2fc6ce1e1 100644 --- a/crates/ra_parser/src/grammar/expressions/atom.rs +++ b/crates/ra_parser/src/grammar/expressions/atom.rs @@ -565,10 +565,10 @@ fn meta_var_expr(p: &mut Parser) -> CompletedMarker { it } _ => { - while !p.at(R_DOLLAR) { + while !p.at(EOF) && !p.at(R_DOLLAR) { p.bump_any() } - p.bump(R_DOLLAR); + p.eat(R_DOLLAR); m.complete(p, ERROR) } } -- cgit v1.2.3 From 035db0fbb94e35c78cc76798844db06bd6ad8451 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Sun, 15 Mar 2020 11:26:54 +0100 Subject: Add test, remove printlns --- crates/ra_hir_expand/src/db.rs | 1 - crates/ra_ide/src/completion/complete_scope.rs | 53 ++++++++++++++++++++++++++ crates/ra_mbe/src/mbe_expander.rs | 1 - crates/ra_mbe/src/syntax_bridge.rs | 1 - 4 files changed, 53 insertions(+), 3 deletions(-) diff --git a/crates/ra_hir_expand/src/db.rs b/crates/ra_hir_expand/src/db.rs index ad4a0732e..f1918817e 100644 --- a/crates/ra_hir_expand/src/db.rs +++ b/crates/ra_hir_expand/src/db.rs @@ -206,7 +206,6 @@ fn macro_expand_with_arg( }; let (tt, err) = macro_rules.0.expand(db, lazy_id, ¯o_arg.0); // Set a hard limit for the expanded tt - eprintln!("expansion size: {}", tt.count()); let count = tt.count(); if count > 65536 { return (None, Some(format!("Total tokens count exceed limit : count = {}", count))); diff --git a/crates/ra_ide/src/completion/complete_scope.rs b/crates/ra_ide/src/completion/complete_scope.rs index 2733922f8..81d3cc1b6 100644 --- a/crates/ra_ide/src/completion/complete_scope.rs +++ b/crates/ra_ide/src/completion/complete_scope.rs @@ -905,6 +905,59 @@ mod tests { ); } + #[test] + fn completes_in_simple_macro_without_closing_parens() { + assert_debug_snapshot!( + do_reference_completion( + r" + macro_rules! m { ($e:expr) => { $e } } + fn quux(x: i32) { + let y = 92; + m!(x<|> + } + " + ), + @r###" + [ + CompletionItem { + label: "m!", + source_range: [145; 146), + delete: [145; 146), + insert: "m!($0)", + kind: Macro, + detail: "macro_rules! m", + }, + CompletionItem { + label: "quux(…)", + source_range: [145; 146), + delete: [145; 146), + insert: "quux(${1:x})$0", + kind: Function, + lookup: "quux", + detail: "fn quux(x: i32)", + trigger_call_info: true, + }, + CompletionItem { + label: "x", + source_range: [145; 146), + delete: [145; 146), + insert: "x", + kind: Binding, + detail: "i32", + }, + CompletionItem { + label: "y", + source_range: [145; 146), + delete: [145; 146), + insert: "y", + kind: Binding, + detail: "i32", + }, + ] + "### + ); + } + #[test] fn completes_unresolved_uses() { assert_debug_snapshot!( diff --git a/crates/ra_mbe/src/mbe_expander.rs b/crates/ra_mbe/src/mbe_expander.rs index c2a5702f0..5fb8414b3 100644 --- a/crates/ra_mbe/src/mbe_expander.rs +++ b/crates/ra_mbe/src/mbe_expander.rs @@ -11,7 +11,6 @@ use rustc_hash::FxHashMap; use crate::{ExpandError, ExpandResult}; pub(crate) fn expand(rules: &crate::MacroRules, input: &tt::Subtree) -> ExpandResult { - eprintln!("expanding input: {:?}", input); let (mut result, mut unmatched_tokens, mut unmatched_patterns, mut err) = ( tt::Subtree::default(), usize::max_value(), diff --git a/crates/ra_mbe/src/syntax_bridge.rs b/crates/ra_mbe/src/syntax_bridge.rs index 8aa3b906b..fcb73fbc7 100644 --- a/crates/ra_mbe/src/syntax_bridge.rs +++ b/crates/ra_mbe/src/syntax_bridge.rs @@ -73,7 +73,6 @@ pub fn token_tree_to_syntax_node( tt: &tt::Subtree, fragment_kind: FragmentKind, ) -> Result<(Parse, TokenMap), ExpandError> { - eprintln!("token_tree_to_syntax_node {:?} as {:?}", tt, fragment_kind); let tmp; let tokens = match tt { tt::Subtree { delimiter: None, token_trees } => token_trees.as_slice(), -- cgit v1.2.3 From e6ec4a329fedb354e64c0e0dea9b7651cab78960 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Sun, 15 Mar 2020 14:03:30 +0100 Subject: Better fix for stuck parser? --- crates/ra_parser/src/grammar/expressions/atom.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/ra_parser/src/grammar/expressions/atom.rs b/crates/ra_parser/src/grammar/expressions/atom.rs index 2fc6ce1e1..2335d99b3 100644 --- a/crates/ra_parser/src/grammar/expressions/atom.rs +++ b/crates/ra_parser/src/grammar/expressions/atom.rs @@ -61,7 +61,7 @@ pub(super) const ATOM_EXPR_FIRST: TokenSet = LIFETIME, ]); -const EXPR_RECOVERY_SET: TokenSet = token_set![LET_KW]; +const EXPR_RECOVERY_SET: TokenSet = token_set![LET_KW, R_DOLLAR]; pub(super) fn atom_expr(p: &mut Parser, r: Restrictions) -> Option<(CompletedMarker, BlockLike)> { if let Some(m) = literal(p) { @@ -565,10 +565,10 @@ fn meta_var_expr(p: &mut Parser) -> CompletedMarker { it } _ => { - while !p.at(EOF) && !p.at(R_DOLLAR) { + while !p.at(R_DOLLAR) { p.bump_any() } - p.eat(R_DOLLAR); + p.bump(R_DOLLAR); m.complete(p, ERROR) } } -- cgit v1.2.3 From 0660dd10d203cba4069b48d00d02ec9935a916f0 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Sun, 15 Mar 2020 14:37:30 +0100 Subject: Fix performance problem --- crates/ra_mbe/src/mbe_expander.rs | 67 +++++++++++++++++++++------------------ crates/ra_mbe/src/tests.rs | 2 +- 2 files changed, 37 insertions(+), 32 deletions(-) diff --git a/crates/ra_mbe/src/mbe_expander.rs b/crates/ra_mbe/src/mbe_expander.rs index 5fb8414b3..1328e4349 100644 --- a/crates/ra_mbe/src/mbe_expander.rs +++ b/crates/ra_mbe/src/mbe_expander.rs @@ -11,40 +11,45 @@ use rustc_hash::FxHashMap; use crate::{ExpandError, ExpandResult}; pub(crate) fn expand(rules: &crate::MacroRules, input: &tt::Subtree) -> ExpandResult { - let (mut result, mut unmatched_tokens, mut unmatched_patterns, mut err) = ( - tt::Subtree::default(), - usize::max_value(), - usize::max_value(), - Some(ExpandError::NoMatchingRule), - ); - for rule in &rules.rules { - let ((res, tokens, patterns), e) = expand_rule(rule, input); - if e.is_none() { + expand_rules(&rules.rules, input) +} + +fn expand_rules(rules: &[crate::Rule], input: &tt::Subtree) -> ExpandResult { + let mut match_: Option<(matcher::Match, &crate::Rule)> = None; + let mut err = Some(ExpandError::NoMatchingRule); + for rule in rules { + let (new_match, bindings_err) = matcher::match_(&rule.lhs, input); + if bindings_err.is_none() { // if we find a rule that applies without errors, we're done - return (res, None); + eprintln!("match without errors: {:?}", new_match); + let (res, transcribe_err) = transcriber::transcribe(&rule.rhs, &new_match.bindings); + eprintln!("transcribe_err = {:?}", transcribe_err); + if transcribe_err.is_none() { + return (res, None); + } } // use the rule if we matched more tokens, or had fewer patterns left - if tokens < unmatched_tokens || tokens == unmatched_tokens && patterns < unmatched_patterns - { - result = res; - err = e; - unmatched_tokens = tokens; - unmatched_patterns = patterns; + if let Some((prev_match, _)) = &match_ { + if new_match.unmatched_tokens < prev_match.unmatched_tokens + || new_match.unmatched_tokens == prev_match.unmatched_tokens + && new_match.unmatched_patterns < prev_match.unmatched_patterns + || err.is_some() && bindings_err.is_none() + { + match_ = Some((new_match, rule)); + err = bindings_err; + } + } else { + match_ = Some((new_match, rule)); + err = bindings_err; } } - (result, err) -} - -fn expand_rule( - rule: &crate::Rule, - input: &tt::Subtree, -) -> ExpandResult<(tt::Subtree, usize, usize)> { - let (match_result, bindings_err) = matcher::match_(&rule.lhs, input); - let (res, transcribe_err) = transcriber::transcribe(&rule.rhs, &match_result.bindings); - ( - (res, match_result.unmatched_tokens, match_result.unmatched_patterns), - bindings_err.or(transcribe_err), - ) + if let Some((match_, rule)) = match_ { + // if we got here, there was no match without errors + let (result, transcribe_err) = transcriber::transcribe(&rule.rhs, &match_.bindings); + (result, err.or(transcribe_err)) + } else { + (tt::Subtree::default(), err) + } } /// The actual algorithm for expansion is not too hard, but is pretty tricky. @@ -167,7 +172,7 @@ mod tests { let (invocation_tt, _) = ast_to_token_tree(¯o_invocation.token_tree().unwrap()).unwrap(); - let expanded = expand_rule(&rules.rules[0], &invocation_tt); - ((expanded.0).0, expanded.1) + let expanded = expand_rules(&rules.rules, &invocation_tt); + (expanded.0, expanded.1) } } diff --git a/crates/ra_mbe/src/tests.rs b/crates/ra_mbe/src/tests.rs index 4d3140fa9..faf88c1b1 100644 --- a/crates/ra_mbe/src/tests.rs +++ b/crates/ra_mbe/src/tests.rs @@ -1663,5 +1663,5 @@ fn test_expand_bad_literal() { macro_rules! foo { ($i:literal) => {}; } "#, ) - .assert_expand_err(r#"foo!(&k");"#, &ExpandError::NoMatchingRule); + .assert_expand_err(r#"foo!(&k");"#, &ExpandError::BindingError("".to_string())); } -- cgit v1.2.3 From f3c6a2e3dbe477a7e0ac714a5bdbda6e8838fcfa Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Sun, 15 Mar 2020 15:15:56 +0100 Subject: Fix remaining test failure --- crates/ra_mbe/src/mbe_expander.rs | 2 -- crates/ra_mbe/src/mbe_expander/matcher.rs | 18 +++++++++++------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/crates/ra_mbe/src/mbe_expander.rs b/crates/ra_mbe/src/mbe_expander.rs index 1328e4349..204c30e3d 100644 --- a/crates/ra_mbe/src/mbe_expander.rs +++ b/crates/ra_mbe/src/mbe_expander.rs @@ -21,9 +21,7 @@ fn expand_rules(rules: &[crate::Rule], input: &tt::Subtree) -> ExpandResult TtIter<'a> { pub(crate) fn expect_fragment( &mut self, fragment_kind: ra_parser::FragmentKind, - ) -> ExpandResult { + ) -> ExpandResult> { pub(crate) struct OffsetTokenSink<'a> { pub(crate) cursor: Cursor<'a>, pub(crate) error: bool, @@ -297,12 +297,16 @@ impl<'a> TtIter<'a> { } } self.inner = self.inner.as_slice()[res.len()..].iter(); + if res.len() == 0 && err.is_none() { + err = Some(err!("no tokens consumed")); + } let res = match res.len() { - 1 => res[0].clone(), - _ => tt::TokenTree::Subtree(tt::Subtree { + 1 => Some(res[0].clone()), + 0 => None, + _ => Some(tt::TokenTree::Subtree(tt::Subtree { delimiter: None, token_trees: res.into_iter().cloned().collect(), - }), + })), }; (res, err) } @@ -312,7 +316,7 @@ impl<'a> TtIter<'a> { match fork.expect_fragment(Visibility) { (tt, None) => { *self = fork; - Some(tt) + tt } (_, Some(_)) => None, } @@ -419,8 +423,8 @@ fn match_meta_var(kind: &str, input: &mut TtIter) -> ExpandResult, pattern: &tt::Subtree) -> Result<(), ExpandError> { -- cgit v1.2.3 From d655749aaeb31461f9af923bbf0b36d219cff343 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Mon, 16 Mar 2020 12:22:10 +0100 Subject: Turn ExpandResult into struct --- crates/ra_hir_expand/src/db.rs | 12 ++++------ crates/ra_mbe/src/lib.rs | 32 +++++++++++++++++++++++++-- crates/ra_mbe/src/mbe_expander.rs | 16 ++++++++------ crates/ra_mbe/src/mbe_expander/matcher.rs | 21 +++++++----------- crates/ra_mbe/src/mbe_expander/transcriber.rs | 22 +++++++++--------- crates/ra_mbe/src/tests.rs | 3 +-- 6 files changed, 63 insertions(+), 43 deletions(-) diff --git a/crates/ra_hir_expand/src/db.rs b/crates/ra_hir_expand/src/db.rs index f1918817e..d171d2dfd 100644 --- a/crates/ra_hir_expand/src/db.rs +++ b/crates/ra_hir_expand/src/db.rs @@ -2,7 +2,7 @@ use std::sync::Arc; -use mbe::MacroRules; +use mbe::{ExpandResult, MacroRules}; use ra_db::{salsa, SourceDatabase}; use ra_parser::FragmentKind; use ra_prof::profile; @@ -31,12 +31,8 @@ impl TokenExpander { match self { TokenExpander::MacroRules(it) => it.expand(tt), // FIXME switch these to ExpandResult as well - TokenExpander::Builtin(it) => it - .expand(db, id, tt) - .map_or_else(|e| (tt::Subtree::default(), Some(e)), |r| (r, None)), - TokenExpander::BuiltinDerive(it) => it - .expand(db, id, tt) - .map_or_else(|e| (tt::Subtree::default(), Some(e)), |r| (r, None)), + TokenExpander::Builtin(it) => it.expand(db, id, tt).into(), + TokenExpander::BuiltinDerive(it) => it.expand(db, id, tt).into(), } } @@ -204,7 +200,7 @@ fn macro_expand_with_arg( Some(it) => it, None => return (None, Some("Fail to find macro definition".into())), }; - let (tt, err) = macro_rules.0.expand(db, lazy_id, ¯o_arg.0); + let ExpandResult(tt, err) = macro_rules.0.expand(db, lazy_id, ¯o_arg.0); // Set a hard limit for the expanded tt let count = tt.count(); if count > 65536 { diff --git a/crates/ra_mbe/src/lib.rs b/crates/ra_mbe/src/lib.rs index 3adec4978..6a9037bfc 100644 --- a/crates/ra_mbe/src/lib.rs +++ b/crates/ra_mbe/src/lib.rs @@ -30,8 +30,6 @@ pub enum ExpandError { InvalidRepeat, } -pub type ExpandResult = (T, Option); - pub use crate::syntax_bridge::{ ast_to_token_tree, parse_to_token_tree, syntax_node_to_token_tree, token_tree_to_syntax_node, TokenMap, @@ -211,5 +209,35 @@ fn validate(pattern: &tt::Subtree) -> Result<(), ParseError> { Ok(()) } +pub struct ExpandResult(pub T, pub Option); + +impl ExpandResult { + pub fn ok(t: T) -> ExpandResult { + ExpandResult(t, None) + } + + pub fn only_err(err: ExpandError) -> ExpandResult + where + T: Default, + { + ExpandResult(Default::default(), Some(err)) + } + + pub fn map(self, f: impl FnOnce(T) -> U) -> ExpandResult { + ExpandResult(f(self.0), self.1) + } + + pub fn result(self) -> Result { + self.1.map(Err).unwrap_or(Ok(self.0)) + } +} + +impl From> for ExpandResult { + fn from(result: Result) -> ExpandResult { + result + .map_or_else(|e| ExpandResult(Default::default(), Some(e)), |it| ExpandResult(it, None)) + } +} + #[cfg(test)] mod tests; diff --git a/crates/ra_mbe/src/mbe_expander.rs b/crates/ra_mbe/src/mbe_expander.rs index 204c30e3d..0a4d73dda 100644 --- a/crates/ra_mbe/src/mbe_expander.rs +++ b/crates/ra_mbe/src/mbe_expander.rs @@ -18,12 +18,13 @@ fn expand_rules(rules: &[crate::Rule], input: &tt::Subtree) -> ExpandResult = None; let mut err = Some(ExpandError::NoMatchingRule); for rule in rules { - let (new_match, bindings_err) = matcher::match_(&rule.lhs, input); + let ExpandResult(new_match, bindings_err) = matcher::match_(&rule.lhs, input); if bindings_err.is_none() { // if we find a rule that applies without errors, we're done - let (res, transcribe_err) = transcriber::transcribe(&rule.rhs, &new_match.bindings); + let ExpandResult(res, transcribe_err) = + transcriber::transcribe(&rule.rhs, &new_match.bindings); if transcribe_err.is_none() { - return (res, None); + return ExpandResult::ok(res); } } // use the rule if we matched more tokens, or had fewer patterns left @@ -43,10 +44,11 @@ fn expand_rules(rules: &[crate::Rule], input: &tt::Subtree) -> ExpandResult ExpandResult { res.bindings.inner.insert(name.clone(), Binding::Fragment(fragment)); @@ -308,17 +308,17 @@ impl<'a> TtIter<'a> { token_trees: res.into_iter().cloned().collect(), })), }; - (res, err) + ExpandResult(res, err) } pub(crate) fn eat_vis(&mut self) -> Option { let mut fork = self.clone(); match fork.expect_fragment(Visibility) { - (tt, None) => { + ExpandResult(tt, None) => { *self = fork; tt } - (_, Some(_)) => None, + ExpandResult(_, Some(_)) => None, } } } @@ -419,12 +419,11 @@ fn match_meta_var(kind: &str, input: &mut TtIter) -> ExpandResult Err(ExpandError::UnexpectedToken), }; - return to_expand_result(tt_result.map(|it| it.map(Fragment::Tokens))); + return tt_result.map(|it| it.map(Fragment::Tokens)).into(); } }; - let (tt, err) = input.expect_fragment(fragment); - let fragment = if kind == "expr" { tt.map(Fragment::Ast) } else { tt.map(Fragment::Tokens) }; - (fragment, err) + let result = input.expect_fragment(fragment); + result.map(|tt| if kind == "expr" { tt.map(Fragment::Ast) } else { tt.map(Fragment::Tokens) }) } fn collect_vars(buf: &mut Vec, pattern: &tt::Subtree) -> Result<(), ExpandError> { @@ -438,7 +437,3 @@ fn collect_vars(buf: &mut Vec, pattern: &tt::Subtree) -> Result<(), Exp } Ok(()) } - -fn to_expand_result(result: Result) -> ExpandResult { - result.map_or_else(|e| (Default::default(), Some(e)), |it| (it, None)) -} diff --git a/crates/ra_mbe/src/mbe_expander/transcriber.rs b/crates/ra_mbe/src/mbe_expander/transcriber.rs index c53c2d35e..4b173edd3 100644 --- a/crates/ra_mbe/src/mbe_expander/transcriber.rs +++ b/crates/ra_mbe/src/mbe_expander/transcriber.rs @@ -87,23 +87,23 @@ fn expand_subtree(ctx: &mut ExpandCtx, template: &tt::Subtree) -> ExpandResult buf.push(tt.clone()), Op::TokenTree(tt::TokenTree::Subtree(tt)) => { - let (tt, e) = expand_subtree(ctx, tt); + let ExpandResult(tt, e) = expand_subtree(ctx, tt); err = err.or(e); buf.push(tt.into()); } Op::Var { name, kind: _ } => { - let (fragment, e) = expand_var(ctx, name); + let ExpandResult(fragment, e) = expand_var(ctx, name); err = err.or(e); push_fragment(&mut buf, fragment); } Op::Repeat { subtree, kind, separator } => { - let (fragment, e) = expand_repeat(ctx, subtree, kind, separator); + let ExpandResult(fragment, e) = expand_repeat(ctx, subtree, kind, separator); err = err.or(e); push_fragment(&mut buf, fragment) } } } - (tt::Subtree { delimiter: template.delimiter, token_trees: buf }, err) + ExpandResult(tt::Subtree { delimiter: template.delimiter, token_trees: buf }, err) } fn expand_var(ctx: &mut ExpandCtx, v: &SmolStr) -> ExpandResult { @@ -112,7 +112,7 @@ fn expand_var(ctx: &mut ExpandCtx, v: &SmolStr) -> ExpandResult { let tt = tt::Leaf::from(tt::Ident { text: "$crate".into(), id: tt::TokenId::unspecified() }) .into(); - (Fragment::Tokens(tt), None) + ExpandResult::ok(Fragment::Tokens(tt)) } else if !ctx.bindings.contains(v) { // Note that it is possible to have a `$var` inside a macro which is not bound. // For example: @@ -141,11 +141,11 @@ fn expand_var(ctx: &mut ExpandCtx, v: &SmolStr) -> ExpandResult { ], } .into(); - (Fragment::Tokens(tt), None) + ExpandResult::ok(Fragment::Tokens(tt)) } else { ctx.bindings.get(&v, &mut ctx.nesting).map_or_else( - |e| (Fragment::Tokens(tt::TokenTree::empty()), Some(e)), - |b| (b.clone(), None), + |e| ExpandResult(Fragment::Tokens(tt::TokenTree::empty()), Some(e)), + |b| ExpandResult::ok(b.clone()), ) } } @@ -165,7 +165,7 @@ fn expand_repeat( let mut counter = 0; loop { - let (mut t, e) = expand_subtree(ctx, template); + let ExpandResult(mut t, e) = expand_subtree(ctx, template); let nesting_state = ctx.nesting.last_mut().unwrap(); if nesting_state.at_end || !nesting_state.hit { break; @@ -225,9 +225,9 @@ fn expand_repeat( let tt = tt::Subtree { delimiter: None, token_trees: buf }.into(); if RepeatKind::OneOrMore == kind && counter == 0 { - return (Fragment::Tokens(tt), Some(ExpandError::UnexpectedToken)); + return ExpandResult(Fragment::Tokens(tt), Some(ExpandError::UnexpectedToken)); } - (Fragment::Tokens(tt), None) + ExpandResult::ok(Fragment::Tokens(tt)) } fn push_fragment(buf: &mut Vec, fragment: Fragment) { diff --git a/crates/ra_mbe/src/tests.rs b/crates/ra_mbe/src/tests.rs index faf88c1b1..44f381938 100644 --- a/crates/ra_mbe/src/tests.rs +++ b/crates/ra_mbe/src/tests.rs @@ -1430,8 +1430,7 @@ impl MacroFixture { let (invocation_tt, _) = ast_to_token_tree(¯o_invocation.token_tree().unwrap()).unwrap(); - let (tt, err) = self.rules.expand(&invocation_tt); - err.map(Err).unwrap_or(Ok(tt)) + self.rules.expand(&invocation_tt).result() } fn assert_expand_err(&self, invocation: &str, err: &ExpandError) { -- cgit v1.2.3 From d6b622cdef6af45aff46650c247940a590afde92 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Mon, 16 Mar 2020 18:04:07 +0100 Subject: Some cleanup --- crates/ra_mbe/src/mbe_expander.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/crates/ra_mbe/src/mbe_expander.rs b/crates/ra_mbe/src/mbe_expander.rs index 0a4d73dda..3c00e3b64 100644 --- a/crates/ra_mbe/src/mbe_expander.rs +++ b/crates/ra_mbe/src/mbe_expander.rs @@ -20,18 +20,20 @@ fn expand_rules(rules: &[crate::Rule], input: &tt::Subtree) -> ExpandResult Date: Mon, 16 Mar 2020 18:38:10 +0100 Subject: Some more refactoring --- crates/ra_mbe/src/mbe_expander.rs | 22 +++--- crates/ra_mbe/src/mbe_expander/matcher.rs | 112 +++++++++++++++++------------- 2 files changed, 76 insertions(+), 58 deletions(-) diff --git a/crates/ra_mbe/src/mbe_expander.rs b/crates/ra_mbe/src/mbe_expander.rs index 3c00e3b64..7adb70d45 100644 --- a/crates/ra_mbe/src/mbe_expander.rs +++ b/crates/ra_mbe/src/mbe_expander.rs @@ -16,10 +16,15 @@ pub(crate) fn expand(rules: &crate::MacroRules, input: &tt::Subtree) -> ExpandRe fn expand_rules(rules: &[crate::Rule], input: &tt::Subtree) -> ExpandResult { let mut match_: Option<(matcher::Match, &crate::Rule)> = None; - let mut err = Some(ExpandError::NoMatchingRule); for rule in rules { - let ExpandResult(new_match, bindings_err) = matcher::match_(&rule.lhs, input); - if bindings_err.is_none() { + let new_match = match matcher::match_(&rule.lhs, input) { + Ok(m) => m, + Err(_e) => { + // error in pattern parsing + continue; + } + }; + if new_match.err.is_none() { // If we find a rule that applies without errors, we're done. // Unconditionally returning the transcription here makes the // `test_repeat_bad_var` test fail. @@ -32,25 +37,22 @@ fn expand_rules(rules: &[crate::Rule], input: &tt::Subtree) -> ExpandResult, + pub err_count: usize, + /// How many top-level token trees were left to match. + pub unmatched_tts: usize, } -pub(super) fn match_(pattern: &tt::Subtree, src: &tt::Subtree) -> ExpandResult { +impl Match { + pub fn add_err(&mut self, err: ExpandError) { + let prev_err = self.err.take(); + self.err = prev_err.or(Some(err)); + self.err_count += 1; + } +} + +// General note: These functions have two channels to return errors, a `Result` +// return value and the `&mut Match`. The returned Result is for pattern parsing +// errors; if a branch of the macro definition doesn't parse, it doesn't make +// sense to try using it. Matching errors are added to the `Match`. It might +// make sense to make pattern parsing a separate step? + +pub(super) fn match_(pattern: &tt::Subtree, src: &tt::Subtree) -> Result { assert!(pattern.delimiter == None); let mut res = Match::default(); let mut src = TtIter::new(src); - let mut err = match_subtree(&mut res, pattern, &mut src).err(); + match_subtree(&mut res, pattern, &mut src)?; - res.unmatched_tokens += src.len(); - if src.len() > 0 && err.is_none() { - err = Some(err!("leftover tokens")); + if src.len() > 0 { + res.unmatched_tts += src.len(); + res.add_err(err!("leftover tokens")); } - ExpandResult(res, err) + Ok(res) } fn match_subtree( @@ -87,19 +104,13 @@ fn match_subtree( pattern: &tt::Subtree, src: &mut TtIter, ) -> Result<(), ExpandError> { - let mut result = Ok(()); for op in parse_pattern(pattern) { - if result.is_err() { - // We're just going through the patterns to count how many we missed - res.unmatched_patterns += 1; - continue; - } match op? { Op::TokenTree(tt::TokenTree::Leaf(lhs)) => { let rhs = match src.expect_leaf() { Ok(l) => l, Err(()) => { - result = Err(err!("expected leaf: `{}`", lhs)); + res.add_err(err!("expected leaf: `{}`", lhs)); continue; } }; @@ -117,7 +128,7 @@ fn match_subtree( tt::Leaf::Literal(tt::Literal { text: rhs, .. }), ) if lhs == rhs => (), _ => { - result = Err(ExpandError::UnexpectedToken); + res.add_err(ExpandError::UnexpectedToken); } } } @@ -125,26 +136,25 @@ fn match_subtree( let rhs = match src.expect_subtree() { Ok(s) => s, Err(()) => { - result = Err(err!("expected subtree")); + res.add_err(err!("expected subtree")); continue; } }; if lhs.delimiter_kind() != rhs.delimiter_kind() { - result = Err(err!("mismatched delimiter")); + res.add_err(err!("mismatched delimiter")); continue; } let mut src = TtIter::new(rhs); - result = match_subtree(res, lhs, &mut src); - res.unmatched_tokens += src.len(); - if src.len() > 0 && result.is_ok() { - result = Err(err!("leftover tokens")); + match_subtree(res, lhs, &mut src)?; + if src.len() > 0 { + res.add_err(err!("leftover tokens")); } } Op::Var { name, kind } => { let kind = match kind { Some(k) => k, None => { - result = Err(ExpandError::UnexpectedToken); + res.add_err(ExpandError::UnexpectedToken); continue; } }; @@ -156,14 +166,16 @@ fn match_subtree( None if match_err.is_none() => res.bindings.push_optional(name), _ => {} } - result = match_err.map_or(Ok(()), Err); + if let Some(err) = match_err { + res.add_err(err); + } } Op::Repeat { subtree, kind, separator } => { - result = match_repeat(res, subtree, kind, separator, src); + match_repeat(res, subtree, kind, separator, src)?; } } } - result + Ok(()) } impl<'a> TtIter<'a> { @@ -345,35 +357,39 @@ pub(super) fn match_repeat( } let mut nested = Match::default(); - match match_subtree(&mut nested, pattern, &mut fork) { - Ok(()) => { - limit -= 1; - if limit == 0 { - log::warn!( - "match_lhs exceeded repeat pattern limit => {:#?}\n{:#?}\n{:#?}\n{:#?}", - pattern, - src, - kind, - separator - ); - break; - } - *src = fork; + match_subtree(&mut nested, pattern, &mut fork)?; + if nested.err.is_none() { + limit -= 1; + if limit == 0 { + log::warn!( + "match_lhs exceeded repeat pattern limit => {:#?}\n{:#?}\n{:#?}\n{:#?}", + pattern, + src, + kind, + separator + ); + break; + } + *src = fork; - res.bindings.push_nested(counter, nested.bindings)?; - counter += 1; - if counter == 1 { - if let RepeatKind::ZeroOrOne = kind { - break; - } + if let Err(err) = res.bindings.push_nested(counter, nested.bindings) { + res.add_err(err); + } + counter += 1; + if counter == 1 { + if let RepeatKind::ZeroOrOne = kind { + break; } } - Err(_) => break, + } else { + break; } } match (kind, counter) { - (RepeatKind::OneOrMore, 0) => return Err(ExpandError::UnexpectedToken), + (RepeatKind::OneOrMore, 0) => { + res.add_err(ExpandError::UnexpectedToken); + } (_, 0) => { // Collect all empty variables in subtrees let mut vars = Vec::new(); -- cgit v1.2.3 From 6c20d7e979b967eb20207414c0a0bf875bbcb98d Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Mon, 16 Mar 2020 18:46:08 +0100 Subject: Small fixes --- crates/ra_mbe/src/mbe_expander.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/ra_mbe/src/mbe_expander.rs b/crates/ra_mbe/src/mbe_expander.rs index 7adb70d45..b1eacf124 100644 --- a/crates/ra_mbe/src/mbe_expander.rs +++ b/crates/ra_mbe/src/mbe_expander.rs @@ -34,8 +34,7 @@ fn expand_rules(rules: &[crate::Rule], input: &tt::Subtree) -> ExpandResult