From c365bd96d30da953f4ed582ac7128820c7491b5c Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Tue, 18 Mar 2025 11:11:36 +0100 Subject: [PATCH] refactor: Do not use `Expander` in assoc item lowering `Expander` is a macro expansion solution for body lowering, there is no need to use it here --- crates/hir-def/src/nameres/assoc.rs | 229 +++++++++++++--------------- 1 file changed, 105 insertions(+), 124 deletions(-) diff --git a/crates/hir-def/src/nameres/assoc.rs b/crates/hir-def/src/nameres/assoc.rs index eb7f8b0061..77d22f3c98 100644 --- a/crates/hir-def/src/nameres/assoc.rs +++ b/crates/hir-def/src/nameres/assoc.rs @@ -1,19 +1,15 @@ //! Expansion of associated items -use hir_expand::{ - AstId, ExpandResult, InFile, Intern, Lookup, MacroCallKind, MacroDefKind, name::Name, -}; -use smallvec::SmallVec; -use span::{HirFileId, MacroCallId}; -use syntax::{Parse, ast}; +use hir_expand::{AstId, InFile, Intern, Lookup, MacroCallKind, MacroDefKind, name::Name}; +use span::MacroCallId; +use syntax::ast; use triomphe::Arc; use crate::{ AssocItemId, AstIdWithPath, ConstLoc, FunctionId, FunctionLoc, ImplId, ItemContainerId, ItemLoc, ModuleId, TraitId, TypeAliasId, TypeAliasLoc, db::DefDatabase, - expander::{Expander, Mark}, - item_tree::{self, AssocItem, ItemTree, ItemTreeId, MacroCall, ModItem, TreeId}, + item_tree::{AssocItem, ItemTree, ItemTreeId, MacroCall, ModItem, TreeId}, macro_call_as_call_id, nameres::{ DefMap, LocalDefMap, MacroSubNs, @@ -26,6 +22,7 @@ use crate::{ pub struct TraitItems { pub items: Box<[(Name, AssocItemId)]>, // box it as the vec is usually empty anyways + // FIXME: AstIds are rather unstable... pub macro_calls: Option, MacroCallId)>>>, } @@ -40,13 +37,11 @@ impl TraitItems { tr: TraitId, ) -> (Arc, DefDiagnostics) { let ItemLoc { container: module_id, id: tree_id } = tr.lookup(db); - let item_tree = tree_id.item_tree(db); - let tr_def = &item_tree[tree_id.value]; - let mut collector = - AssocItemCollector::new(db, module_id, tree_id.file_id(), ItemContainerId::TraitId(tr)); - collector.collect(&item_tree, tree_id.tree_id(), &tr_def.items); - let (items, macro_calls, diagnostics) = collector.finish(); + let collector = AssocItemCollector::new(db, module_id, ItemContainerId::TraitId(tr)); + let item_tree = tree_id.item_tree(db); + let (items, macro_calls, diagnostics) = + collector.collect(&item_tree, tree_id.tree_id(), &item_tree[tree_id.value].items); (Arc::new(TraitItems { macro_calls, items }), DefDiagnostics::new(diagnostics)) } @@ -81,6 +76,7 @@ impl TraitItems { pub struct ImplItems { pub items: Box<[(Name, AssocItemId)]>, // box it as the vec is usually empty anyways + // FIXME: AstIds are rather unstable... pub macro_calls: Option, MacroCallId)>>>, } @@ -97,13 +93,10 @@ impl ImplItems { let _p = tracing::info_span!("impl_items_with_diagnostics_query").entered(); let ItemLoc { container: module_id, id: tree_id } = id.lookup(db); + let collector = AssocItemCollector::new(db, module_id, ItemContainerId::ImplId(id)); let item_tree = tree_id.item_tree(db); - let impl_def = &item_tree[tree_id.value]; - let mut collector = - AssocItemCollector::new(db, module_id, tree_id.file_id(), ItemContainerId::ImplId(id)); - collector.collect(&item_tree, tree_id.tree_id(), &impl_def.items); - - let (items, macro_calls, diagnostics) = collector.finish(); + let (items, macro_calls, diagnostics) = + collector.collect(&item_tree, tree_id.tree_id(), &item_tree[tree_id.value].items); (Arc::new(ImplItems { items, macro_calls }), DefDiagnostics::new(diagnostics)) } @@ -120,19 +113,14 @@ struct AssocItemCollector<'a> { local_def_map: Arc, diagnostics: Vec, container: ItemContainerId, - expander: Expander, + depth: usize, items: Vec<(Name, AssocItemId)>, macro_calls: Vec<(AstId, MacroCallId)>, } impl<'a> AssocItemCollector<'a> { - fn new( - db: &'a dyn DefDatabase, - module_id: ModuleId, - file_id: HirFileId, - container: ItemContainerId, - ) -> Self { + fn new(db: &'a dyn DefDatabase, module_id: ModuleId, container: ItemContainerId) -> Self { let (def_map, local_def_map) = module_id.local_def_map(db); Self { db, @@ -140,20 +128,28 @@ impl<'a> AssocItemCollector<'a> { def_map, local_def_map, container, - expander: Expander::new(db, file_id, module_id), items: Vec::new(), + + depth: 0, macro_calls: Vec::new(), diagnostics: Vec::new(), } } - fn finish( - self, + fn collect( + mut self, + item_tree: &ItemTree, + tree_id: TreeId, + assoc_items: &[AssocItem], ) -> ( Box<[(Name, AssocItemId)]>, Option, MacroCallId)>>>, Vec, ) { + self.items.reserve(assoc_items.len()); + for &item in assoc_items { + self.collect_item(item_tree, tree_id, item); + } ( self.items.into_boxed_slice(), if self.macro_calls.is_empty() { None } else { Some(Box::new(self.macro_calls)) }, @@ -161,116 +157,99 @@ impl<'a> AssocItemCollector<'a> { ) } - fn collect(&mut self, item_tree: &ItemTree, tree_id: TreeId, assoc_items: &[AssocItem]) { - let container = self.container; - self.items.reserve(assoc_items.len()); + fn collect_item(&mut self, item_tree: &ItemTree, tree_id: TreeId, item: AssocItem) { + let attrs = item_tree.attrs(self.db, self.module_id.krate, ModItem::from(item).into()); + if !attrs.is_cfg_enabled(self.module_id.krate.cfg_options(self.db)) { + self.diagnostics.push(DefDiagnostic::unconfigured_code( + self.module_id.local_id, + tree_id, + ModItem::from(item).into(), + attrs.cfg().unwrap(), + self.module_id.krate.cfg_options(self.db).clone(), + )); + return; + } - 'items: for &item in assoc_items { - let attrs = item_tree.attrs(self.db, self.module_id.krate, ModItem::from(item).into()); - if !attrs.is_cfg_enabled(self.expander.cfg_options(self.db)) { - self.diagnostics.push(DefDiagnostic::unconfigured_code( - self.module_id.local_id, - tree_id, - ModItem::from(item).into(), - attrs.cfg().unwrap(), - self.expander.cfg_options(self.db).clone(), - )); - continue; - } + 'attrs: for attr in &*attrs { + let ast_id = AstId::new(tree_id.file_id(), item.ast_id(item_tree).upcast()); + let ast_id_with_path = AstIdWithPath { path: attr.path.clone(), ast_id }; - 'attrs: for attr in &*attrs { - let ast_id = - AstId::new(self.expander.current_file_id(), item.ast_id(item_tree).upcast()); - let ast_id_with_path = AstIdWithPath { path: attr.path.clone(), ast_id }; - - match self.def_map.resolve_attr_macro( - &self.local_def_map, - self.db, - self.module_id.local_id, - ast_id_with_path, - attr, - ) { - Ok(ResolvedAttr::Macro(call_id)) => { - let loc = self.db.lookup_intern_macro_call(call_id); - if let MacroDefKind::ProcMacro(_, exp, _) = loc.def.kind { - // If there's no expander for the proc macro (e.g. the - // proc macro is ignored, or building the proc macro - // crate failed), skip expansion like we would if it was - // disabled. This is analogous to the handling in - // `DefCollector::collect_macros`. - if let Some(err) = exp.as_expand_error(self.module_id.krate) { - self.diagnostics.push(DefDiagnostic::macro_error( - self.module_id.local_id, - ast_id, - (*attr.path).clone(), - err, - )); - continue 'attrs; - } - } - - self.macro_calls.push((ast_id, call_id)); - let res = - self.expander.enter_expand_id::(self.db, call_id); - self.collect_macro_items(res); - continue 'items; - } - Ok(_) => (), - Err(_) => { - self.diagnostics.push(DefDiagnostic::unresolved_macro_call( - self.module_id.local_id, - MacroCallKind::Attr { + match self.def_map.resolve_attr_macro( + &self.local_def_map, + self.db, + self.module_id.local_id, + ast_id_with_path, + attr, + ) { + Ok(ResolvedAttr::Macro(call_id)) => { + let loc = self.db.lookup_intern_macro_call(call_id); + if let MacroDefKind::ProcMacro(_, exp, _) = loc.def.kind { + // If there's no expander for the proc macro (e.g. the + // proc macro is ignored, or building the proc macro + // crate failed), skip expansion like we would if it was + // disabled. This is analogous to the handling in + // `DefCollector::collect_macros`. + if let Some(err) = exp.as_expand_error(self.module_id.krate) { + self.diagnostics.push(DefDiagnostic::macro_error( + self.module_id.local_id, ast_id, - attr_args: None, - invoc_attr_index: attr.id, - }, - attr.path().clone(), - )); + (*attr.path).clone(), + err, + )); + continue 'attrs; + } } + + self.macro_calls.push((ast_id, call_id)); + self.collect_macro_items(call_id); + return; + } + Ok(_) => (), + Err(_) => { + self.diagnostics.push(DefDiagnostic::unresolved_macro_call( + self.module_id.local_id, + MacroCallKind::Attr { ast_id, attr_args: None, invoc_attr_index: attr.id }, + attr.path().clone(), + )); } } - - self.collect_item(item_tree, tree_id, container, item); } + + self.record_item(item_tree, tree_id, item); } - fn collect_item( - &mut self, - item_tree: &ItemTree, - tree_id: TreeId, - container: ItemContainerId, - item: AssocItem, - ) { + fn record_item(&mut self, item_tree: &ItemTree, tree_id: TreeId, item: AssocItem) { match item { AssocItem::Function(id) => { let item = &item_tree[id]; let def = - FunctionLoc { container, id: ItemTreeId::new(tree_id, id) }.intern(self.db); + FunctionLoc { container: self.container, id: ItemTreeId::new(tree_id, id) } + .intern(self.db); self.items.push((item.name.clone(), def.into())); } AssocItem::TypeAlias(id) => { let item = &item_tree[id]; let def = - TypeAliasLoc { container, id: ItemTreeId::new(tree_id, id) }.intern(self.db); + TypeAliasLoc { container: self.container, id: ItemTreeId::new(tree_id, id) } + .intern(self.db); self.items.push((item.name.clone(), def.into())); } AssocItem::Const(id) => { let item = &item_tree[id]; let Some(name) = item.name.clone() else { return }; - let def = ConstLoc { container, id: ItemTreeId::new(tree_id, id) }.intern(self.db); + let def = ConstLoc { container: self.container, id: ItemTreeId::new(tree_id, id) } + .intern(self.db); self.items.push((name, def.into())); } AssocItem::MacroCall(call) => { - let file_id = self.expander.current_file_id(); let MacroCall { ast_id, expand_to, ctxt, ref path } = item_tree[call]; - let module = self.expander.module.local_id; let resolver = |path: &_| { self.def_map .resolve_path( &self.local_def_map, self.db, - module, + self.module_id.local_id, path, crate::item_scope::BuiltinShadowMode::Other, Some(MacroSubNs::Bang), @@ -281,24 +260,23 @@ impl<'a> AssocItemCollector<'a> { }; match macro_call_as_call_id( self.db.upcast(), - &AstIdWithPath::new(file_id, ast_id, Clone::clone(path)), + &AstIdWithPath::new(tree_id.file_id(), ast_id, Clone::clone(path)), ctxt, expand_to, - self.expander.krate(), + self.module_id.krate(), resolver, ) { Ok(Some(call_id)) => { - let res = - self.expander.enter_expand_id::(self.db, call_id); - self.macro_calls.push((InFile::new(file_id, ast_id.upcast()), call_id)); - self.collect_macro_items(res); + self.macro_calls + .push((InFile::new(tree_id.file_id(), ast_id.upcast()), call_id)); + self.collect_macro_items(call_id); } Ok(None) => (), Err(_) => { self.diagnostics.push(DefDiagnostic::unresolved_macro_call( self.module_id.local_id, MacroCallKind::FnLike { - ast_id: InFile::new(file_id, ast_id), + ast_id: InFile::new(tree_id.file_id(), ast_id), expand_to, eager: None, }, @@ -310,16 +288,19 @@ impl<'a> AssocItemCollector<'a> { } } - fn collect_macro_items(&mut self, res: ExpandResult)>>) { - let Some((mark, _parse)) = res.value else { return }; + fn collect_macro_items(&mut self, macro_call_id: MacroCallId) { + if self.depth > self.def_map.recursion_limit() as usize { + tracing::warn!("macro expansion is too deep"); + return; + } + let file_id = macro_call_id.as_file(); + let tree_id = TreeId::new(file_id, None); + let item_tree = self.db.file_item_tree(file_id); - let tree_id = item_tree::TreeId::new(self.expander.current_file_id(), None); - let item_tree = tree_id.item_tree(self.db); - let iter: SmallVec<[_; 2]> = - item_tree.top_level_items().iter().filter_map(ModItem::as_assoc_item).collect(); - - self.collect(&item_tree, tree_id, &iter); - - self.expander.exit(mark); + self.depth += 1; + for item in item_tree.top_level_items().iter().filter_map(ModItem::as_assoc_item) { + self.collect_item(&item_tree, tree_id, item); + } + self.depth -= 1; } }