internal: Make block_def_map infallible

Author: Veykril

crates/hir-def/src/body.rs

@@ -461,9 +461,7 @@ impl Body {
         &'a self,
         db: &'a dyn DefDatabase,
     ) -> impl Iterator<Item = (BlockId, Arc<DefMap>)> + '_ {
-        self.block_scopes
-            .iter()
-            .map(move |&block| (block, db.block_def_map(block).expect("block ID without DefMap")))
+        self.block_scopes.iter().map(move |&block| (block, db.block_def_map(block)))
     }
 
     pub fn pretty_print(&self, db: &dyn DefDatabase, owner: DefWithBodyId) -> String {

crates/hir-def/src/body/lower.rs

@@ -948,15 +948,14 @@ impl ExprCollector<'_> {
             None
         };
 
-        let (module, def_map) = match block_id
-            .and_then(|block_id| self.db.block_def_map(block_id).zip(Some(block_id)))
-        {
-            Some((def_map, block_id)) => {
-                self.body.block_scopes.push(block_id);
-                (def_map.root(), def_map)
-            }
-            None => (self.expander.module, self.expander.def_map.clone()),
-        };
+        let (module, def_map) =
+            match block_id.map(|block_id| (self.db.block_def_map(block_id), block_id)) {
+                Some((def_map, block_id)) => {
+                    self.body.block_scopes.push(block_id);
+                    (def_map.root(), def_map)
+                }
+                None => (self.expander.module, self.expander.def_map.clone()),
+            };
         let prev_def_map = mem::replace(&mut self.expander.def_map, def_map);
         let prev_local_module = mem::replace(&mut self.expander.module, module);
 

crates/hir-def/src/db.rs

@@ -96,7 +96,7 @@ pub trait DefDatabase: InternDatabase + ExpandDatabase + Upcast<dyn ExpandDataba
     // FIXME: This actually can't return None anymore as we no longer allocate block scopes for
     // non item declaring blocks
     #[salsa::invoke(DefMap::block_def_map_query)]
-    fn block_def_map(&self, block: BlockId) -> Option<Arc<DefMap>>;
+    fn block_def_map(&self, block: BlockId) -> Arc<DefMap>;
 
     #[salsa::invoke(StructData::struct_data_query)]
     fn struct_data(&self, id: StructId) -> Arc<StructData>;

crates/hir-def/src/lib.rs

@@ -101,13 +101,7 @@ pub struct ModuleId {
 impl ModuleId {
     pub fn def_map(&self, db: &dyn db::DefDatabase) -> Arc<DefMap> {
         match self.block {
-            Some(block) => {
-                db.block_def_map(block).unwrap_or_else(|| {
-                    // NOTE: This should be unreachable - all `ModuleId`s come from their `DefMap`s,
-                    // so the `DefMap` here must exist.
-                    unreachable!("no `block_def_map` for `ModuleId` {:?}", self);
-                })
-            }
+            Some(block) => db.block_def_map(block),
             None => db.crate_def_map(self.krate),
         }
     }

crates/hir-def/src/nameres.rs

@@ -65,7 +65,7 @@ use itertools::Itertools;
 use la_arena::Arena;
 use profile::Count;
 use rustc_hash::{FxHashMap, FxHashSet};
-use stdx::format_to;
+use stdx::{format_to, never};
 use syntax::{ast, SmolStr};
 
 use crate::{
@@ -243,17 +243,12 @@ impl DefMap {
         Arc::new(def_map)
     }
 
-    pub(crate) fn block_def_map_query(
-        db: &dyn DefDatabase,
-        block_id: BlockId,
-    ) -> Option<Arc<DefMap>> {
+    pub(crate) fn block_def_map_query(db: &dyn DefDatabase, block_id: BlockId) -> Arc<DefMap> {
         let block: BlockLoc = db.lookup_intern_block(block_id);
 
         let tree_id = TreeId::new(block.ast_id.file_id, Some(block_id));
         let item_tree = tree_id.item_tree(db);
-        if item_tree.top_level_items().is_empty() {
-            return None;
-        }
+        never!(item_tree.top_level_items().is_empty(), "block should have items");
 
         let parent_map = block.module.def_map(db);
         let krate = block.module.krate;
@@ -269,7 +264,7 @@ impl DefMap {
         def_map.block = Some(BlockInfo { block: block_id, parent: block.module });
 
         let def_map = collector::collect_defs(db, def_map, tree_id);
-        Some(Arc::new(def_map))
+        Arc::new(def_map)
     }
 
     fn empty(krate: CrateId, edition: Edition, module_data: ModuleData) -> DefMap {

crates/hir-def/src/resolver.rs

@@ -572,15 +572,12 @@ impl Resolver {
                 scope_id,
             }));
             if let Some(block) = expr_scopes.block(scope_id) {
-                if let Some(def_map) = db.block_def_map(block) {
-                    let root = def_map.root();
-                    resolver
-                        .scopes
-                        .push(Scope::BlockScope(ModuleItemMap { def_map, module_id: root }));
-                    // FIXME: This adds as many module scopes as there are blocks, but resolving in each
-                    // already traverses all parents, so this is O(n²). I think we could only store the
-                    // innermost module scope instead?
-                }
+                let def_map = db.block_def_map(block);
+                let root = def_map.root();
+                resolver.scopes.push(Scope::BlockScope(ModuleItemMap { def_map, module_id: root }));
+                // FIXME: This adds as many module scopes as there are blocks, but resolving in each
+                // already traverses all parents, so this is O(n²). I think we could only store the
+                // innermost module scope instead?
             }
         }
 
@@ -741,13 +738,12 @@ fn resolver_for_scope_(
 
     for scope in scope_chain.into_iter().rev() {
         if let Some(block) = scopes.block(scope) {
-            if let Some(def_map) = db.block_def_map(block) {
-                let root = def_map.root();
-                r = r.push_block_scope(def_map, root);
-                // FIXME: This adds as many module scopes as there are blocks, but resolving in each
-                // already traverses all parents, so this is O(n²). I think we could only store the
-                // innermost module scope instead?
-            }
+            let def_map = db.block_def_map(block);
+            let root = def_map.root();
+            r = r.push_block_scope(def_map, root);
+            // FIXME: This adds as many module scopes as there are blocks, but resolving in each
+            // already traverses all parents, so this is O(n²). I think we could only store the
+            // innermost module scope instead?
         }
 
         r = r.push_expr_scope(owner, Arc::clone(&scopes), scope);

crates/hir-def/src/test_db.rs

@@ -210,13 +210,11 @@ impl TestDB {
             });
 
         for scope in scope_iter {
-            let containing_blocks =
+            let mut containing_blocks =
                 scopes.scope_chain(Some(scope)).filter_map(|scope| scopes.block(scope));
 
-            for block in containing_blocks {
-                if let Some(def_map) = self.block_def_map(block) {
-                    return Some(def_map);
-                }
+            if let Some(block) = containing_blocks.next().map(|block| self.block_def_map(block)) {
+                return Some(block);
             }
         }
 

crates/hir-ty/src/chalk_db.rs

@@ -129,10 +129,7 @@ impl<'a> chalk_solve::RustIrDatabase<Interner> for ChalkContext<'a> {
         let impl_maps = [in_deps, in_self];
         let block_impls = iter::successors(self.block, |&block_id| {
             cov_mark::hit!(block_local_impls);
-            self.db
-                .block_def_map(block_id)
-                .and_then(|map| map.parent())
-                .and_then(|module| module.containing_block())
+            self.db.block_def_map(block_id).parent().and_then(|module| module.containing_block())
         })
         .inspect(|&block_id| {
             // make sure we don't search the same block twice

crates/hir-ty/src/method_resolution.rs

@@ -156,7 +156,7 @@ impl TraitImpls {
         let _p = profile::span("trait_impls_in_block_query");
         let mut impls = Self { map: FxHashMap::default() };
 
-        let block_def_map = db.block_def_map(block)?;
+        let block_def_map = db.block_def_map(block);
         impls.collect_def_map(db, &block_def_map);
         impls.shrink_to_fit();
 
@@ -290,7 +290,7 @@ impl InherentImpls {
         let _p = profile::span("inherent_impls_in_block_query");
         let mut impls = Self { map: FxHashMap::default(), invalid_impls: Vec::default() };
 
-        let block_def_map = db.block_def_map(block)?;
+        let block_def_map = db.block_def_map(block);
         impls.collect_def_map(db, &block_def_map);
         impls.shrink_to_fit();
 
@@ -1191,10 +1191,7 @@ fn iterate_inherent_methods(
             )?;
         }
 
-        block = db
-            .block_def_map(block_id)
-            .and_then(|map| map.parent())
-            .and_then(|module| module.containing_block());
+        block = db.block_def_map(block_id).parent().and_then(|module| module.containing_block());
     }
 
     for krate in def_crates {