From 5c241b07666bc7b29e97b8206e505944775266a0 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Mon, 25 Jan 2021 15:21:33 +0100 Subject: [PATCH] Create all `ModuleId`s through a `DefMap` method `ModuleId` needs to be able to represent blocks, and only the associated `DefMap` will know how to construct that `ModuleId` --- crates/hir/src/code_model.rs | 27 +++++++------------ crates/hir/src/semantics/source_to_def.rs | 10 +++---- crates/hir_def/src/find_path.rs | 27 ++++++------------- crates/hir_def/src/import_map.rs | 2 +- crates/hir_def/src/lib.rs | 4 --- crates/hir_def/src/nameres.rs | 4 +++ crates/hir_def/src/nameres/collector.rs | 27 +++++++------------ crates/hir_def/src/nameres/path_resolution.rs | 24 +++++------------ crates/hir_def/src/resolver.rs | 2 +- crates/hir_def/src/test_db.rs | 6 ++--- crates/hir_ty/src/test_db.rs | 2 +- 11 files changed, 48 insertions(+), 87 deletions(-) diff --git a/crates/hir/src/code_model.rs b/crates/hir/src/code_model.rs index d9b4cdfcef..e9bb4f5416 100644 --- a/crates/hir/src/code_model.rs +++ b/crates/hir/src/code_model.rs @@ -18,8 +18,8 @@ use hir_def::{ type_ref::{Mutability, TypeRef}, AdtId, AssocContainerId, AssocItemId, AssocItemLoc, AttrDefId, ConstId, ConstParamId, DefWithBodyId, EnumId, FunctionId, GenericDefId, HasModule, ImplId, LifetimeParamId, - LocalEnumVariantId, LocalFieldId, LocalModuleId, Lookup, ModuleId, StaticId, StructId, TraitId, - TypeAliasId, TypeParamId, UnionId, + LocalEnumVariantId, LocalFieldId, Lookup, ModuleId, StaticId, StructId, TraitId, TypeAliasId, + TypeParamId, UnionId, }; use hir_def::{find_path::PrefixKind, item_scope::ItemInNs, visibility::Visibility}; use hir_expand::{ @@ -90,8 +90,8 @@ impl Crate { } pub fn root_module(self, db: &dyn HirDatabase) -> Module { - let module_id = db.crate_def_map(self.id).root(); - Module::new(self, module_id) + let def_map = db.crate_def_map(self.id); + Module { id: def_map.module_id(def_map.root()) } } pub fn root_file(self, db: &dyn HirDatabase) -> FileId { @@ -275,10 +275,6 @@ impl ModuleDef { } impl Module { - pub(crate) fn new(krate: Crate, crate_module_id: LocalModuleId) -> Module { - Module { id: ModuleId::top_level(krate.id, crate_module_id) } - } - /// Name of this module. pub fn name(self, db: &dyn HirDatabase) -> Option { let def_map = self.id.def_map(db.upcast()); @@ -302,7 +298,7 @@ impl Module { /// in the module tree of any target in `Cargo.toml`. pub fn crate_root(self, db: &dyn HirDatabase) -> Module { let def_map = db.crate_def_map(self.id.krate()); - self.with_module_id(def_map.root()) + Module { id: def_map.module_id(def_map.root()) } } /// Iterates over all child modules. @@ -311,7 +307,7 @@ impl Module { let children = def_map[self.id.local_id] .children .iter() - .map(|(_, module_id)| self.with_module_id(*module_id)) + .map(|(_, module_id)| Module { id: def_map.module_id(*module_id) }) .collect::>(); children.into_iter() } @@ -321,7 +317,7 @@ impl Module { // FIXME: handle block expressions as modules (their parent is in a different DefMap) let def_map = self.id.def_map(db.upcast()); let parent_id = def_map[self.id.local_id].parent?; - Some(self.with_module_id(parent_id)) + Some(Module { id: def_map.module_id(parent_id) }) } pub fn path_to_root(self, db: &dyn HirDatabase) -> Vec { @@ -406,10 +402,6 @@ impl Module { def_map[self.id.local_id].scope.impls().map(Impl::from).collect() } - pub(crate) fn with_module_id(self, module_id: LocalModuleId) -> Module { - Module::new(self.krate(), module_id) - } - /// Finds a path that can be used to refer to the given item from within /// this module, if possible. pub fn find_use_path(self, db: &dyn DefDatabase, item: impl Into) -> Option { @@ -1013,8 +1005,9 @@ impl MacroDef { /// early, in `hir_expand`, where modules simply do not exist yet. pub fn module(self, db: &dyn HirDatabase) -> Option { let krate = self.id.krate; - let module_id = db.crate_def_map(krate).root(); - Some(Module::new(Crate { id: krate }, module_id)) + let def_map = db.crate_def_map(krate); + let module_id = def_map.root(); + Some(Module { id: def_map.module_id(module_id) }) } /// XXX: this parses the file diff --git a/crates/hir/src/semantics/source_to_def.rs b/crates/hir/src/semantics/source_to_def.rs index faede32699..6c612ee863 100644 --- a/crates/hir/src/semantics/source_to_def.rs +++ b/crates/hir/src/semantics/source_to_def.rs @@ -30,13 +30,12 @@ pub(super) struct SourceToDefCtx<'a, 'b> { impl SourceToDefCtx<'_, '_> { pub(super) fn file_to_def(&mut self, file: FileId) -> Option { let _p = profile::span("SourceBinder::to_module_def"); - let (krate, local_id) = self.db.relevant_crates(file).iter().find_map(|&crate_id| { + self.db.relevant_crates(file).iter().find_map(|&crate_id| { // FIXME: inner items let crate_def_map = self.db.crate_def_map(crate_id); let local_id = crate_def_map.modules_for_file(file).next()?; - Some((crate_id, local_id)) - })?; - Some(ModuleId::top_level(krate, local_id)) + Some(crate_def_map.module_id(local_id)) + }) } pub(super) fn module_to_def(&mut self, src: InFile) -> Option { @@ -63,8 +62,7 @@ impl SourceToDefCtx<'_, '_> { let child_name = src.value.name()?.as_name(); let def_map = parent_module.def_map(self.db.upcast()); let child_id = *def_map[parent_module.local_id].children.get(&child_name)?; - // FIXME: handle block expression modules - Some(ModuleId::top_level(parent_module.krate(), child_id)) + Some(def_map.module_id(child_id)) } pub(super) fn trait_to_def(&mut self, src: InFile) -> Option { diff --git a/crates/hir_def/src/find_path.rs b/crates/hir_def/src/find_path.rs index c01b6daf2c..94a1d567d8 100644 --- a/crates/hir_def/src/find_path.rs +++ b/crates/hir_def/src/find_path.rs @@ -53,12 +53,8 @@ fn check_self_super(def_map: &DefMap, item: ItemInNs, from: ModuleId) -> Option< Some(ModPath::from_segments(PathKind::Super(0), Vec::new())) } else if let Some(parent_id) = def_map[from.local_id].parent { // - if the item is the parent module, use `super` (this is not used recursively, since `super::super` is ugly) - if item - == ItemInNs::Types(ModuleDefId::ModuleId(ModuleId { - krate: from.krate, - local_id: parent_id, - })) - { + let parent_id = def_map.module_id(parent_id); + if item == ItemInNs::Types(ModuleDefId::ModuleId(parent_id)) { Some(ModPath::from_segments(PathKind::Super(1), Vec::new())) } else { None @@ -120,12 +116,8 @@ fn find_path_inner( } // - if the item is the crate root, return `crate` - if item - == ItemInNs::Types(ModuleDefId::ModuleId(ModuleId { - krate: from.krate, - local_id: def_map.root(), - })) - { + let root = def_map.module_id(def_map.root()); + if item == ItemInNs::Types(ModuleDefId::ModuleId(root)) { return Some(ModPath::from_segments(PathKind::Crate, Vec::new())); } @@ -175,7 +167,7 @@ fn find_path_inner( // - otherwise, look for modules containing (reexporting) it and import it from one of those - let crate_root = ModuleId { local_id: def_map.root(), krate: from.krate }; + let crate_root = def_map.module_id(def_map.root()); let crate_attrs = db.attrs(crate_root.into()); let prefer_no_std = crate_attrs.by_key("no_std").exists(); let mut best_path = None; @@ -288,14 +280,11 @@ fn find_local_import_locations( // Compute the initial worklist. We start with all direct child modules of `from` as well as all // of its (recursive) parent modules. let data = &def_map[from.local_id]; - let mut worklist = data - .children - .values() - .map(|child| ModuleId { krate: from.krate, local_id: *child }) - .collect::>(); + let mut worklist = + data.children.values().map(|child| def_map.module_id(*child)).collect::>(); let mut parent = data.parent; while let Some(p) = parent { - worklist.push(ModuleId { krate: from.krate, local_id: p }); + worklist.push(def_map.module_id(p)); parent = def_map[p].parent; } diff --git a/crates/hir_def/src/import_map.rs b/crates/hir_def/src/import_map.rs index 0b78304456..0a3dc79564 100644 --- a/crates/hir_def/src/import_map.rs +++ b/crates/hir_def/src/import_map.rs @@ -75,7 +75,7 @@ impl ImportMap { // We look only into modules that are public(ly reexported), starting with the crate root. let empty = ImportPath { segments: vec![] }; - let root = ModuleId { krate, local_id: def_map.root() }; + let root = def_map.module_id(def_map.root()); let mut worklist = vec![(root, empty)]; while let Some((module, mod_path)) = worklist.pop() { let ext_def_map; diff --git a/crates/hir_def/src/lib.rs b/crates/hir_def/src/lib.rs index c8dbb2aeb3..cf09ebd3f0 100644 --- a/crates/hir_def/src/lib.rs +++ b/crates/hir_def/src/lib.rs @@ -78,10 +78,6 @@ pub struct ModuleId { } impl ModuleId { - pub fn top_level(krate: CrateId, local_id: LocalModuleId) -> Self { - Self { krate, local_id } - } - pub fn def_map(&self, db: &dyn db::DefDatabase) -> Arc { db.crate_def_map(self.krate) } diff --git a/crates/hir_def/src/nameres.rs b/crates/hir_def/src/nameres.rs index bd3ea9b8b1..4fbbecb386 100644 --- a/crates/hir_def/src/nameres.rs +++ b/crates/hir_def/src/nameres.rs @@ -265,6 +265,10 @@ impl DefMap { self.extern_prelude.iter() } + pub fn module_id(&self, local_id: LocalModuleId) -> ModuleId { + ModuleId { krate: self.krate, local_id } + } + pub(crate) fn resolve_path( &self, db: &dyn DefDatabase, diff --git a/crates/hir_def/src/nameres/collector.rs b/crates/hir_def/src/nameres/collector.rs index adfcf879a4..393170b32f 100644 --- a/crates/hir_def/src/nameres/collector.rs +++ b/crates/hir_def/src/nameres/collector.rs @@ -37,8 +37,8 @@ use crate::{ per_ns::PerNs, visibility::{RawVisibility, Visibility}, AdtId, AsMacroCall, AstId, AstIdWithPath, ConstLoc, ContainerId, EnumLoc, EnumVariantId, - FunctionLoc, ImplLoc, Intern, LocalModuleId, ModuleDefId, ModuleId, StaticLoc, StructLoc, - TraitLoc, TypeAliasLoc, UnionLoc, + FunctionLoc, ImplLoc, Intern, LocalModuleId, ModuleDefId, StaticLoc, StructLoc, TraitLoc, + TypeAliasLoc, UnionLoc, }; const GLOB_RECURSION_LIMIT: usize = 100; @@ -56,10 +56,9 @@ pub(super) fn collect_defs( for dep in &crate_graph[def_map.krate].dependencies { log::debug!("crate dep {:?} -> {:?}", dep.name, dep.crate_id); let dep_def_map = db.crate_def_map(dep.crate_id); - def_map.extern_prelude.insert( - dep.as_name(), - ModuleId { krate: dep.crate_id, local_id: dep_def_map.root }.into(), - ); + def_map + .extern_prelude + .insert(dep.as_name(), dep_def_map.module_id(dep_def_map.root).into()); // look for the prelude // If the dependency defines a prelude, we overwrite an already defined @@ -332,11 +331,9 @@ impl DefCollector<'_> { // exported in type/value namespace. This function reduces the visibility of all items // in the crate root that aren't proc macros. let root = self.def_map.root; + let module_id = self.def_map.module_id(root); let root = &mut self.def_map.modules[root]; - root.scope.censor_non_proc_macros(ModuleId { - krate: self.def_map.krate, - local_id: self.def_map.root, - }); + root.scope.censor_non_proc_macros(module_id); } } @@ -1029,8 +1026,7 @@ impl ModCollector<'_, '_> { continue; } } - let module = - ModuleId { krate: self.def_collector.def_map.krate, local_id: self.module_id }; + let module = self.def_collector.def_map.module_id(self.module_id); let container = ContainerId::ModuleId(module); let mut def = None; @@ -1097,10 +1093,7 @@ impl ModCollector<'_, '_> { } } ModItem::Impl(imp) => { - let module = ModuleId { - krate: self.def_collector.def_map.krate, - local_id: self.module_id, - }; + let module = self.def_collector.def_map.module_id(self.module_id); let container = ContainerId::ModuleId(module); let impl_id = ImplLoc { container, id: ItemTreeId::new(self.file_id, imp) } .intern(self.def_collector.db); @@ -1343,7 +1336,7 @@ impl ModCollector<'_, '_> { modules[res].scope.define_legacy_macro(name, mac) } modules[self.module_id].children.insert(name.clone(), res); - let module = ModuleId { krate: self.def_collector.def_map.krate, local_id: res }; + let module = self.def_collector.def_map.module_id(res); let def: ModuleDefId = module.into(); self.def_collector.def_map.modules[self.module_id].scope.define_def(def); self.def_collector.update( diff --git a/crates/hir_def/src/nameres/path_resolution.rs b/crates/hir_def/src/nameres/path_resolution.rs index 7b5fe24a7c..c1eded5f2b 100644 --- a/crates/hir_def/src/nameres/path_resolution.rs +++ b/crates/hir_def/src/nameres/path_resolution.rs @@ -24,7 +24,7 @@ use crate::{ path::{ModPath, PathKind}, per_ns::PerNs, visibility::{RawVisibility, Visibility}, - AdtId, CrateId, EnumVariantId, LocalModuleId, ModuleDefId, ModuleId, + AdtId, CrateId, EnumVariantId, LocalModuleId, ModuleDefId, }; #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -66,10 +66,7 @@ impl DefMap { pub(super) fn resolve_name_in_extern_prelude(&self, name: &Name) -> PerNs { if name == &name!(self) { mark::hit!(extern_crate_self_as); - return PerNs::types( - ModuleId { krate: self.krate, local_id: self.root }.into(), - Visibility::Public, - ); + return PerNs::types(self.module_id(self.root).into(), Visibility::Public); } self.extern_prelude .get(name) @@ -154,21 +151,15 @@ impl DefMap { PathKind::DollarCrate(krate) => { if krate == self.krate { mark::hit!(macro_dollar_crate_self); - PerNs::types( - ModuleId { krate: self.krate, local_id: self.root }.into(), - Visibility::Public, - ) + PerNs::types(self.module_id(self.root).into(), Visibility::Public) } else { let def_map = db.crate_def_map(krate); - let module = ModuleId { krate, local_id: def_map.root }; + let module = def_map.module_id(def_map.root); mark::hit!(macro_dollar_crate_other); PerNs::types(module.into(), Visibility::Public) } } - PathKind::Crate => PerNs::types( - ModuleId { krate: self.krate, local_id: self.root }.into(), - Visibility::Public, - ), + PathKind::Crate => PerNs::types(self.module_id(self.root).into(), Visibility::Public), // plain import or absolute path in 2015: crate-relative with // fallback to extern prelude (with the simplification in // rust-lang/rust#57745) @@ -205,10 +196,7 @@ impl DefMap { let m = successors(Some(original_module), |m| self.modules[*m].parent) .nth(lvl as usize); if let Some(local_id) = m { - PerNs::types( - ModuleId { krate: self.krate, local_id }.into(), - Visibility::Public, - ) + PerNs::types(self.module_id(local_id).into(), Visibility::Public) } else { log::debug!("super path in root module"); return ResolvePathResult::empty(ReachedFixedPoint::Yes); diff --git a/crates/hir_def/src/resolver.rs b/crates/hir_def/src/resolver.rs index 130c074f08..9021ea7125 100644 --- a/crates/hir_def/src/resolver.rs +++ b/crates/hir_def/src/resolver.rs @@ -459,7 +459,7 @@ impl Resolver { pub fn module(&self) -> Option { let (def_map, local_id) = self.module_scope()?; - Some(ModuleId { krate: def_map.krate(), local_id }) + Some(def_map.module_id(local_id)) } pub fn krate(&self) -> Option { diff --git a/crates/hir_def/src/test_db.rs b/crates/hir_def/src/test_db.rs index 4ff219fb70..c4e36eda5e 100644 --- a/crates/hir_def/src/test_db.rs +++ b/crates/hir_def/src/test_db.rs @@ -15,7 +15,7 @@ use rustc_hash::FxHashSet; use syntax::{TextRange, TextSize}; use test_utils::extract_annotations; -use crate::{db::DefDatabase, ModuleDefId}; +use crate::{db::DefDatabase, ModuleDefId, ModuleId}; #[salsa::database( base_db::SourceDatabaseExtStorage, @@ -72,12 +72,12 @@ impl FileLoader for TestDB { } impl TestDB { - pub(crate) fn module_for_file(&self, file_id: FileId) -> crate::ModuleId { + pub(crate) fn module_for_file(&self, file_id: FileId) -> ModuleId { for &krate in self.relevant_crates(file_id).iter() { let crate_def_map = self.crate_def_map(krate); for (local_id, data) in crate_def_map.modules() { if data.origin.file_id() == Some(file_id) { - return crate::ModuleId { krate, local_id }; + return crate_def_map.module_id(local_id); } } } diff --git a/crates/hir_ty/src/test_db.rs b/crates/hir_ty/src/test_db.rs index 09696fcf46..381b98ba80 100644 --- a/crates/hir_ty/src/test_db.rs +++ b/crates/hir_ty/src/test_db.rs @@ -83,7 +83,7 @@ impl TestDB { let crate_def_map = self.crate_def_map(krate); for (local_id, data) in crate_def_map.modules() { if data.origin.file_id() == Some(file_id) { - return ModuleId::top_level(krate, local_id); + return crate_def_map.module_id(local_id); } } }