From e5078078375ffeac80770973920446d9cb97302d Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 20 Jul 2022 15:12:00 +0200 Subject: [PATCH 1/3] internal: Don't eagerly construct `AstIdMap`s --- crates/hir-def/src/body.rs | 14 +++++++------- crates/hir-def/src/body/lower.rs | 23 ++++++++++++++--------- crates/hir-def/src/data.rs | 17 ++++++++++------- crates/hir-def/src/type_ref.rs | 2 +- 4 files changed, 32 insertions(+), 24 deletions(-) diff --git a/crates/hir-def/src/body.rs b/crates/hir-def/src/body.rs index 33851d90a2f..7e9acd3f86e 100644 --- a/crates/hir-def/src/body.rs +++ b/crates/hir-def/src/body.rs @@ -50,7 +50,7 @@ pub struct Expander { cfg_expander: CfgExpander, def_map: Arc, current_file_id: HirFileId, - ast_id_map: Arc, + ast_id_map: Option>, module: LocalModuleId, recursion_limit: usize, } @@ -80,12 +80,11 @@ impl Expander { pub fn new(db: &dyn DefDatabase, current_file_id: HirFileId, module: ModuleId) -> Expander { let cfg_expander = CfgExpander::new(db, current_file_id, module.krate); let def_map = module.def_map(db); - let ast_id_map = db.ast_id_map(current_file_id); Expander { cfg_expander, def_map, current_file_id, - ast_id_map, + ast_id_map: None, module: module.local_id, recursion_limit: 0, } @@ -175,7 +174,7 @@ impl Expander { }; self.cfg_expander.hygiene = Hygiene::new(db.upcast(), file_id); self.current_file_id = file_id; - self.ast_id_map = db.ast_id_map(file_id); + self.ast_id_map = None; ExpandResult { value: Some((mark, node)), err } } @@ -213,8 +212,9 @@ impl Expander { self.def_map.resolve_path(db, self.module, path, BuiltinShadowMode::Other).0.take_macros() } - fn ast_id(&self, item: &N) -> AstId { - let file_local_id = self.ast_id_map.ast_id(item); + fn ast_id(&mut self, db: &dyn DefDatabase, item: &N) -> AstId { + let file_local_id = + self.ast_id_map.get_or_insert_with(|| db.ast_id_map(self.current_file_id)).ast_id(item); AstId::new(self.current_file_id, file_local_id) } @@ -233,7 +233,7 @@ impl Expander { #[derive(Debug)] pub struct Mark { file_id: HirFileId, - ast_id_map: Arc, + ast_id_map: Option>, bomb: DropBomb, } diff --git a/crates/hir-def/src/body/lower.rs b/crates/hir-def/src/body/lower.rs index 049afa82279..006376e362c 100644 --- a/crates/hir-def/src/body/lower.rs +++ b/crates/hir-def/src/body/lower.rs @@ -11,6 +11,7 @@ use hir_expand::{ ExpandError, HirFileId, InFile, }; use la_arena::Arena; +use once_cell::unsync::OnceCell; use profile::Count; use rustc_hash::FxHashMap; use syntax::{ @@ -41,8 +42,7 @@ use crate::{ pub struct LowerCtx<'a> { pub db: &'a dyn DefDatabase, hygiene: Hygiene, - file_id: Option, - source_ast_id_map: Option>, + ast_id_map: Option<(HirFileId, OnceCell>)>, } impl<'a> LowerCtx<'a> { @@ -50,13 +50,12 @@ impl<'a> LowerCtx<'a> { LowerCtx { db, hygiene: Hygiene::new(db.upcast(), file_id), - file_id: Some(file_id), - source_ast_id_map: Some(db.ast_id_map(file_id)), + ast_id_map: Some((file_id, OnceCell::new())), } } pub fn with_hygiene(db: &'a dyn DefDatabase, hygiene: &Hygiene) -> Self { - LowerCtx { db, hygiene: hygiene.clone(), file_id: None, source_ast_id_map: None } + LowerCtx { db, hygiene: hygiene.clone(), ast_id_map: None } } pub(crate) fn hygiene(&self) -> &Hygiene { @@ -64,15 +63,21 @@ impl<'a> LowerCtx<'a> { } pub(crate) fn file_id(&self) -> HirFileId { - self.file_id.unwrap() + self.ast_id_map.as_ref().unwrap().0 } pub(crate) fn lower_path(&self, ast: ast::Path) -> Option { Path::from_src(ast, self) } - pub(crate) fn ast_id(&self, item: &N) -> Option> { - self.source_ast_id_map.as_ref().map(|ast_id_map| ast_id_map.ast_id(item)) + pub(crate) fn ast_id( + &self, + db: &dyn DefDatabase, + item: &N, + ) -> Option> { + let (file_id, ast_id_map) = self.ast_id_map.as_ref()?; + let ast_id_map = ast_id_map.get_or_init(|| db.ast_id_map(*file_id)); + Some(ast_id_map.ast_id(item)) } } @@ -675,7 +680,7 @@ impl ExprCollector<'_> { } fn collect_block(&mut self, block: ast::BlockExpr) -> ExprId { - let ast_id = self.expander.ast_id(&block); + let ast_id = self.expander.ast_id(self.db, &block); let block_loc = BlockLoc { ast_id, module: self.expander.def_map.module_id(self.expander.module) }; let block_id = self.db.intern_block(block_loc); diff --git a/crates/hir-def/src/data.rs b/crates/hir-def/src/data.rs index 8e8b5c322f9..ca15e7c4100 100644 --- a/crates/hir-def/src/data.rs +++ b/crates/hir-def/src/data.rs @@ -550,14 +550,17 @@ impl<'a> AssocItemCollector<'a> { AssocItem::MacroCall(call) => { let call = &item_tree[call]; let ast_id_map = self.db.ast_id_map(self.expander.current_file_id()); - let root = self.db.parse_or_expand(self.expander.current_file_id()).unwrap(); - let call = ast_id_map.get(call.ast_id).to_node(&root); - let _cx = - stdx::panic_context::enter(format!("collect_items MacroCall: {}", call)); - let res = self.expander.enter_expand(self.db, call); + if let Some(root) = self.db.parse_or_expand(self.expander.current_file_id()) { + let call = ast_id_map.get(call.ast_id).to_node(&root); + let _cx = stdx::panic_context::enter(format!( + "collect_items MacroCall: {}", + call + )); + let res = self.expander.enter_expand(self.db, call); - if let Ok(ExpandResult { value: Some((mark, mac)), .. }) = res { - self.collect_macro_items(mark, mac); + if let Ok(ExpandResult { value: Some((mark, mac)), .. }) = res { + self.collect_macro_items(mark, mac); + } } } } diff --git a/crates/hir-def/src/type_ref.rs b/crates/hir-def/src/type_ref.rs index 6be78745f12..dd990b0c784 100644 --- a/crates/hir-def/src/type_ref.rs +++ b/crates/hir-def/src/type_ref.rs @@ -237,7 +237,7 @@ impl TypeRef { } ast::Type::MacroType(mt) => match mt.macro_call() { Some(mc) => ctx - .ast_id(&mc) + .ast_id(ctx.db, &mc) .map(|mc| TypeRef::Macro(InFile::new(ctx.file_id(), mc))) .unwrap_or(TypeRef::Error), None => TypeRef::Error, From c83f14a44a6f89f55cdb32bb7dc5c2277d9aafe2 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Thu, 21 Jul 2022 02:00:58 +0200 Subject: [PATCH 2/3] Remove AstIdMap from Expander as it is seldom needed --- crates/hir-def/src/body.rs | 27 +++++---------------------- crates/hir-def/src/body/lower.rs | 12 ++++++++++-- 2 files changed, 15 insertions(+), 24 deletions(-) diff --git a/crates/hir-def/src/body.rs b/crates/hir-def/src/body.rs index 7e9acd3f86e..080a307b1f8 100644 --- a/crates/hir-def/src/body.rs +++ b/crates/hir-def/src/body.rs @@ -5,21 +5,18 @@ mod lower; mod tests; pub mod scope; -use std::{mem, ops::Index, sync::Arc}; +use std::{ops::Index, sync::Arc}; use base_db::CrateId; use cfg::{CfgExpr, CfgOptions}; use drop_bomb::DropBomb; use either::Either; -use hir_expand::{ - ast_id_map::AstIdMap, hygiene::Hygiene, AstId, ExpandError, ExpandResult, HirFileId, InFile, - MacroCallId, -}; +use hir_expand::{hygiene::Hygiene, ExpandError, ExpandResult, HirFileId, InFile, MacroCallId}; use la_arena::{Arena, ArenaMap}; use limit::Limit; use profile::Count; use rustc_hash::FxHashMap; -use syntax::{ast, AstNode, AstPtr, SyntaxNodePtr}; +use syntax::{ast, AstPtr, SyntaxNodePtr}; use crate::{ attr::{Attrs, RawAttrs}, @@ -50,7 +47,6 @@ pub struct Expander { cfg_expander: CfgExpander, def_map: Arc, current_file_id: HirFileId, - ast_id_map: Option>, module: LocalModuleId, recursion_limit: usize, } @@ -84,7 +80,6 @@ impl Expander { cfg_expander, def_map, current_file_id, - ast_id_map: None, module: module.local_id, recursion_limit: 0, } @@ -167,14 +162,10 @@ impl Expander { tracing::debug!("macro expansion {:#?}", node.syntax()); self.recursion_limit += 1; - let mark = Mark { - file_id: self.current_file_id, - ast_id_map: mem::take(&mut self.ast_id_map), - bomb: DropBomb::new("expansion mark dropped"), - }; + let mark = + Mark { file_id: self.current_file_id, bomb: DropBomb::new("expansion mark dropped") }; self.cfg_expander.hygiene = Hygiene::new(db.upcast(), file_id); self.current_file_id = file_id; - self.ast_id_map = None; ExpandResult { value: Some((mark, node)), err } } @@ -182,7 +173,6 @@ impl Expander { pub fn exit(&mut self, db: &dyn DefDatabase, mut mark: Mark) { self.cfg_expander.hygiene = Hygiene::new(db.upcast(), mark.file_id); self.current_file_id = mark.file_id; - self.ast_id_map = mem::take(&mut mark.ast_id_map); self.recursion_limit -= 1; mark.bomb.defuse(); } @@ -212,12 +202,6 @@ impl Expander { self.def_map.resolve_path(db, self.module, path, BuiltinShadowMode::Other).0.take_macros() } - fn ast_id(&mut self, db: &dyn DefDatabase, item: &N) -> AstId { - let file_local_id = - self.ast_id_map.get_or_insert_with(|| db.ast_id_map(self.current_file_id)).ast_id(item); - AstId::new(self.current_file_id, file_local_id) - } - fn recursion_limit(&self, db: &dyn DefDatabase) -> Limit { let limit = db.crate_limits(self.cfg_expander.krate).recursion_limit as _; @@ -233,7 +217,6 @@ impl Expander { #[derive(Debug)] pub struct Mark { file_id: HirFileId, - ast_id_map: Option>, bomb: DropBomb, } diff --git a/crates/hir-def/src/body/lower.rs b/crates/hir-def/src/body/lower.rs index 006376e362c..8cef31ed151 100644 --- a/crates/hir-def/src/body/lower.rs +++ b/crates/hir-def/src/body/lower.rs @@ -8,7 +8,7 @@ use hir_expand::{ ast_id_map::{AstIdMap, FileAstId}, hygiene::Hygiene, name::{name, AsName, Name}, - ExpandError, HirFileId, InFile, + AstId, ExpandError, HirFileId, InFile, }; use la_arena::Arena; use once_cell::unsync::OnceCell; @@ -90,6 +90,7 @@ pub(super) fn lower( ExprCollector { db, source_map: BodySourceMap::default(), + ast_id_map: db.ast_id_map(expander.current_file_id), body: Body { exprs: Arena::default(), pats: Arena::default(), @@ -110,6 +111,7 @@ pub(super) fn lower( struct ExprCollector<'a> { db: &'a dyn DefDatabase, expander: Expander, + ast_id_map: Arc, body: Body, source_map: BodySourceMap, // a poor-mans union-find? @@ -591,8 +593,13 @@ impl ExprCollector<'_> { match res.value { Some((mark, expansion)) => { self.source_map.expansions.insert(macro_call_ptr, self.expander.current_file_id); + let prev_ast_id_map = mem::replace( + &mut self.ast_id_map, + self.db.ast_id_map(self.expander.current_file_id), + ); let id = collector(self, Some(expansion)); + self.ast_id_map = prev_ast_id_map; self.expander.exit(self.db, mark); id } @@ -680,7 +687,8 @@ impl ExprCollector<'_> { } fn collect_block(&mut self, block: ast::BlockExpr) -> ExprId { - let ast_id = self.expander.ast_id(self.db, &block); + let file_local_id = self.ast_id_map.ast_id(&block); + let ast_id = AstId::new(self.expander.current_file_id, file_local_id); let block_loc = BlockLoc { ast_id, module: self.expander.def_map.module_id(self.expander.module) }; let block_id = self.db.intern_block(block_loc); From 7bd2e305d667e53314601ef04435d59b25e65235 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Thu, 21 Jul 2022 02:06:26 +0200 Subject: [PATCH 3/3] Simplify --- crates/hir-def/src/body/lower.rs | 18 +++++------------- crates/hir-def/src/type_ref.rs | 7 ++----- 2 files changed, 7 insertions(+), 18 deletions(-) diff --git a/crates/hir-def/src/body/lower.rs b/crates/hir-def/src/body/lower.rs index 8cef31ed151..c3f26112278 100644 --- a/crates/hir-def/src/body/lower.rs +++ b/crates/hir-def/src/body/lower.rs @@ -5,7 +5,7 @@ use std::{mem, sync::Arc}; use either::Either; use hir_expand::{ - ast_id_map::{AstIdMap, FileAstId}, + ast_id_map::AstIdMap, hygiene::Hygiene, name::{name, AsName, Name}, AstId, ExpandError, HirFileId, InFile, @@ -62,22 +62,14 @@ impl<'a> LowerCtx<'a> { &self.hygiene } - pub(crate) fn file_id(&self) -> HirFileId { - self.ast_id_map.as_ref().unwrap().0 - } - pub(crate) fn lower_path(&self, ast: ast::Path) -> Option { Path::from_src(ast, self) } - pub(crate) fn ast_id( - &self, - db: &dyn DefDatabase, - item: &N, - ) -> Option> { - let (file_id, ast_id_map) = self.ast_id_map.as_ref()?; - let ast_id_map = ast_id_map.get_or_init(|| db.ast_id_map(*file_id)); - Some(ast_id_map.ast_id(item)) + pub(crate) fn ast_id(&self, db: &dyn DefDatabase, item: &N) -> Option> { + let &(file_id, ref ast_id_map) = self.ast_id_map.as_ref()?; + let ast_id_map = ast_id_map.get_or_init(|| db.ast_id_map(file_id)); + Some(InFile::new(file_id, ast_id_map.ast_id(item))) } } diff --git a/crates/hir-def/src/type_ref.rs b/crates/hir-def/src/type_ref.rs index dd990b0c784..59f0a8458a5 100644 --- a/crates/hir-def/src/type_ref.rs +++ b/crates/hir-def/src/type_ref.rs @@ -5,7 +5,7 @@ use std::fmt::Write; use hir_expand::{ name::{AsName, Name}, - AstId, InFile, + AstId, }; use syntax::ast::{self, HasName}; @@ -236,10 +236,7 @@ impl TypeRef { TypeRef::DynTrait(type_bounds_from_ast(ctx, inner.type_bound_list())) } ast::Type::MacroType(mt) => match mt.macro_call() { - Some(mc) => ctx - .ast_id(ctx.db, &mc) - .map(|mc| TypeRef::Macro(InFile::new(ctx.file_id(), mc))) - .unwrap_or(TypeRef::Error), + Some(mc) => ctx.ast_id(ctx.db, &mc).map(TypeRef::Macro).unwrap_or(TypeRef::Error), None => TypeRef::Error, }, }