Skip to content

Commit b1bd478

Browse files
VeykrilVeykril
authored
Merge pull request #19742 from Veykril/push-ykmuwtkzruqm
fix: Fix incorrect handling of unresolved non-module imports in name resolution
2 parents 3b57c00 + 9a62507 commit b1bd478

File tree

6 files changed

+303
-22
lines changed

6 files changed

+303
-22
lines changed

‎crates/hir-def/src/lib.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -701,6 +701,16 @@ pub enum AssocItemId {
701701
// casting them, and somehow making the constructors private, which would be annoying.
702702
impl_from!(FunctionId, ConstId, TypeAliasId for AssocItemId);
703703

704+
impl From<AssocItemId> for ModuleDefId {
705+
fn from(item: AssocItemId) -> Self {
706+
match item {
707+
AssocItemId::FunctionId(f) => f.into(),
708+
AssocItemId::ConstId(c) => c.into(),
709+
AssocItemId::TypeAliasId(t) => t.into(),
710+
}
711+
}
712+
}
713+
704714
#[derive(Debug, PartialOrd, Ord, Clone, Copy, PartialEq, Eq, Hash, salsa_macros::Supertype)]
705715
pub enum GenericDefId {
706716
AdtId(AdtId),

‎crates/hir-def/src/nameres/assoc.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,15 @@ impl TraitItems {
6666
})
6767
}
6868

69+
pub fn assoc_item_by_name(&self, name: &Name) -> Option<AssocItemId> {
70+
self.items.iter().find_map(|&(ref item_name, item)| match item {
71+
AssocItemId::FunctionId(_) if item_name == name => Some(item),
72+
AssocItemId::TypeAliasId(_) if item_name == name => Some(item),
73+
AssocItemId::ConstId(_) if item_name == name => Some(item),
74+
_ => None,
75+
})
76+
}
77+
6978
pub fn attribute_calls(&self) -> impl Iterator<Item = (AstId<ast::Item>, MacroCallId)> + '_ {
7079
self.macro_calls.iter().flat_map(|it| it.iter()).copied()
7180
}

‎crates/hir-def/src/nameres/collector.rs

Lines changed: 58 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ use syntax::ast;
2626
use triomphe::Arc;
2727

2828
use crate::{
29-
AdtId, AstId, AstIdWithPath, ConstLoc, CrateRootModuleId, EnumLoc, ExternBlockLoc,
29+
AdtId, AssocItemId, AstId, AstIdWithPath, ConstLoc, CrateRootModuleId, EnumLoc, ExternBlockLoc,
3030
ExternCrateId, ExternCrateLoc, FunctionId, FunctionLoc, ImplLoc, Intern, ItemContainerId,
3131
LocalModuleId, Lookup, Macro2Id, Macro2Loc, MacroExpander, MacroId, MacroRulesId,
3232
MacroRulesLoc, MacroRulesLocFlags, ModuleDefId, ModuleId, ProcMacroId, ProcMacroLoc, StaticLoc,
@@ -45,7 +45,7 @@ use crate::{
4545
attr_resolution::{attr_macro_as_call_id, derive_macro_as_call_id},
4646
diagnostics::DefDiagnostic,
4747
mod_resolution::ModDir,
48-
path_resolution::ReachedFixedPoint,
48+
path_resolution::{ReachedFixedPoint, ResolvePathResult},
4949
proc_macro::{ProcMacroDef, ProcMacroKind, parse_macro_name_and_helper_attrs},
5050
sub_namespace_match,
5151
},
@@ -811,32 +811,35 @@ impl DefCollector<'_> {
811811
let _p = tracing::info_span!("resolve_import", import_path = %import.path.display(self.db, Edition::LATEST))
812812
.entered();
813813
tracing::debug!("resolving import: {:?} ({:?})", import, self.def_map.data.edition);
814-
let res = self.def_map.resolve_path_fp_with_macro(
815-
self.crate_local_def_map.as_deref().unwrap_or(&self.local_def_map),
816-
self.db,
817-
ResolveMode::Import,
818-
module_id,
819-
&import.path,
820-
BuiltinShadowMode::Module,
821-
None, // An import may resolve to any kind of macro.
822-
);
814+
let ResolvePathResult { resolved_def, segment_index, reached_fixedpoint, prefix_info } =
815+
self.def_map.resolve_path_fp_with_macro(
816+
self.crate_local_def_map.as_deref().unwrap_or(&self.local_def_map),
817+
self.db,
818+
ResolveMode::Import,
819+
module_id,
820+
&import.path,
821+
BuiltinShadowMode::Module,
822+
None, // An import may resolve to any kind of macro.
823+
);
823824

824-
let def = res.resolved_def;
825-
if res.reached_fixedpoint == ReachedFixedPoint::No || def.is_none() {
825+
if reached_fixedpoint == ReachedFixedPoint::No
826+
|| resolved_def.is_none()
827+
|| segment_index.is_some()
828+
{
826829
return PartialResolvedImport::Unresolved;
827830
}
828831

829-
if res.prefix_info.differing_crate {
832+
if prefix_info.differing_crate {
830833
return PartialResolvedImport::Resolved(
831-
def.filter_visibility(|v| matches!(v, Visibility::Public)),
834+
resolved_def.filter_visibility(|v| matches!(v, Visibility::Public)),
832835
);
833836
}
834837

835838
// Check whether all namespaces are resolved.
836-
if def.is_full() {
837-
PartialResolvedImport::Resolved(def)
839+
if resolved_def.is_full() {
840+
PartialResolvedImport::Resolved(resolved_def)
838841
} else {
839-
PartialResolvedImport::Indeterminate(def)
842+
PartialResolvedImport::Indeterminate(resolved_def)
840843
}
841844
}
842845

@@ -986,6 +989,43 @@ impl DefCollector<'_> {
986989
Some(ImportOrExternCrate::Glob(glob)),
987990
);
988991
}
992+
Some(ModuleDefId::TraitId(it)) => {
993+
// FIXME: Implement this correctly
994+
// We can't actually call `trait_items`, the reason being that if macro calls
995+
// occur, they will call back into the def map which we might be computing right
996+
// now resulting in a cycle.
997+
// To properly implement this, trait item collection needs to be done in def map
998+
// collection...
999+
let resolutions = if true {
1000+
vec![]
1001+
} else {
1002+
self.db
1003+
.trait_items(it)
1004+
.items
1005+
.iter()
1006+
.map(|&(ref name, variant)| {
1007+
let res = match variant {
1008+
AssocItemId::FunctionId(it) => {
1009+
PerNs::values(it.into(), vis, None)
1010+
}
1011+
AssocItemId::ConstId(it) => {
1012+
PerNs::values(it.into(), vis, None)
1013+
}
1014+
AssocItemId::TypeAliasId(it) => {
1015+
PerNs::types(it.into(), vis, None)
1016+
}
1017+
};
1018+
(Some(name.clone()), res)
1019+
})
1020+
.collect::<Vec<_>>()
1021+
};
1022+
self.update(
1023+
module_id,
1024+
&resolutions,
1025+
vis,
1026+
Some(ImportOrExternCrate::Glob(glob)),
1027+
);
1028+
}
9891029
Some(d) => {
9901030
tracing::debug!("glob import {:?} from non-module/enum {:?}", import, d);
9911031
}

‎crates/hir-def/src/nameres/path_resolution.rs

Lines changed: 61 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ use hir_expand::{
1717
name::Name,
1818
};
1919
use span::Edition;
20+
use stdx::TupleExt;
2021
use triomphe::Arc;
2122

2223
use crate::{
@@ -44,6 +45,7 @@ pub(super) enum ReachedFixedPoint {
4445
#[derive(Debug, Clone)]
4546
pub(super) struct ResolvePathResult {
4647
pub(super) resolved_def: PerNs,
48+
/// The index of the last resolved segment, or `None` if the full path has been resolved.
4749
pub(super) segment_index: Option<usize>,
4850
pub(super) reached_fixedpoint: ReachedFixedPoint,
4951
pub(super) prefix_info: ResolvePathResultPrefixInfo,
@@ -364,7 +366,15 @@ impl DefMap {
364366
},
365367
};
366368

367-
self.resolve_remaining_segments(segments, curr_per_ns, path, db, shadow, original_module)
369+
self.resolve_remaining_segments(
370+
db,
371+
mode,
372+
segments,
373+
curr_per_ns,
374+
path,
375+
shadow,
376+
original_module,
377+
)
368378
}
369379

370380
/// Resolves a path only in the preludes, without accounting for item scopes.
@@ -413,7 +423,15 @@ impl DefMap {
413423
}
414424
};
415425

416-
self.resolve_remaining_segments(segments, curr_per_ns, path, db, shadow, original_module)
426+
self.resolve_remaining_segments(
427+
db,
428+
mode,
429+
segments,
430+
curr_per_ns,
431+
path,
432+
shadow,
433+
original_module,
434+
)
417435
}
418436

419437
/// 2018-style absolute path -- only extern prelude
@@ -441,10 +459,11 @@ impl DefMap {
441459

442460
fn resolve_remaining_segments<'a>(
443461
&self,
462+
db: &dyn DefDatabase,
463+
mode: ResolveMode,
444464
mut segments: impl Iterator<Item = (usize, &'a Name)>,
445465
mut curr_per_ns: PerNs,
446466
path: &ModPath,
447-
db: &dyn DefDatabase,
448467
shadow: BuiltinShadowMode,
449468
original_module: LocalModuleId,
450469
) -> ResolvePathResult {
@@ -478,7 +497,7 @@ impl DefMap {
478497
let resolution = defp_map.resolve_path_fp_with_macro(
479498
LocalDefMap::EMPTY,
480499
db,
481-
ResolveMode::Other,
500+
mode,
482501
module.local_id,
483502
&path,
484503
shadow,
@@ -553,6 +572,44 @@ impl DefMap {
553572
),
554573
};
555574
}
575+
def @ ModuleDefId::TraitId(t) if mode == ResolveMode::Import => {
576+
// FIXME: Implement this correctly
577+
// We can't actually call `trait_items`, the reason being that if macro calls
578+
// occur, they will call back into the def map which we might be computing right
579+
// now resulting in a cycle.
580+
// To properly implement this, trait item collection needs to be done in def map
581+
// collection...
582+
let item =
583+
if true { None } else { db.trait_items(t).assoc_item_by_name(segment) };
584+
return match item {
585+
Some(item) => ResolvePathResult::new(
586+
match item {
587+
crate::AssocItemId::FunctionId(function_id) => PerNs::values(
588+
function_id.into(),
589+
curr.vis,
590+
curr.import.and_then(|it| it.import_or_glob()),
591+
),
592+
crate::AssocItemId::ConstId(const_id) => PerNs::values(
593+
const_id.into(),
594+
curr.vis,
595+
curr.import.and_then(|it| it.import_or_glob()),
596+
),
597+
crate::AssocItemId::TypeAliasId(type_alias_id) => {
598+
PerNs::types(type_alias_id.into(), curr.vis, curr.import)
599+
}
600+
},
601+
ReachedFixedPoint::Yes,
602+
segments.next().map(TupleExt::head),
603+
ResolvePathResultPrefixInfo::default(),
604+
),
605+
None => ResolvePathResult::new(
606+
PerNs::types(def, curr.vis, curr.import),
607+
ReachedFixedPoint::Yes,
608+
Some(i),
609+
ResolvePathResultPrefixInfo::default(),
610+
),
611+
};
612+
}
556613
s => {
557614
// could be an inherent method call in UFCS form
558615
// (`Struct::method`), or some other kind of associated item

0 commit comments

Comments
 (0)