internal: remove erroneous default impl

Author: matklad

crates/hir_def/src/nameres.rs

@@ -200,20 +200,19 @@ pub struct ModuleData {
     pub origin: ModuleOrigin,
 }
 
-impl Default for ModuleData {
-    fn default() -> Self {
-        ModuleData::new(ModuleOrigin::CrateRoot { definition: FileId(!0) })
-    }
-}
-
 impl DefMap {
     pub(crate) fn crate_def_map_query(db: &dyn DefDatabase, krate: CrateId) -> Arc<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<ModuleData> = Arena::default();
-        let root = modules.alloc(ModuleData::default());
+        let root = modules.alloc(ModuleData::new(root_module_origin));
         DefMap {
             _c: Count::new(),
             block: None,

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<ast::BlockExpr>) {
         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()
@@ -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)
     }
 

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.