Skip to content

Commit 44f40e9

Browse files
authored
Rollup merge of #143728 - LorrensP-2158466:refactor-resolve-extraction, r=petrochenkov
Resolve refactor: extraction of `finalize_module_binding` and `single_import_can_define_name` This pr the work Vadim asked for in #142547 (comment). This part: > finalize_module_binding/single_import_can_define_name extraction Cherry-picked commits of b-naber. Extraction of 2 processes in `resolve_ident_in_module_unadjusted`: - `finalize_module_binding` - `single_import_can_define_name` r? ```@petrochenkov```
2 parents 6e3d017 + 7632c55 commit 44f40e9

File tree

1 file changed

+143
-118
lines changed

1 file changed

+143
-118
lines changed

compiler/rustc_resolve/src/ident.rs

Lines changed: 143 additions & 118 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use rustc_span::{Ident, Span, kw, sym};
1313
use tracing::{debug, instrument};
1414

1515
use crate::errors::{ParamKindInEnumDiscriminant, ParamKindInNonTrivialAnonConst};
16-
use crate::imports::Import;
16+
use crate::imports::{Import, NameResolution};
1717
use crate::late::{ConstantHasGenerics, NoConstantGenericsReason, PathSource, Rib, RibKind};
1818
use crate::macros::{MacroRulesScope, sub_namespace_match};
1919
use crate::{
@@ -37,7 +37,7 @@ impl From<UsePrelude> for bool {
3737
}
3838
}
3939

40-
#[derive(Debug, PartialEq)]
40+
#[derive(Debug, PartialEq, Clone, Copy)]
4141
enum Shadowing {
4242
Restricted,
4343
Unrestricted,
@@ -879,53 +879,15 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
879879
.into_iter()
880880
.find_map(|binding| if binding == ignore_binding { None } else { binding });
881881

882-
if let Some(Finalize { path_span, report_private, used, root_span, .. }) = finalize {
883-
let Some(binding) = binding else {
884-
return Err((Determined, Weak::No));
885-
};
886-
887-
if !self.is_accessible_from(binding.vis, parent_scope.module) {
888-
if report_private {
889-
self.privacy_errors.push(PrivacyError {
890-
ident,
891-
binding,
892-
dedup_span: path_span,
893-
outermost_res: None,
894-
parent_scope: *parent_scope,
895-
single_nested: path_span != root_span,
896-
});
897-
} else {
898-
return Err((Determined, Weak::No));
899-
}
900-
}
901-
902-
// Forbid expanded shadowing to avoid time travel.
903-
if let Some(shadowed_glob) = resolution.shadowed_glob
904-
&& shadowing == Shadowing::Restricted
905-
&& binding.expansion != LocalExpnId::ROOT
906-
&& binding.res() != shadowed_glob.res()
907-
{
908-
self.ambiguity_errors.push(AmbiguityError {
909-
kind: AmbiguityKind::GlobVsExpanded,
910-
ident,
911-
b1: binding,
912-
b2: shadowed_glob,
913-
warning: false,
914-
misc1: AmbiguityErrorMisc::None,
915-
misc2: AmbiguityErrorMisc::None,
916-
});
917-
}
918-
919-
if shadowing == Shadowing::Unrestricted
920-
&& binding.expansion != LocalExpnId::ROOT
921-
&& let NameBindingKind::Import { import, .. } = binding.kind
922-
&& matches!(import.kind, ImportKind::MacroExport)
923-
{
924-
self.macro_expanded_macro_export_errors.insert((path_span, binding.span));
925-
}
926-
927-
self.record_use(ident, binding, used);
928-
return Ok(binding);
882+
if let Some(finalize) = finalize {
883+
return self.finalize_module_binding(
884+
ident,
885+
binding,
886+
resolution.shadowed_glob,
887+
parent_scope,
888+
finalize,
889+
shadowing,
890+
);
929891
}
930892

931893
let check_usable = |this: &mut Self, binding: NameBinding<'ra>| {
@@ -944,75 +906,15 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
944906

945907
// Check if one of single imports can still define the name,
946908
// if it can then our result is not determined and can be invalidated.
947-
for single_import in &resolution.single_imports {
948-
if ignore_import == Some(*single_import) {
949-
// This branch handles a cycle in single imports.
950-
//
951-
// For example:
952-
// ```
953-
// use a::b;
954-
// use b as a;
955-
// ```
956-
// 1. Record `use a::b` as the `ignore_import` and attempt to locate `a` in the
957-
// current module.
958-
// 2. Encounter the import `use b as a`, which is a `single_import` for `a`,
959-
// and try to find `b` in the current module.
960-
// 3. Re-encounter the `use a::b` import since it's a `single_import` of `b`.
961-
// This leads to entering this branch.
962-
continue;
963-
}
964-
if !self.is_accessible_from(single_import.vis, parent_scope.module) {
965-
continue;
966-
}
967-
if let Some(ignored) = ignore_binding
968-
&& let NameBindingKind::Import { import, .. } = ignored.kind
969-
&& import == *single_import
970-
{
971-
// Ignore not just the binding itself, but if it has a shadowed_glob,
972-
// ignore that, too, because this loop is supposed to only process
973-
// named imports.
974-
continue;
975-
}
976-
977-
let Some(module) = single_import.imported_module.get() else {
978-
return Err((Undetermined, Weak::No));
979-
};
980-
let ImportKind::Single { source, target, target_bindings, .. } = &single_import.kind
981-
else {
982-
unreachable!();
983-
};
984-
if source != target {
985-
// This branch allows the binding to be defined or updated later if the target name
986-
// can hide the source.
987-
if target_bindings.iter().all(|binding| binding.get().is_none()) {
988-
// None of the target bindings are available, so we can't determine
989-
// if this binding is correct or not.
990-
// See more details in #124840
991-
return Err((Undetermined, Weak::No));
992-
} else if target_bindings[ns].get().is_none() && binding.is_some() {
993-
// `binding.is_some()` avoids the condition where the binding
994-
// truly doesn't exist in this namespace and should return `Err(Determined)`.
995-
return Err((Undetermined, Weak::No));
996-
}
997-
}
998-
999-
match self.resolve_ident_in_module(
1000-
module,
1001-
*source,
1002-
ns,
1003-
&single_import.parent_scope,
1004-
None,
1005-
ignore_binding,
1006-
ignore_import,
1007-
) {
1008-
Err((Determined, _)) => continue,
1009-
Ok(binding)
1010-
if !self.is_accessible_from(binding.vis, single_import.parent_scope.module) =>
1011-
{
1012-
continue;
1013-
}
1014-
Ok(_) | Err((Undetermined, _)) => return Err((Undetermined, Weak::No)),
1015-
}
909+
if self.single_import_can_define_name(
910+
&resolution,
911+
binding,
912+
ns,
913+
ignore_import,
914+
ignore_binding,
915+
parent_scope,
916+
) {
917+
return Err((Undetermined, Weak::No));
1016918
}
1017919

1018920
// So we have a resolution that's from a glob import. This resolution is determined
@@ -1101,6 +1003,129 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
11011003
Err((Determined, Weak::No))
11021004
}
11031005

1006+
fn finalize_module_binding(
1007+
&mut self,
1008+
ident: Ident,
1009+
binding: Option<NameBinding<'ra>>,
1010+
shadowed_glob: Option<NameBinding<'ra>>,
1011+
parent_scope: &ParentScope<'ra>,
1012+
finalize: Finalize,
1013+
shadowing: Shadowing,
1014+
) -> Result<NameBinding<'ra>, (Determinacy, Weak)> {
1015+
let Finalize { path_span, report_private, used, root_span, .. } = finalize;
1016+
1017+
let Some(binding) = binding else {
1018+
return Err((Determined, Weak::No));
1019+
};
1020+
1021+
if !self.is_accessible_from(binding.vis, parent_scope.module) {
1022+
if report_private {
1023+
self.privacy_errors.push(PrivacyError {
1024+
ident,
1025+
binding,
1026+
dedup_span: path_span,
1027+
outermost_res: None,
1028+
parent_scope: *parent_scope,
1029+
single_nested: path_span != root_span,
1030+
});
1031+
} else {
1032+
return Err((Determined, Weak::No));
1033+
}
1034+
}
1035+
1036+
// Forbid expanded shadowing to avoid time travel.
1037+
if let Some(shadowed_glob) = shadowed_glob
1038+
&& shadowing == Shadowing::Restricted
1039+
&& binding.expansion != LocalExpnId::ROOT
1040+
&& binding.res() != shadowed_glob.res()
1041+
{
1042+
self.ambiguity_errors.push(AmbiguityError {
1043+
kind: AmbiguityKind::GlobVsExpanded,
1044+
ident,
1045+
b1: binding,
1046+
b2: shadowed_glob,
1047+
warning: false,
1048+
misc1: AmbiguityErrorMisc::None,
1049+
misc2: AmbiguityErrorMisc::None,
1050+
});
1051+
}
1052+
1053+
if shadowing == Shadowing::Unrestricted
1054+
&& binding.expansion != LocalExpnId::ROOT
1055+
&& let NameBindingKind::Import { import, .. } = binding.kind
1056+
&& matches!(import.kind, ImportKind::MacroExport)
1057+
{
1058+
self.macro_expanded_macro_export_errors.insert((path_span, binding.span));
1059+
}
1060+
1061+
self.record_use(ident, binding, used);
1062+
return Ok(binding);
1063+
}
1064+
1065+
// Checks if a single import can define the `Ident` corresponding to `binding`.
1066+
// This is used to check whether we can definitively accept a glob as a resolution.
1067+
fn single_import_can_define_name(
1068+
&mut self,
1069+
resolution: &NameResolution<'ra>,
1070+
binding: Option<NameBinding<'ra>>,
1071+
ns: Namespace,
1072+
ignore_import: Option<Import<'ra>>,
1073+
ignore_binding: Option<NameBinding<'ra>>,
1074+
parent_scope: &ParentScope<'ra>,
1075+
) -> bool {
1076+
for single_import in &resolution.single_imports {
1077+
if ignore_import == Some(*single_import) {
1078+
continue;
1079+
}
1080+
if !self.is_accessible_from(single_import.vis, parent_scope.module) {
1081+
continue;
1082+
}
1083+
if let Some(ignored) = ignore_binding
1084+
&& let NameBindingKind::Import { import, .. } = ignored.kind
1085+
&& import == *single_import
1086+
{
1087+
continue;
1088+
}
1089+
1090+
let Some(module) = single_import.imported_module.get() else {
1091+
return true;
1092+
};
1093+
let ImportKind::Single { source, target, target_bindings, .. } = &single_import.kind
1094+
else {
1095+
unreachable!();
1096+
};
1097+
if source != target {
1098+
if target_bindings.iter().all(|binding| binding.get().is_none()) {
1099+
return true;
1100+
} else if target_bindings[ns].get().is_none() && binding.is_some() {
1101+
return true;
1102+
}
1103+
}
1104+
1105+
match self.resolve_ident_in_module(
1106+
module,
1107+
*source,
1108+
ns,
1109+
&single_import.parent_scope,
1110+
None,
1111+
ignore_binding,
1112+
ignore_import,
1113+
) {
1114+
Err((Determined, _)) => continue,
1115+
Ok(binding)
1116+
if !self.is_accessible_from(binding.vis, single_import.parent_scope.module) =>
1117+
{
1118+
continue;
1119+
}
1120+
Ok(_) | Err((Undetermined, _)) => {
1121+
return true;
1122+
}
1123+
}
1124+
}
1125+
1126+
false
1127+
}
1128+
11041129
/// Validate a local resolution (from ribs).
11051130
#[instrument(level = "debug", skip(self, all_ribs))]
11061131
fn validate_res_from_ribs(

0 commit comments

Comments
 (0)