diff --git a/crates/hir_def/src/nameres.rs b/crates/hir_def/src/nameres.rs index 9e6aa96075b9..c5f8d45d68b3 100644 --- a/crates/hir_def/src/nameres.rs +++ b/crates/hir_def/src/nameres.rs @@ -145,12 +145,6 @@ pub enum ModuleOrigin { }, } -impl Default for ModuleOrigin { - fn default() -> Self { - ModuleOrigin::CrateRoot { definition: FileId(0) } - } -} - impl ModuleOrigin { fn declaration(&self) -> Option> { match self { @@ -196,7 +190,7 @@ impl ModuleOrigin { } } -#[derive(Default, Debug, PartialEq, Eq)] +#[derive(Debug, PartialEq, Eq)] pub struct ModuleData { pub parent: Option, pub children: FxHashMap, @@ -211,9 +205,14 @@ impl DefMap { let _p = profile::span("crate_def_map_query").detail(|| { db.crate_graph()[krate].display_name.as_deref().unwrap_or_default().to_string() }); - let edition = db.crate_graph()[krate].edition; - let def_map = DefMap::empty(krate, edition); + + let crate_graph = db.crate_graph(); + + let edition = crate_graph[krate].edition; + let origin = ModuleOrigin::CrateRoot { definition: crate_graph[krate].root_file_id }; + let def_map = DefMap::empty(krate, edition, origin); let def_map = collector::collect_defs(db, def_map, None); + Arc::new(def_map) } @@ -231,16 +230,20 @@ impl DefMap { let block_info = BlockInfo { block: block_id, parent: block.module }; let parent_map = block.module.def_map(db); - let mut def_map = DefMap::empty(block.module.krate, parent_map.edition); + let mut def_map = DefMap::empty( + block.module.krate, + parent_map.edition, + ModuleOrigin::BlockExpr { block: block.ast_id }, + ); def_map.block = Some(block_info); let def_map = collector::collect_defs(db, def_map, Some(block.ast_id)); Some(Arc::new(def_map)) } - fn empty(krate: CrateId, edition: Edition) -> DefMap { + fn empty(krate: CrateId, edition: Edition, root_module_origin: ModuleOrigin) -> DefMap { let mut modules: Arena = Arena::default(); - let root = modules.alloc(ModuleData::default()); + let root = modules.alloc(ModuleData::new(root_module_origin)); DefMap { _c: Count::new(), block: None, @@ -445,6 +448,15 @@ impl DefMap { } impl ModuleData { + pub(crate) fn new(origin: ModuleOrigin) -> Self { + ModuleData { + parent: None, + children: FxHashMap::default(), + scope: ItemScope::default(), + origin, + } + } + /// Returns a node which defines this module. That is, a file or a `mod foo {}` with items. pub fn definition_source(&self, db: &dyn DefDatabase) -> InFile { self.origin.definition_source(db) diff --git a/crates/hir_def/src/nameres/collector.rs b/crates/hir_def/src/nameres/collector.rs index f4f481f4dc04..dd31365a80db 100644 --- a/crates/hir_def/src/nameres/collector.rs +++ b/crates/hir_def/src/nameres/collector.rs @@ -272,7 +272,6 @@ impl DefCollector<'_> { let file_id = self.db.crate_graph()[self.def_map.krate].root_file_id; let item_tree = self.db.file_item_tree(file_id.into()); let module_id = self.def_map.root; - self.def_map.modules[module_id].origin = ModuleOrigin::CrateRoot { definition: file_id }; let attrs = item_tree.top_level_attrs(self.db, self.def_map.krate); if attrs.cfg().map_or(true, |cfg| self.cfg_options.check(&cfg) != Some(false)) { @@ -323,7 +322,6 @@ impl DefCollector<'_> { fn seed_with_inner(&mut self, block: AstId) { let item_tree = self.db.file_item_tree(block.file_id); let module_id = self.def_map.root; - self.def_map.modules[module_id].origin = ModuleOrigin::BlockExpr { block }; if item_tree .top_level_attrs(self.db, self.def_map.krate) .cfg() @@ -1625,14 +1623,14 @@ impl ModCollector<'_, '_> { .resolve_visibility(self.def_collector.db, self.module_id, visibility) .unwrap_or(Visibility::Public); let modules = &mut self.def_collector.def_map.modules; - let res = modules.alloc(ModuleData::default()); - modules[res].parent = Some(self.module_id); - modules[res].origin = match definition { + let origin = match definition { None => ModuleOrigin::Inline { definition: declaration }, Some((definition, is_mod_rs)) => { ModuleOrigin::File { declaration, definition, is_mod_rs } } }; + let res = modules.alloc(ModuleData::new(origin)); + modules[res].parent = Some(self.module_id); for (name, mac) in modules[self.module_id].scope.collect_legacy_macros() { modules[res].scope.define_legacy_macro(name, mac) } @@ -2005,11 +2003,12 @@ mod tests { } fn do_resolve(not_ra_fixture: &str) -> DefMap { - let (db, _file_id) = TestDB::with_single_file(not_ra_fixture); + let (db, file_id) = TestDB::with_single_file(not_ra_fixture); let krate = db.test_crate(); let edition = db.crate_graph()[krate].edition; - let def_map = DefMap::empty(krate, edition); + let module_origin = ModuleOrigin::CrateRoot { definition: file_id }; + let def_map = DefMap::empty(krate, edition, module_origin); do_collect_defs(&db, def_map) } diff --git a/docs/dev/style.md b/docs/dev/style.md index 696a175a8477..7e8e41a41788 100644 --- a/docs/dev/style.md +++ b/docs/dev/style.md @@ -312,7 +312,7 @@ Even when generality is not required, consistency is important. ## Constructors -Prefer `Default` to zero-argument `new` function +Prefer `Default` to zero-argument `new` function. ```rust // GOOD @@ -341,6 +341,10 @@ Use `Vec::new` rather than `vec![]`. **Rationale:** uniformity, strength reduction. +Avoid using "dummy" states to implement a `Default`. +If a type doesn't have a sensible default, empty value, don't hide it. +Let the caller explicitly decide what's the right initial state is. + ## Functions Over Objects Avoid creating "doer" objects.