about summary refs log tree commit diff
diff options
context:
space:
mode:
authorChayim Refael Friedman <chayimfr@gmail.com>2024-09-22 03:27:30 +0300
committerChayim Refael Friedman <chayimfr@gmail.com>2024-09-22 04:19:10 +0300
commit0a259082dfc6e1bf35399b759a366db61c5f6c23 (patch)
tree23d1b0528ce958b1983ebca531daa80a2a8fe7a3
parent814da15d8baf2391f4d19cbc4bf73f81ffbeabdf (diff)
downloadrust-0a259082dfc6e1bf35399b759a366db61c5f6c23.tar.gz
rust-0a259082dfc6e1bf35399b759a366db61c5f6c23.zip
Fix name resolution when an import is resolved to some namespace and then later in the algorithm another namespace is added
The import is flagged as "indeterminate", and previously it was re-resolved, but only at the end of name resolution, when it's already too late for anything that depends on it.

This issue was tried to fix in https://github.com/rust-lang/rust-analyzer/pull/2466, but it was not fixed fully.
-rw-r--r--src/tools/rust-analyzer/crates/hir-def/src/nameres/collector.rs43
-rw-r--r--src/tools/rust-analyzer/crates/hir-def/src/per_ns.rs20
-rw-r--r--src/tools/rust-analyzer/crates/ide/src/goto_definition.rs32
3 files changed, 82 insertions, 13 deletions
diff --git a/src/tools/rust-analyzer/crates/hir-def/src/nameres/collector.rs b/src/tools/rust-analyzer/crates/hir-def/src/nameres/collector.rs
index 96db3db8f0d..e09ef4f205d 100644
--- a/src/tools/rust-analyzer/crates/hir-def/src/nameres/collector.rs
+++ b/src/tools/rust-analyzer/crates/hir-def/src/nameres/collector.rs
@@ -221,7 +221,7 @@ struct DefCollector<'a> {
     deps: FxHashMap<Name, Dependency>,
     glob_imports: FxHashMap<LocalModuleId, Vec<(LocalModuleId, Visibility, UseId)>>,
     unresolved_imports: Vec<ImportDirective>,
-    indeterminate_imports: Vec<ImportDirective>,
+    indeterminate_imports: Vec<(ImportDirective, PerNs)>,
     unresolved_macros: Vec<MacroDirective>,
     mod_dirs: FxHashMap<LocalModuleId, ModDir>,
     cfg_options: &'a CfgOptions,
@@ -415,16 +415,6 @@ impl DefCollector<'_> {
 
         self.resolution_loop();
 
-        // Resolve all indeterminate resolved imports again
-        // As some of the macros will expand newly import shadowing partial resolved imports
-        // FIXME: We maybe could skip this, if we handle the indeterminate imports in `resolve_imports`
-        // correctly
-        let partial_resolved = self.indeterminate_imports.drain(..).map(|directive| {
-            ImportDirective { status: PartialResolvedImport::Unresolved, ..directive }
-        });
-        self.unresolved_imports.extend(partial_resolved);
-        self.resolve_imports();
-
         let unresolved_imports = mem::take(&mut self.unresolved_imports);
         // show unresolved imports in completion, etc
         for directive in &unresolved_imports {
@@ -749,9 +739,9 @@ impl DefCollector<'_> {
             .filter_map(|mut directive| {
                 directive.status = self.resolve_import(directive.module_id, &directive.import);
                 match directive.status {
-                    PartialResolvedImport::Indeterminate(_) => {
+                    PartialResolvedImport::Indeterminate(resolved) => {
                         self.record_resolved_import(&directive);
-                        self.indeterminate_imports.push(directive);
+                        self.indeterminate_imports.push((directive, resolved));
                         res = ReachedFixedPoint::No;
                         None
                     }
@@ -764,6 +754,33 @@ impl DefCollector<'_> {
                 }
             })
             .collect();
+
+        // Resolve all indeterminate resolved imports again
+        // As some of the macros will expand newly import shadowing partial resolved imports
+        // FIXME: We maybe could skip this, if we handle the indeterminate imports in `resolve_imports`
+        // correctly
+        let mut indeterminate_imports = std::mem::take(&mut self.indeterminate_imports);
+        indeterminate_imports.retain_mut(|(directive, partially_resolved)| {
+            let partially_resolved = partially_resolved.availability();
+            directive.status = self.resolve_import(directive.module_id, &directive.import);
+            match directive.status {
+                PartialResolvedImport::Indeterminate(import)
+                    if partially_resolved != import.availability() =>
+                {
+                    self.record_resolved_import(directive);
+                    res = ReachedFixedPoint::No;
+                    false
+                }
+                PartialResolvedImport::Resolved(_) => {
+                    self.record_resolved_import(directive);
+                    res = ReachedFixedPoint::No;
+                    false
+                }
+                _ => true,
+            }
+        });
+        self.indeterminate_imports = indeterminate_imports;
+
         res
     }
 
diff --git a/src/tools/rust-analyzer/crates/hir-def/src/per_ns.rs b/src/tools/rust-analyzer/crates/hir-def/src/per_ns.rs
index 19485c476f7..3f3b98c6b5b 100644
--- a/src/tools/rust-analyzer/crates/hir-def/src/per_ns.rs
+++ b/src/tools/rust-analyzer/crates/hir-def/src/per_ns.rs
@@ -3,6 +3,8 @@
 //!
 //! `PerNs` (per namespace) captures this.
 
+use bitflags::bitflags;
+
 use crate::{
     item_scope::{ImportId, ImportOrExternCrate, ItemInNs},
     visibility::Visibility,
@@ -16,6 +18,16 @@ pub enum Namespace {
     Macros,
 }
 
+bitflags! {
+    /// Describes only the presence/absence of each namespace, without its value.
+    #[derive(Debug, PartialEq, Eq)]
+    pub(crate) struct NsAvailability : u32 {
+        const TYPES = 1 << 0;
+        const VALUES = 1 << 1;
+        const MACROS = 1 << 2;
+    }
+}
+
 #[derive(Clone, Copy, Debug, Default, Eq, Hash, PartialEq)]
 pub struct PerNs {
     pub types: Option<(ModuleDefId, Visibility, Option<ImportOrExternCrate>)>,
@@ -24,6 +36,14 @@ pub struct PerNs {
 }
 
 impl PerNs {
+    pub(crate) fn availability(&self) -> NsAvailability {
+        let mut result = NsAvailability::empty();
+        result.set(NsAvailability::TYPES, self.types.is_some());
+        result.set(NsAvailability::VALUES, self.values.is_some());
+        result.set(NsAvailability::MACROS, self.macros.is_some());
+        result
+    }
+
     pub fn none() -> PerNs {
         PerNs { types: None, values: None, macros: None }
     }
diff --git a/src/tools/rust-analyzer/crates/ide/src/goto_definition.rs b/src/tools/rust-analyzer/crates/ide/src/goto_definition.rs
index 971cd3ef585..8836166d969 100644
--- a/src/tools/rust-analyzer/crates/ide/src/goto_definition.rs
+++ b/src/tools/rust-analyzer/crates/ide/src/goto_definition.rs
@@ -2750,4 +2750,36 @@ fn foo() {
         "#,
         );
     }
+
+    #[test]
+    fn issue_18138() {
+        check(
+            r#"
+mod foo {
+    macro_rules! x {
+        () => {
+            pub struct Foo;
+                    // ^^^
+        };
+    }
+    pub(crate) use x as m;
+}
+
+mod bar {
+    use crate::m;
+
+    m!();
+ // ^^^^^
+
+    fn qux() {
+        Foo$0;
+    }
+}
+
+mod m {}
+
+use foo::m;
+"#,
+        );
+    }
 }