From 212e82fd4183f17670a343a8fabf220ab4b6f7a2 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Mon, 7 Feb 2022 12:54:08 +0100 Subject: [PATCH 01/15] Add test for giving attribute proc macros valid syntax --- crates/base_db/src/fixture.rs | 115 +++++++++++++----- crates/hir_def/src/macro_expansion_tests.rs | 72 ++++++++++- .../src/macro_expansion_tests/proc_macros.rs | 37 ++++++ 3 files changed, 186 insertions(+), 38 deletions(-) diff --git a/crates/base_db/src/fixture.rs b/crates/base_db/src/fixture.rs index 5326a5a440..af825a2e00 100644 --- a/crates/base_db/src/fixture.rs +++ b/crates/base_db/src/fixture.rs @@ -43,6 +43,17 @@ pub trait WithFixture: Default + SourceDatabaseExt + 'static { db } + fn with_files_extra_proc_macros( + ra_fixture: &str, + proc_macros: Vec<(String, ProcMacro)>, + ) -> Self { + let fixture = ChangeFixture::parse_with_proc_macros(ra_fixture, proc_macros); + let mut db = Self::default(); + fixture.change.apply(&mut db); + assert!(fixture.file_position.is_none()); + db + } + fn with_position(ra_fixture: &str) -> (Self, FilePosition) { let (db, file_id, range_or_offset) = Self::with_range_or_offset(ra_fixture); let offset = range_or_offset.expect_offset(); @@ -84,7 +95,14 @@ pub struct ChangeFixture { impl ChangeFixture { pub fn parse(ra_fixture: &str) -> ChangeFixture { - let (mini_core, proc_macros, fixture) = Fixture::parse(ra_fixture); + Self::parse_with_proc_macros(ra_fixture, Vec::new()) + } + + pub fn parse_with_proc_macros( + ra_fixture: &str, + mut proc_macros: Vec<(String, ProcMacro)>, + ) -> ChangeFixture { + let (mini_core, proc_macro_names, fixture) = Fixture::parse(ra_fixture); let mut change = Change::new(); let mut files = Vec::new(); @@ -222,11 +240,12 @@ impl ChangeFixture { } } - if !proc_macros.is_empty() { + if !proc_macro_names.is_empty() { let proc_lib_file = file_id; file_id.0 += 1; - let (proc_macro, source) = test_proc_macros(&proc_macros); + proc_macros.extend(default_test_proc_macros()); + let (proc_macro, source) = filter_test_proc_macros(&proc_macro_names, proc_macros); let mut fs = FileSet::default(); fs.insert( proc_lib_file, @@ -272,52 +291,84 @@ impl ChangeFixture { } } -fn test_proc_macros(proc_macros: &[String]) -> (Vec, String) { - // The source here is only required so that paths to the macros exist and are resolvable. - let source = r#" +fn default_test_proc_macros() -> [(String, ProcMacro); 4] { + [ + ( + r#" #[proc_macro_attribute] pub fn identity(_attr: TokenStream, item: TokenStream) -> TokenStream { item } +"# + .into(), + ProcMacro { + name: "identity".into(), + kind: crate::ProcMacroKind::Attr, + expander: Arc::new(IdentityProcMacroExpander), + }, + ), + ( + r#" #[proc_macro_derive(DeriveIdentity)] pub fn derive_identity(item: TokenStream) -> TokenStream { item } +"# + .into(), + ProcMacro { + name: "DeriveIdentity".into(), + kind: crate::ProcMacroKind::CustomDerive, + expander: Arc::new(IdentityProcMacroExpander), + }, + ), + ( + r#" #[proc_macro_attribute] pub fn input_replace(attr: TokenStream, _item: TokenStream) -> TokenStream { attr } +"# + .into(), + ProcMacro { + name: "input_replace".into(), + kind: crate::ProcMacroKind::Attr, + expander: Arc::new(AttributeInputReplaceProcMacroExpander), + }, + ), + ( + r#" #[proc_macro] pub fn mirror(input: TokenStream) -> TokenStream { input } -"#; - let proc_macros = [ - ProcMacro { - name: "identity".into(), - kind: crate::ProcMacroKind::Attr, - expander: Arc::new(IdentityProcMacroExpander), - }, - ProcMacro { - name: "DeriveIdentity".into(), - kind: crate::ProcMacroKind::CustomDerive, - expander: Arc::new(IdentityProcMacroExpander), - }, - ProcMacro { - name: "input_replace".into(), - kind: crate::ProcMacroKind::Attr, - expander: Arc::new(AttributeInputReplaceProcMacroExpander), - }, - ProcMacro { - name: "mirror".into(), - kind: crate::ProcMacroKind::FuncLike, - expander: Arc::new(MirrorProcMacroExpander), - }, +"# + .into(), + ProcMacro { + name: "mirror".into(), + kind: crate::ProcMacroKind::FuncLike, + expander: Arc::new(MirrorProcMacroExpander), + }, + ), ] - .into_iter() - .filter(|pm| proc_macros.iter().any(|name| name == &stdx::to_lower_snake_case(&pm.name))) - .collect(); - (proc_macros, source.into()) +} + +fn filter_test_proc_macros( + proc_macro_names: &[String], + proc_macro_defs: Vec<(String, ProcMacro)>, +) -> (Vec, String) { + // The source here is only required so that paths to the macros exist and are resolvable. + let mut source = String::new(); + let mut proc_macros = Vec::new(); + + for (c, p) in proc_macro_defs { + if !proc_macro_names.iter().any(|name| name == &stdx::to_lower_snake_case(&p.name)) { + continue; + } + proc_macros.push(p); + source += &c; + } + + (proc_macros, source) } #[derive(Debug, Clone, Copy)] diff --git a/crates/hir_def/src/macro_expansion_tests.rs b/crates/hir_def/src/macro_expansion_tests.rs index c3116edc88..80747f6c63 100644 --- a/crates/hir_def/src/macro_expansion_tests.rs +++ b/crates/hir_def/src/macro_expansion_tests.rs @@ -14,10 +14,10 @@ mod builtin_fn_macro; mod builtin_derive_macro; mod proc_macros; -use std::{iter, ops::Range}; +use std::{iter, ops::Range, sync::Arc}; use ::mbe::TokenMap; -use base_db::{fixture::WithFixture, SourceDatabase}; +use base_db::{fixture::WithFixture, ProcMacro, SourceDatabase}; use expect_test::Expect; use hir_expand::{ db::{AstDatabase, TokenExpander}, @@ -39,7 +39,21 @@ use crate::{ #[track_caller] fn check(ra_fixture: &str, mut expect: Expect) { - let db = TestDB::with_files(ra_fixture); + let extra_proc_macros = vec![( + r#" +#[proc_macro_attribute] +pub fn identity_when_valid(_attr: TokenStream, item: TokenStream) -> TokenStream { + item +} +"# + .into(), + ProcMacro { + name: "identity_when_valid".into(), + kind: base_db::ProcMacroKind::Attr, + expander: Arc::new(IdentityWhenValidProcMacroExpander), + }, + )]; + let db = TestDB::with_files_extra_proc_macros(ra_fixture, extra_proc_macros); let krate = db.crate_graph().iter().next().unwrap(); let def_map = db.crate_def_map(krate); let local_id = def_map.root(); @@ -201,10 +215,19 @@ fn check(ra_fixture: &str, mut expect: Expect) { } for decl_id in def_map[local_id].scope.declarations() { - if let ModuleDefId::AdtId(AdtId::StructId(struct_id)) = decl_id { - let src = struct_id.lookup(&db).source(&db); + // FIXME: I'm sure there's already better way to do this + let src = match decl_id { + ModuleDefId::AdtId(AdtId::StructId(struct_id)) => { + Some(struct_id.lookup(&db).source(&db).syntax().cloned()) + } + ModuleDefId::FunctionId(function_id) => { + Some(function_id.lookup(&db).source(&db).syntax().cloned()) + } + _ => None, + }; + if let Some(src) = src { if src.file_id.is_attr_macro(&db) || src.file_id.is_custom_derive(&db) { - let pp = pretty_print_macro_expansion(src.value.syntax().clone(), None); + let pp = pretty_print_macro_expansion(src.value, None); format_to!(expanded_text, "\n{}", pp) } } @@ -304,3 +327,40 @@ fn pretty_print_macro_expansion(expn: SyntaxNode, map: Option<&TokenMap>) -> Str } res } + +// Identity mapping, but only works when the input is syntactically valid. This +// simulates common proc macros that unnecessarily parse their input and return +// compile errors. +#[derive(Debug)] +struct IdentityWhenValidProcMacroExpander; +impl base_db::ProcMacroExpander for IdentityWhenValidProcMacroExpander { + fn expand( + &self, + subtree: &Subtree, + _: Option<&Subtree>, + _: &base_db::Env, + ) -> Result { + let (parse, _) = + ::mbe::token_tree_to_syntax_node(subtree, ::mbe::TopEntryPoint::MacroItems); + if parse.errors().is_empty() { + Ok(subtree.clone()) + } else { + use tt::{Delimiter, DelimiterKind, Ident, Leaf, Literal, Punct, TokenTree}; + let mut subtree = Subtree::default(); + subtree.token_trees.push(TokenTree::Leaf( + Ident { text: "compile_error!".into(), id: TokenId(0) }.into(), + )); + subtree.token_trees.push(TokenTree::Subtree(Subtree { + delimiter: Some(Delimiter { id: TokenId(2), kind: DelimiterKind::Parenthesis }), + token_trees: vec![TokenTree::Leaf(Leaf::Literal(Literal { + text: r#""parse error""#.into(), + id: TokenId::unspecified(), + }))], + })); + subtree.token_trees.push(TokenTree::Leaf( + Punct { char: ';', spacing: tt::Spacing::Alone, id: TokenId::unspecified() }.into(), + )); + Ok(subtree) + } + } +} diff --git a/crates/hir_def/src/macro_expansion_tests/proc_macros.rs b/crates/hir_def/src/macro_expansion_tests/proc_macros.rs index 901872edda..dead99a40c 100644 --- a/crates/hir_def/src/macro_expansion_tests/proc_macros.rs +++ b/crates/hir_def/src/macro_expansion_tests/proc_macros.rs @@ -52,3 +52,40 @@ struct S; #[attr2] struct S;"##]], ); } + +#[test] +fn attribute_macro_syntax_completion_1() { + // this is just the case where the input is actually valid + check( + r#" +//- proc_macros: identity_when_valid +#[proc_macros::identity_when_valid] +fn foo() { bar.baz(); blub } +"#, + expect![[r##" +#[proc_macros::identity_when_valid] +fn foo() { bar.baz(); blub } + +fn foo() { + bar.baz(); + blub +}"##]], + ); +} + +#[test] +fn attribute_macro_syntax_completion_2() { + // common case of dot completion while typing + // right now not working + check( + r#" +//- proc_macros: identity_when_valid +#[proc_macros::identity_when_valid] +fn foo() { bar.; blub } +"#, + expect![[r##" +#[proc_macros::identity_when_valid] +fn foo() { bar.; blub } +"##]], + ); +} From cff209f1520e8ac0270a71814f993bda74668203 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Mon, 7 Feb 2022 18:08:31 +0100 Subject: [PATCH 02/15] WIP: Actually fix up syntax errors in attribute macro input --- crates/hir_def/src/macro_expansion_tests.rs | 1 + .../src/macro_expansion_tests/proc_macros.rs | 6 +- crates/hir_expand/src/db.rs | 21 +++- crates/hir_expand/src/lib.rs | 1 + crates/mbe/src/lib.rs | 2 +- crates/mbe/src/syntax_bridge.rs | 115 ++++++++++++++---- 6 files changed, 112 insertions(+), 34 deletions(-) diff --git a/crates/hir_def/src/macro_expansion_tests.rs b/crates/hir_def/src/macro_expansion_tests.rs index 80747f6c63..16df7ce4cf 100644 --- a/crates/hir_def/src/macro_expansion_tests.rs +++ b/crates/hir_def/src/macro_expansion_tests.rs @@ -345,6 +345,7 @@ impl base_db::ProcMacroExpander for IdentityWhenValidProcMacroExpander { if parse.errors().is_empty() { Ok(subtree.clone()) } else { + eprintln!("parse errors: {:?}", parse.errors()); use tt::{Delimiter, DelimiterKind, Ident, Leaf, Literal, Punct, TokenTree}; let mut subtree = Subtree::default(); subtree.token_trees.push(TokenTree::Leaf( diff --git a/crates/hir_def/src/macro_expansion_tests/proc_macros.rs b/crates/hir_def/src/macro_expansion_tests/proc_macros.rs index dead99a40c..e0c5367cf3 100644 --- a/crates/hir_def/src/macro_expansion_tests/proc_macros.rs +++ b/crates/hir_def/src/macro_expansion_tests/proc_macros.rs @@ -86,6 +86,10 @@ fn foo() { bar.; blub } expect![[r##" #[proc_macros::identity_when_valid] fn foo() { bar.; blub } -"##]], + +fn foo() { + bar.; + blub +}"##]], ); } diff --git a/crates/hir_expand/src/db.rs b/crates/hir_expand/src/db.rs index d9bfdd6fd4..6576701817 100644 --- a/crates/hir_expand/src/db.rs +++ b/crates/hir_expand/src/db.rs @@ -5,8 +5,8 @@ use std::sync::Arc; use base_db::{salsa, SourceDatabase}; use either::Either; use limit::Limit; -use mbe::{syntax_node_to_token_tree, ExpandError, ExpandResult}; -use rustc_hash::FxHashSet; +use mbe::{syntax_node_to_token_tree, ExpandError, ExpandResult, SyntheticToken}; +use rustc_hash::{FxHashMap, FxHashSet}; use syntax::{ algo::diff, ast::{self, HasAttrs, HasDocComments}, @@ -14,7 +14,7 @@ use syntax::{ }; use crate::{ - ast_id_map::AstIdMap, hygiene::HygieneFrame, BuiltinAttrExpander, BuiltinDeriveExpander, + ast_id_map::AstIdMap, fixup, hygiene::HygieneFrame, BuiltinAttrExpander, BuiltinDeriveExpander, BuiltinFnLikeExpander, ExpandTo, HirFileId, HirFileIdRepr, MacroCallId, MacroCallKind, MacroCallLoc, MacroDefId, MacroDefKind, MacroFile, ProcMacroExpander, }; @@ -146,8 +146,10 @@ pub fn expand_speculative( // Build the subtree and token mapping for the speculative args let censor = censor_for_macro_input(&loc, &speculative_args); + let mut fixups = fixup::fixup_syntax(&speculative_args); + fixups.replace.extend(censor.into_iter().map(|node| (node, Vec::new()))); let (mut tt, spec_args_tmap) = - mbe::syntax_node_to_token_tree_censored(&speculative_args, &censor); + mbe::syntax_node_to_token_tree_censored(&speculative_args, fixups.replace, fixups.append); let (attr_arg, token_id) = match loc.kind { MacroCallKind::Attr { invoc_attr_index, .. } => { @@ -294,8 +296,17 @@ fn macro_arg(db: &dyn AstDatabase, id: MacroCallId) -> Option (tt::Subtree, TokenMap) { - syntax_node_to_token_tree_censored(node, &Default::default()) + syntax_node_to_token_tree_censored(node, Default::default(), Default::default()) } +// TODO rename /// Convert the syntax node to a `TokenTree` (what macro will consume) /// with the censored range excluded. pub fn syntax_node_to_token_tree_censored( node: &SyntaxNode, - censor: &FxHashSet, + replace: FxHashMap>, + append: FxHashMap>, ) -> (tt::Subtree, TokenMap) { let global_offset = node.text_range().start(); - let mut c = Convertor::new(node, global_offset, censor); + let mut c = Convertor::new(node, global_offset, replace, append); let subtree = convert_tokens(&mut c); c.id_alloc.map.shrink_to_fit(); (subtree, c.id_alloc.map) } +pub type SyntheticToken = (SyntaxKind, SmolStr); + // The following items are what `rustc` macro can be parsed into : // link: https://github.com/rust-lang/rust/blob/9ebf47851a357faa4cd97f4b1dc7835f6376e639/src/libsyntax/ext/expand.rs#L141 // * Expr(P) -> token_tree_to_expr @@ -465,86 +469,124 @@ impl<'a> TokenConvertor for RawConvertor<'a> { } } -struct Convertor<'c> { +struct Convertor { id_alloc: TokenIdAlloc, current: Option, + current_synthetic: Vec, preorder: PreorderWithTokens, - censor: &'c FxHashSet, + replace: FxHashMap>, + append: FxHashMap>, range: TextRange, punct_offset: Option<(SyntaxToken, TextSize)>, } -impl<'c> Convertor<'c> { +impl Convertor { fn new( node: &SyntaxNode, global_offset: TextSize, - censor: &'c FxHashSet, - ) -> Convertor<'c> { + replace: FxHashMap>, + append: FxHashMap>, + ) -> Convertor { let range = node.text_range(); let mut preorder = node.preorder_with_tokens(); - let first = Self::next_token(&mut preorder, censor); + let (first, synthetic) = Self::next_token(&mut preorder, &replace, &append); Convertor { id_alloc: { TokenIdAlloc { map: TokenMap::default(), global_offset, next_id: 0 } }, current: first, + current_synthetic: synthetic, preorder, range, - censor, + replace, + append, punct_offset: None, } } fn next_token( preorder: &mut PreorderWithTokens, - censor: &FxHashSet, - ) -> Option { + replace: &FxHashMap>, + append: &FxHashMap>, + ) -> (Option, Vec) { while let Some(ev) = preorder.next() { let ele = match ev { WalkEvent::Enter(ele) => ele, + WalkEvent::Leave(SyntaxElement::Node(node)) => { + if let Some(v) = append.get(&node) { + eprintln!("after {:?}, appending {:?}", node, v); + if !v.is_empty() { + let mut reversed = v.clone(); + reversed.reverse(); + return (None, reversed); + } + } + continue; + } _ => continue, }; match ele { - SyntaxElement::Token(t) => return Some(t), - SyntaxElement::Node(node) if censor.contains(&node) => preorder.skip_subtree(), - SyntaxElement::Node(_) => (), + SyntaxElement::Token(t) => return (Some(t), Vec::new()), + SyntaxElement::Node(node) => { + if let Some(v) = replace.get(&node) { + preorder.skip_subtree(); + eprintln!("replacing {:?} by {:?}", node, v); + if !v.is_empty() { + let mut reversed = v.clone(); + reversed.reverse(); + return (None, reversed); + } + } + } } } - None + (None, Vec::new()) } } #[derive(Debug)] enum SynToken { Ordinary(SyntaxToken), + // FIXME is this supposed to be `Punct`? Punch(SyntaxToken, TextSize), + Synthetic(SyntheticToken), } impl SynToken { - fn token(&self) -> &SyntaxToken { + fn token(&self) -> Option<&SyntaxToken> { match self { - SynToken::Ordinary(it) | SynToken::Punch(it, _) => it, + SynToken::Ordinary(it) | SynToken::Punch(it, _) => Some(it), + SynToken::Synthetic(_) => None, } } } -impl<'a> SrcToken> for SynToken { - fn kind(&self, _ctx: &Convertor<'a>) -> SyntaxKind { - self.token().kind() +impl SrcToken for SynToken { + fn kind(&self, _ctx: &Convertor) -> SyntaxKind { + match self { + SynToken::Ordinary(token) => token.kind(), + SynToken::Punch(token, _) => token.kind(), + SynToken::Synthetic((kind, _)) => *kind, + } } - fn to_char(&self, _ctx: &Convertor<'a>) -> Option { + fn to_char(&self, _ctx: &Convertor) -> Option { match self { SynToken::Ordinary(_) => None, SynToken::Punch(it, i) => it.text().chars().nth((*i).into()), + SynToken::Synthetic(_) => None, } } - fn to_text(&self, _ctx: &Convertor<'a>) -> SmolStr { - self.token().text().into() + fn to_text(&self, _ctx: &Convertor) -> SmolStr { + match self { + SynToken::Ordinary(token) => token.text().into(), + SynToken::Punch(token, _) => token.text().into(), + SynToken::Synthetic((_, text)) => text.clone(), + } } } -impl TokenConvertor for Convertor<'_> { +impl TokenConvertor for Convertor { type Token = SynToken; fn convert_doc_comment(&self, token: &Self::Token) -> Option> { - convert_doc_comment(token.token()) + convert_doc_comment(token.token()?) } fn bump(&mut self) -> Option<(Self::Token, TextRange)> { @@ -558,11 +600,25 @@ impl TokenConvertor for Convertor<'_> { } } + if let Some(synth_token) = self.current_synthetic.pop() { + if self.current_synthetic.is_empty() { + let (new_current, new_synth) = + Self::next_token(&mut self.preorder, &self.replace, &self.append); + self.current = new_current; + self.current_synthetic = new_synth; + } + // TODO fix range? + return Some((SynToken::Synthetic(synth_token), self.range)); + } + let curr = self.current.clone()?; if !&self.range.contains_range(curr.text_range()) { return None; } - self.current = Self::next_token(&mut self.preorder, self.censor); + let (new_current, new_synth) = + Self::next_token(&mut self.preorder, &self.replace, &self.append); + self.current = new_current; + self.current_synthetic = new_synth; let token = if curr.kind().is_punct() { self.punct_offset = Some((curr.clone(), 0.into())); let range = curr.text_range(); @@ -585,6 +641,11 @@ impl TokenConvertor for Convertor<'_> { } } + if let Some(synth_token) = self.current_synthetic.last() { + // TODO fix range? + return Some(SynToken::Synthetic(synth_token.clone())); + } + let curr = self.current.clone()?; if !self.range.contains_range(curr.text_range()) { return None; From b9c5d23f69ab45f6bcd16c8f83317ed2c0a4b1a8 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Mon, 7 Feb 2022 18:17:28 +0100 Subject: [PATCH 03/15] Simplify a bit --- crates/mbe/src/syntax_bridge.rs | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/crates/mbe/src/syntax_bridge.rs b/crates/mbe/src/syntax_bridge.rs index b46e959466..d5639cadc4 100644 --- a/crates/mbe/src/syntax_bridge.rs +++ b/crates/mbe/src/syntax_bridge.rs @@ -484,12 +484,12 @@ impl Convertor { fn new( node: &SyntaxNode, global_offset: TextSize, - replace: FxHashMap>, - append: FxHashMap>, + mut replace: FxHashMap>, + mut append: FxHashMap>, ) -> Convertor { let range = node.text_range(); let mut preorder = node.preorder_with_tokens(); - let (first, synthetic) = Self::next_token(&mut preorder, &replace, &append); + let (first, synthetic) = Self::next_token(&mut preorder, &mut replace, &mut append); Convertor { id_alloc: { TokenIdAlloc { map: TokenMap::default(), global_offset, next_id: 0 } }, current: first, @@ -504,19 +504,18 @@ impl Convertor { fn next_token( preorder: &mut PreorderWithTokens, - replace: &FxHashMap>, - append: &FxHashMap>, + replace: &mut FxHashMap>, + append: &mut FxHashMap>, ) -> (Option, Vec) { while let Some(ev) = preorder.next() { let ele = match ev { WalkEvent::Enter(ele) => ele, WalkEvent::Leave(SyntaxElement::Node(node)) => { - if let Some(v) = append.get(&node) { + if let Some(mut v) = append.remove(&node) { eprintln!("after {:?}, appending {:?}", node, v); if !v.is_empty() { - let mut reversed = v.clone(); - reversed.reverse(); - return (None, reversed); + v.reverse(); + return (None, v); } } continue; @@ -526,13 +525,12 @@ impl Convertor { match ele { SyntaxElement::Token(t) => return (Some(t), Vec::new()), SyntaxElement::Node(node) => { - if let Some(v) = replace.get(&node) { + if let Some(mut v) = replace.remove(&node) { preorder.skip_subtree(); eprintln!("replacing {:?} by {:?}", node, v); if !v.is_empty() { - let mut reversed = v.clone(); - reversed.reverse(); - return (None, reversed); + v.reverse(); + return (None, v); } } } @@ -603,7 +601,7 @@ impl TokenConvertor for Convertor { if let Some(synth_token) = self.current_synthetic.pop() { if self.current_synthetic.is_empty() { let (new_current, new_synth) = - Self::next_token(&mut self.preorder, &self.replace, &self.append); + Self::next_token(&mut self.preorder, &mut self.replace, &mut self.append); self.current = new_current; self.current_synthetic = new_synth; } @@ -616,7 +614,7 @@ impl TokenConvertor for Convertor { return None; } let (new_current, new_synth) = - Self::next_token(&mut self.preorder, &self.replace, &self.append); + Self::next_token(&mut self.preorder, &mut self.replace, &mut self.append); self.current = new_current; self.current_synthetic = new_synth; let token = if curr.kind().is_punct() { From 86b968ba94c30986ef7731b44af49907307c29a3 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Mon, 7 Feb 2022 18:19:00 +0100 Subject: [PATCH 04/15] Add a check --- crates/mbe/src/syntax_bridge.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/mbe/src/syntax_bridge.rs b/crates/mbe/src/syntax_bridge.rs index d5639cadc4..d3ba28f3bc 100644 --- a/crates/mbe/src/syntax_bridge.rs +++ b/crates/mbe/src/syntax_bridge.rs @@ -1,7 +1,7 @@ //! Conversions between [`SyntaxNode`] and [`tt::TokenTree`]. use rustc_hash::{FxHashMap, FxHashSet}; -use stdx::non_empty_vec::NonEmptyVec; +use stdx::{non_empty_vec::NonEmptyVec, always}; use syntax::{ ast::{self, make::tokens::doc_comment}, AstToken, Parse, PreorderWithTokens, SmolStr, SyntaxElement, SyntaxKind, @@ -30,6 +30,8 @@ pub fn syntax_node_to_token_tree_censored( let mut c = Convertor::new(node, global_offset, replace, append); let subtree = convert_tokens(&mut c); c.id_alloc.map.shrink_to_fit(); + always!(c.replace.is_empty()); + always!(c.append.is_empty()); (subtree, c.id_alloc.map) } From 79ebf618ecb097ff4e8b6cc4842ca3f648fe371b Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Mon, 7 Feb 2022 18:21:31 +0100 Subject: [PATCH 05/15] Simplify --- crates/hir_def/src/macro_expansion_tests.rs | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/crates/hir_def/src/macro_expansion_tests.rs b/crates/hir_def/src/macro_expansion_tests.rs index 16df7ce4cf..28a5d96085 100644 --- a/crates/hir_def/src/macro_expansion_tests.rs +++ b/crates/hir_def/src/macro_expansion_tests.rs @@ -345,23 +345,7 @@ impl base_db::ProcMacroExpander for IdentityWhenValidProcMacroExpander { if parse.errors().is_empty() { Ok(subtree.clone()) } else { - eprintln!("parse errors: {:?}", parse.errors()); - use tt::{Delimiter, DelimiterKind, Ident, Leaf, Literal, Punct, TokenTree}; - let mut subtree = Subtree::default(); - subtree.token_trees.push(TokenTree::Leaf( - Ident { text: "compile_error!".into(), id: TokenId(0) }.into(), - )); - subtree.token_trees.push(TokenTree::Subtree(Subtree { - delimiter: Some(Delimiter { id: TokenId(2), kind: DelimiterKind::Parenthesis }), - token_trees: vec![TokenTree::Leaf(Leaf::Literal(Literal { - text: r#""parse error""#.into(), - id: TokenId::unspecified(), - }))], - })); - subtree.token_trees.push(TokenTree::Leaf( - Punct { char: ';', spacing: tt::Spacing::Alone, id: TokenId::unspecified() }.into(), - )); - Ok(subtree) + panic!("got invalid macro input: {:?}", parse.errors()); } } } From c3601e9860e533c7990d90dbd773a49039bb037e Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Mon, 7 Feb 2022 19:53:31 +0100 Subject: [PATCH 06/15] Reverse fixups --- crates/hir_def/src/macro_expansion_tests/proc_macros.rs | 3 +-- crates/hir_expand/src/db.rs | 4 +++- crates/mbe/src/syntax_bridge.rs | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/hir_def/src/macro_expansion_tests/proc_macros.rs b/crates/hir_def/src/macro_expansion_tests/proc_macros.rs index e0c5367cf3..0ca30fb799 100644 --- a/crates/hir_def/src/macro_expansion_tests/proc_macros.rs +++ b/crates/hir_def/src/macro_expansion_tests/proc_macros.rs @@ -76,7 +76,6 @@ fn foo() { #[test] fn attribute_macro_syntax_completion_2() { // common case of dot completion while typing - // right now not working check( r#" //- proc_macros: identity_when_valid @@ -88,7 +87,7 @@ fn foo() { bar.; blub } fn foo() { bar.; blub } fn foo() { - bar.; + bar. ; blub }"##]], ); diff --git a/crates/hir_expand/src/db.rs b/crates/hir_expand/src/db.rs index 6576701817..935fb30fa9 100644 --- a/crates/hir_expand/src/db.rs +++ b/crates/hir_expand/src/db.rs @@ -430,7 +430,7 @@ fn macro_expand(db: &dyn AstDatabase, id: MacroCallId) -> ExpandResult return ExpandResult::str_err(format!("invalid macro definition: {}", err)), }; - let ExpandResult { value: tt, err } = expander.expand(db, id, ¯o_arg.0); + let ExpandResult { value: mut tt, err } = expander.expand(db, id, ¯o_arg.0); // Set a hard limit for the expanded tt let count = tt.count(); // XXX: Make ExpandResult a real error and use .map_err instead? @@ -442,6 +442,8 @@ fn macro_expand(db: &dyn AstDatabase, id: MacroCallId) -> ExpandResult Date: Mon, 7 Feb 2022 20:30:28 +0100 Subject: [PATCH 07/15] Actually check in fixup.rs --- Cargo.lock | 1 + crates/hir_expand/Cargo.toml | 3 + crates/hir_expand/src/fixup.rs | 136 +++++++++++++++++++++++++++++++++ 3 files changed, 140 insertions(+) create mode 100644 crates/hir_expand/src/fixup.rs diff --git a/Cargo.lock b/Cargo.lock index 0ab9f89fd2..11ae5c6721 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -526,6 +526,7 @@ dependencies = [ "cfg", "cov-mark", "either", + "expect-test", "hashbrown 0.12.0", "itertools", "la-arena", diff --git a/crates/hir_expand/Cargo.toml b/crates/hir_expand/Cargo.toml index e5455660d3..d7b4cbf82e 100644 --- a/crates/hir_expand/Cargo.toml +++ b/crates/hir_expand/Cargo.toml @@ -27,3 +27,6 @@ profile = { path = "../profile", version = "0.0.0" } tt = { path = "../tt", version = "0.0.0" } mbe = { path = "../mbe", version = "0.0.0" } limit = { path = "../limit", version = "0.0.0" } + +[dev-dependencies] +expect-test = "1.2.0-pre.1" diff --git a/crates/hir_expand/src/fixup.rs b/crates/hir_expand/src/fixup.rs new file mode 100644 index 0000000000..f82fba46e9 --- /dev/null +++ b/crates/hir_expand/src/fixup.rs @@ -0,0 +1,136 @@ +use mbe::SyntheticToken; +use rustc_hash::FxHashMap; +use syntax::{ + ast::{self, AstNode}, + match_ast, SyntaxKind, SyntaxNode, SyntaxToken, +}; +use tt::{Leaf, Subtree}; + +#[derive(Debug)] +pub struct SyntaxFixups { + pub append: FxHashMap>, + pub replace: FxHashMap>, +} + +pub fn fixup_syntax(node: &SyntaxNode) -> SyntaxFixups { + let mut append = FxHashMap::default(); + let mut replace = FxHashMap::default(); + let mut preorder = node.preorder(); + while let Some(event) = preorder.next() { + let node = match event { + syntax::WalkEvent::Enter(node) => node, + syntax::WalkEvent::Leave(_) => continue, + }; + if node.kind() == SyntaxKind::ERROR { + // TODO this might not be helpful + replace.insert(node, Vec::new()); + preorder.skip_subtree(); + continue; + } + match_ast! { + match node { + ast::FieldExpr(it) => { + if it.name_ref().is_none() { + // incomplete field access: some_expr.| + append.insert(node.clone(), vec![(SyntaxKind::IDENT, "__ra_fixup".into())]); + } + }, + _ => (), + } + } + } + SyntaxFixups { append, replace } +} + +pub fn reverse_fixups(tt: &mut Subtree) { + tt.token_trees.retain(|tt| match tt { + tt::TokenTree::Leaf(Leaf::Ident(ident)) => ident.text != "__ra_fixup", + _ => true, + }); + tt.token_trees.iter_mut().for_each(|tt| match tt { + tt::TokenTree::Subtree(tt) => reverse_fixups(tt), + _ => {} + }); +} + +#[cfg(test)] +mod tests { + use expect_test::{Expect, expect}; + + use super::reverse_fixups; + + #[track_caller] + fn check(ra_fixture: &str, mut expect: Expect) { + let parsed = syntax::SourceFile::parse(ra_fixture); + let fixups = super::fixup_syntax(&parsed.syntax_node()); + let (mut tt, _tmap) = mbe::syntax_node_to_token_tree_censored( + &parsed.syntax_node(), + fixups.replace, + fixups.append, + ); + + let mut actual = tt.to_string(); + actual.push_str("\n"); + + expect.indent(false); + expect.assert_eq(&actual); + + // the fixed-up tree should be syntactically valid + let (parse, _) = mbe::token_tree_to_syntax_node(&tt, ::mbe::TopEntryPoint::MacroItems); + assert_eq!(parse.errors(), &[], "parse has syntax errors. parse tree:\n{:#?}", parse.syntax_node()); + + reverse_fixups(&mut tt); + + // the fixed-up + reversed version should be equivalent to the original input + // (but token IDs don't matter) + let (original_as_tt, _) = mbe::syntax_node_to_token_tree(&parsed.syntax_node()); + assert_eq!(tt.to_string(), original_as_tt.to_string()); + } + + #[test] + fn incomplete_field_expr_1() { + check(r#" +fn foo() { + a. +} +"#, expect![[r#" +fn foo () {a . __ra_fixup} +"#]]) + } + + #[test] + fn incomplete_field_expr_2() { + check(r#" +fn foo() { + a. ; +} +"#, expect![[r#" +fn foo () {a . __ra_fixup ;} +"#]]) + } + + #[test] + fn incomplete_field_expr_3() { + check(r#" +fn foo() { + a. ; + bar(); +} +"#, expect![[r#" +fn foo () {a . __ra_fixup ; bar () ;} +"#]]) + } + + #[test] + fn field_expr_before_call() { + // another case that easily happens while typing + check(r#" +fn foo() { + a.b + bar(); +} +"#, expect![[r#" +fn foo () {a . b bar () ;} +"#]]) + } +} From 1a5aa84e9f48f652e584cdbe0393616730ea73d8 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Tue, 8 Feb 2022 18:13:18 +0100 Subject: [PATCH 08/15] Track synthetic tokens, to be able to remove them again later --- crates/hir_expand/src/db.rs | 6 +-- crates/hir_expand/src/fixup.rs | 87 ++++++++++++++++++++++++--------- crates/mbe/src/lib.rs | 1 + crates/mbe/src/syntax_bridge.rs | 57 ++++++++++++++++----- crates/mbe/src/token_map.rs | 11 +++++ crates/tt/src/lib.rs | 10 ++++ 6 files changed, 133 insertions(+), 39 deletions(-) diff --git a/crates/hir_expand/src/db.rs b/crates/hir_expand/src/db.rs index 935fb30fa9..7a21e3e870 100644 --- a/crates/hir_expand/src/db.rs +++ b/crates/hir_expand/src/db.rs @@ -5,8 +5,8 @@ use std::sync::Arc; use base_db::{salsa, SourceDatabase}; use either::Either; use limit::Limit; -use mbe::{syntax_node_to_token_tree, ExpandError, ExpandResult, SyntheticToken}; -use rustc_hash::{FxHashMap, FxHashSet}; +use mbe::{syntax_node_to_token_tree, ExpandError, ExpandResult}; +use rustc_hash::FxHashSet; use syntax::{ algo::diff, ast::{self, HasAttrs, HasDocComments}, @@ -442,7 +442,7 @@ fn macro_expand(db: &dyn AstDatabase, id: MacroCallId) -> ExpandResult SyntaxFixups { let mut append = FxHashMap::default(); let mut replace = FxHashMap::default(); let mut preorder = node.preorder(); + let empty_id = SyntheticTokenId(0); while let Some(event) = preorder.next() { let node = match event { syntax::WalkEvent::Enter(node) => node, @@ -27,12 +28,32 @@ pub fn fixup_syntax(node: &SyntaxNode) -> SyntaxFixups { preorder.skip_subtree(); continue; } + let end_range = TextRange::empty(node.text_range().end()); match_ast! { match node { ast::FieldExpr(it) => { if it.name_ref().is_none() { // incomplete field access: some_expr.| - append.insert(node.clone(), vec![(SyntaxKind::IDENT, "__ra_fixup".into())]); + append.insert(node.clone(), vec![ + SyntheticToken { + kind: SyntaxKind::IDENT, + text: "__ra_fixup".into(), + range: end_range, + id: empty_id, + }, + ]); + } + }, + ast::ExprStmt(it) => { + if it.semicolon_token().is_none() { + append.insert(node.clone(), vec![ + SyntheticToken { + kind: SyntaxKind::SEMICOLON, + text: ";".into(), + range: end_range, + id: empty_id, + }, + ]); } }, _ => (), @@ -42,20 +63,21 @@ pub fn fixup_syntax(node: &SyntaxNode) -> SyntaxFixups { SyntaxFixups { append, replace } } -pub fn reverse_fixups(tt: &mut Subtree) { +pub fn reverse_fixups(tt: &mut Subtree, token_map: &TokenMap) { + eprintln!("token_map: {:?}", token_map); tt.token_trees.retain(|tt| match tt { - tt::TokenTree::Leaf(Leaf::Ident(ident)) => ident.text != "__ra_fixup", + tt::TokenTree::Leaf(leaf) => token_map.synthetic_token_id(leaf.id()).is_none(), _ => true, }); tt.token_trees.iter_mut().for_each(|tt| match tt { - tt::TokenTree::Subtree(tt) => reverse_fixups(tt), + tt::TokenTree::Subtree(tt) => reverse_fixups(tt, token_map), _ => {} }); } #[cfg(test)] mod tests { - use expect_test::{Expect, expect}; + use expect_test::{expect, Expect}; use super::reverse_fixups; @@ -63,7 +85,7 @@ mod tests { fn check(ra_fixture: &str, mut expect: Expect) { let parsed = syntax::SourceFile::parse(ra_fixture); let fixups = super::fixup_syntax(&parsed.syntax_node()); - let (mut tt, _tmap) = mbe::syntax_node_to_token_tree_censored( + let (mut tt, tmap) = mbe::syntax_node_to_token_tree_censored( &parsed.syntax_node(), fixups.replace, fixups.append, @@ -77,9 +99,14 @@ mod tests { // the fixed-up tree should be syntactically valid let (parse, _) = mbe::token_tree_to_syntax_node(&tt, ::mbe::TopEntryPoint::MacroItems); - assert_eq!(parse.errors(), &[], "parse has syntax errors. parse tree:\n{:#?}", parse.syntax_node()); + assert_eq!( + parse.errors(), + &[], + "parse has syntax errors. parse tree:\n{:#?}", + parse.syntax_node() + ); - reverse_fixups(&mut tt); + reverse_fixups(&mut tt, &tmap); // the fixed-up + reversed version should be equivalent to the original input // (but token IDs don't matter) @@ -89,48 +116,60 @@ mod tests { #[test] fn incomplete_field_expr_1() { - check(r#" + check( + r#" fn foo() { a. } -"#, expect![[r#" +"#, + expect![[r#" fn foo () {a . __ra_fixup} -"#]]) +"#]], + ) } #[test] fn incomplete_field_expr_2() { - check(r#" + check( + r#" fn foo() { a. ; } -"#, expect![[r#" +"#, + expect![[r#" fn foo () {a . __ra_fixup ;} -"#]]) +"#]], + ) } #[test] fn incomplete_field_expr_3() { - check(r#" + check( + r#" fn foo() { a. ; bar(); } -"#, expect![[r#" +"#, + expect![[r#" fn foo () {a . __ra_fixup ; bar () ;} -"#]]) +"#]], + ) } #[test] fn field_expr_before_call() { // another case that easily happens while typing - check(r#" + check( + r#" fn foo() { a.b bar(); } -"#, expect![[r#" -fn foo () {a . b bar () ;} -"#]]) +"#, + expect![[r#" +fn foo () {a . b ; bar () ;} +"#]], + ) } } diff --git a/crates/mbe/src/lib.rs b/crates/mbe/src/lib.rs index 3633624c64..a35c22c2e1 100644 --- a/crates/mbe/src/lib.rs +++ b/crates/mbe/src/lib.rs @@ -31,6 +31,7 @@ pub use crate::{ syntax_bridge::{ parse_exprs_with_sep, parse_to_token_tree, syntax_node_to_token_tree, syntax_node_to_token_tree_censored, token_tree_to_syntax_node, SyntheticToken, + SyntheticTokenId, }, token_map::TokenMap, }; diff --git a/crates/mbe/src/syntax_bridge.rs b/crates/mbe/src/syntax_bridge.rs index d3489813e1..7feaaaa62d 100644 --- a/crates/mbe/src/syntax_bridge.rs +++ b/crates/mbe/src/syntax_bridge.rs @@ -1,6 +1,6 @@ //! Conversions between [`SyntaxNode`] and [`tt::TokenTree`]. -use rustc_hash::{FxHashMap, FxHashSet}; +use rustc_hash::FxHashMap; use stdx::{always, non_empty_vec::NonEmptyVec}; use syntax::{ ast::{self, make::tokens::doc_comment}, @@ -35,7 +35,16 @@ pub fn syntax_node_to_token_tree_censored( (subtree, c.id_alloc.map) } -pub type SyntheticToken = (SyntaxKind, SmolStr); +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +pub struct SyntheticTokenId(pub u32); + +#[derive(Debug, Clone)] +pub struct SyntheticToken { + pub kind: SyntaxKind, + pub text: SmolStr, + pub range: TextRange, + pub id: SyntheticTokenId, +} // The following items are what `rustc` macro can be parsed into : // link: https://github.com/rust-lang/rust/blob/9ebf47851a357faa4cd97f4b1dc7835f6376e639/src/libsyntax/ext/expand.rs#L141 @@ -153,13 +162,14 @@ fn convert_tokens(conv: &mut C) -> tt::Subtree { Some(it) => it, None => break, }; + let synth_id = token.synthetic_id(&conv); let kind = token.kind(&conv); if kind == COMMENT { if let Some(tokens) = conv.convert_doc_comment(&token) { // FIXME: There has to be a better way to do this // Add the comments token id to the converted doc string - let id = conv.id_alloc().alloc(range); + let id = conv.id_alloc().alloc(range, synth_id); result.extend(tokens.into_iter().map(|mut tt| { if let tt::TokenTree::Subtree(sub) = &mut tt { if let Some(tt::TokenTree::Leaf(tt::Leaf::Literal(lit))) = @@ -174,7 +184,7 @@ fn convert_tokens(conv: &mut C) -> tt::Subtree { continue; } let tt = if kind.is_punct() && kind != UNDERSCORE { - assert_eq!(range.len(), TextSize::of('.')); + // assert_eq!(range.len(), TextSize::of('.')); if let Some(delim) = subtree.delimiter { let expected = match delim.kind { @@ -226,11 +236,13 @@ fn convert_tokens(conv: &mut C) -> tt::Subtree { panic!("Token from lexer must be single char: token = {:#?}", token); } }; - tt::Leaf::from(tt::Punct { char, spacing, id: conv.id_alloc().alloc(range) }).into() + tt::Leaf::from(tt::Punct { char, spacing, id: conv.id_alloc().alloc(range, synth_id) }) + .into() } else { macro_rules! make_leaf { ($i:ident) => { - tt::$i { id: conv.id_alloc().alloc(range), text: token.to_text(conv) }.into() + tt::$i { id: conv.id_alloc().alloc(range, synth_id), text: token.to_text(conv) } + .into() }; } let leaf: tt::Leaf = match kind { @@ -245,14 +257,14 @@ fn convert_tokens(conv: &mut C) -> tt::Subtree { let apostrophe = tt::Leaf::from(tt::Punct { char: '\'', spacing: tt::Spacing::Joint, - id: conv.id_alloc().alloc(r), + id: conv.id_alloc().alloc(r, synth_id), }); result.push(apostrophe.into()); let r = TextRange::at(range.start() + char_unit, range.len() - char_unit); let ident = tt::Leaf::from(tt::Ident { text: SmolStr::new(&token.to_text(conv)[1..]), - id: conv.id_alloc().alloc(r), + id: conv.id_alloc().alloc(r, synth_id), }); result.push(ident.into()); continue; @@ -273,7 +285,7 @@ fn convert_tokens(conv: &mut C) -> tt::Subtree { conv.id_alloc().close_delim(entry.idx, None); let leaf: tt::Leaf = tt::Punct { - id: conv.id_alloc().alloc(entry.open_range), + id: conv.id_alloc().alloc(entry.open_range, None), char: match entry.subtree.delimiter.unwrap().kind { tt::DelimiterKind::Parenthesis => '(', tt::DelimiterKind::Brace => '{', @@ -367,11 +379,18 @@ struct TokenIdAlloc { } impl TokenIdAlloc { - fn alloc(&mut self, absolute_range: TextRange) -> tt::TokenId { + fn alloc( + &mut self, + absolute_range: TextRange, + synthetic_id: Option, + ) -> tt::TokenId { let relative_range = absolute_range - self.global_offset; let token_id = tt::TokenId(self.next_id); self.next_id += 1; self.map.insert(token_id, relative_range); + if let Some(id) = synthetic_id { + self.map.insert_synthetic(token_id, id); + } token_id } @@ -411,6 +430,8 @@ trait SrcToken: std::fmt::Debug { fn to_char(&self, ctx: &Ctx) -> Option; fn to_text(&self, ctx: &Ctx) -> SmolStr; + + fn synthetic_id(&self, ctx: &Ctx) -> Option; } trait TokenConvertor: Sized { @@ -437,6 +458,10 @@ impl<'a> SrcToken> for usize { fn to_text(&self, ctx: &RawConvertor<'_>) -> SmolStr { ctx.lexed.text(*self).into() } + + fn synthetic_id(&self, _ctx: &RawConvertor<'a>) -> Option { + None + } } impl<'a> TokenConvertor for RawConvertor<'a> { @@ -564,13 +589,14 @@ impl SrcToken for SynToken { match self { SynToken::Ordinary(token) => token.kind(), SynToken::Punch(token, _) => token.kind(), - SynToken::Synthetic((kind, _)) => *kind, + SynToken::Synthetic(token) => token.kind, } } fn to_char(&self, _ctx: &Convertor) -> Option { match self { SynToken::Ordinary(_) => None, SynToken::Punch(it, i) => it.text().chars().nth((*i).into()), + SynToken::Synthetic(token) if token.text.len() == 1 => token.text.chars().next(), SynToken::Synthetic(_) => None, } } @@ -578,7 +604,14 @@ impl SrcToken for SynToken { match self { SynToken::Ordinary(token) => token.text().into(), SynToken::Punch(token, _) => token.text().into(), - SynToken::Synthetic((_, text)) => text.clone(), + SynToken::Synthetic(token) => token.text.clone(), + } + } + + fn synthetic_id(&self, _ctx: &Convertor) -> Option { + match self { + SynToken::Synthetic(token) => Some(token.id), + _ => None, } } } diff --git a/crates/mbe/src/token_map.rs b/crates/mbe/src/token_map.rs index 9053526d20..ee1090945c 100644 --- a/crates/mbe/src/token_map.rs +++ b/crates/mbe/src/token_map.rs @@ -5,6 +5,8 @@ use std::hash::Hash; use parser::{SyntaxKind, T}; use syntax::{TextRange, TextSize}; +use crate::syntax_bridge::SyntheticTokenId; + #[derive(Debug, PartialEq, Eq, Clone, Copy, Hash)] enum TokenTextRange { Token(TextRange), @@ -31,6 +33,7 @@ impl TokenTextRange { pub struct TokenMap { /// Maps `tt::TokenId` to the *relative* source range. entries: Vec<(tt::TokenId, TokenTextRange)>, + pub synthetic_entries: Vec<(tt::TokenId, SyntheticTokenId)>, } impl TokenMap { @@ -57,6 +60,10 @@ impl TokenMap { .filter_map(move |(_, range)| range.by_kind(kind)) } + pub fn synthetic_token_id(&self, token_id: tt::TokenId) -> Option { + self.synthetic_entries.iter().find(|(tid, _)| *tid == token_id).map(|(_, id)| *id) + } + pub fn first_range_by_token( &self, token_id: tt::TokenId, @@ -73,6 +80,10 @@ impl TokenMap { self.entries.push((token_id, TokenTextRange::Token(relative_range))); } + pub(crate) fn insert_synthetic(&mut self, token_id: tt::TokenId, id: SyntheticTokenId) { + self.synthetic_entries.push((token_id, id)); + } + pub(crate) fn insert_delim( &mut self, token_id: tt::TokenId, diff --git a/crates/tt/src/lib.rs b/crates/tt/src/lib.rs index 9eca970ee2..0316b15038 100644 --- a/crates/tt/src/lib.rs +++ b/crates/tt/src/lib.rs @@ -87,6 +87,16 @@ pub struct Ident { pub id: TokenId, } +impl Leaf { + pub fn id(&self) -> TokenId { + match self { + Leaf::Literal(l) => l.id, + Leaf::Punct(p) => p.id, + Leaf::Ident(i) => i.id, + } + } +} + fn print_debug_subtree(f: &mut fmt::Formatter<'_>, subtree: &Subtree, level: usize) -> fmt::Result { let align = " ".repeat(level); From 30287e6051f82085d62d38f26f42a53fa48b434c Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Tue, 8 Feb 2022 20:44:46 +0100 Subject: [PATCH 09/15] Fix test --- crates/ide_completion/src/tests/attribute.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/ide_completion/src/tests/attribute.rs b/crates/ide_completion/src/tests/attribute.rs index 5a9c48a327..2e643453af 100644 --- a/crates/ide_completion/src/tests/attribute.rs +++ b/crates/ide_completion/src/tests/attribute.rs @@ -62,8 +62,7 @@ fn proc_macros_qualified() { struct Foo; "#, expect![[r#" - at input_replace pub macro input_replace - at identity pub macro identity + at identity pub macro identity "#]], ) } From ecf3cff4a6c7c06d1fe30e636d69227ab6310ebb Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Wed, 9 Feb 2022 11:58:52 +0100 Subject: [PATCH 10/15] Replace expressions with errors in them --- crates/hir_def/src/macro_expansion_tests.rs | 2 +- crates/hir_expand/src/db.rs | 20 +++--- crates/hir_expand/src/fixup.rs | 78 +++++++++++++++++---- crates/hir_expand/src/hygiene.rs | 3 +- crates/hir_expand/src/lib.rs | 2 +- crates/mbe/src/syntax_bridge.rs | 11 ++- crates/mbe/src/token_map.rs | 1 + 7 files changed, 84 insertions(+), 33 deletions(-) diff --git a/crates/hir_def/src/macro_expansion_tests.rs b/crates/hir_def/src/macro_expansion_tests.rs index 28a5d96085..1a7d9aa841 100644 --- a/crates/hir_def/src/macro_expansion_tests.rs +++ b/crates/hir_def/src/macro_expansion_tests.rs @@ -186,7 +186,7 @@ pub fn identity_when_valid(_attr: TokenStream, item: TokenStream) -> TokenStream let range: Range = range.into(); if show_token_ids { - if let Some((tree, map)) = arg.as_deref() { + if let Some((tree, map, _)) = arg.as_deref() { let tt_range = call.token_tree().unwrap().syntax().text_range(); let mut ranges = Vec::new(); extract_id_ranges(&mut ranges, &map, &tree); diff --git a/crates/hir_expand/src/db.rs b/crates/hir_expand/src/db.rs index 7a21e3e870..eea02898d2 100644 --- a/crates/hir_expand/src/db.rs +++ b/crates/hir_expand/src/db.rs @@ -108,7 +108,10 @@ pub trait AstDatabase: SourceDatabase { /// Lowers syntactic macro call to a token tree representation. #[salsa::transparent] - fn macro_arg(&self, id: MacroCallId) -> Option>; + fn macro_arg( + &self, + id: MacroCallId, + ) -> Option>; /// Extracts syntax node, corresponding to a macro call. That's a firewall /// query, only typing in the macro call itself changes the returned /// subtree. @@ -291,29 +294,27 @@ fn parse_macro_expansion( } } -fn macro_arg(db: &dyn AstDatabase, id: MacroCallId) -> Option> { +fn macro_arg( + db: &dyn AstDatabase, + id: MacroCallId, +) -> Option> { let arg = db.macro_arg_text(id)?; let loc = db.lookup_intern_macro_call(id); let node = SyntaxNode::new_root(arg); - eprintln!("input text:\n{node}"); - eprintln!("input syntax:\n{node:#?}"); let censor = censor_for_macro_input(&loc, &node); // TODO only fixup for attribute macro input let mut fixups = fixup::fixup_syntax(&node); fixups.replace.extend(censor.into_iter().map(|node| (node, Vec::new()))); - eprintln!("fixups: {fixups:?}"); let (mut tt, tmap) = mbe::syntax_node_to_token_tree_censored(&node, fixups.replace, fixups.append); - eprintln!("fixed-up input: {}", tt); - if loc.def.is_proc_macro() { // proc macros expect their inputs without parentheses, MBEs expect it with them included tt.delimiter = None; } - Some(Arc::new((tt, tmap))) + Some(Arc::new((tt, tmap, fixups.map))) } fn censor_for_macro_input(loc: &MacroCallLoc, node: &SyntaxNode) -> FxHashSet { @@ -433,7 +434,6 @@ fn macro_expand(db: &dyn AstDatabase, id: MacroCallId) -> ExpandResult ExpandResult>, pub replace: FxHashMap>, + pub map: SyntaxFixupMap, } +#[derive(Debug, PartialEq, Eq)] +pub struct SyntaxFixupMap { + original: Vec<(Subtree, TokenMap)>, +} + +const EMPTY_ID: SyntheticTokenId = SyntheticTokenId(!0); + pub fn fixup_syntax(node: &SyntaxNode) -> SyntaxFixups { let mut append = FxHashMap::default(); let mut replace = FxHashMap::default(); let mut preorder = node.preorder(); - let empty_id = SyntheticTokenId(0); + let mut original = Vec::new(); while let Some(event) = preorder.next() { let node = match event { syntax::WalkEvent::Enter(node) => node, syntax::WalkEvent::Leave(_) => continue, }; - if node.kind() == SyntaxKind::ERROR { - // TODO this might not be helpful - replace.insert(node, Vec::new()); + if can_handle_error(&node) && has_error_to_handle(&node) { + // the node contains an error node, we have to completely replace it by something valid + let original_tree = mbe::syntax_node_to_token_tree(&node); + // TODO handle token ids / token map + let idx = original.len() as u32; + original.push(original_tree); + let replacement = SyntheticToken { + kind: SyntaxKind::IDENT, + text: "__ra_fixup".into(), + range: node.text_range(), + id: SyntheticTokenId(idx), + }; + replace.insert(node.clone(), vec![replacement]); preorder.skip_subtree(); continue; } @@ -39,7 +57,7 @@ pub fn fixup_syntax(node: &SyntaxNode) -> SyntaxFixups { kind: SyntaxKind::IDENT, text: "__ra_fixup".into(), range: end_range, - id: empty_id, + id: EMPTY_ID, }, ]); } @@ -51,7 +69,7 @@ pub fn fixup_syntax(node: &SyntaxNode) -> SyntaxFixups { kind: SyntaxKind::SEMICOLON, text: ";".into(), range: end_range, - id: empty_id, + id: EMPTY_ID, }, ]); } @@ -60,18 +78,37 @@ pub fn fixup_syntax(node: &SyntaxNode) -> SyntaxFixups { } } } - SyntaxFixups { append, replace } + SyntaxFixups { append, replace, map: SyntaxFixupMap { original } } } -pub fn reverse_fixups(tt: &mut Subtree, token_map: &TokenMap) { - eprintln!("token_map: {:?}", token_map); +fn has_error(node: &SyntaxNode) -> bool { + node.children().any(|c| c.kind() == SyntaxKind::ERROR) +} + +fn can_handle_error(node: &SyntaxNode) -> bool { + ast::Expr::can_cast(node.kind()) +} + +fn has_error_to_handle(node: &SyntaxNode) -> bool { + has_error(node) || node.children().any(|c| !can_handle_error(&c) && has_error_to_handle(&c)) +} + +pub fn reverse_fixups(tt: &mut Subtree, token_map: &TokenMap, fixup_map: &SyntaxFixupMap) { tt.token_trees.retain(|tt| match tt { - tt::TokenTree::Leaf(leaf) => token_map.synthetic_token_id(leaf.id()).is_none(), + tt::TokenTree::Leaf(leaf) => { + token_map.synthetic_token_id(leaf.id()).is_none() + || token_map.synthetic_token_id(leaf.id()) != Some(EMPTY_ID) + } _ => true, }); tt.token_trees.iter_mut().for_each(|tt| match tt { - tt::TokenTree::Subtree(tt) => reverse_fixups(tt, token_map), - _ => {} + tt::TokenTree::Subtree(tt) => reverse_fixups(tt, token_map, fixup_map), + tt::TokenTree::Leaf(leaf) => { + if let Some(id) = token_map.synthetic_token_id(leaf.id()) { + let (original, _original_tmap) = &fixup_map.original[id.0 as usize]; + *tt = tt::TokenTree::Subtree(original.clone()); + } + } }); } @@ -84,6 +121,7 @@ mod tests { #[track_caller] fn check(ra_fixture: &str, mut expect: Expect) { let parsed = syntax::SourceFile::parse(ra_fixture); + eprintln!("parse: {:#?}", parsed.syntax_node()); let fixups = super::fixup_syntax(&parsed.syntax_node()); let (mut tt, tmap) = mbe::syntax_node_to_token_tree_censored( &parsed.syntax_node(), @@ -106,7 +144,7 @@ mod tests { parse.syntax_node() ); - reverse_fixups(&mut tt, &tmap); + reverse_fixups(&mut tt, &tmap, &fixups.map); // the fixed-up + reversed version should be equivalent to the original input // (but token IDs don't matter) @@ -169,6 +207,20 @@ fn foo() { "#, expect![[r#" fn foo () {a . b ; bar () ;} +"#]], + ) + } + + #[test] + fn extraneous_comma() { + check( + r#" +fn foo() { + bar(,); +} +"#, + expect![[r#" +fn foo () {__ra_fixup ;} "#]], ) } diff --git a/crates/hir_expand/src/hygiene.rs b/crates/hir_expand/src/hygiene.rs index d2b719bf57..0ac5ee8306 100644 --- a/crates/hir_expand/src/hygiene.rs +++ b/crates/hir_expand/src/hygiene.rs @@ -15,6 +15,7 @@ use syntax::{ use crate::{ db::{self, AstDatabase}, + fixup, name::{AsName, Name}, HirFileId, HirFileIdRepr, InFile, MacroCallKind, MacroCallLoc, MacroDefKind, MacroFile, }; @@ -127,7 +128,7 @@ struct HygieneInfo { attr_input_or_mac_def_start: Option>, macro_def: Arc, - macro_arg: Arc<(tt::Subtree, mbe::TokenMap)>, + macro_arg: Arc<(tt::Subtree, mbe::TokenMap, fixup::SyntaxFixupMap)>, macro_arg_shift: mbe::Shift, exp_map: Arc, } diff --git a/crates/hir_expand/src/lib.rs b/crates/hir_expand/src/lib.rs index b23d547242..6ec507d00f 100644 --- a/crates/hir_expand/src/lib.rs +++ b/crates/hir_expand/src/lib.rs @@ -427,7 +427,7 @@ pub struct ExpansionInfo { attr_input_or_mac_def: Option>, macro_def: Arc, - macro_arg: Arc<(tt::Subtree, mbe::TokenMap)>, + macro_arg: Arc<(tt::Subtree, mbe::TokenMap, fixup::SyntaxFixupMap)>, /// A shift built from `macro_arg`'s subtree, relevant for attributes as the item is the macro arg /// and as such we need to shift tokens if they are part of an attributes input instead of their item. macro_arg_shift: mbe::Shift, diff --git a/crates/mbe/src/syntax_bridge.rs b/crates/mbe/src/syntax_bridge.rs index 7feaaaa62d..da7fdb74ee 100644 --- a/crates/mbe/src/syntax_bridge.rs +++ b/crates/mbe/src/syntax_bridge.rs @@ -30,8 +30,8 @@ pub fn syntax_node_to_token_tree_censored( let mut c = Convertor::new(node, global_offset, replace, append); let subtree = convert_tokens(&mut c); c.id_alloc.map.shrink_to_fit(); - always!(c.replace.is_empty()); - always!(c.append.is_empty()); + always!(c.replace.is_empty(), "replace: {:?}", c.replace); + always!(c.append.is_empty(), "append: {:?}", c.append); (subtree, c.id_alloc.map) } @@ -539,7 +539,6 @@ impl Convertor { WalkEvent::Enter(ele) => ele, WalkEvent::Leave(SyntaxElement::Node(node)) => { if let Some(mut v) = append.remove(&node) { - eprintln!("after {:?}, appending {:?}", node, v); if !v.is_empty() { v.reverse(); return (None, v); @@ -554,7 +553,6 @@ impl Convertor { SyntaxElement::Node(node) => { if let Some(mut v) = replace.remove(&node) { preorder.skip_subtree(); - eprintln!("replacing {:?} by {:?}", node, v); if !v.is_empty() { v.reverse(); return (None, v); @@ -640,8 +638,8 @@ impl TokenConvertor for Convertor { self.current = new_current; self.current_synthetic = new_synth; } - // TODO fix range? - return Some((SynToken::Synthetic(synth_token), self.range)); + let range = synth_token.range; + return Some((SynToken::Synthetic(synth_token), range)); } let curr = self.current.clone()?; @@ -675,7 +673,6 @@ impl TokenConvertor for Convertor { } if let Some(synth_token) = self.current_synthetic.last() { - // TODO fix range? return Some(SynToken::Synthetic(synth_token.clone())); } diff --git a/crates/mbe/src/token_map.rs b/crates/mbe/src/token_map.rs index ee1090945c..c923e7a69a 100644 --- a/crates/mbe/src/token_map.rs +++ b/crates/mbe/src/token_map.rs @@ -74,6 +74,7 @@ impl TokenMap { pub(crate) fn shrink_to_fit(&mut self) { self.entries.shrink_to_fit(); + self.synthetic_entries.shrink_to_fit(); } pub(crate) fn insert(&mut self, token_id: tt::TokenId, relative_range: TextRange) { From bdb7ae5dd055e5ee4ab7b2e008f4299172a67709 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Wed, 9 Feb 2022 12:00:03 +0100 Subject: [PATCH 11/15] Rename syntax_node_to_token_tree_censored --- crates/hir_expand/src/db.rs | 10 ++++++---- crates/hir_expand/src/fixup.rs | 2 +- crates/mbe/src/lib.rs | 2 +- crates/mbe/src/syntax_bridge.rs | 5 ++--- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/crates/hir_expand/src/db.rs b/crates/hir_expand/src/db.rs index eea02898d2..65799e0be5 100644 --- a/crates/hir_expand/src/db.rs +++ b/crates/hir_expand/src/db.rs @@ -151,8 +151,11 @@ pub fn expand_speculative( let censor = censor_for_macro_input(&loc, &speculative_args); let mut fixups = fixup::fixup_syntax(&speculative_args); fixups.replace.extend(censor.into_iter().map(|node| (node, Vec::new()))); - let (mut tt, spec_args_tmap) = - mbe::syntax_node_to_token_tree_censored(&speculative_args, fixups.replace, fixups.append); + let (mut tt, spec_args_tmap) = mbe::syntax_node_to_token_tree_with_modifications( + &speculative_args, + fixups.replace, + fixups.append, + ); let (attr_arg, token_id) = match loc.kind { MacroCallKind::Attr { invoc_attr_index, .. } => { @@ -303,11 +306,10 @@ fn macro_arg( let node = SyntaxNode::new_root(arg); let censor = censor_for_macro_input(&loc, &node); - // TODO only fixup for attribute macro input let mut fixups = fixup::fixup_syntax(&node); fixups.replace.extend(censor.into_iter().map(|node| (node, Vec::new()))); let (mut tt, tmap) = - mbe::syntax_node_to_token_tree_censored(&node, fixups.replace, fixups.append); + mbe::syntax_node_to_token_tree_with_modifications(&node, fixups.replace, fixups.append); if loc.def.is_proc_macro() { // proc macros expect their inputs without parentheses, MBEs expect it with them included diff --git a/crates/hir_expand/src/fixup.rs b/crates/hir_expand/src/fixup.rs index c98d20e456..f2d43f4d69 100644 --- a/crates/hir_expand/src/fixup.rs +++ b/crates/hir_expand/src/fixup.rs @@ -123,7 +123,7 @@ mod tests { let parsed = syntax::SourceFile::parse(ra_fixture); eprintln!("parse: {:#?}", parsed.syntax_node()); let fixups = super::fixup_syntax(&parsed.syntax_node()); - let (mut tt, tmap) = mbe::syntax_node_to_token_tree_censored( + let (mut tt, tmap) = mbe::syntax_node_to_token_tree_with_modifications( &parsed.syntax_node(), fixups.replace, fixups.append, diff --git a/crates/mbe/src/lib.rs b/crates/mbe/src/lib.rs index a35c22c2e1..07b7f4d1a5 100644 --- a/crates/mbe/src/lib.rs +++ b/crates/mbe/src/lib.rs @@ -30,7 +30,7 @@ pub use tt::{Delimiter, DelimiterKind, Punct}; pub use crate::{ syntax_bridge::{ parse_exprs_with_sep, parse_to_token_tree, syntax_node_to_token_tree, - syntax_node_to_token_tree_censored, token_tree_to_syntax_node, SyntheticToken, + syntax_node_to_token_tree_with_modifications, token_tree_to_syntax_node, SyntheticToken, SyntheticTokenId, }, token_map::TokenMap, diff --git a/crates/mbe/src/syntax_bridge.rs b/crates/mbe/src/syntax_bridge.rs index da7fdb74ee..78a37993f7 100644 --- a/crates/mbe/src/syntax_bridge.rs +++ b/crates/mbe/src/syntax_bridge.rs @@ -15,13 +15,12 @@ use crate::{to_parser_input::to_parser_input, tt_iter::TtIter, TokenMap}; /// Convert the syntax node to a `TokenTree` (what macro /// will consume). pub fn syntax_node_to_token_tree(node: &SyntaxNode) -> (tt::Subtree, TokenMap) { - syntax_node_to_token_tree_censored(node, Default::default(), Default::default()) + syntax_node_to_token_tree_with_modifications(node, Default::default(), Default::default()) } -// TODO rename /// Convert the syntax node to a `TokenTree` (what macro will consume) /// with the censored range excluded. -pub fn syntax_node_to_token_tree_censored( +pub fn syntax_node_to_token_tree_with_modifications( node: &SyntaxNode, replace: FxHashMap>, append: FxHashMap>, From 63fd643d725512cf41fb818a76f25ce7f5465327 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Wed, 9 Feb 2022 16:30:10 +0100 Subject: [PATCH 12/15] Various fixes --- crates/hir_expand/src/db.rs | 22 ++++++++----- crates/hir_expand/src/fixup.rs | 53 +++++++++++++++++++++++++------- crates/hir_expand/src/hygiene.rs | 2 +- crates/hir_expand/src/lib.rs | 2 +- crates/mbe/src/syntax_bridge.rs | 21 ++++++++++--- 5 files changed, 75 insertions(+), 25 deletions(-) diff --git a/crates/hir_expand/src/db.rs b/crates/hir_expand/src/db.rs index 65799e0be5..75766a54a7 100644 --- a/crates/hir_expand/src/db.rs +++ b/crates/hir_expand/src/db.rs @@ -111,7 +111,7 @@ pub trait AstDatabase: SourceDatabase { fn macro_arg( &self, id: MacroCallId, - ) -> Option>; + ) -> Option>; /// Extracts syntax node, corresponding to a macro call. That's a firewall /// query, only typing in the macro call itself changes the returned /// subtree. @@ -151,8 +151,10 @@ pub fn expand_speculative( let censor = censor_for_macro_input(&loc, &speculative_args); let mut fixups = fixup::fixup_syntax(&speculative_args); fixups.replace.extend(censor.into_iter().map(|node| (node, Vec::new()))); - let (mut tt, spec_args_tmap) = mbe::syntax_node_to_token_tree_with_modifications( + let (mut tt, spec_args_tmap, _) = mbe::syntax_node_to_token_tree_with_modifications( &speculative_args, + fixups.token_map, + fixups.next_id, fixups.replace, fixups.append, ); @@ -202,7 +204,7 @@ pub fn expand_speculative( // Do the actual expansion, we need to directly expand the proc macro due to the attribute args // Otherwise the expand query will fetch the non speculative attribute args and pass those instead. - let speculative_expansion = if let MacroDefKind::ProcMacro(expander, ..) = loc.def.kind { + let mut speculative_expansion = if let MacroDefKind::ProcMacro(expander, ..) = loc.def.kind { tt.delimiter = None; expander.expand(db, loc.krate, &tt, attr_arg.as_ref()) } else { @@ -210,6 +212,7 @@ pub fn expand_speculative( }; let expand_to = macro_expand_to(db, actual_macro_call); + fixup::reverse_fixups(&mut speculative_expansion.value, &spec_args_tmap, &fixups.undo_info); let (node, rev_tmap) = token_tree_to_syntax_node(&speculative_expansion.value, expand_to); let range = rev_tmap.first_range_by_token(token_id, token_to_map.kind())?; @@ -300,7 +303,7 @@ fn parse_macro_expansion( fn macro_arg( db: &dyn AstDatabase, id: MacroCallId, -) -> Option> { +) -> Option> { let arg = db.macro_arg_text(id)?; let loc = db.lookup_intern_macro_call(id); @@ -308,15 +311,20 @@ fn macro_arg( let censor = censor_for_macro_input(&loc, &node); let mut fixups = fixup::fixup_syntax(&node); fixups.replace.extend(censor.into_iter().map(|node| (node, Vec::new()))); - let (mut tt, tmap) = - mbe::syntax_node_to_token_tree_with_modifications(&node, fixups.replace, fixups.append); + let (mut tt, tmap, _) = mbe::syntax_node_to_token_tree_with_modifications( + &node, + fixups.token_map, + fixups.next_id, + fixups.replace, + fixups.append, + ); if loc.def.is_proc_macro() { // proc macros expect their inputs without parentheses, MBEs expect it with them included tt.delimiter = None; } - Some(Arc::new((tt, tmap, fixups.map))) + Some(Arc::new((tt, tmap, fixups.undo_info))) } fn censor_for_macro_input(loc: &MacroCallLoc, node: &SyntaxNode) -> FxHashSet { diff --git a/crates/hir_expand/src/fixup.rs b/crates/hir_expand/src/fixup.rs index f2d43f4d69..36425c8078 100644 --- a/crates/hir_expand/src/fixup.rs +++ b/crates/hir_expand/src/fixup.rs @@ -1,3 +1,7 @@ +//! To make attribute macros work reliably when typing, we need to take care to +//! fix up syntax errors in the code we're passing to them. +use std::mem; + use mbe::{SyntheticToken, SyntheticTokenId, TokenMap}; use rustc_hash::FxHashMap; use syntax::{ @@ -6,16 +10,22 @@ use syntax::{ }; use tt::Subtree; +/// The result of calculating fixes for a syntax node -- a bunch of changes +/// (appending to and replacing nodes), the information that is needed to +/// reverse those changes afterwards, and a token map. #[derive(Debug)] pub struct SyntaxFixups { pub append: FxHashMap>, pub replace: FxHashMap>, - pub map: SyntaxFixupMap, + pub undo_info: SyntaxFixupUndoInfo, + pub token_map: TokenMap, + pub next_id: u32, } +/// This is the information needed to reverse the fixups. #[derive(Debug, PartialEq, Eq)] -pub struct SyntaxFixupMap { - original: Vec<(Subtree, TokenMap)>, +pub struct SyntaxFixupUndoInfo { + original: Vec, } const EMPTY_ID: SyntheticTokenId = SyntheticTokenId(!0); @@ -25,15 +35,26 @@ pub fn fixup_syntax(node: &SyntaxNode) -> SyntaxFixups { let mut replace = FxHashMap::default(); let mut preorder = node.preorder(); let mut original = Vec::new(); + let mut token_map = TokenMap::default(); + let mut next_id = 0; while let Some(event) = preorder.next() { let node = match event { syntax::WalkEvent::Enter(node) => node, syntax::WalkEvent::Leave(_) => continue, }; + if can_handle_error(&node) && has_error_to_handle(&node) { // the node contains an error node, we have to completely replace it by something valid - let original_tree = mbe::syntax_node_to_token_tree(&node); - // TODO handle token ids / token map + let (original_tree, new_tmap, new_next_id) = + mbe::syntax_node_to_token_tree_with_modifications( + &node, + mem::take(&mut token_map), + next_id, + Default::default(), + Default::default(), + ); + token_map = new_tmap; + next_id = new_next_id; let idx = original.len() as u32; original.push(original_tree); let replacement = SyntheticToken { @@ -46,6 +67,8 @@ pub fn fixup_syntax(node: &SyntaxNode) -> SyntaxFixups { preorder.skip_subtree(); continue; } + + // In some other situations, we can fix things by just appending some tokens. let end_range = TextRange::empty(node.text_range().end()); match_ast! { match node { @@ -78,7 +101,13 @@ pub fn fixup_syntax(node: &SyntaxNode) -> SyntaxFixups { } } } - SyntaxFixups { append, replace, map: SyntaxFixupMap { original } } + SyntaxFixups { + append, + replace, + token_map, + next_id, + undo_info: SyntaxFixupUndoInfo { original }, + } } fn has_error(node: &SyntaxNode) -> bool { @@ -93,7 +122,7 @@ fn has_error_to_handle(node: &SyntaxNode) -> bool { has_error(node) || node.children().any(|c| !can_handle_error(&c) && has_error_to_handle(&c)) } -pub fn reverse_fixups(tt: &mut Subtree, token_map: &TokenMap, fixup_map: &SyntaxFixupMap) { +pub fn reverse_fixups(tt: &mut Subtree, token_map: &TokenMap, undo_info: &SyntaxFixupUndoInfo) { tt.token_trees.retain(|tt| match tt { tt::TokenTree::Leaf(leaf) => { token_map.synthetic_token_id(leaf.id()).is_none() @@ -102,10 +131,10 @@ pub fn reverse_fixups(tt: &mut Subtree, token_map: &TokenMap, fixup_map: &Syntax _ => true, }); tt.token_trees.iter_mut().for_each(|tt| match tt { - tt::TokenTree::Subtree(tt) => reverse_fixups(tt, token_map, fixup_map), + tt::TokenTree::Subtree(tt) => reverse_fixups(tt, token_map, undo_info), tt::TokenTree::Leaf(leaf) => { if let Some(id) = token_map.synthetic_token_id(leaf.id()) { - let (original, _original_tmap) = &fixup_map.original[id.0 as usize]; + let original = &undo_info.original[id.0 as usize]; *tt = tt::TokenTree::Subtree(original.clone()); } } @@ -123,8 +152,10 @@ mod tests { let parsed = syntax::SourceFile::parse(ra_fixture); eprintln!("parse: {:#?}", parsed.syntax_node()); let fixups = super::fixup_syntax(&parsed.syntax_node()); - let (mut tt, tmap) = mbe::syntax_node_to_token_tree_with_modifications( + let (mut tt, tmap, _) = mbe::syntax_node_to_token_tree_with_modifications( &parsed.syntax_node(), + fixups.token_map, + fixups.next_id, fixups.replace, fixups.append, ); @@ -144,7 +175,7 @@ mod tests { parse.syntax_node() ); - reverse_fixups(&mut tt, &tmap, &fixups.map); + reverse_fixups(&mut tt, &tmap, &fixups.undo_info); // the fixed-up + reversed version should be equivalent to the original input // (but token IDs don't matter) diff --git a/crates/hir_expand/src/hygiene.rs b/crates/hir_expand/src/hygiene.rs index 0ac5ee8306..d60734372c 100644 --- a/crates/hir_expand/src/hygiene.rs +++ b/crates/hir_expand/src/hygiene.rs @@ -128,7 +128,7 @@ struct HygieneInfo { attr_input_or_mac_def_start: Option>, macro_def: Arc, - macro_arg: Arc<(tt::Subtree, mbe::TokenMap, fixup::SyntaxFixupMap)>, + macro_arg: Arc<(tt::Subtree, mbe::TokenMap, fixup::SyntaxFixupUndoInfo)>, macro_arg_shift: mbe::Shift, exp_map: Arc, } diff --git a/crates/hir_expand/src/lib.rs b/crates/hir_expand/src/lib.rs index 6ec507d00f..37186532bb 100644 --- a/crates/hir_expand/src/lib.rs +++ b/crates/hir_expand/src/lib.rs @@ -427,7 +427,7 @@ pub struct ExpansionInfo { attr_input_or_mac_def: Option>, macro_def: Arc, - macro_arg: Arc<(tt::Subtree, mbe::TokenMap, fixup::SyntaxFixupMap)>, + macro_arg: Arc<(tt::Subtree, mbe::TokenMap, fixup::SyntaxFixupUndoInfo)>, /// A shift built from `macro_arg`'s subtree, relevant for attributes as the item is the macro arg /// and as such we need to shift tokens if they are part of an attributes input instead of their item. macro_arg_shift: mbe::Shift, diff --git a/crates/mbe/src/syntax_bridge.rs b/crates/mbe/src/syntax_bridge.rs index 78a37993f7..83f97c4970 100644 --- a/crates/mbe/src/syntax_bridge.rs +++ b/crates/mbe/src/syntax_bridge.rs @@ -15,23 +15,32 @@ use crate::{to_parser_input::to_parser_input, tt_iter::TtIter, TokenMap}; /// Convert the syntax node to a `TokenTree` (what macro /// will consume). pub fn syntax_node_to_token_tree(node: &SyntaxNode) -> (tt::Subtree, TokenMap) { - syntax_node_to_token_tree_with_modifications(node, Default::default(), Default::default()) + let (subtree, token_map, _) = syntax_node_to_token_tree_with_modifications( + node, + Default::default(), + 0, + Default::default(), + Default::default(), + ); + (subtree, token_map) } /// Convert the syntax node to a `TokenTree` (what macro will consume) /// with the censored range excluded. pub fn syntax_node_to_token_tree_with_modifications( node: &SyntaxNode, + existing_token_map: TokenMap, + next_id: u32, replace: FxHashMap>, append: FxHashMap>, -) -> (tt::Subtree, TokenMap) { +) -> (tt::Subtree, TokenMap, u32) { let global_offset = node.text_range().start(); - let mut c = Convertor::new(node, global_offset, replace, append); + let mut c = Convertor::new(node, global_offset, existing_token_map, next_id, replace, append); let subtree = convert_tokens(&mut c); c.id_alloc.map.shrink_to_fit(); always!(c.replace.is_empty(), "replace: {:?}", c.replace); always!(c.append.is_empty(), "append: {:?}", c.append); - (subtree, c.id_alloc.map) + (subtree, c.id_alloc.map, c.id_alloc.next_id) } #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] @@ -510,6 +519,8 @@ impl Convertor { fn new( node: &SyntaxNode, global_offset: TextSize, + existing_token_map: TokenMap, + next_id: u32, mut replace: FxHashMap>, mut append: FxHashMap>, ) -> Convertor { @@ -517,7 +528,7 @@ impl Convertor { let mut preorder = node.preorder_with_tokens(); let (first, synthetic) = Self::next_token(&mut preorder, &mut replace, &mut append); Convertor { - id_alloc: { TokenIdAlloc { map: TokenMap::default(), global_offset, next_id: 0 } }, + id_alloc: { TokenIdAlloc { map: existing_token_map, global_offset, next_id } }, current: first, current_synthetic: synthetic, preorder, From dfd2cef0d0a8d3fa8b76eb1a5553658e40c33667 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Wed, 9 Feb 2022 16:36:06 +0100 Subject: [PATCH 13/15] Add back an assertion --- crates/hir_expand/src/fixup.rs | 1 - crates/mbe/src/syntax_bridge.rs | 4 +++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/hir_expand/src/fixup.rs b/crates/hir_expand/src/fixup.rs index 36425c8078..622d019c47 100644 --- a/crates/hir_expand/src/fixup.rs +++ b/crates/hir_expand/src/fixup.rs @@ -150,7 +150,6 @@ mod tests { #[track_caller] fn check(ra_fixture: &str, mut expect: Expect) { let parsed = syntax::SourceFile::parse(ra_fixture); - eprintln!("parse: {:#?}", parsed.syntax_node()); let fixups = super::fixup_syntax(&parsed.syntax_node()); let (mut tt, tmap, _) = mbe::syntax_node_to_token_tree_with_modifications( &parsed.syntax_node(), diff --git a/crates/mbe/src/syntax_bridge.rs b/crates/mbe/src/syntax_bridge.rs index 83f97c4970..7e12647cd8 100644 --- a/crates/mbe/src/syntax_bridge.rs +++ b/crates/mbe/src/syntax_bridge.rs @@ -192,7 +192,9 @@ fn convert_tokens(conv: &mut C) -> tt::Subtree { continue; } let tt = if kind.is_punct() && kind != UNDERSCORE { - // assert_eq!(range.len(), TextSize::of('.')); + if synth_id.is_none() { + assert_eq!(range.len(), TextSize::of('.')); + } if let Some(delim) = subtree.delimiter { let expected = match delim.kind { From 577f70cc9ccd7442d3a33da50ab152ef566d5cbc Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Wed, 9 Feb 2022 17:43:37 +0100 Subject: [PATCH 14/15] Reduce visibility --- crates/hir_expand/src/fixup.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/hir_expand/src/fixup.rs b/crates/hir_expand/src/fixup.rs index 622d019c47..24b942f8fa 100644 --- a/crates/hir_expand/src/fixup.rs +++ b/crates/hir_expand/src/fixup.rs @@ -14,12 +14,12 @@ use tt::Subtree; /// (appending to and replacing nodes), the information that is needed to /// reverse those changes afterwards, and a token map. #[derive(Debug)] -pub struct SyntaxFixups { - pub append: FxHashMap>, - pub replace: FxHashMap>, - pub undo_info: SyntaxFixupUndoInfo, - pub token_map: TokenMap, - pub next_id: u32, +pub(crate) struct SyntaxFixups { + pub(crate) append: FxHashMap>, + pub(crate) replace: FxHashMap>, + pub(crate) undo_info: SyntaxFixupUndoInfo, + pub(crate) token_map: TokenMap, + pub(crate) next_id: u32, } /// This is the information needed to reverse the fixups. @@ -30,7 +30,7 @@ pub struct SyntaxFixupUndoInfo { const EMPTY_ID: SyntheticTokenId = SyntheticTokenId(!0); -pub fn fixup_syntax(node: &SyntaxNode) -> SyntaxFixups { +pub(crate) fn fixup_syntax(node: &SyntaxNode) -> SyntaxFixups { let mut append = FxHashMap::default(); let mut replace = FxHashMap::default(); let mut preorder = node.preorder(); @@ -122,7 +122,7 @@ fn has_error_to_handle(node: &SyntaxNode) -> bool { has_error(node) || node.children().any(|c| !can_handle_error(&c) && has_error_to_handle(&c)) } -pub fn reverse_fixups(tt: &mut Subtree, token_map: &TokenMap, undo_info: &SyntaxFixupUndoInfo) { +pub(crate) fn reverse_fixups(tt: &mut Subtree, token_map: &TokenMap, undo_info: &SyntaxFixupUndoInfo) { tt.token_trees.retain(|tt| match tt { tt::TokenTree::Leaf(leaf) => { token_map.synthetic_token_id(leaf.id()).is_none() From ccb789b94ab2b4bebb3da8be046efb13028380f1 Mon Sep 17 00:00:00 2001 From: Florian Diebold Date: Wed, 9 Feb 2022 17:52:15 +0100 Subject: [PATCH 15/15] Format again --- crates/hir_expand/src/fixup.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/hir_expand/src/fixup.rs b/crates/hir_expand/src/fixup.rs index 24b942f8fa..2eb3da79dc 100644 --- a/crates/hir_expand/src/fixup.rs +++ b/crates/hir_expand/src/fixup.rs @@ -122,7 +122,11 @@ fn has_error_to_handle(node: &SyntaxNode) -> bool { has_error(node) || node.children().any(|c| !can_handle_error(&c) && has_error_to_handle(&c)) } -pub(crate) fn reverse_fixups(tt: &mut Subtree, token_map: &TokenMap, undo_info: &SyntaxFixupUndoInfo) { +pub(crate) fn reverse_fixups( + tt: &mut Subtree, + token_map: &TokenMap, + undo_info: &SyntaxFixupUndoInfo, +) { tt.token_trees.retain(|tt| match tt { tt::TokenTree::Leaf(leaf) => { token_map.synthetic_token_id(leaf.id()).is_none()