From 8876f44054f57c1f4ee305eb47340609683bd566 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 4 May 2019 15:26:55 +0300 Subject: Revert "eagarly clean astd maps" This reverts commit 6c63a59425e256ce46d058807b64149297231982. This causes massive slowdowns: looks like we accidentally have some source-depndent --- crates/ra_hir/src/db.rs | 2 +- crates/ra_hir/src/source_id.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'crates/ra_hir') diff --git a/crates/ra_hir/src/db.rs b/crates/ra_hir/src/db.rs index 3ac3c79a3..8af0a3176 100644 --- a/crates/ra_hir/src/db.rs +++ b/crates/ra_hir/src/db.rs @@ -60,7 +60,7 @@ pub trait DefDatabase: SourceDatabase { #[salsa::invoke(crate::source_id::AstIdMap::ast_id_map_query)] fn ast_id_map(&self, file_id: HirFileId) -> Arc; - #[salsa::invoke(crate::source_id::AstIdMap::ast_id_to_node_query)] + #[salsa::invoke(crate::source_id::AstIdMap::file_item_query)] fn ast_id_to_node(&self, file_id: HirFileId, ast_id: ErasedFileAstId) -> TreeArc; #[salsa::invoke(RawItems::raw_items_query)] diff --git a/crates/ra_hir/src/source_id.rs b/crates/ra_hir/src/source_id.rs index a2bc9a799..0a8fb6d32 100644 --- a/crates/ra_hir/src/source_id.rs +++ b/crates/ra_hir/src/source_id.rs @@ -92,7 +92,7 @@ impl AstIdMap { Arc::new(AstIdMap::from_source_file(&source_file)) } - pub(crate) fn ast_id_to_node_query( + pub(crate) fn file_item_query( db: &impl DefDatabase, file_id: HirFileId, ast_id: ErasedFileAstId, -- cgit v1.2.3 From 5dc384132f7fe6f610ec21e62830ab83f45274a9 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 4 May 2019 17:38:09 +0300 Subject: introduce macro_arg intermediate query Currently, when expanding macros, we look at the source code directly (we invoke ast_id_to_node query via to_node method). This is less then ideal, because it make us re-expand macros after every source change. This commit establishes a salsa-firewall: a query to get macro call's token tree. Unlike the syntax tree, token tree changes only if we actually modify the macro itself. --- crates/ra_hir/src/db.rs | 5 ++++- crates/ra_hir/src/expr.rs | 33 ++++++++++++++------------------- crates/ra_hir/src/ids.rs | 14 +++++++++----- 3 files changed, 27 insertions(+), 25 deletions(-) (limited to 'crates/ra_hir') diff --git a/crates/ra_hir/src/db.rs b/crates/ra_hir/src/db.rs index 8af0a3176..f88ae61bb 100644 --- a/crates/ra_hir/src/db.rs +++ b/crates/ra_hir/src/db.rs @@ -42,7 +42,10 @@ pub trait DefDatabase: SourceDatabase { #[salsa::invoke(crate::ids::macro_def_query)] fn macro_def(&self, macro_id: MacroDefId) -> Option>; - #[salsa::invoke(HirFileId::hir_parse_query)] + #[salsa::invoke(crate::ids::macro_arg_query)] + fn macro_arg(&self, macro_call: ids::MacroCallId) -> Option>; + + #[salsa::invoke(crate::ids::HirFileId::hir_parse_query)] fn hir_parse(&self, file_id: HirFileId) -> TreeArc; #[salsa::invoke(crate::adt::StructData::struct_data_query)] diff --git a/crates/ra_hir/src/expr.rs b/crates/ra_hir/src/expr.rs index cdadf3ba6..f9b3f5443 100644 --- a/crates/ra_hir/src/expr.rs +++ b/crates/ra_hir/src/expr.rs @@ -826,21 +826,20 @@ where .with_file_id(self.current_file_id); if let Some(call_id) = self.resolver.resolve_macro_call(self.db, path, ast_id) { - if let Some(expr) = expand_macro_to_expr(self.db, call_id, e.token_tree()) { - log::debug!("macro expansion {}", expr.syntax().debug_dump()); - let old_file_id = - std::mem::replace(&mut self.current_file_id, call_id.into()); - let id = self.collect_expr(&expr); - self.current_file_id = old_file_id; - id - } else { - // FIXME: Instead of just dropping the error from expansion - // report it - self.alloc_expr(Expr::Missing, syntax_ptr) + if let Some(arg) = self.db.macro_arg(call_id) { + if let Some(expr) = expand_macro_to_expr(self.db, call_id, &arg) { + log::debug!("macro expansion {}", expr.syntax().debug_dump()); + let old_file_id = + std::mem::replace(&mut self.current_file_id, call_id.into()); + let id = self.collect_expr(&expr); + self.current_file_id = old_file_id; + return id; + } } - } else { - self.alloc_expr(Expr::Missing, syntax_ptr) } + // FIXME: Instead of just dropping the error from expansion + // report it + self.alloc_expr(Expr::Missing, syntax_ptr) } } } @@ -1002,14 +1001,10 @@ where fn expand_macro_to_expr( db: &impl HirDatabase, macro_call: MacroCallId, - args: Option<&ast::TokenTree>, + arg: &tt::Subtree, ) -> Option> { let rules = db.macro_def(macro_call.loc(db).def)?; - - let args = mbe::ast_to_token_tree(args?)?.0; - - let expanded = rules.expand(&args).ok()?; - + let expanded = rules.expand(&arg).ok()?; mbe::token_tree_to_expr(&expanded).ok() } diff --git a/crates/ra_hir/src/ids.rs b/crates/ra_hir/src/ids.rs index b0e9b1f9a..24c5f412b 100644 --- a/crates/ra_hir/src/ids.rs +++ b/crates/ra_hir/src/ids.rs @@ -86,11 +86,7 @@ fn parse_macro( macro_call_id: MacroCallId, ) -> Result, String> { let loc = macro_call_id.loc(db); - let macro_call = loc.ast_id.to_node(db); - let (macro_arg, _) = macro_call - .token_tree() - .and_then(mbe::ast_to_token_tree) - .ok_or("Fail to args in to tt::TokenTree")?; + let macro_arg = db.macro_arg(macro_call_id).ok_or("Fail to args in to tt::TokenTree")?; let macro_rules = db.macro_def(loc.def).ok_or("Fail to find macro definition")?; let tt = macro_rules.expand(¯o_arg).map_err(|err| format!("{:?}", err))?; @@ -139,6 +135,14 @@ pub(crate) fn macro_def_query(db: &impl DefDatabase, id: MacroDefId) -> Option Option> { + let loc = id.loc(db); + let macro_call = loc.ast_id.to_node(db); + let arg = macro_call.token_tree()?; + let (tt, _) = mbe::ast_to_token_tree(arg)?; + Some(Arc::new(tt)) +} + macro_rules! impl_intern_key { ($name:ident) => { impl salsa::InternKey for $name { -- cgit v1.2.3 From 87a1e276d56a3cb5f9d9c59f8c52c5573e19982b Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 4 May 2019 17:42:24 +0300 Subject: minor, move --- crates/ra_hir/src/ids.rs | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) (limited to 'crates/ra_hir') diff --git a/crates/ra_hir/src/ids.rs b/crates/ra_hir/src/ids.rs index 24c5f412b..cf0e934a9 100644 --- a/crates/ra_hir/src/ids.rs +++ b/crates/ra_hir/src/ids.rs @@ -81,25 +81,6 @@ impl HirFileId { } } -fn parse_macro( - db: &impl DefDatabase, - macro_call_id: MacroCallId, -) -> Result, String> { - let loc = macro_call_id.loc(db); - let macro_arg = db.macro_arg(macro_call_id).ok_or("Fail to args in to tt::TokenTree")?; - - let macro_rules = db.macro_def(loc.def).ok_or("Fail to find macro definition")?; - let tt = macro_rules.expand(¯o_arg).map_err(|err| format!("{:?}", err))?; - - // Set a hard limit for the expanded tt - let count = tt.count(); - if count > 65536 { - return Err(format!("Total tokens count exceed limit : count = {}", count)); - } - - Ok(mbe::token_tree_to_ast_item_list(&tt)) -} - #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] enum HirFileIdRepr { File(FileId), @@ -143,6 +124,25 @@ pub(crate) fn macro_arg_query(db: &impl DefDatabase, id: MacroCallId) -> Option< Some(Arc::new(tt)) } +fn parse_macro( + db: &impl DefDatabase, + macro_call_id: MacroCallId, +) -> Result, String> { + let loc = macro_call_id.loc(db); + let macro_arg = db.macro_arg(macro_call_id).ok_or("Fail to args in to tt::TokenTree")?; + + let macro_rules = db.macro_def(loc.def).ok_or("Fail to find macro definition")?; + let tt = macro_rules.expand(¯o_arg).map_err(|err| format!("{:?}", err))?; + + // Set a hard limit for the expanded tt + let count = tt.count(); + if count > 65536 { + return Err(format!("Total tokens count exceed limit : count = {}", count)); + } + + Ok(mbe::token_tree_to_ast_item_list(&tt)) +} + macro_rules! impl_intern_key { ($name:ident) => { impl salsa::InternKey for $name { -- cgit v1.2.3 From bcf45371ff19882e67300cc483b481450ee129fb Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 4 May 2019 18:01:43 +0300 Subject: make macro expansion into a proper query --- crates/ra_hir/src/db.rs | 3 +++ crates/ra_hir/src/expr.rs | 17 +++-------------- crates/ra_hir/src/ids.rs | 43 ++++++++++++++++++++++--------------------- 3 files changed, 28 insertions(+), 35 deletions(-) (limited to 'crates/ra_hir') diff --git a/crates/ra_hir/src/db.rs b/crates/ra_hir/src/db.rs index f88ae61bb..398e00c42 100644 --- a/crates/ra_hir/src/db.rs +++ b/crates/ra_hir/src/db.rs @@ -45,6 +45,9 @@ pub trait DefDatabase: SourceDatabase { #[salsa::invoke(crate::ids::macro_arg_query)] fn macro_arg(&self, macro_call: ids::MacroCallId) -> Option>; + #[salsa::invoke(crate::ids::macro_expand_query)] + fn macro_expand(&self, macro_call: ids::MacroCallId) -> Result, String>; + #[salsa::invoke(crate::ids::HirFileId::hir_parse_query)] fn hir_parse(&self, file_id: HirFileId) -> TreeArc; diff --git a/crates/ra_hir/src/expr.rs b/crates/ra_hir/src/expr.rs index f9b3f5443..692da2895 100644 --- a/crates/ra_hir/src/expr.rs +++ b/crates/ra_hir/src/expr.rs @@ -5,14 +5,13 @@ use rustc_hash::FxHashMap; use ra_arena::{Arena, RawId, impl_arena_id, map::ArenaMap}; use ra_syntax::{ - SyntaxNodePtr, AstPtr, AstNode,TreeArc, + SyntaxNodePtr, AstPtr, AstNode, ast::{self, LoopBodyOwner, ArgListOwner, NameOwner, LiteralKind,ArrayExprKind, TypeAscriptionOwner} }; use crate::{ Path, Name, HirDatabase, Resolver,DefWithBody, Either, HirFileId, name::AsName, - ids::{MacroCallId}, type_ref::{Mutability, TypeRef}, }; use crate::{path::GenericArgs, ty::primitive::{IntTy, UncertainIntTy, FloatTy, UncertainFloatTy}}; @@ -826,8 +825,8 @@ where .with_file_id(self.current_file_id); if let Some(call_id) = self.resolver.resolve_macro_call(self.db, path, ast_id) { - if let Some(arg) = self.db.macro_arg(call_id) { - if let Some(expr) = expand_macro_to_expr(self.db, call_id, &arg) { + if let Some(tt) = self.db.macro_expand(call_id).ok() { + if let Some(expr) = mbe::token_tree_to_expr(&tt).ok() { log::debug!("macro expansion {}", expr.syntax().debug_dump()); let old_file_id = std::mem::replace(&mut self.current_file_id, call_id.into()); @@ -998,16 +997,6 @@ where } } -fn expand_macro_to_expr( - db: &impl HirDatabase, - macro_call: MacroCallId, - arg: &tt::Subtree, -) -> Option> { - let rules = db.macro_def(macro_call.loc(db).def)?; - let expanded = rules.expand(&arg).ok()?; - mbe::token_tree_to_expr(&expanded).ok() -} - pub(crate) fn body_with_source_map_query( db: &impl HirDatabase, def: DefWithBody, diff --git a/crates/ra_hir/src/ids.rs b/crates/ra_hir/src/ids.rs index cf0e934a9..4102951c9 100644 --- a/crates/ra_hir/src/ids.rs +++ b/crates/ra_hir/src/ids.rs @@ -63,19 +63,22 @@ impl HirFileId { match file_id.0 { HirFileIdRepr::File(file_id) => db.parse(file_id), HirFileIdRepr::Macro(macro_call_id) => { - parse_macro(db, macro_call_id).unwrap_or_else(|err| { - // Note: - // The final goal we would like to make all parse_macro success, - // such that the following log will not call anyway. - log::warn!( - "fail on macro_parse: (reason: {}) {}", - err, - macro_call_id.debug_dump(db) - ); - - // returning an empty string looks fishy... - SourceFile::parse("") - }) + match db.macro_expand(macro_call_id) { + Ok(tt) => mbe::token_tree_to_ast_item_list(&tt), + Err(err) => { + // Note: + // The final goal we would like to make all parse_macro success, + // such that the following log will not call anyway. + log::warn!( + "fail on macro_parse: (reason: {}) {}", + err, + macro_call_id.debug_dump(db) + ); + + // returning an empty string looks fishy... + SourceFile::parse("") + } + } } } } @@ -124,23 +127,21 @@ pub(crate) fn macro_arg_query(db: &impl DefDatabase, id: MacroCallId) -> Option< Some(Arc::new(tt)) } -fn parse_macro( +pub(crate) fn macro_expand_query( db: &impl DefDatabase, - macro_call_id: MacroCallId, -) -> Result, String> { - let loc = macro_call_id.loc(db); - let macro_arg = db.macro_arg(macro_call_id).ok_or("Fail to args in to tt::TokenTree")?; + id: MacroCallId, +) -> Result, String> { + let loc = id.loc(db); + let macro_arg = db.macro_arg(id).ok_or("Fail to args in to tt::TokenTree")?; let macro_rules = db.macro_def(loc.def).ok_or("Fail to find macro definition")?; let tt = macro_rules.expand(¯o_arg).map_err(|err| format!("{:?}", err))?; - // Set a hard limit for the expanded tt let count = tt.count(); if count > 65536 { return Err(format!("Total tokens count exceed limit : count = {}", count)); } - - Ok(mbe::token_tree_to_ast_item_list(&tt)) + Ok(Arc::new(tt)) } macro_rules! impl_intern_key { -- cgit v1.2.3