mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #15148 - lowr:fix/super-nameres-in-block, r=Veykril

Browse Source 
Fix `self` and `super` path resolution in block modules

This PR fixes `self` and `super` path resolution with block modules involved.

Previously, we were just going up the module tree count-of-`super` times without considering block modules in the way, and then if we ended up in a block `DefMap`, we adjust "to the containing crate-rooted module". While this seems to work in most real-world cases, we failed to resolve them within peculiar module structures.

`self` and `super` should actually be resolved to the nearest non-block module, and the paths don't necessarily resolve to a crate-rooted module. This PR makes sure every `self` and `super` segment in paths are resolved to a non-block module.
This commit is contained in:
bors 2023-06-28 10:43:56 +00:00
commit 4b06d3c595
7 changed files with 183 additions and 74 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
crates/hir-def/src/body/tests.rs
View File

@ -3,12 +3,12 @@ mod block;
use base_db::{fixture::WithFixture, SourceDatabase}; use base_db::{fixture::WithFixture, SourceDatabase};
use expect_test::Expect; use expect_test::Expect;
use crate::ModuleDefId; use crate::{test_db::TestDB, ModuleDefId};
use super::*; use super::*;
fn lower(ra_fixture: &str) -> Arc<Body> { fn lower(ra_fixture: &str) -> Arc<Body> {
let db = crate::test_db::TestDB::with_files(ra_fixture); let db = TestDB::with_files(ra_fixture);
let krate = db.crate_graph().iter().next().unwrap(); let krate = db.crate_graph().iter().next().unwrap();
let def_map = db.crate_def_map(krate); let def_map = db.crate_def_map(krate);
@ -25,15 +25,15 @@ fn lower(ra_fixture: &str) -> Arc<Body> {
db.body(fn_def.unwrap().into()) db.body(fn_def.unwrap().into())
} }
fn block_def_map_at(ra_fixture: &str) -> String { fn def_map_at(ra_fixture: &str) -> String {
let (db, position) = crate::test_db::TestDB::with_position(ra_fixture); let (db, position) = TestDB::with_position(ra_fixture);
let module = db.module_at_position(position); let module = db.module_at_position(position);
module.def_map(&db).dump(&db) module.def_map(&db).dump(&db)
} }
fn check_block_scopes_at(ra_fixture: &str, expect: Expect) { fn check_block_scopes_at(ra_fixture: &str, expect: Expect) {
let (db, position) = crate::test_db::TestDB::with_position(ra_fixture); let (db, position) = TestDB::with_position(ra_fixture);
let module = db.module_at_position(position); let module = db.module_at_position(position);
let actual = module.def_map(&db).dump_block_scopes(&db); let actual = module.def_map(&db).dump_block_scopes(&db);
@ -41,7 +41,7 @@ fn check_block_scopes_at(ra_fixture: &str, expect: Expect) {
} }
fn check_at(ra_fixture: &str, expect: Expect) { fn check_at(ra_fixture: &str, expect: Expect) {
let actual = block_def_map_at(ra_fixture); let actual = def_map_at(ra_fixture);
expect.assert_eq(&actual); expect.assert_eq(&actual);
} }

77
crates/hir-def/src/body/tests/block.rs
View File

@ -133,6 +133,47 @@ struct Struct {}
); );
} }
#[test]
fn super_imports_2() {
check_at(
r#"
fn outer() {
mod m {
struct ResolveMe {}
fn middle() {
mod m2 {
fn inner() {
use super::ResolveMe;
$0
}
}
}
}
}
"#,
expect![[r#"
block scope
ResolveMe: t
block scope
m2: t
block scope::m2
inner: v
block scope
m: t
block scope::m
ResolveMe: t
middle: v
crate
outer: v
"#]],
);
}
#[test] #[test]
fn nested_module_scoping() { fn nested_module_scoping() {
check_block_scopes_at( check_block_scopes_at(
@ -155,6 +196,42 @@ fn f() {
); );
} }
#[test]
fn self_imports() {
check_at(
r#"
fn f() {
mod m {
struct ResolveMe {}
fn g() {
fn h() {
use self::ResolveMe;
$0
}
}
}
}
"#,
expect![[r#"
block scope
ResolveMe: t
block scope
h: v
block scope
m: t
block scope::m
ResolveMe: t
g: v
crate
f: v
"#]],
);
}
#[test] #[test]
fn legacy_macro_items() { fn legacy_macro_items() {
// Checks that legacy-scoped `macro_rules!` from parent namespaces are resolved and expanded // Checks that legacy-scoped `macro_rules!` from parent namespaces are resolved and expanded

12
crates/hir-def/src/lib.rs
View File

@ -145,24 +145,28 @@ pub struct ModuleId {
} }
impl ModuleId { impl ModuleId {
pub fn def_map(&self, db: &dyn db::DefDatabase) -> Arc<DefMap> { pub fn def_map(self, db: &dyn db::DefDatabase) -> Arc<DefMap> {
match self.block { match self.block {
Some(block) => db.block_def_map(block), Some(block) => db.block_def_map(block),
None => db.crate_def_map(self.krate), None => db.crate_def_map(self.krate),
} }
} }
pub fn krate(&self) -> CrateId { pub fn krate(self) -> CrateId {
self.krate self.krate
} }
pub fn containing_module(&self, db: &dyn db::DefDatabase) -> Option<ModuleId> { pub fn containing_module(self, db: &dyn db::DefDatabase) -> Option<ModuleId> {
self.def_map(db).containing_module(self.local_id) self.def_map(db).containing_module(self.local_id)
} }
pub fn containing_block(&self) -> Option<BlockId> { pub fn containing_block(self) -> Option<BlockId> {
self.block self.block
} }
pub fn is_block_module(self) -> bool {
self.block.is_some() && self.local_id == DefMap::ROOT
}
} }
/// An ID of a module, **local** to a `DefMap`. /// An ID of a module, **local** to a `DefMap`.

8
crates/hir-def/src/nameres.rs
View File

@ -196,6 +196,10 @@ impl BlockRelativeModuleId {
fn into_module(self, krate: CrateId) -> ModuleId { fn into_module(self, krate: CrateId) -> ModuleId {
ModuleId { krate, block: self.block, local_id: self.local_id } ModuleId { krate, block: self.block, local_id: self.local_id }
} }
fn is_block_module(self) -> bool {
self.block.is_some() && self.local_id == DefMap::ROOT
}
} }
impl std::ops::Index<LocalModuleId> for DefMap { impl std::ops::Index<LocalModuleId> for DefMap {
@ -278,7 +282,9 @@ pub struct ModuleData {
pub origin: ModuleOrigin, pub origin: ModuleOrigin,
/// Declared visibility of this module. /// Declared visibility of this module.
pub visibility: Visibility, pub visibility: Visibility,
/// Always [`None`] for block modules /// Parent module in the same `DefMap`.
///
/// [`None`] for block modules because they are always its `DefMap`'s root.
pub parent: Option<LocalModuleId>, pub parent: Option<LocalModuleId>,
pub children: FxHashMap<Name, LocalModuleId>, pub children: FxHashMap<Name, LocalModuleId>,
pub scope: ItemScope, pub scope: ItemScope,

9
crates/hir-def/src/nameres/collector.rs
View File

@ -808,11 +808,8 @@ impl DefCollector<'_> {
} }
} }
// Check whether all namespace is resolved // Check whether all namespaces are resolved.
if def.take_types().is_some() if def.is_full() {
&& def.take_values().is_some()
&& def.take_macros().is_some()
{
PartialResolvedImport::Resolved(def) PartialResolvedImport::Resolved(def)
} else { } else {
PartialResolvedImport::Indeterminate(def) PartialResolvedImport::Indeterminate(def)
@ -821,7 +818,7 @@ impl DefCollector<'_> {
} }
fn resolve_extern_crate(&self, name: &Name) -> Option<CrateRootModuleId> { fn resolve_extern_crate(&self, name: &Name) -> Option<CrateRootModuleId> {
if *name == name!(self) { if *name == name![self] {
cov_mark::hit!(extern_crate_self_as); cov_mark::hit!(extern_crate_self_as);
Some(self.def_map.crate_root()) Some(self.def_map.crate_root())
} else { } else {

126
crates/hir-def/src/nameres/path_resolution.rs
View File

@ -12,11 +12,12 @@
use base_db::Edition; use base_db::Edition;
use hir_expand::name::Name; use hir_expand::name::Name;
use triomphe::Arc;
use crate::{ use crate::{
db::DefDatabase, db::DefDatabase,
item_scope::BUILTIN_SCOPE, item_scope::BUILTIN_SCOPE,
nameres::{sub_namespace_match, BuiltinShadowMode, DefMap, MacroSubNs}, nameres::{sub_namespace_match, BlockInfo, BuiltinShadowMode, DefMap, MacroSubNs},
path::{ModPath, PathKind}, path::{ModPath, PathKind},
per_ns::PerNs, per_ns::PerNs,
visibility::{RawVisibility, Visibility}, visibility::{RawVisibility, Visibility},
@ -159,13 +160,15 @@ impl DefMap {
(None, new) => new, (None, new) => new,
}; };
match &current_map.block { match current_map.block {
Some(block) => { Some(block) if original_module == Self::ROOT => {
// Block modules "inherit" names from its parent module.
original_module = block.parent.local_id; original_module = block.parent.local_id;
arc = block.parent.def_map(db, current_map.krate); arc = block.parent.def_map(db, current_map.krate);
current_map = &*arc; current_map = &arc;
} }
None => return result, // Proper (non-block) modules, including those in block `DefMap`s, don't.
_ => return result,
} }
} }
} }
@ -189,7 +192,7 @@ impl DefMap {
)); ));
let mut segments = path.segments().iter().enumerate(); let mut segments = path.segments().iter().enumerate();
let mut curr_per_ns: PerNs = match path.kind { let mut curr_per_ns = match path.kind {
PathKind::DollarCrate(krate) => { PathKind::DollarCrate(krate) => {
if krate == self.krate { if krate == self.krate {
cov_mark::hit!(macro_dollar_crate_self); cov_mark::hit!(macro_dollar_crate_self);
@ -241,51 +244,54 @@ impl DefMap {
) )
} }
PathKind::Super(lvl) => { PathKind::Super(lvl) => {
let mut module = original_module; let mut local_id = original_module;
for i in 0..lvl { let mut ext;
match self.modules[module].parent { let mut def_map = self;
Some(it) => module = it,
None => match &self.block { // Adjust `local_id` to `self`, i.e. the nearest non-block module.
Some(block) => { if def_map.module_id(local_id).is_block_module() {
// Look up remaining path in parent `DefMap` (ext, local_id) = adjust_to_nearest_non_block_module(db, def_map, local_id);
let new_path = ModPath::from_segments( def_map = &ext;
PathKind::Super(lvl - i), }
path.segments().to_vec(),
); // Go up the module tree but skip block modules as `super` always refers to the
tracing::debug!( // nearest non-block module.
"`super` path: {} -> {} in parent map", for _ in 0..lvl {
path.display(db.upcast()), // Loop invariant: at the beginning of each loop, `local_id` must refer to a
new_path.display(db.upcast()) // non-block module.
); if let Some(parent) = def_map.modules[local_id].parent {
return block local_id = parent;
.parent if def_map.module_id(local_id).is_block_module() {
.def_map(db, self.krate) (ext, local_id) =
.resolve_path_fp_with_macro( adjust_to_nearest_non_block_module(db, def_map, local_id);
db, def_map = &ext;
mode, }
block.parent.local_id, } else {
&new_path, stdx::always!(def_map.block.is_none());
shadow, tracing::debug!("super path in root module");
expected_macro_subns, return ResolvePathResult::empty(ReachedFixedPoint::Yes);
);
}
None => {
tracing::debug!("super path in root module");
return ResolvePathResult::empty(ReachedFixedPoint::Yes);
}
},
} }
} }
// Resolve `self` to the containing crate-rooted module if we're a block let module = def_map.module_id(local_id);
self.with_ancestor_maps(db, module, &mut |def_map, module| { stdx::never!(module.is_block_module());
if def_map.block.is_some() {
None // keep ascending if self.block != def_map.block {
} else { // If we have a different `DefMap` from `self` (the orignal `DefMap` we started
Some(PerNs::types(def_map.module_id(module).into(), Visibility::Public)) // with), resolve the remaining path segments in that `DefMap`.
} let path =
}) ModPath::from_segments(PathKind::Super(0), path.segments().iter().cloned());
.expect("block DefMap not rooted in crate DefMap") return def_map.resolve_path_fp_with_macro(
db,
mode,
local_id,
&path,
shadow,
expected_macro_subns,
);
}
PerNs::types(module.into(), Visibility::Public)
} }
PathKind::Abs => { PathKind::Abs => {
// 2018-style absolute path -- only extern prelude // 2018-style absolute path -- only extern prelude
@ -508,3 +514,27 @@ impl DefMap {
} }
} }
} }
/// Given a block module, returns its nearest non-block module and the `DefMap` it blongs to.
fn adjust_to_nearest_non_block_module(
db: &dyn DefDatabase,
def_map: &DefMap,
mut local_id: LocalModuleId,
) -> (Arc<DefMap>, LocalModuleId) {
// INVARIANT: `local_id` in `def_map` must be a block module.
stdx::always!(def_map.module_id(local_id).is_block_module());
let mut ext;
// This needs to be a local variable due to our mighty lifetime.
let mut def_map = def_map;
loop {
let BlockInfo { parent, .. } = def_map.block.expect("block module without parent module");
ext = parent.def_map(db, def_map.krate);
def_map = &ext;
local_id = parent.local_id;
if !parent.is_block_module() {
return (ext, local_id);
}
}
}

13
crates/hir/src/lib.rs
View File

@ -47,7 +47,7 @@ use hir_def::{
lang_item::LangItemTarget, lang_item::LangItemTarget,
layout::{self, ReprOptions, TargetDataLayout}, layout::{self, ReprOptions, TargetDataLayout},
macro_id_to_def_id, macro_id_to_def_id,
nameres::{self, diagnostics::DefDiagnostic, ModuleOrigin}, nameres::{self, diagnostics::DefDiagnostic},
per_ns::PerNs, per_ns::PerNs,
resolver::{HasResolver, Resolver}, resolver::{HasResolver, Resolver},
src::HasSource as _, src::HasSource as _,
@ -505,15 +505,10 @@ impl Module {
/// Finds nearest non-block ancestor `Module` (`self` included). /// Finds nearest non-block ancestor `Module` (`self` included).
pub fn nearest_non_block_module(self, db: &dyn HirDatabase) -> Module { pub fn nearest_non_block_module(self, db: &dyn HirDatabase) -> Module {
let mut id = self.id; let mut id = self.id;
loop { while id.is_block_module() {
let def_map = id.def_map(db.upcast()); id = id.containing_module(db.upcast()).expect("block without parent module");
let origin = def_map[id.local_id].origin;
if matches!(origin, ModuleOrigin::BlockExpr { .. }) {
id = id.containing_module(db.upcast()).expect("block without parent module")
} else {
return Module { id };
}
} }
Module { id }
} }
pub fn path_to_root(self, db: &dyn HirDatabase) -> Vec<Module> { pub fn path_to_root(self, db: &dyn HirDatabase) -> Vec<Module> {