From 433888c4b2bec73d7d53475a2c881cc82179dd66 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Fri, 24 Jan 2025 15:12:48 +0100 Subject: [PATCH] Record the use tree index in glob imports --- crates/hir-def/src/import_map.rs | 4 +- crates/hir-def/src/item_scope.rs | 122 ++++++++++++------------ crates/hir-def/src/nameres/collector.rs | 75 +++++++++------ crates/hir-def/src/per_ns.rs | 2 +- crates/hir/src/symbols.rs | 4 +- 5 files changed, 112 insertions(+), 95 deletions(-) diff --git a/crates/hir-def/src/import_map.rs b/crates/hir-def/src/import_map.rs index bb780277ef..34635997bd 100644 --- a/crates/hir-def/src/import_map.rs +++ b/crates/hir-def/src/import_map.rs @@ -167,8 +167,8 @@ impl ImportMap { let attr_id = if let Some(import) = import { match import { ImportOrExternCrate::ExternCrate(id) => Some(id.into()), - ImportOrExternCrate::Import(id) => Some(id.import.into()), - ImportOrExternCrate::Glob(id) => Some(id.into()), + ImportOrExternCrate::Import(id) => Some(id.use_.into()), + ImportOrExternCrate::Glob(id) => Some(id.use_.into()), } } else { match item { diff --git a/crates/hir-def/src/item_scope.rs b/crates/hir-def/src/item_scope.rs index 0ece878b83..65a39c5656 100644 --- a/crates/hir-def/src/item_scope.rs +++ b/crates/hir-def/src/item_scope.rs @@ -31,7 +31,7 @@ pub struct PerNsGlobImports { #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] pub enum ImportOrExternCrate { - Glob(UseId), + Glob(GlobId), Import(ImportId), ExternCrate(ExternCrateId), } @@ -45,29 +45,41 @@ impl From for ImportOrExternCrate { } } -impl From for ImportOrExternCrate { - fn from(value: ImportType) -> Self { - match value { - ImportType::Glob(it) => ImportOrExternCrate::Glob(it), - ImportType::Import(it) => ImportOrExternCrate::Import(it), - ImportType::ExternCrate(it) => ImportOrExternCrate::ExternCrate(it), - } - } -} - impl ImportOrExternCrate { - pub fn into_import(self) -> Option { + pub fn import_or_glob(self) -> Option { match self { ImportOrExternCrate::Import(it) => Some(ImportOrGlob::Import(it)), ImportOrExternCrate::Glob(it) => Some(ImportOrGlob::Glob(it)), _ => None, } } + + pub fn import(self) -> Option { + match self { + ImportOrExternCrate::Import(it) => Some(it), + _ => None, + } + } + + pub fn glob(self) -> Option { + match self { + ImportOrExternCrate::Glob(id) => Some(id), + _ => None, + } + } + + pub fn use_(self) -> Option { + match self { + ImportOrExternCrate::Glob(id) => Some(id.use_), + ImportOrExternCrate::Import(id) => Some(id.use_), + _ => None, + } + } } #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] pub enum ImportOrGlob { - Glob(UseId), + Glob(GlobId), Import(ImportId), } @@ -79,17 +91,11 @@ impl ImportOrGlob { } } } -#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] -pub(crate) enum ImportType { - Import(ImportId), - Glob(UseId), - ExternCrate(ExternCrateId), -} #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] pub enum ImportOrDef { Import(ImportId), - Glob(UseId), + Glob(GlobId), ExternCrate(ExternCrateId), Def(ModuleDefId), } @@ -115,7 +121,13 @@ impl From for ImportOrDef { #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Ord, PartialOrd)] pub struct ImportId { - pub import: UseId, + pub use_: UseId, + pub idx: Idx, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Ord, PartialOrd)] +pub struct GlobId { + pub use_: UseId, pub idx: Idx, } @@ -236,7 +248,7 @@ impl ItemScope { self.use_imports_types .keys() .copied() - .filter_map(ImportOrExternCrate::into_import) + .filter_map(ImportOrExternCrate::import_or_glob) .chain(self.use_imports_values.keys().copied()) .chain(self.use_imports_macros.keys().copied()) .filter_map(ImportOrGlob::into_import) @@ -252,7 +264,7 @@ impl ItemScope { while let Some(&m) = scope.use_imports_macros.get(&ImportOrGlob::Import(import)) { match m { ImportOrDef::Import(i) => { - let module_id = i.import.lookup(db).container; + let module_id = i.use_.lookup(db).container; def_map = module_id.def_map(db); scope = &def_map[module_id.local_id].scope; import = i; @@ -268,7 +280,7 @@ impl ItemScope { while let Some(&m) = scope.use_imports_types.get(&ImportOrExternCrate::Import(import)) { match m { ImportOrDef::Import(i) => { - let module_id = i.import.lookup(db).container; + let module_id = i.use_.lookup(db).container; def_map = module_id.def_map(db); scope = &def_map[module_id.local_id].scope; import = i; @@ -284,7 +296,7 @@ impl ItemScope { while let Some(&m) = scope.use_imports_values.get(&ImportOrGlob::Import(import)) { match m { ImportOrDef::Import(i) => { - let module_id = i.import.lookup(db).container; + let module_id = i.use_.lookup(db).container; def_map = module_id.def_map(db); scope = &def_map[module_id.local_id].scope; import = i; @@ -545,9 +557,13 @@ impl ItemScope { self.unnamed_trait_imports.get(&tr).map(|trait_| trait_.vis) } - pub(crate) fn push_unnamed_trait(&mut self, tr: TraitId, vis: Visibility) { - // FIXME: import - self.unnamed_trait_imports.insert(tr, Item { def: (), vis, import: None }); + pub(crate) fn push_unnamed_trait( + &mut self, + tr: TraitId, + vis: Visibility, + import: Option, + ) { + self.unnamed_trait_imports.insert(tr, Item { def: (), vis, import }); } pub(crate) fn push_res_with_import( @@ -555,7 +571,7 @@ impl ItemScope { glob_imports: &mut PerNsGlobImports, lookup: (LocalModuleId, Name), def: PerNs, - import: Option, + import: Option, ) -> bool { let mut changed = false; @@ -566,12 +582,11 @@ impl ItemScope { match existing { Entry::Vacant(entry) => { match import { - Some(ImportType::Glob(_)) => { + Some(ImportOrExternCrate::Glob(_)) => { glob_imports.types.insert(lookup.clone()); } _ => _ = glob_imports.types.remove(&lookup), } - let import = import.map(Into::into); let prev = std::mem::replace(&mut fld.import, import); if let Some(import) = import { self.use_imports_types @@ -582,7 +597,7 @@ impl ItemScope { } Entry::Occupied(mut entry) => { match import { - Some(ImportType::Glob(..)) => { + Some(ImportOrExternCrate::Glob(..)) => { // Multiple globs may import the same item and they may // override visibility from previously resolved globs. This is // currently handled by `DefCollector`, because we need to @@ -591,7 +606,6 @@ impl ItemScope { } _ => { if glob_imports.types.remove(&lookup) { - let import = import.map(Into::into); let prev = std::mem::replace(&mut fld.import, import); if let Some(import) = import { self.use_imports_types.insert( @@ -614,16 +628,12 @@ impl ItemScope { match existing { Entry::Vacant(entry) => { match import { - Some(ImportType::Glob(_)) => { + Some(ImportOrExternCrate::Glob(_)) => { glob_imports.values.insert(lookup.clone()); } _ => _ = glob_imports.values.remove(&lookup), } - let import = match import { - Some(ImportType::Import(import)) => Some(ImportOrGlob::Import(import)), - Some(ImportType::Glob(u)) => Some(ImportOrGlob::Glob(u)), - _ => None, - }; + let import = import.and_then(ImportOrExternCrate::import_or_glob); let prev = std::mem::replace(&mut fld.import, import); if let Some(import) = import { self.use_imports_values @@ -632,15 +642,13 @@ impl ItemScope { entry.insert(fld); changed = true; } - Entry::Occupied(mut entry) if !matches!(import, Some(ImportType::Glob(..))) => { + Entry::Occupied(mut entry) + if !matches!(import, Some(ImportOrExternCrate::Glob(..))) => + { if glob_imports.values.remove(&lookup) { cov_mark::hit!(import_shadowed); - let import = match import { - Some(ImportType::Import(import)) => Some(ImportOrGlob::Import(import)), - Some(ImportType::Glob(u)) => Some(ImportOrGlob::Glob(u)), - _ => None, - }; + let import = import.and_then(ImportOrExternCrate::import_or_glob); let prev = std::mem::replace(&mut fld.import, import); if let Some(import) = import { self.use_imports_values @@ -659,16 +667,12 @@ impl ItemScope { match existing { Entry::Vacant(entry) => { match import { - Some(ImportType::Glob(_)) => { + Some(ImportOrExternCrate::Glob(_)) => { glob_imports.macros.insert(lookup.clone()); } _ => _ = glob_imports.macros.remove(&lookup), } - let import = match import { - Some(ImportType::Import(import)) => Some(ImportOrGlob::Import(import)), - Some(ImportType::Glob(u)) => Some(ImportOrGlob::Glob(u)), - _ => None, - }; + let import = import.and_then(ImportOrExternCrate::import_or_glob); let prev = std::mem::replace(&mut fld.import, import); if let Some(import) = import { self.use_imports_macros.insert( @@ -679,14 +683,12 @@ impl ItemScope { entry.insert(fld); changed = true; } - Entry::Occupied(mut entry) if !matches!(import, Some(ImportType::Glob(..))) => { + Entry::Occupied(mut entry) + if !matches!(import, Some(ImportOrExternCrate::Glob(..))) => + { if glob_imports.macros.remove(&lookup) { cov_mark::hit!(import_shadowed); - let import = match import { - Some(ImportType::Import(import)) => Some(ImportOrGlob::Import(import)), - Some(ImportType::Glob(u)) => Some(ImportOrGlob::Glob(u)), - _ => None, - }; + let import = import.and_then(ImportOrExternCrate::import_or_glob); let prev = std::mem::replace(&mut fld.import, import); if let Some(import) = import { self.use_imports_macros.insert( @@ -856,7 +858,7 @@ impl PerNs { match def { ModuleDefId::ModuleId(_) => PerNs::types(def, v, import), ModuleDefId::FunctionId(_) => { - PerNs::values(def, v, import.and_then(ImportOrExternCrate::into_import)) + PerNs::values(def, v, import.and_then(ImportOrExternCrate::import_or_glob)) } ModuleDefId::AdtId(adt) => match adt { AdtId::UnionId(_) => PerNs::types(def, v, import), @@ -871,14 +873,14 @@ impl PerNs { }, ModuleDefId::EnumVariantId(_) => PerNs::both(def, def, v, import), ModuleDefId::ConstId(_) | ModuleDefId::StaticId(_) => { - PerNs::values(def, v, import.and_then(ImportOrExternCrate::into_import)) + PerNs::values(def, v, import.and_then(ImportOrExternCrate::import_or_glob)) } ModuleDefId::TraitId(_) => PerNs::types(def, v, import), ModuleDefId::TraitAliasId(_) => PerNs::types(def, v, import), ModuleDefId::TypeAliasId(_) => PerNs::types(def, v, import), ModuleDefId::BuiltinType(_) => PerNs::types(def, v, import), ModuleDefId::MacroId(mac) => { - PerNs::macros(mac, v, import.and_then(ImportOrExternCrate::into_import)) + PerNs::macros(mac, v, import.and_then(ImportOrExternCrate::import_or_glob)) } } } diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs index 5058d15f34..06276335b7 100644 --- a/crates/hir-def/src/nameres/collector.rs +++ b/crates/hir-def/src/nameres/collector.rs @@ -28,7 +28,7 @@ use triomphe::Arc; use crate::{ attr::Attrs, db::DefDatabase, - item_scope::{ImportId, ImportOrExternCrate, ImportOrGlob, ImportType, PerNsGlobImports}, + item_scope::{GlobId, ImportId, ImportOrExternCrate, PerNsGlobImports}, item_tree::{ self, AttrOwner, FieldsShape, FileItemTreeId, ImportKind, ItemTree, ItemTreeId, ItemTreeNode, Macro2, MacroCall, MacroRules, Mod, ModItem, ModKind, TreeId, UseTreeKind, @@ -208,7 +208,7 @@ struct DefCollector<'a> { def_map: DefMap, // The dependencies of the current crate, including optional deps like `test`. deps: FxHashMap, - glob_imports: FxHashMap>, + glob_imports: FxHashMap>, unresolved_imports: Vec, indeterminate_imports: Vec<(ImportDirective, PerNs)>, unresolved_macros: Vec, @@ -524,14 +524,7 @@ impl DefCollector<'_> { match per_ns.types { Some(Item { def: ModuleDefId::ModuleId(m), import, .. }) => { - // FIXME: This should specifically look for a glob import somehow and record that here - self.def_map.prelude = Some(( - m, - import - .and_then(ImportOrExternCrate::into_import) - .and_then(ImportOrGlob::into_import) - .map(|it| it.import), - )); + self.def_map.prelude = Some((m, import.and_then(ImportOrExternCrate::use_))); } types => { tracing::debug!( @@ -848,13 +841,14 @@ impl DefCollector<'_> { def.values = None; def.macros = None; } - let imp = ImportType::Import(ImportId { import: id, idx: use_tree }); + let imp = ImportOrExternCrate::Import(ImportId { use_: id, idx: use_tree }); tracing::debug!("resolved import {:?} ({:?}) to {:?}", name, import, def); self.update(module_id, &[(name.cloned(), def)], vis, Some(imp)); } - ImportSource { kind: ImportKind::Glob, id, is_prelude, .. } => { + ImportSource { kind: ImportKind::Glob, id, is_prelude, use_tree } => { tracing::debug!("glob import: {:?}", import); + let glob = GlobId { use_: id, idx: use_tree }; match def.take_types() { Some(ModuleDefId::ModuleId(m)) => { if is_prelude { @@ -878,7 +872,12 @@ impl DefCollector<'_> { .filter(|(_, res)| !res.is_none()) .collect::>(); - self.update(module_id, &items, vis, Some(ImportType::Glob(id))); + self.update( + module_id, + &items, + vis, + Some(ImportOrExternCrate::Glob(glob)), + ); } else { // glob import from same crate => we do an initial // import, and then need to propagate any further @@ -910,11 +909,16 @@ impl DefCollector<'_> { .filter(|(_, res)| !res.is_none()) .collect::>(); - self.update(module_id, &items, vis, Some(ImportType::Glob(id))); + self.update( + module_id, + &items, + vis, + Some(ImportOrExternCrate::Glob(glob)), + ); // record the glob import in case we add further items - let glob = self.glob_imports.entry(m.local_id).or_default(); - match glob.iter_mut().find(|(mid, _, _)| *mid == module_id) { - None => glob.push((module_id, vis, id)), + let glob_imports = self.glob_imports.entry(m.local_id).or_default(); + match glob_imports.iter_mut().find(|(mid, _, _)| *mid == module_id) { + None => glob_imports.push((module_id, vis, glob)), Some((_, old_vis, _)) => { if let Some(new_vis) = old_vis.max(vis, &self.def_map) { *old_vis = new_vis; @@ -947,7 +951,12 @@ impl DefCollector<'_> { (Some(name), res) }) .collect::>(); - self.update(module_id, &resolutions, vis, Some(ImportType::Glob(id))); + self.update( + module_id, + &resolutions, + vis, + Some(ImportOrExternCrate::Glob(glob)), + ); } Some(d) => { tracing::debug!("glob import {:?} from non-module/enum {:?}", import, d); @@ -967,7 +976,7 @@ impl DefCollector<'_> { resolutions: &[(Option, PerNs)], // Visibility this import will have vis: Visibility, - import: Option, + import: Option, ) { self.db.unwind_if_cancelled(); self.update_recursive(module_id, resolutions, vis, import, 0) @@ -981,7 +990,7 @@ impl DefCollector<'_> { // All resolutions are imported with this visibility; the visibilities in // the `PerNs` values are ignored and overwritten vis: Visibility, - import: Option, + import: Option, depth: usize, ) { if GLOB_RECURSION_LIMIT.check(depth).is_err() { @@ -997,8 +1006,10 @@ impl DefCollector<'_> { self.push_res_and_update_glob_vis(module_id, name, *res, vis, import); } None => { - let tr = match res.take_types() { - Some(ModuleDefId::TraitId(tr)) => tr, + let (tr, import) = match res.take_types_full() { + Some(Item { def: ModuleDefId::TraitId(tr), vis: _, import }) => { + (tr, import) + } Some(other) => { tracing::debug!("non-trait `_` import of {:?}", other); continue; @@ -1024,7 +1035,11 @@ impl DefCollector<'_> { if should_update { changed = true; - self.def_map.modules[module_id].scope.push_unnamed_trait(tr, vis); + self.def_map.modules[module_id].scope.push_unnamed_trait( + tr, + vis, + import.and_then(ImportOrExternCrate::import), + ); } } } @@ -1046,13 +1061,13 @@ impl DefCollector<'_> { .cloned() .collect::>(); - for (glob_importing_module, glob_import_vis, use_) in glob_imports { + for (glob_importing_module, glob_import_vis, glob) in glob_imports { let vis = glob_import_vis.min(vis, &self.def_map).unwrap_or(glob_import_vis); self.update_recursive( glob_importing_module, resolutions, vis, - Some(ImportType::Glob(use_)), + Some(ImportOrExternCrate::Glob(glob)), depth + 1, ); } @@ -1064,7 +1079,7 @@ impl DefCollector<'_> { name: &Name, mut defs: PerNs, vis: Visibility, - def_import_type: Option, + def_import_type: Option, ) -> bool { // `extern crate crate_name` things can be re-exported as `pub use crate_name`. // But they cannot be re-exported as `pub use self::crate_name`, `pub use crate::crate_name` @@ -1077,10 +1092,10 @@ impl DefCollector<'_> { let Some(ImportOrExternCrate::ExternCrate(_)) = def.import else { return false; }; - let Some(ImportType::Import(id)) = def_import_type else { + let Some(ImportOrExternCrate::Import(id)) = def_import_type else { return false; }; - let use_id = id.import.lookup(self.db).id; + let use_id = id.use_.lookup(self.db).id; let item_tree = use_id.item_tree(self.db); let use_kind = item_tree[use_id.value].use_tree.kind(); let UseTreeKind::Single { path, .. } = use_kind else { @@ -1103,7 +1118,7 @@ impl DefCollector<'_> { let mut changed = false; - if let Some(ImportType::Glob(_)) = def_import_type { + if let Some(ImportOrExternCrate::Glob(_)) = def_import_type { let prev_defs = self.def_map[module_id].scope.get(name); // Multiple globs may import the same item and they may override visibility from @@ -1730,7 +1745,7 @@ impl ModCollector<'_, '_> { ), )], vis, - Some(ImportType::ExternCrate(id)), + Some(ImportOrExternCrate::ExternCrate(id)), ); } else { if let Some(name) = name { diff --git a/crates/hir-def/src/per_ns.rs b/crates/hir-def/src/per_ns.rs index 339786fc01..c2d3f67f17 100644 --- a/crates/hir-def/src/per_ns.rs +++ b/crates/hir-def/src/per_ns.rs @@ -78,7 +78,7 @@ impl PerNs { values: Some(Item { def: values, vis, - import: import.and_then(ImportOrExternCrate::into_import), + import: import.and_then(ImportOrExternCrate::import_or_glob), }), macros: None, } diff --git a/crates/hir/src/symbols.rs b/crates/hir/src/symbols.rs index f379a2d4d6..c1c45d8175 100644 --- a/crates/hir/src/symbols.rs +++ b/crates/hir/src/symbols.rs @@ -160,8 +160,8 @@ impl<'a> SymbolCollector<'a> { let mut push_import = |this: &mut Self, i: ImportId, name: &Name, def: ModuleDefId| { let source = import_child_source_cache - .entry(i.import) - .or_insert_with(|| i.import.child_source(this.db.upcast())); + .entry(i.use_) + .or_insert_with(|| i.use_.child_source(this.db.upcast())); let Some(use_tree_src) = source.value.get(i.idx) else { return }; let Some(name_ptr) = use_tree_src .rename()