diff --git a/crates/hir_def/src/body.rs b/crates/hir_def/src/body.rs index b9ecf22fa5f2..41abd8f83b5e 100644 --- a/crates/hir_def/src/body.rs +++ b/crates/hir_def/src/body.rs @@ -46,7 +46,7 @@ pub(crate) struct CfgExpander { pub(crate) struct Expander { cfg_expander: CfgExpander, - crate_def_map: Arc, + def_map: Arc, current_file_id: HirFileId, ast_id_map: Arc, module: ModuleId, @@ -91,7 +91,7 @@ pub(crate) fn new( let ast_id_map = db.ast_id_map(current_file_id); Expander { cfg_expander, - crate_def_map, + def_map: crate_def_map, current_file_id, ast_id_map, module, @@ -102,7 +102,6 @@ pub(crate) fn new( pub(crate) fn enter_expand( &mut self, db: &dyn DefDatabase, - local_scope: Option<&ItemScope>, macro_call: ast::MacroCall, ) -> ExpandResult> { if self.recursion_limit + 1 > EXPANSION_RECURSION_LIMIT { @@ -112,18 +111,12 @@ pub(crate) fn enter_expand( let macro_call = InFile::new(self.current_file_id, ¯o_call); - let resolver = |path: ModPath| -> Option { - if let Some(local_scope) = local_scope { - if let Some(def) = path.as_ident().and_then(|n| local_scope.get_legacy_macro(n)) { - return Some(def); - } - } - self.resolve_path_as_macro(db, &path) - }; + let resolver = + |path: ModPath| -> Option { self.resolve_path_as_macro(db, &path) }; let mut err = None; let call_id = - macro_call.as_call_id_with_errors(db, self.crate_def_map.krate(), resolver, &mut |e| { + macro_call.as_call_id_with_errors(db, self.def_map.krate(), resolver, &mut |e| { err.get_or_insert(e); }); let call_id = match call_id { @@ -204,7 +197,7 @@ fn parse_path(&mut self, path: ast::Path) -> Option { } fn resolve_path_as_macro(&self, db: &dyn DefDatabase, path: &ModPath) -> Option { - self.crate_def_map + self.def_map .resolve_path(db, self.module.local_id, path, BuiltinShadowMode::Other) .0 .take_macros() diff --git a/crates/hir_def/src/body/lower.rs b/crates/hir_def/src/body/lower.rs index 209965fcaf74..540c6c9ada7b 100644 --- a/crates/hir_def/src/body/lower.rs +++ b/crates/hir_def/src/body/lower.rs @@ -1,7 +1,7 @@ //! Transforms `ast::Expr` into an equivalent `hir_def::expr::Expr` //! representation. -use std::{any::type_name, sync::Arc}; +use std::{any::type_name, mem, sync::Arc}; use either::Either; use hir_expand::{ @@ -36,8 +36,8 @@ item_tree::{ItemTree, ItemTreeId, ItemTreeNode}, path::{GenericArgs, Path}, type_ref::{Mutability, Rawness, TypeRef}, - AdtId, ConstLoc, ContainerId, DefWithBodyId, EnumLoc, FunctionLoc, Intern, ModuleDefId, - StaticLoc, StructLoc, TraitLoc, TypeAliasLoc, UnionLoc, + AdtId, BlockLoc, ConstLoc, ContainerId, DefWithBodyId, EnumLoc, FunctionLoc, Intern, + ModuleDefId, StaticLoc, StructLoc, TraitLoc, TypeAliasLoc, UnionLoc, }; use super::{diagnostics::BodyDiagnostic, ExprSource, PatSource}; @@ -152,8 +152,8 @@ fn alloc_expr(&mut self, expr: Expr, ptr: AstPtr) -> ExprId { fn alloc_expr_desugared(&mut self, expr: Expr) -> ExprId { self.make_expr(expr, Err(SyntheticSyntax)) } - fn empty_block(&mut self) -> ExprId { - self.alloc_expr_desugared(Expr::Block { statements: Vec::new(), tail: None, label: None }) + fn unit(&mut self) -> ExprId { + self.alloc_expr_desugared(Expr::Tuple { exprs: Vec::new() }) } fn missing_expr(&mut self) -> ExprId { self.alloc_expr_desugared(Expr::Missing) @@ -222,7 +222,7 @@ fn collect_expr(&mut self, expr: ast::Expr) -> ExprId { MatchArm { pat, expr: then_branch, guard: None }, MatchArm { pat: placeholder_pat, - expr: else_branch.unwrap_or_else(|| self.empty_block()), + expr: else_branch.unwrap_or_else(|| self.unit()), guard: None, }, ]; @@ -561,7 +561,7 @@ fn collect_macro_call), T: ast::AstNode>( let outer_file = self.expander.current_file_id; let macro_call = self.expander.to_source(AstPtr::new(&e)); - let res = self.expander.enter_expand(self.db, Some(&self.body.item_scope), e); + let res = self.expander.enter_expand(self.db, e); match &res.err { Some(ExpandError::UnresolvedProcMacro) => { @@ -697,12 +697,30 @@ fn collect_stmt(&mut self, s: ast::Stmt) -> Option> { } fn collect_block(&mut self, block: ast::BlockExpr) -> ExprId { - let syntax_node_ptr = AstPtr::new(&block.clone().into()); + let ast_id = self.expander.ast_id(&block); + let block_loc = BlockLoc { ast_id, module: self.expander.module }; + let block_id = self.db.intern_block(block_loc); + let opt_def_map = self.db.block_def_map(block_id); + let has_def_map = opt_def_map.is_some(); + let def_map = opt_def_map.unwrap_or_else(|| self.expander.def_map.clone()); + let module = + if has_def_map { def_map.module_id(def_map.root()) } else { self.expander.module }; + let prev_def_map = mem::replace(&mut self.expander.def_map, def_map); + let prev_module = mem::replace(&mut self.expander.module, module); + self.collect_stmts_items(block.statements()); let statements = block.statements().filter_map(|s| self.collect_stmt(s)).flatten().collect(); let tail = block.tail_expr().map(|e| self.collect_expr(e)); - self.alloc_expr(Expr::Block { statements, tail, label: None }, syntax_node_ptr) + let syntax_node_ptr = AstPtr::new(&block.clone().into()); + let expr_id = self.alloc_expr( + Expr::Block { id: block_id, statements, tail, label: None }, + syntax_node_ptr, + ); + + self.expander.def_map = prev_def_map; + self.expander.module = prev_module; + expr_id } fn collect_stmts_items(&mut self, stmts: ast::AstChildren) { @@ -832,7 +850,7 @@ fn collect_pat(&mut self, pat: ast::Pat) -> PatId { if annotation == BindingAnnotation::Unannotated && subpat.is_none() { // This could also be a single-segment path pattern. To // decide that, we need to try resolving the name. - let (resolved, _) = self.expander.crate_def_map.resolve_path( + let (resolved, _) = self.expander.def_map.resolve_path( self.db, self.expander.module.local_id, &name.clone().into(), diff --git a/crates/hir_def/src/body/tests.rs b/crates/hir_def/src/body/tests.rs index 2e5d0a01e76e..404603360844 100644 --- a/crates/hir_def/src/body/tests.rs +++ b/crates/hir_def/src/body/tests.rs @@ -1,7 +1,10 @@ -use base_db::{fixture::WithFixture, SourceDatabase}; +mod block; + +use base_db::{fixture::WithFixture, FilePosition, SourceDatabase}; +use expect_test::Expect; use test_utils::mark; -use crate::{test_db::TestDB, ModuleDefId}; +use crate::{test_db::TestDB, BlockId, ModuleDefId}; use super::*; @@ -31,6 +34,115 @@ fn check_diagnostics(ra_fixture: &str) { db.check_diagnostics(); } +fn block_def_map_at(ra_fixture: &str) -> Arc { + let (db, position) = crate::test_db::TestDB::with_position(ra_fixture); + + let krate = db.crate_graph().iter().next().unwrap(); + let def_map = db.crate_def_map(krate); + + let mut block = + block_at_pos(&db, &def_map, position).expect("couldn't find enclosing function or block"); + loop { + let def_map = db.block_def_map(block).unwrap_or_else(|| def_map.clone()); + let new_block = block_at_pos(&db, &def_map, position); + match new_block { + Some(new_block) => { + assert_ne!(block, new_block); + block = new_block; + } + None => { + return def_map; + } + } + } +} + +fn block_at_pos(db: &dyn DefDatabase, def_map: &DefMap, position: FilePosition) -> Option { + // Find the smallest (innermost) function containing the cursor. + let mut size = None; + let mut fn_def = None; + for (_, module) in def_map.modules() { + let file_id = module.definition_source(db).file_id; + if file_id != position.file_id.into() { + continue; + } + let root = db.parse_or_expand(file_id).unwrap(); + let ast_map = db.ast_id_map(file_id); + let item_tree = db.item_tree(file_id); + for decl in module.scope.declarations() { + if let ModuleDefId::FunctionId(it) = decl { + let ast = ast_map.get(item_tree[it.lookup(db).id.value].ast_id).to_node(&root); + let range = ast.syntax().text_range(); + + if !range.contains(position.offset) { + continue; + } + + let new_size = match size { + None => range.len(), + Some(size) => { + if range.len() < size { + range.len() + } else { + size + } + } + }; + if size != Some(new_size) { + size = Some(new_size); + fn_def = Some(it); + } + } + } + } + + let (body, source_map) = db.body_with_source_map(fn_def?.into()); + + // Now find the smallest encompassing block expression in the function body. + let mut size = None; + let mut block_id = None; + for (expr_id, expr) in body.exprs.iter() { + if let Expr::Block { id, .. } = expr { + if let Ok(ast) = source_map.expr_syntax(expr_id) { + if ast.file_id != position.file_id.into() { + continue; + } + + let root = db.parse_or_expand(ast.file_id).unwrap(); + let ast = ast.value.to_node(&root); + let range = ast.syntax().text_range(); + + if !range.contains(position.offset) { + continue; + } + + let new_size = match size { + None => range.len(), + Some(size) => { + if range.len() < size { + range.len() + } else { + size + } + } + }; + if size != Some(new_size) { + size = Some(new_size); + block_id = Some(*id); + } + } + } + } + + Some(block_id.expect("can't find block containing cursor")) +} + +fn check_at(ra_fixture: &str, expect: Expect) { + let def_map = block_def_map_at(ra_fixture); + let actual = def_map.dump(); + expect.assert_eq(&actual); +} + #[test] fn your_stack_belongs_to_me() { mark::check!(your_stack_belongs_to_me); diff --git a/crates/hir_def/src/nameres/tests/block.rs b/crates/hir_def/src/body/tests/block.rs similarity index 86% rename from crates/hir_def/src/nameres/tests/block.rs rename to crates/hir_def/src/body/tests/block.rs index 6cc65951352c..d5f3ac4c5612 100644 --- a/crates/hir_def/src/nameres/tests/block.rs +++ b/crates/hir_def/src/body/tests/block.rs @@ -1,4 +1,5 @@ use super::*; +use expect_test::expect; #[test] fn inner_item_smoke() { @@ -184,3 +185,36 @@ struct $name {} "#]], ); } + +#[test] +fn macro_resolve_legacy() { + check_at( + r#" +//- /lib.rs +mod module; + +//- /module.rs +macro_rules! m { + () => { + struct Def {} + }; +} + +fn f() { + { + m!(); + $0 + } +} + "#, + expect![[r#" + block scope + Def: t + crate + module: t + + crate::module + f: v + "#]], + ) +} diff --git a/crates/hir_def/src/data.rs b/crates/hir_def/src/data.rs index e7b7724f75fb..c2b0dc00714a 100644 --- a/crates/hir_def/src/data.rs +++ b/crates/hir_def/src/data.rs @@ -262,7 +262,7 @@ fn collect_items( let root = db.parse_or_expand(file_id).unwrap(); let call = ast_id_map.get(call.ast_id).to_node(&root); - if let Some((mark, mac)) = expander.enter_expand(db, None, call).value { + if let Some((mark, mac)) = expander.enter_expand(db, call).value { let src: InFile = expander.to_source(mac); let item_tree = db.item_tree(src.file_id); let iter = diff --git a/crates/hir_def/src/db.rs b/crates/hir_def/src/db.rs index aef7e1f6cd03..7fe6f6346a59 100644 --- a/crates/hir_def/src/db.rs +++ b/crates/hir_def/src/db.rs @@ -59,7 +59,7 @@ pub trait DefDatabase: InternDatabase + AstDatabase + Upcast { fn crate_def_map_query(&self, krate: CrateId) -> Arc; #[salsa::invoke(DefMap::block_def_map_query)] - fn block_def_map(&self, block: BlockId) -> Arc; + fn block_def_map(&self, block: BlockId) -> Option>; #[salsa::invoke(StructData::struct_data_query)] fn struct_data(&self, id: StructId) -> Arc; diff --git a/crates/hir_def/src/expr.rs b/crates/hir_def/src/expr.rs index 5be838f4a7d8..4d72eaeaff9a 100644 --- a/crates/hir_def/src/expr.rs +++ b/crates/hir_def/src/expr.rs @@ -20,6 +20,7 @@ builtin_type::{BuiltinFloat, BuiltinInt}, path::{GenericArgs, Path}, type_ref::{Mutability, Rawness, TypeRef}, + BlockId, }; pub type ExprId = Idx; @@ -56,6 +57,7 @@ pub enum Expr { else_branch: Option, }, Block { + id: BlockId, statements: Vec, tail: Option, label: Option, diff --git a/crates/hir_def/src/item_tree.rs b/crates/hir_def/src/item_tree.rs index 42d9f09472da..4bde676490ec 100644 --- a/crates/hir_def/src/item_tree.rs +++ b/crates/hir_def/src/item_tree.rs @@ -24,7 +24,7 @@ use profile::Count; use rustc_hash::FxHashMap; use smallvec::SmallVec; -use syntax::{ast, match_ast}; +use syntax::{ast, match_ast, SyntaxKind}; use test_utils::mark; use crate::{ @@ -80,6 +80,10 @@ impl ItemTree { pub(crate) fn item_tree_query(db: &dyn DefDatabase, file_id: HirFileId) -> Arc { let _p = profile::span("item_tree_query").detail(|| format!("{:?}", file_id)); let syntax = if let Some(node) = db.parse_or_expand(file_id) { + if node.kind() == SyntaxKind::ERROR { + // FIXME: not 100% sure why these crop up, but return an empty tree to avoid a panic + return Default::default(); + } node } else { return Default::default(); diff --git a/crates/hir_def/src/lib.rs b/crates/hir_def/src/lib.rs index 42b50b5b75fa..5dd3705b0bbd 100644 --- a/crates/hir_def/src/lib.rs +++ b/crates/hir_def/src/lib.rs @@ -81,7 +81,13 @@ 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), + 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. + panic!("no `block_def_map` for `ModuleId` {:?}", self); + }) + } None => db.crate_def_map(self.krate), } } @@ -239,6 +245,7 @@ pub struct FieldId { #[derive(Debug, Hash, PartialEq, Eq, Clone)] pub struct BlockLoc { ast_id: AstId, + /// The containing module. module: ModuleId, } impl_intern!(BlockId, BlockLoc, intern_block, lookup_intern_block); diff --git a/crates/hir_def/src/nameres.rs b/crates/hir_def/src/nameres.rs index 0a15fc470407..ece5958f4a49 100644 --- a/crates/hir_def/src/nameres.rs +++ b/crates/hir_def/src/nameres.rs @@ -197,12 +197,17 @@ pub(crate) fn crate_def_map_query(db: &dyn DefDatabase, krate: CrateId) -> Arc Arc { + pub(crate) fn block_def_map_query( + db: &dyn DefDatabase, + block_id: BlockId, + ) -> Option> { let block: BlockLoc = db.lookup_intern_block(block_id); let parent = block.module.def_map(db); - // FIXME: It would be good to just return the parent map when the block has no items, but - // we rely on `def_map.block` in a few places, which is `Some` for the inner `DefMap`. + let item_tree = db.item_tree(block.ast_id.file_id); + if item_tree.inner_items_of_block(block.ast_id.value).is_empty() { + return None; + } let block_info = BlockInfo { block: block_id, parent, parent_module: block.module.local_id }; @@ -211,7 +216,7 @@ pub(crate) fn block_def_map_query(db: &dyn DefDatabase, block_id: BlockId) -> Ar def_map.block = Some(block_info); let def_map = collector::collect_defs(db, def_map, Some(block.ast_id)); - Arc::new(def_map) + Some(Arc::new(def_map)) } fn empty(krate: CrateId, edition: Edition) -> DefMap { diff --git a/crates/hir_def/src/nameres/tests.rs b/crates/hir_def/src/nameres/tests.rs index b36d0b59bd73..723481c367fc 100644 --- a/crates/hir_def/src/nameres/tests.rs +++ b/crates/hir_def/src/nameres/tests.rs @@ -4,16 +4,14 @@ mod mod_resolution; mod diagnostics; mod primitives; -mod block; use std::sync::Arc; -use base_db::{fixture::WithFixture, FilePosition, SourceDatabase}; +use base_db::{fixture::WithFixture, SourceDatabase}; use expect_test::{expect, Expect}; -use syntax::AstNode; use test_utils::mark; -use crate::{db::DefDatabase, nameres::*, test_db::TestDB, Lookup}; +use crate::{db::DefDatabase, nameres::*, test_db::TestDB}; fn compute_crate_def_map(ra_fixture: &str) -> Arc { let db = TestDB::with_files(ra_fixture); @@ -21,74 +19,12 @@ fn compute_crate_def_map(ra_fixture: &str) -> Arc { db.crate_def_map(krate) } -fn compute_block_def_map(ra_fixture: &str) -> Arc { - let (db, position) = TestDB::with_position(ra_fixture); - - // FIXME: perhaps we should make this use body lowering tests instead? - - let module = db.module_for_file(position.file_id); - let mut def_map = db.crate_def_map(module.krate); - while let Some(new_def_map) = descend_def_map_at_position(&db, position, def_map.clone()) { - def_map = new_def_map; - } - - // FIXME: select the right module, not the root - - def_map -} - -fn descend_def_map_at_position( - db: &dyn DefDatabase, - position: FilePosition, - def_map: Arc, -) -> Option> { - for (local_id, module_data) in def_map.modules() { - let mod_def = module_data.origin.definition_source(db); - let ast_map = db.ast_id_map(mod_def.file_id); - let item_tree = db.item_tree(mod_def.file_id); - let root = db.parse_or_expand(mod_def.file_id).unwrap(); - for item in module_data.scope.declarations() { - match item { - ModuleDefId::FunctionId(it) => { - // Technically blocks can be inside any type (due to arrays and const generics), - // and also in const/static initializers. For tests we only really care about - // functions though. - - let ast = ast_map.get(item_tree[it.lookup(db).id.value].ast_id).to_node(&root); - - if ast.syntax().text_range().contains(position.offset) { - // Cursor inside function, descend into its body's DefMap. - // Note that we don't handle block *expressions* inside function bodies. - let ast_map = db.ast_id_map(position.file_id.into()); - let ast_id = ast_map.ast_id(&ast.body().unwrap()); - let block = BlockLoc { - ast_id: InFile::new(position.file_id.into(), ast_id), - module: def_map.module_id(local_id), - }; - let block_id = db.intern_block(block); - return Some(db.block_def_map(block_id)); - } - } - _ => continue, - } - } - } - - None -} - fn check(ra_fixture: &str, expect: Expect) { let def_map = compute_crate_def_map(ra_fixture); let actual = def_map.dump(); expect.assert_eq(&actual); } -fn check_at(ra_fixture: &str, expect: Expect) { - let def_map = compute_block_def_map(ra_fixture); - let actual = def_map.dump(); - expect.assert_eq(&actual); -} - #[test] fn crate_def_map_smoke_test() { check( diff --git a/crates/hir_ty/src/infer/expr.rs b/crates/hir_ty/src/infer/expr.rs index d7351d212701..12f1591c81f2 100644 --- a/crates/hir_ty/src/infer/expr.rs +++ b/crates/hir_ty/src/infer/expr.rs @@ -137,7 +137,7 @@ fn infer_expr_inner(&mut self, tgt_expr: ExprId, expected: &Expectation) -> Ty { self.coerce_merge_branch(&then_ty, &else_ty) } - Expr::Block { statements, tail, label } => match label { + Expr::Block { statements, tail, label, id: _ } => match label { Some(_) => { let break_ty = self.table.new_type_var(); self.breakables.push(BreakableContext {