From c32d51979d023bf0580a72f24d148ff82b2d71ed Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Fri, 14 Apr 2023 12:15:48 +0200 Subject: [PATCH] internal: Make block_def_map infallible --- crates/hir-def/src/body.rs | 4 +--- crates/hir-def/src/body/lower.rs | 17 ++++++++-------- crates/hir-def/src/db.rs | 2 +- crates/hir-def/src/lib.rs | 8 +------- crates/hir-def/src/nameres.rs | 11 ++-------- crates/hir-def/src/resolver.rs | 28 +++++++++++--------------- crates/hir-def/src/test_db.rs | 8 +++----- crates/hir-ty/src/chalk_db.rs | 5 +---- crates/hir-ty/src/method_resolution.rs | 9 +++------ 9 files changed, 32 insertions(+), 60 deletions(-) diff --git a/crates/hir-def/src/body.rs b/crates/hir-def/src/body.rs index 9caa084f2a2..6b887bb1b31 100644 --- a/crates/hir-def/src/body.rs +++ b/crates/hir-def/src/body.rs @@ -461,9 +461,7 @@ pub fn blocks<'a>( &'a self, db: &'a dyn DefDatabase, ) -> impl Iterator)> + '_ { - 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 { diff --git a/crates/hir-def/src/body/lower.rs b/crates/hir-def/src/body/lower.rs index 688c9e86bbb..b5487dda1b9 100644 --- a/crates/hir-def/src/body/lower.rs +++ b/crates/hir-def/src/body/lower.rs @@ -948,15 +948,14 @@ fn collect_block_( 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); diff --git a/crates/hir-def/src/db.rs b/crates/hir-def/src/db.rs index 2dfe4b62648..998a7bf0048 100644 --- a/crates/hir-def/src/db.rs +++ b/crates/hir-def/src/db.rs @@ -96,7 +96,7 @@ pub trait DefDatabase: InternDatabase + ExpandDatabase + Upcast Option>; + fn block_def_map(&self, block: BlockId) -> Arc; #[salsa::invoke(StructData::struct_data_query)] fn struct_data(&self, id: StructId) -> Arc; diff --git a/crates/hir-def/src/lib.rs b/crates/hir-def/src/lib.rs index 65c33322b1d..ec9a8e80068 100644 --- a/crates/hir-def/src/lib.rs +++ b/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 { 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), } } diff --git a/crates/hir-def/src/nameres.rs b/crates/hir-def/src/nameres.rs index 121b25c6c97..655004dcfe2 100644 --- a/crates/hir-def/src/nameres.rs +++ b/crates/hir-def/src/nameres.rs @@ -243,17 +243,10 @@ pub(crate) fn crate_def_map_query(db: &dyn DefDatabase, krate: CrateId) -> Arc Option> { + pub(crate) fn block_def_map_query(db: &dyn DefDatabase, block_id: BlockId) -> Arc { 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; - } let parent_map = block.module.def_map(db); let krate = block.module.krate; @@ -269,7 +262,7 @@ pub(crate) fn block_def_map_query( 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 { diff --git a/crates/hir-def/src/resolver.rs b/crates/hir-def/src/resolver.rs index 670495e4d16..12499faeb62 100644 --- a/crates/hir-def/src/resolver.rs +++ b/crates/hir-def/src/resolver.rs @@ -572,15 +572,12 @@ fn append_expr_scope( 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); diff --git a/crates/hir-def/src/test_db.rs b/crates/hir-def/src/test_db.rs index 5f3b2a72949..1cd652e7f02 100644 --- a/crates/hir-def/src/test_db.rs +++ b/crates/hir-def/src/test_db.rs @@ -210,13 +210,11 @@ fn block_at_position(&self, def_map: &DefMap, position: FilePosition) -> Option< }); 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); } } diff --git a/crates/hir-ty/src/chalk_db.rs b/crates/hir-ty/src/chalk_db.rs index c30a99e06ca..11c4dc43410 100644 --- a/crates/hir-ty/src/chalk_db.rs +++ b/crates/hir-ty/src/chalk_db.rs @@ -129,10 +129,7 @@ fn binder_kind( 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 diff --git a/crates/hir-ty/src/method_resolution.rs b/crates/hir-ty/src/method_resolution.rs index 740b6ddc27a..9b8839f3576 100644 --- a/crates/hir-ty/src/method_resolution.rs +++ b/crates/hir-ty/src/method_resolution.rs @@ -156,7 +156,7 @@ pub(crate) fn trait_impls_in_block_query( 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 @@ pub(crate) fn inherent_impls_in_block_query( 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 {