From 7202ce6c963f047b8890ee50acc5aaf5d65f175d Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Tue, 2 Feb 2021 11:46:58 +0100 Subject: Revert "Use block_def_map in body lowering" --- crates/hir_def/src/body.rs | 19 ++- crates/hir_def/src/body/lower.rs | 35 ++---- crates/hir_def/src/body/tests.rs | 116 +----------------- crates/hir_def/src/body/tests/block.rs | 187 ------------------------------ crates/hir_def/src/data.rs | 2 +- crates/hir_def/src/expr.rs | 2 - crates/hir_def/src/item_tree.rs | 6 +- crates/hir_def/src/nameres.rs | 6 +- crates/hir_def/src/nameres/tests.rs | 68 ++++++++++- crates/hir_def/src/nameres/tests/block.rs | 186 +++++++++++++++++++++++++++++ crates/hir_ty/src/infer/expr.rs | 2 +- 11 files changed, 282 insertions(+), 347 deletions(-) delete mode 100644 crates/hir_def/src/body/tests/block.rs create mode 100644 crates/hir_def/src/nameres/tests/block.rs diff --git a/crates/hir_def/src/body.rs b/crates/hir_def/src/body.rs index 41abd8f83..b9ecf22fa 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, - def_map: Arc, + crate_def_map: Arc, current_file_id: HirFileId, ast_id_map: Arc, module: ModuleId, @@ -91,7 +91,7 @@ impl Expander { let ast_id_map = db.ast_id_map(current_file_id); Expander { cfg_expander, - def_map: crate_def_map, + crate_def_map, current_file_id, ast_id_map, module, @@ -102,6 +102,7 @@ impl Expander { 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 { @@ -111,12 +112,18 @@ impl Expander { let macro_call = InFile::new(self.current_file_id, ¯o_call); - let resolver = - |path: ModPath| -> Option { self.resolve_path_as_macro(db, &path) }; + 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 mut err = None; let call_id = - macro_call.as_call_id_with_errors(db, self.def_map.krate(), resolver, &mut |e| { + macro_call.as_call_id_with_errors(db, self.crate_def_map.krate(), resolver, &mut |e| { err.get_or_insert(e); }); let call_id = match call_id { @@ -197,7 +204,7 @@ impl Expander { } fn resolve_path_as_macro(&self, db: &dyn DefDatabase, path: &ModPath) -> Option { - self.def_map + self.crate_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 bc61730a7..209965fca 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, mem, sync::Arc}; +use std::{any::type_name, sync::Arc}; use either::Either; use hir_expand::{ @@ -36,8 +36,8 @@ use crate::{ item_tree::{ItemTree, ItemTreeId, ItemTreeNode}, path::{GenericArgs, Path}, type_ref::{Mutability, Rawness, TypeRef}, - AdtId, BlockLoc, ConstLoc, ContainerId, DefWithBodyId, EnumLoc, FunctionLoc, Intern, - ModuleDefId, StaticLoc, StructLoc, TraitLoc, TypeAliasLoc, UnionLoc, + AdtId, ConstLoc, ContainerId, DefWithBodyId, EnumLoc, FunctionLoc, Intern, ModuleDefId, + StaticLoc, StructLoc, TraitLoc, TypeAliasLoc, UnionLoc, }; use super::{diagnostics::BodyDiagnostic, ExprSource, PatSource}; @@ -152,8 +152,8 @@ impl ExprCollector<'_> { fn alloc_expr_desugared(&mut self, expr: Expr) -> ExprId { self.make_expr(expr, Err(SyntheticSyntax)) } - fn unit(&mut self) -> ExprId { - self.alloc_expr_desugared(Expr::Tuple { exprs: Vec::new() }) + fn empty_block(&mut self) -> ExprId { + self.alloc_expr_desugared(Expr::Block { statements: Vec::new(), tail: None, label: None }) } fn missing_expr(&mut self) -> ExprId { self.alloc_expr_desugared(Expr::Missing) @@ -222,7 +222,7 @@ impl ExprCollector<'_> { MatchArm { pat, expr: then_branch, guard: None }, MatchArm { pat: placeholder_pat, - expr: else_branch.unwrap_or_else(|| self.unit()), + expr: else_branch.unwrap_or_else(|| self.empty_block()), guard: None, }, ]; @@ -561,7 +561,7 @@ impl ExprCollector<'_> { 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, e); + let res = self.expander.enter_expand(self.db, Some(&self.body.item_scope), e); match &res.err { Some(ExpandError::UnresolvedProcMacro) => { @@ -697,27 +697,12 @@ impl ExprCollector<'_> { } fn collect_block(&mut self, block: ast::BlockExpr) -> ExprId { - 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 def_map = self.db.block_def_map(block_id); - let root = def_map.module_id(def_map.root()); - let prev_def_map = mem::replace(&mut self.expander.def_map, def_map); - let prev_module = mem::replace(&mut self.expander.module, root); - + let syntax_node_ptr = AstPtr::new(&block.clone().into()); 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)); - 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 + self.alloc_expr(Expr::Block { statements, tail, label: None }, syntax_node_ptr) } fn collect_stmts_items(&mut self, stmts: ast::AstChildren) { @@ -847,7 +832,7 @@ impl ExprCollector<'_> { 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.def_map.resolve_path( + let (resolved, _) = self.expander.crate_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 da60072ce..2e5d0a01e 100644 --- a/crates/hir_def/src/body/tests.rs +++ b/crates/hir_def/src/body/tests.rs @@ -1,10 +1,7 @@ -mod block; - -use base_db::{fixture::WithFixture, FilePosition, SourceDatabase}; -use expect_test::Expect; +use base_db::{fixture::WithFixture, SourceDatabase}; use test_utils::mark; -use crate::{test_db::TestDB, BlockId, ModuleDefId}; +use crate::{test_db::TestDB, ModuleDefId}; use super::*; @@ -34,115 +31,6 @@ 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); - 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 { - 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(); - - // Find the smallest (innermost) function containing the cursor. - 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/body/tests/block.rs b/crates/hir_def/src/body/tests/block.rs deleted file mode 100644 index 6b1ed2555..000000000 --- a/crates/hir_def/src/body/tests/block.rs +++ /dev/null @@ -1,187 +0,0 @@ -use super::*; -use expect_test::expect; - -#[test] -fn inner_item_smoke() { - check_at( - r#" -struct inner {} -fn outer() { - $0 - fn inner() {} -} -"#, - expect![[r#" - block scope - inner: v - crate - inner: t - outer: v - "#]], - ); -} - -#[test] -fn use_from_crate() { - check_at( - r#" -struct Struct; -fn outer() { - use Struct; - use crate::Struct as CrateStruct; - use self::Struct as SelfStruct; - $0 -} -"#, - expect![[r#" - block scope - CrateStruct: t v - SelfStruct: t v - Struct: t v - crate - Struct: t v - outer: v - "#]], - ); -} - -#[test] -fn merge_namespaces() { - check_at( - r#" -struct name {} -fn outer() { - fn name() {} - - use name as imported; // should import both `name`s - - $0 -} -"#, - expect![[r#" - block scope - imported: t v - name: v - crate - name: t - outer: v - "#]], - ); -} - -#[test] -fn nested_blocks() { - check_at( - r#" -fn outer() { - struct inner1 {} - fn inner() { - use inner1; - use outer; - fn inner2() {} - $0 - } -} -"#, - expect![[r#" - block scope - inner1: t - inner2: v - outer: v - block scope - inner: v - inner1: t - crate - outer: v - "#]], - ); -} - -#[test] -fn super_imports() { - check_at( - r#" -mod module { - fn f() { - use super::Struct; - $0 - } -} - -struct Struct {} -"#, - expect![[r#" - block scope - Struct: t - crate - Struct: t - module: t - - crate::module - f: v - "#]], - ); -} - -#[test] -fn legacy_macro_items() { - // Checks that legacy-scoped `macro_rules!` from parent namespaces are resolved and expanded - // correctly. - check_at( - r#" -macro_rules! hit { - () => { - struct Hit {} - } -} - -fn f() { - hit!(); - $0 -} -"#, - expect![[r#" - block scope - Hit: t - crate - f: v - "#]], - ); -} - -#[test] -fn macro_resolve() { - check_at( - r#" -//- /lib.rs crate:lib deps:core -use core::mark; - -fn f() { - fn nested() { - mark::hit!(Hit); - $0 - } -} -//- /core.rs crate:core -pub mod mark { - #[macro_export] - macro_rules! _hit { - ($name:ident) => { - struct $name {} - } - } - - pub use crate::_hit as hit; -} -"#, - expect![[r#" - block scope - Hit: t - block scope - nested: v - crate - f: v - mark: t - "#]], - ); -} diff --git a/crates/hir_def/src/data.rs b/crates/hir_def/src/data.rs index c2b0dc007..e7b7724f7 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, call).value { + if let Some((mark, mac)) = expander.enter_expand(db, None, 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/expr.rs b/crates/hir_def/src/expr.rs index 4d72eaeaf..5be838f4a 100644 --- a/crates/hir_def/src/expr.rs +++ b/crates/hir_def/src/expr.rs @@ -20,7 +20,6 @@ use crate::{ builtin_type::{BuiltinFloat, BuiltinInt}, path::{GenericArgs, Path}, type_ref::{Mutability, Rawness, TypeRef}, - BlockId, }; pub type ExprId = Idx; @@ -57,7 +56,6 @@ 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 4bde67649..42d9f0947 100644 --- a/crates/hir_def/src/item_tree.rs +++ b/crates/hir_def/src/item_tree.rs @@ -24,7 +24,7 @@ use la_arena::{Arena, Idx, RawIdx}; use profile::Count; use rustc_hash::FxHashMap; use smallvec::SmallVec; -use syntax::{ast, match_ast, SyntaxKind}; +use syntax::{ast, match_ast}; use test_utils::mark; use crate::{ @@ -80,10 +80,6 @@ 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/nameres.rs b/crates/hir_def/src/nameres.rs index 9839761d1..6169b3bbc 100644 --- a/crates/hir_def/src/nameres.rs +++ b/crates/hir_def/src/nameres.rs @@ -201,10 +201,8 @@ impl DefMap { let block: BlockLoc = db.lookup_intern_block(block_id); let parent = block.module.def_map(db); - 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 parent.clone(); - } + // 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 block_info = BlockInfo { block: block_id, parent, parent_module: block.module.local_id }; diff --git a/crates/hir_def/src/nameres/tests.rs b/crates/hir_def/src/nameres/tests.rs index 723481c36..b36d0b59b 100644 --- a/crates/hir_def/src/nameres/tests.rs +++ b/crates/hir_def/src/nameres/tests.rs @@ -4,14 +4,16 @@ mod macros; mod mod_resolution; mod diagnostics; mod primitives; +mod block; use std::sync::Arc; -use base_db::{fixture::WithFixture, SourceDatabase}; +use base_db::{fixture::WithFixture, FilePosition, SourceDatabase}; use expect_test::{expect, Expect}; +use syntax::AstNode; use test_utils::mark; -use crate::{db::DefDatabase, nameres::*, test_db::TestDB}; +use crate::{db::DefDatabase, nameres::*, test_db::TestDB, Lookup}; fn compute_crate_def_map(ra_fixture: &str) -> Arc { let db = TestDB::with_files(ra_fixture); @@ -19,12 +21,74 @@ 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_def/src/nameres/tests/block.rs b/crates/hir_def/src/nameres/tests/block.rs new file mode 100644 index 000000000..6cc659513 --- /dev/null +++ b/crates/hir_def/src/nameres/tests/block.rs @@ -0,0 +1,186 @@ +use super::*; + +#[test] +fn inner_item_smoke() { + check_at( + r#" +struct inner {} +fn outer() { + $0 + fn inner() {} +} +"#, + expect![[r#" + block scope + inner: v + crate + inner: t + outer: v + "#]], + ); +} + +#[test] +fn use_from_crate() { + check_at( + r#" +struct Struct; +fn outer() { + use Struct; + use crate::Struct as CrateStruct; + use self::Struct as SelfStruct; + $0 +} +"#, + expect![[r#" + block scope + CrateStruct: t v + SelfStruct: t v + Struct: t v + crate + Struct: t v + outer: v + "#]], + ); +} + +#[test] +fn merge_namespaces() { + check_at( + r#" +struct name {} +fn outer() { + fn name() {} + + use name as imported; // should import both `name`s + + $0 +} +"#, + expect![[r#" + block scope + imported: t v + name: v + crate + name: t + outer: v + "#]], + ); +} + +#[test] +fn nested_blocks() { + check_at( + r#" +fn outer() { + struct inner1 {} + fn inner() { + use inner1; + use outer; + fn inner2() {} + $0 + } +} +"#, + expect![[r#" + block scope + inner1: t + inner2: v + outer: v + block scope + inner: v + inner1: t + crate + outer: v + "#]], + ); +} + +#[test] +fn super_imports() { + check_at( + r#" +mod module { + fn f() { + use super::Struct; + $0 + } +} + +struct Struct {} +"#, + expect![[r#" + block scope + Struct: t + crate + Struct: t + module: t + + crate::module + f: v + "#]], + ); +} + +#[test] +fn legacy_macro_items() { + // Checks that legacy-scoped `macro_rules!` from parent namespaces are resolved and expanded + // correctly. + check_at( + r#" +macro_rules! hit { + () => { + struct Hit {} + } +} + +fn f() { + hit!(); + $0 +} +"#, + expect![[r#" + block scope + Hit: t + crate + f: v + "#]], + ); +} + +#[test] +fn macro_resolve() { + check_at( + r#" +//- /lib.rs crate:lib deps:core +use core::mark; + +fn f() { + fn nested() { + mark::hit!(Hit); + $0 + } +} +//- /core.rs crate:core +pub mod mark { + #[macro_export] + macro_rules! _hit { + ($name:ident) => { + struct $name {} + } + } + + pub use crate::_hit as hit; +} +"#, + expect![[r#" + block scope + Hit: t + block scope + nested: v + crate + f: v + mark: t + "#]], + ); +} diff --git a/crates/hir_ty/src/infer/expr.rs b/crates/hir_ty/src/infer/expr.rs index 12f1591c8..d7351d212 100644 --- a/crates/hir_ty/src/infer/expr.rs +++ b/crates/hir_ty/src/infer/expr.rs @@ -137,7 +137,7 @@ impl<'a> InferenceContext<'a> { self.coerce_merge_branch(&then_ty, &else_ty) } - Expr::Block { statements, tail, label, id: _ } => match label { + Expr::Block { statements, tail, label } => match label { Some(_) => { let break_ty = self.table.new_type_var(); self.breakables.push(BreakableContext { -- cgit v1.2.3