Skip to content

Commit

Permalink
Fix name resolution when an import is resolved to some namespace and …
Browse files Browse the repository at this point in the history
…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 rust-lang#2466, but it was not fixed fully.
  • Loading branch information
ChayimFriedman2 committed Sep 22, 2024
1 parent 6dad8c5 commit 8e55ddc
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 13 deletions.
43 changes: 30 additions & 13 deletions crates/hir-def/src/nameres/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}

Expand Down
20 changes: 20 additions & 0 deletions crates/hir-def/src/per_ns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
//!
//! `PerNs` (per namespace) captures this.

use bitflags::bitflags;

use crate::{
item_scope::{ImportId, ImportOrExternCrate, ItemInNs},
visibility::Visibility,
Expand All @@ -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>)>,
Expand All @@ -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 }
}
Expand Down
32 changes: 32 additions & 0 deletions crates/ide/src/goto_definition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
"#,
);
}
}

0 comments on commit 8e55ddc

Please sign in to comment.