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

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.
author: bors <bors@rust-lang.org> 2023-06-28 10:43:56 +0000
committer: bors <bors@rust-lang.org> 2023-06-28 10:43:56 +0000
commit: 4b06d3c595a75fd84bfce2b7f2861a913ed6e530 (patch)
tree: cdaafdde8aa4ec5ff176550c533ad335406592d4
parent: 77ccd64326c686d21b9cbf42110b626255f7ca8d (diff)
parent: 56dd5368f5c46aeeb4e823d1dabdbfb59ca317b2 (diff)
download: rust-4b06d3c595a75fd84bfce2b7f2861a913ed6e530.tar.gz
rust-4b06d3c595a75fd84bfce2b7f2861a913ed6e530.zip
7 files changed, 182 insertions, 73 deletions
diff --git a/crates/hir-def/src/body/tests.rs b/crates/hir-def/src/body/tests.rs
index edee2c7ff96..d5582011645 100644
--- a/crates/hir-def/src/body/tests.rs
+++ b/crates/hir-def/src/body/tests.rs
@@ -3,12 +3,12 @@ mod block;
 use base_db::{fixture::WithFixture, SourceDatabase};
 use expect_test::Expect;
 
-use crate::ModuleDefId;
+use crate::{test_db::TestDB, ModuleDefId};
 
 use super::*;
 
 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 def_map = db.crate_def_map(krate);
@@ -25,15 +25,15 @@ fn lower(ra_fixture: &str) -> Arc<Body> {
     db.body(fn_def.unwrap().into())
 }
 
-fn block_def_map_at(ra_fixture: &str) -> String {
-    let (db, position) = crate::test_db::TestDB::with_position(ra_fixture);
+fn def_map_at(ra_fixture: &str) -> String {
+    let (db, position) = TestDB::with_position(ra_fixture);
 
     let module = db.module_at_position(position);
     module.def_map(&db).dump(&db)
 }
 
 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 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) {
-    let actual = block_def_map_at(ra_fixture);
+    let actual = def_map_at(ra_fixture);
     expect.assert_eq(&actual);
 }
 
diff --git a/crates/hir-def/src/body/tests/block.rs b/crates/hir-def/src/body/tests/block.rs
index 6e77744f215..4e015a7fbbb 100644
--- a/crates/hir-def/src/body/tests/block.rs
+++ b/crates/hir-def/src/body/tests/block.rs
@@ -134,6 +134,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]
 fn nested_module_scoping() {
     check_block_scopes_at(
         r#"
@@ -156,6 +197,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]
 fn legacy_macro_items() {
     // Checks that legacy-scoped `macro_rules!` from parent namespaces are resolved and expanded
     // correctly.
diff --git a/crates/hir-def/src/lib.rs b/crates/hir-def/src/lib.rs
index 65e8c83b5d7..e5149ab577c 100644
--- a/crates/hir-def/src/lib.rs
+++ b/crates/hir-def/src/lib.rs
@@ -145,24 +145,28 @@ pub struct 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 {
             Some(block) => db.block_def_map(block),
             None => db.crate_def_map(self.krate),
         }
     }
 
-    pub fn krate(&self) -> CrateId {
+    pub fn krate(self) -> CrateId {
         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)
     }
 
-    pub fn containing_block(&self) -> Option<BlockId> {
+    pub fn containing_block(self) -> Option<BlockId> {
         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`.
diff --git a/crates/hir-def/src/nameres.rs b/crates/hir-def/src/nameres.rs
index e7a4355d258..86818ce26dd 100644
--- a/crates/hir-def/src/nameres.rs
+++ b/crates/hir-def/src/nameres.rs
@@ -196,6 +196,10 @@ impl BlockRelativeModuleId {
     fn into_module(self, krate: CrateId) -> ModuleId {
         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 {
@@ -278,7 +282,9 @@ pub struct ModuleData {
     pub origin: ModuleOrigin,
     /// Declared visibility of this module.
     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 children: FxHashMap<Name, LocalModuleId>,
     pub scope: ItemScope,
diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs
index d5cfe5bc706..fb332c06d0a 100644
--- a/crates/hir-def/src/nameres/collector.rs
+++ b/crates/hir-def/src/nameres/collector.rs
@@ -808,11 +808,8 @@ impl DefCollector<'_> {
                 }
             }
 
-            // Check whether all namespace is resolved
-            if def.take_types().is_some()
-                && def.take_values().is_some()
-                && def.take_macros().is_some()
-            {
+            // Check whether all namespaces are resolved.
+            if def.is_full() {
                 PartialResolvedImport::Resolved(def)
             } else {
                 PartialResolvedImport::Indeterminate(def)
@@ -821,7 +818,7 @@ impl DefCollector<'_> {
     }
 
     fn resolve_extern_crate(&self, name: &Name) -> Option<CrateRootModuleId> {
-        if *name == name!(self) {
+        if *name == name![self] {
             cov_mark::hit!(extern_crate_self_as);
             Some(self.def_map.crate_root())
         } else {
diff --git a/crates/hir-def/src/nameres/path_resolution.rs b/crates/hir-def/src/nameres/path_resolution.rs
index 5f6163175a7..de22ea10146 100644
--- a/crates/hir-def/src/nameres/path_resolution.rs
+++ b/crates/hir-def/src/nameres/path_resolution.rs
@@ -12,11 +12,12 @@
 
 use base_db::Edition;
 use hir_expand::name::Name;
+use triomphe::Arc;
 
 use crate::{
     db::DefDatabase,
     item_scope::BUILTIN_SCOPE,
-    nameres::{sub_namespace_match, BuiltinShadowMode, DefMap, MacroSubNs},
+    nameres::{sub_namespace_match, BlockInfo, BuiltinShadowMode, DefMap, MacroSubNs},
     path::{ModPath, PathKind},
     per_ns::PerNs,
     visibility::{RawVisibility, Visibility},
@@ -159,13 +160,15 @@ impl DefMap {
                 (None, new) => new,
             };
 
-            match &current_map.block {
-                Some(block) => {
+            match current_map.block {
+                Some(block) if original_module == Self::ROOT => {
+                    // Block modules "inherit" names from its parent module.
                     original_module = block.parent.local_id;
                     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 curr_per_ns: PerNs = match path.kind {
+        let mut curr_per_ns = match path.kind {
             PathKind::DollarCrate(krate) => {
                 if krate == self.krate {
                     cov_mark::hit!(macro_dollar_crate_self);
@@ -241,51 +244,54 @@ impl DefMap {
                 )
             }
             PathKind::Super(lvl) => {
-                let mut module = original_module;
-                for i in 0..lvl {
-                    match self.modules[module].parent {
-                        Some(it) => module = it,
-                        None => match &self.block {
-                            Some(block) => {
-                                // Look up remaining path in parent `DefMap`
-                                let new_path = ModPath::from_segments(
-                                    PathKind::Super(lvl - i),
-                                    path.segments().to_vec(),
-                                );
-                                tracing::debug!(
-                                    "`super` path: {} -> {} in parent map",
-                                    path.display(db.upcast()),
-                                    new_path.display(db.upcast())
-                                );
-                                return block
-                                    .parent
-                                    .def_map(db, self.krate)
-                                    .resolve_path_fp_with_macro(
-                                        db,
-                                        mode,
-                                        block.parent.local_id,
-                                        &new_path,
-                                        shadow,
-                                        expected_macro_subns,
-                                    );
-                            }
-                            None => {
-                                tracing::debug!("super path in root module");
-                                return ResolvePathResult::empty(ReachedFixedPoint::Yes);
-                            }
-                        },
-                    }
+                let mut local_id = original_module;
+                let mut ext;
+                let mut def_map = self;
+
+                // Adjust `local_id` to `self`, i.e. the nearest non-block module.
+                if def_map.module_id(local_id).is_block_module() {
+                    (ext, local_id) = adjust_to_nearest_non_block_module(db, def_map, local_id);
+                    def_map = &ext;
                 }
 
-                // Resolve `self` to the containing crate-rooted module if we're a block
-                self.with_ancestor_maps(db, module, &mut |def_map, module| {
-                    if def_map.block.is_some() {
-                        None // keep ascending
+                // Go up the module tree but skip block modules as `super` always refers to the
+                // nearest non-block module.
+                for _ in 0..lvl {
+                    // Loop invariant: at the beginning of each loop, `local_id` must refer to a
+                    // non-block module.
+                    if let Some(parent) = def_map.modules[local_id].parent {
+                        local_id = parent;
+                        if def_map.module_id(local_id).is_block_module() {
+                            (ext, local_id) =
+                                adjust_to_nearest_non_block_module(db, def_map, local_id);
+                            def_map = &ext;
+                        }
                     } else {
-                        Some(PerNs::types(def_map.module_id(module).into(), Visibility::Public))
+                        stdx::always!(def_map.block.is_none());
+                        tracing::debug!("super path in root module");
+                        return ResolvePathResult::empty(ReachedFixedPoint::Yes);
                     }
-                })
-                .expect("block DefMap not rooted in crate DefMap")
+                }
+
+                let module = def_map.module_id(local_id);
+                stdx::never!(module.is_block_module());
+
+                if self.block != def_map.block {
+                    // If we have a different `DefMap` from `self` (the orignal `DefMap` we started
+                    // with), resolve the remaining path segments in that `DefMap`.
+                    let path =
+                        ModPath::from_segments(PathKind::Super(0), path.segments().iter().cloned());
+                    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 => {
                 // 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);
+        }
+    }
+}
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
index 352fa481509..74eb9f65883 100644
--- a/crates/hir/src/lib.rs
+++ b/crates/hir/src/lib.rs
@@ -47,7 +47,7 @@ use hir_def::{
     lang_item::LangItemTarget,
     layout::{self, ReprOptions, TargetDataLayout},
     macro_id_to_def_id,
-    nameres::{self, diagnostics::DefDiagnostic, ModuleOrigin},
+    nameres::{self, diagnostics::DefDiagnostic},
     per_ns::PerNs,
     resolver::{HasResolver, Resolver},
     src::HasSource as _,
@@ -505,15 +505,10 @@ impl Module {
     /// Finds nearest non-block ancestor `Module` (`self` included).
     pub fn nearest_non_block_module(self, db: &dyn HirDatabase) -> Module {
         let mut id = self.id;
-        loop {
-            let def_map = id.def_map(db.upcast());
-            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 };
-            }
+        while id.is_block_module() {
+            id = id.containing_module(db.upcast()).expect("block without parent module");
         }
+        Module { id }
     }
 
     pub fn path_to_root(self, db: &dyn HirDatabase) -> Vec<Module> {
author	bors <bors@rust-lang.org>	2023-06-28 10:43:56 +0000
committer	bors <bors@rust-lang.org>	2023-06-28 10:43:56 +0000
commit	4b06d3c595a75fd84bfce2b7f2861a913ed6e530 (patch)
tree	cdaafdde8aa4ec5ff176550c533ad335406592d4
parent	77ccd64326c686d21b9cbf42110b626255f7ca8d (diff)
parent	56dd5368f5c46aeeb4e823d1dabdbfb59ca317b2 (diff)
download	rust-4b06d3c595a75fd84bfce2b7f2861a913ed6e530.tar.gz rust-4b06d3c595a75fd84bfce2b7f2861a913ed6e530.zip