Skip to content

Commit 1b9ae9e

Browse files
rust-bors[bot]rust-bors[bot]
authored
Auto merge of #150502 - petrochenkov:overglob, r=yaahc
resolve: Factor out and document the glob binding overwriting logic Also, avoid creating fresh name declarations and overwriting declarations in modules to update some fields in `DeclData`, when possible. Instead, change the fields directly in `DeclData` using cells. Unblocks #149195.
2 parents 2b82e05 + a251ae2 commit 1b9ae9e

11 files changed

Lines changed: 189 additions & 123 deletions

File tree

‎compiler/rustc_resolve/src/build_reduced_graph.rs‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -89,10 +89,10 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
8989
) {
9090
let decl = self.arenas.alloc_decl(DeclData {
9191
kind: DeclKind::Def(res),
92-
ambiguity,
92+
ambiguity: CmCell::new(ambiguity),
9393
// External ambiguities always report the `AMBIGUOUS_GLOB_IMPORTS` lint at the moment.
94-
warn_ambiguity: true,
95-
vis,
94+
warn_ambiguity: CmCell::new(true),
95+
vis: CmCell::new(vis),
9696
span,
9797
expansion,
9898
});
@@ -1158,18 +1158,18 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
11581158
self.r.potentially_unused_imports.push(import);
11591159
module.for_each_child_mut(self, |this, ident, ns, binding| {
11601160
if ns == MacroNS {
1161-
let import = if this.r.is_accessible_from(binding.vis, this.parent_scope.module)
1162-
{
1163-
import
1164-
} else {
1165-
// FIXME: This branch is used for reporting the `private_macro_use` lint
1166-
// and should eventually be removed.
1167-
if this.r.macro_use_prelude.contains_key(&ident.name) {
1168-
// Do not override already existing entries with compatibility entries.
1169-
return;
1170-
}
1171-
macro_use_import(this, span, true)
1172-
};
1161+
let import =
1162+
if this.r.is_accessible_from(binding.vis(), this.parent_scope.module) {
1163+
import
1164+
} else {
1165+
// FIXME: This branch is used for reporting the `private_macro_use` lint
1166+
// and should eventually be removed.
1167+
if this.r.macro_use_prelude.contains_key(&ident.name) {
1168+
// Do not override already existing entries with compatibility entries.
1169+
return;
1170+
}
1171+
macro_use_import(this, span, true)
1172+
};
11731173
let import_decl = this.r.new_import_decl(binding, import);
11741174
this.add_macro_use_decl(ident.name, import_decl, span, allow_shadowing);
11751175
}

‎compiler/rustc_resolve/src/diagnostics.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1338,7 +1338,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
13381338
}
13391339

13401340
let child_accessible =
1341-
accessible && this.is_accessible_from(name_binding.vis, parent_scope.module);
1341+
accessible && this.is_accessible_from(name_binding.vis(), parent_scope.module);
13421342

13431343
// do not venture inside inaccessible items of other crates
13441344
if in_module_is_extern && !child_accessible {
@@ -1358,8 +1358,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
13581358

13591359
// #90113: Do not count an inaccessible reexported item as a candidate.
13601360
if let DeclKind::Import { source_decl, .. } = name_binding.kind
1361-
&& this.is_accessible_from(source_decl.vis, parent_scope.module)
1362-
&& !this.is_accessible_from(name_binding.vis, parent_scope.module)
1361+
&& this.is_accessible_from(source_decl.vis(), parent_scope.module)
1362+
&& !this.is_accessible_from(name_binding.vis(), parent_scope.module)
13631363
{
13641364
return;
13651365
}
@@ -2243,7 +2243,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
22432243
let first = binding == first_binding;
22442244
let def_span = self.tcx.sess.source_map().guess_head_span(binding.span);
22452245
let mut note_span = MultiSpan::from_span(def_span);
2246-
if !first && binding.vis.is_public() {
2246+
if !first && binding.vis().is_public() {
22472247
let desc = match binding.kind {
22482248
DeclKind::Import { .. } => "re-export",
22492249
_ => "directly",

‎compiler/rustc_resolve/src/effective_visibilities.rs‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,9 @@ impl<'a, 'ra, 'tcx> EffectiveVisibilitiesVisitor<'a, 'ra, 'tcx> {
100100
if let Some(node_id) = import.id() {
101101
r.effective_visibilities.update_eff_vis(r.local_def_id(node_id), eff_vis, r.tcx)
102102
}
103-
} else if decl.ambiguity.is_some() && eff_vis.is_public_at_level(Level::Reexported) {
103+
} else if decl.ambiguity.get().is_some()
104+
&& eff_vis.is_public_at_level(Level::Reexported)
105+
{
104106
exported_ambiguities.insert(*decl);
105107
}
106108
}
@@ -125,9 +127,10 @@ impl<'a, 'ra, 'tcx> EffectiveVisibilitiesVisitor<'a, 'ra, 'tcx> {
125127
// If the binding is ambiguous, put the root ambiguity binding and all reexports
126128
// leading to it into the table. They are used by the `ambiguous_glob_reexports`
127129
// lint. For all bindings added to the table this way `is_ambiguity` returns true.
128-
let is_ambiguity = |decl: Decl<'ra>, warn: bool| decl.ambiguity.is_some() && !warn;
130+
let is_ambiguity =
131+
|decl: Decl<'ra>, warn: bool| decl.ambiguity.get().is_some() && !warn;
129132
let mut parent_id = ParentId::Def(module_id);
130-
let mut warn_ambiguity = decl.warn_ambiguity;
133+
let mut warn_ambiguity = decl.warn_ambiguity.get();
131134
while let DeclKind::Import { source_decl, .. } = decl.kind {
132135
self.update_import(decl, parent_id);
133136

@@ -140,12 +143,12 @@ impl<'a, 'ra, 'tcx> EffectiveVisibilitiesVisitor<'a, 'ra, 'tcx> {
140143

141144
parent_id = ParentId::Import(decl);
142145
decl = source_decl;
143-
warn_ambiguity |= source_decl.warn_ambiguity;
146+
warn_ambiguity |= source_decl.warn_ambiguity.get();
144147
}
145148
if !is_ambiguity(decl, warn_ambiguity)
146149
&& let Some(def_id) = decl.res().opt_def_id().and_then(|id| id.as_local())
147150
{
148-
self.update_def(def_id, decl.vis.expect_local(), parent_id);
151+
self.update_def(def_id, decl.vis().expect_local(), parent_id);
149152
}
150153
}
151154
}
@@ -188,7 +191,7 @@ impl<'a, 'ra, 'tcx> EffectiveVisibilitiesVisitor<'a, 'ra, 'tcx> {
188191
}
189192

190193
fn update_import(&mut self, decl: Decl<'ra>, parent_id: ParentId<'ra>) {
191-
let nominal_vis = decl.vis.expect_local();
194+
let nominal_vis = decl.vis().expect_local();
192195
let Some(cheap_private_vis) = self.may_update(nominal_vis, parent_id) else { return };
193196
let inherited_eff_vis = self.effective_vis_or_private(parent_id);
194197
let tcx = self.r.tcx;

‎compiler/rustc_resolve/src/ident.rs‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1017,7 +1017,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
10171017

10181018
// Items and single imports are not shadowable, if we have one, then it's determined.
10191019
if let Some(binding) = binding {
1020-
let accessible = self.is_accessible_from(binding.vis, parent_scope.module);
1020+
let accessible = self.is_accessible_from(binding.vis(), parent_scope.module);
10211021
return if accessible { Ok(binding) } else { Err(ControlFlow::Break(Determined)) };
10221022
}
10231023

@@ -1103,7 +1103,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
11031103
// shadowing is enabled, see `macro_expanded_macro_export_errors`).
11041104
if let Some(binding) = binding {
11051105
return if binding.determined() || ns == MacroNS || shadowing == Shadowing::Restricted {
1106-
let accessible = self.is_accessible_from(binding.vis, parent_scope.module);
1106+
let accessible = self.is_accessible_from(binding.vis(), parent_scope.module);
11071107
if accessible { Ok(binding) } else { Err(ControlFlow::Break(Determined)) }
11081108
} else {
11091109
Err(ControlFlow::Break(Undetermined))
@@ -1160,7 +1160,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
11601160
match result {
11611161
Err(Determined) => continue,
11621162
Ok(binding)
1163-
if !self.is_accessible_from(binding.vis, glob_import.parent_scope.module) =>
1163+
if !self.is_accessible_from(binding.vis(), glob_import.parent_scope.module) =>
11641164
{
11651165
continue;
11661166
}
@@ -1187,7 +1187,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
11871187
return Err(ControlFlow::Continue(Determined));
11881188
};
11891189

1190-
if !self.is_accessible_from(binding.vis, parent_scope.module) {
1190+
if !self.is_accessible_from(binding.vis(), parent_scope.module) {
11911191
if report_private {
11921192
self.privacy_errors.push(PrivacyError {
11931193
ident,
@@ -1304,7 +1304,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
13041304
) {
13051305
Err(Determined) => continue,
13061306
Ok(binding)
1307-
if !self.is_accessible_from(binding.vis, single_import.parent_scope.module) =>
1307+
if !self
1308+
.is_accessible_from(binding.vis(), single_import.parent_scope.module) =>
13081309
{
13091310
continue;
13101311
}

0 commit comments

Comments
 (0)