Skip to content

Commit 2818f1d

Browse files
b-naberb-naber
committed
address review
1 parent 4d7fa7c commit 2818f1d

File tree

2 files changed

+38
-84
lines changed

2 files changed

+38
-84
lines changed

‎compiler/rustc_resolve/src/ident.rs‎

Lines changed: 37 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use crate::{
1919
AmbiguityError, AmbiguityErrorMisc, AmbiguityKind, BindingKey, CmResolver, Determinacy,
2020
Finalize, ImportKind, LexicalScopeBinding, Module, ModuleKind, ModuleOrUniformRoot,
2121
NameBinding, NameBindingKind, ParentScope, PathResult, PrivacyError, Res, ResolutionError,
22-
Resolver, Scope, ScopeSet, Segment, Shadowing, Used, Weak, errors,
22+
Resolver, Scope, ScopeSet, Segment, Used, Weak, errors,
2323
};
2424

2525
#[derive(Copy, Clone)]
@@ -34,6 +34,12 @@ impl From<UsePrelude> for bool {
3434
}
3535
}
3636

37+
#[derive(Debug, PartialEq, Clone, Copy)]
38+
enum Shadowing {
39+
Restricted,
40+
Unrestricted,
41+
}
42+
3743
bitflags::bitflags! {
3844
#[derive(Clone, Copy, Debug)]
3945
struct Flags: u8 {
@@ -107,7 +113,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
107113
let (ns, macro_kind) = match scope_set {
108114
ScopeSet::All(ns)
109115
| ScopeSet::ModuleAndExternPrelude(ns, _)
110-
| ScopeSet::Module(_, ns, _)
116+
| ScopeSet::Module(ns, ..)
111117
| ScopeSet::Late(ns, ..) => (ns, None),
112118
ScopeSet::ExternPrelude => (TypeNS, None),
113119
ScopeSet::Macro(macro_kind) => (MacroNS, Some(macro_kind)),
@@ -116,7 +122,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
116122
// Start with the specified module.
117123
ScopeSet::Late(_, module, _)
118124
| ScopeSet::ModuleAndExternPrelude(_, module)
119-
| ScopeSet::Module(module, ..) => module,
125+
| ScopeSet::Module(_, module) => module,
120126
// Jump out of trait or enum modules, they do not act as scopes.
121127
_ => parent_scope.module.nearest_item_scope(),
122128
};
@@ -125,7 +131,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
125131
let module_and_extern_prelude = matches!(scope_set, ScopeSet::ModuleAndExternPrelude(..));
126132
let extern_prelude = matches!(scope_set, ScopeSet::ExternPrelude);
127133
let mut scope = match ns {
128-
_ if (module_and_extern_prelude || module_scope) => Scope::NonGlobModule(module, None),
134+
_ if module_and_extern_prelude || module_scope => Scope::NonGlobModule(module, None),
129135
_ if extern_prelude => Scope::ExternPreludeItems,
130136
TypeNS | ValueNS => Scope::NonGlobModule(module, None),
131137
MacroNS => Scope::DeriveHelpers(parent_scope.expansion),
@@ -355,7 +361,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
355361
ident,
356362
ns,
357363
parent_scope,
358-
Shadowing::Unrestricted,
359364
finalize.map(|finalize| Finalize { used: Used::Scope, ..finalize }),
360365
ignore_binding,
361366
None,
@@ -418,7 +423,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
418423
let (ns, macro_kind) = match scope_set {
419424
ScopeSet::All(ns)
420425
| ScopeSet::ModuleAndExternPrelude(ns, _)
421-
| ScopeSet::Module(_, ns, _)
426+
| ScopeSet::Module(ns, ..)
422427
| ScopeSet::Late(ns, ..) => (ns, None),
423428
ScopeSet::ExternPrelude => (TypeNS, None),
424429
ScopeSet::Macro(macro_kind) => (MacroNS, Some(macro_kind)),
@@ -520,13 +525,13 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
520525
ns,
521526
adjusted_parent_scope,
522527
match scope_set {
523-
ScopeSet::Late(..) => Shadowing::Unrestricted,
524-
ScopeSet::Module(_, _, shadowing) => shadowing,
528+
ScopeSet::Late(..) | ScopeSet::Module(..) => {
529+
Shadowing::Unrestricted
530+
}
525531
_ => Shadowing::Restricted,
526532
},
527533
adjusted_finalize,
528534
ignore_binding,
529-
ignore_import,
530535
);
531536

532537
match binding {
@@ -590,8 +595,9 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
590595
ns,
591596
adjusted_parent_scope,
592597
match scope_set {
593-
ScopeSet::Late(..) => Shadowing::Unrestricted,
594-
ScopeSet::Module(_, _, shadowing) => shadowing,
598+
ScopeSet::Late(..) | ScopeSet::Module(..) => {
599+
Shadowing::Unrestricted
600+
}
595601
_ => Shadowing::Restricted,
596602
},
597603
finalize.map(|finalize| Finalize { used: Used::Scope, ..finalize }),
@@ -665,7 +671,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
665671
ident,
666672
ns,
667673
parent_scope,
668-
Shadowing::Unrestricted,
669674
None,
670675
ignore_binding,
671676
ignore_import,
@@ -883,7 +888,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
883888
ident,
884889
ns,
885890
adjusted_parent_scope,
886-
Shadowing::Unrestricted,
887891
finalize,
888892
ignore_binding,
889893
ignore_import,
@@ -898,7 +902,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
898902
ident: Ident,
899903
ns: Namespace,
900904
parent_scope: &ParentScope<'ra>,
901-
shadowing: Shadowing,
902905
finalize: Option<Finalize>,
903906
// This binding should be ignored during in-module resolution, so that we don't get
904907
// "self-confirming" import resolutions during import validation and checking.
@@ -908,27 +911,24 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
908911
match module {
909912
ModuleOrUniformRoot::Module(module) => self.early_resolve_ident_in_lexical_scope(
910913
ident,
911-
ScopeSet::Module(module, ns, shadowing),
914+
ScopeSet::Module(ns, module),
912915
parent_scope,
913916
finalize,
914917
finalize.is_some(),
915918
ignore_binding,
916919
ignore_import,
917920
),
918-
ModuleOrUniformRoot::ModuleAndExternPrelude(module) => {
919-
assert_eq!(shadowing, Shadowing::Unrestricted);
920-
self.early_resolve_ident_in_lexical_scope(
921+
ModuleOrUniformRoot::ModuleAndExternPrelude(module) => self
922+
.early_resolve_ident_in_lexical_scope(
921923
ident,
922924
ScopeSet::ModuleAndExternPrelude(ns, module),
923925
parent_scope,
924926
finalize,
925927
finalize.is_some(),
926928
ignore_binding,
927929
ignore_import,
928-
)
929-
}
930+
),
930931
ModuleOrUniformRoot::ExternPrelude => {
931-
assert_eq!(shadowing, Shadowing::Unrestricted);
932932
return if ns != TypeNS {
933933
Err(Determined)
934934
} else {
@@ -944,7 +944,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
944944
};
945945
}
946946
ModuleOrUniformRoot::CurrentScope => {
947-
assert_eq!(shadowing, Shadowing::Unrestricted);
948947
if ns == TypeNS {
949948
if ident.name == kw::Crate || ident.name == kw::DollarCrate {
950949
let module = self.resolve_crate_root(ident);
@@ -980,7 +979,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
980979
// This binding should be ignored during in-module resolution, so that we don't get
981980
// "self-confirming" import resolutions during import validation and checking.
982981
ignore_binding: Option<NameBinding<'ra>>,
983-
_ignore_import: Option<Import<'ra>>, // not used, but kept for signature consistency
984982
) -> Result<NameBinding<'ra>, (Determinacy, Weak)> {
985983
let key = BindingKey::new(ident, ns);
986984
// `try_borrow_mut` is required to ensure exclusive access, even if the resulting binding
@@ -991,16 +989,11 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
991989
.try_borrow_mut()
992990
.map_err(|_| (Determined, Weak::No))?;
993991

994-
let check_usable = |this: CmResolver<'r, 'ra, 'tcx>, binding: NameBinding<'ra>| {
995-
let usable = this.is_accessible_from(binding.vis, parent_scope.module);
996-
if usable { Ok(binding) } else { Err((Determined, Weak::No)) }
997-
};
998-
999992
if let Some(binding) = resolution.non_glob_binding
1000993
&& ignore_binding.map_or(true, |b| binding != b)
1001994
{
1002995
if let Some(finalize) = finalize {
1003-
return self.get_mut().finalize_non_glob_module_binding(
996+
return self.get_mut().finalize_module_binding(
1004997
ident,
1005998
binding,
1006999
resolution.glob_binding,
@@ -1009,7 +1002,12 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
10091002
shadowing,
10101003
);
10111004
} else {
1012-
return check_usable(self, binding);
1005+
let usable = self.is_accessible_from(binding.vis, parent_scope.module);
1006+
if usable {
1007+
return Ok(binding);
1008+
} else {
1009+
return Err((Determined, Weak::No));
1010+
};
10131011
}
10141012
}
10151013

@@ -1035,18 +1033,14 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
10351033
.try_borrow_mut()
10361034
.map_err(|_| (Determined, Weak::No))?; // This happens when there is a cycle of imports.
10371035

1038-
let check_usable = |this: CmResolver<'r, 'ra, 'tcx>, binding: NameBinding<'ra>| {
1039-
let usable = this.is_accessible_from(binding.vis, parent_scope.module);
1040-
if usable { Ok(binding) } else { Err((Determined, Weak::No)) }
1041-
};
1042-
10431036
if let Some(binding) = resolution.glob_binding
10441037
&& ignore_binding.map_or(true, |b| binding != b)
10451038
{
10461039
if let Some(finalize) = finalize {
1047-
return self.get_mut().finalize_glob_module_binding(
1040+
return self.get_mut().finalize_module_binding(
10481041
ident,
10491042
binding,
1043+
None,
10501044
parent_scope,
10511045
finalize,
10521046
shadowing,
@@ -1079,7 +1073,12 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
10791073
// and prohibit access to macro-expanded `macro_export` macros instead (unless restricted
10801074
// shadowing is enabled, see `macro_expanded_macro_export_errors`).
10811075
if binding.determined() || ns == MacroNS || shadowing == Shadowing::Restricted {
1082-
return check_usable(self, binding);
1076+
let usable = self.is_accessible_from(binding.vis, parent_scope.module);
1077+
if usable {
1078+
return Ok(binding);
1079+
} else {
1080+
return Err((Determined, Weak::No));
1081+
};
10831082
} else {
10841083
return Err((Undetermined, Weak::No));
10851084
}
@@ -1163,7 +1162,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
11631162
ident,
11641163
ns,
11651164
adjusted_parent_scope,
1166-
Shadowing::Unrestricted,
11671165
None,
11681166
ignore_binding,
11691167
ignore_import,
@@ -1184,7 +1182,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
11841182
(Determined, Weak::No)
11851183
}
11861184

1187-
fn finalize_non_glob_module_binding(
1185+
fn finalize_module_binding(
11881186
&mut self,
11891187
ident: Ident,
11901188
binding: NameBinding<'ra>,
@@ -1242,44 +1240,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
12421240
return Ok(binding);
12431241
}
12441242

1245-
fn finalize_glob_module_binding(
1246-
&mut self,
1247-
ident: Ident,
1248-
binding: NameBinding<'ra>,
1249-
parent_scope: &ParentScope<'ra>,
1250-
finalize: Finalize,
1251-
shadowing: Shadowing,
1252-
) -> Result<NameBinding<'ra>, (Determinacy, Weak)> {
1253-
let Finalize { path_span, report_private, used, root_span, .. } = finalize;
1254-
1255-
if !self.is_accessible_from(binding.vis, parent_scope.module) {
1256-
if report_private {
1257-
self.privacy_errors.push(PrivacyError {
1258-
ident,
1259-
binding,
1260-
dedup_span: path_span,
1261-
outermost_res: None,
1262-
source: None,
1263-
parent_scope: *parent_scope,
1264-
single_nested: path_span != root_span,
1265-
});
1266-
} else {
1267-
return Err((Determined, Weak::No));
1268-
}
1269-
}
1270-
1271-
if shadowing == Shadowing::Unrestricted
1272-
&& binding.expansion != LocalExpnId::ROOT
1273-
&& let NameBindingKind::Import { import, .. } = binding.kind
1274-
&& matches!(import.kind, ImportKind::MacroExport)
1275-
{
1276-
self.macro_expanded_macro_export_errors.insert((path_span, binding.span));
1277-
}
1278-
1279-
self.record_use(ident, binding, used);
1280-
return Ok(binding);
1281-
}
1282-
12831243
// Checks if a single import can define the `Ident` corresponding to `binding`.
12841244
// This is used to check whether we can definitively accept a glob as a resolution.
12851245
fn single_import_can_define_name<'r>(

‎compiler/rustc_resolve/src/lib.rs‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -147,12 +147,6 @@ enum Scope<'ra> {
147147
BuiltinTypes,
148148
}
149149

150-
#[derive(Debug, PartialEq, Clone, Copy)]
151-
enum Shadowing {
152-
Restricted,
153-
Unrestricted,
154-
}
155-
156150
/// Names from different contexts may want to visit different subsets of all specific scopes
157151
/// with different restrictions when looking up the resolution.
158152
#[derive(Clone, Copy, Debug)]
@@ -169,7 +163,7 @@ enum ScopeSet<'ra> {
169163
/// The node id enables lints and is used for reporting them.
170164
Late(Namespace, Module<'ra>, Option<NodeId>),
171165
/// Scope::NonGlobModule and Scope::GlobModule.
172-
Module(Module<'ra>, Namespace, Shadowing),
166+
Module(Namespace, Module<'ra>),
173167
}
174168

175169
/// Everything you need to know about a name's location to resolve it.

0 commit comments

Comments
 (0)