From 227e7bd48b5a7f3479bc4b231cf042e092fadd89 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Tue, 30 Dec 2025 01:47:50 +0300 Subject: [PATCH] resolve: Update `NameBindingData::vis` in place instead of overwriting bindings in modules. --- .../rustc_resolve/src/build_reduced_graph.rs | 26 +++++++------- compiler/rustc_resolve/src/diagnostics.rs | 8 ++--- .../src/effective_visibilities.rs | 4 +-- compiler/rustc_resolve/src/ident.rs | 11 +++--- compiler/rustc_resolve/src/imports.rs | 35 +++++++++---------- .../rustc_resolve/src/late/diagnostics.rs | 2 +- compiler/rustc_resolve/src/lib.rs | 8 +++-- 7 files changed, 49 insertions(+), 45 deletions(-) diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs index 0159e9c9eda4..0167b8038649 100644 --- a/compiler/rustc_resolve/src/build_reduced_graph.rs +++ b/compiler/rustc_resolve/src/build_reduced_graph.rs @@ -92,7 +92,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ambiguity, // External ambiguities always report the `AMBIGUOUS_GLOB_IMPORTS` lint at the moment. warn_ambiguity: CmCell::new(true), - vis, + vis: CmCell::new(vis), span, expansion, }); @@ -1158,18 +1158,18 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> { self.r.potentially_unused_imports.push(import); module.for_each_child_mut(self, |this, ident, ns, binding| { if ns == MacroNS { - let import = if this.r.is_accessible_from(binding.vis, this.parent_scope.module) - { - import - } else { - // FIXME: This branch is used for reporting the `private_macro_use` lint - // and should eventually be removed. - if this.r.macro_use_prelude.contains_key(&ident.name) { - // Do not override already existing entries with compatibility entries. - return; - } - macro_use_import(this, span, true) - }; + let import = + if this.r.is_accessible_from(binding.vis(), this.parent_scope.module) { + import + } else { + // FIXME: This branch is used for reporting the `private_macro_use` lint + // and should eventually be removed. + if this.r.macro_use_prelude.contains_key(&ident.name) { + // Do not override already existing entries with compatibility entries. + return; + } + macro_use_import(this, span, true) + }; let import_decl = this.r.new_import_decl(binding, import); this.add_macro_use_decl(ident.name, import_decl, span, allow_shadowing); } diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index 7c86ed91a07a..d704280f3fa2 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -1338,7 +1338,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { } let child_accessible = - accessible && this.is_accessible_from(name_binding.vis, parent_scope.module); + accessible && this.is_accessible_from(name_binding.vis(), parent_scope.module); // do not venture inside inaccessible items of other crates if in_module_is_extern && !child_accessible { @@ -1358,8 +1358,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { // #90113: Do not count an inaccessible reexported item as a candidate. if let DeclKind::Import { source_decl, .. } = name_binding.kind - && this.is_accessible_from(source_decl.vis, parent_scope.module) - && !this.is_accessible_from(name_binding.vis, parent_scope.module) + && this.is_accessible_from(source_decl.vis(), parent_scope.module) + && !this.is_accessible_from(name_binding.vis(), parent_scope.module) { return; } @@ -2243,7 +2243,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { let first = binding == first_binding; let def_span = self.tcx.sess.source_map().guess_head_span(binding.span); let mut note_span = MultiSpan::from_span(def_span); - if !first && binding.vis.is_public() { + if !first && binding.vis().is_public() { let desc = match binding.kind { DeclKind::Import { .. } => "re-export", _ => "directly", diff --git a/compiler/rustc_resolve/src/effective_visibilities.rs b/compiler/rustc_resolve/src/effective_visibilities.rs index eee12922e511..7272314b9aa7 100644 --- a/compiler/rustc_resolve/src/effective_visibilities.rs +++ b/compiler/rustc_resolve/src/effective_visibilities.rs @@ -145,7 +145,7 @@ impl<'a, 'ra, 'tcx> EffectiveVisibilitiesVisitor<'a, 'ra, 'tcx> { if !is_ambiguity(decl, warn_ambiguity) && let Some(def_id) = decl.res().opt_def_id().and_then(|id| id.as_local()) { - self.update_def(def_id, decl.vis.expect_local(), parent_id); + self.update_def(def_id, decl.vis().expect_local(), parent_id); } } } @@ -188,7 +188,7 @@ impl<'a, 'ra, 'tcx> EffectiveVisibilitiesVisitor<'a, 'ra, 'tcx> { } fn update_import(&mut self, decl: Decl<'ra>, parent_id: ParentId<'ra>) { - let nominal_vis = decl.vis.expect_local(); + let nominal_vis = decl.vis().expect_local(); let Some(cheap_private_vis) = self.may_update(nominal_vis, parent_id) else { return }; let inherited_eff_vis = self.effective_vis_or_private(parent_id); let tcx = self.r.tcx; diff --git a/compiler/rustc_resolve/src/ident.rs b/compiler/rustc_resolve/src/ident.rs index e0acf043ffcf..f47bb28d3fa1 100644 --- a/compiler/rustc_resolve/src/ident.rs +++ b/compiler/rustc_resolve/src/ident.rs @@ -1017,7 +1017,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { // Items and single imports are not shadowable, if we have one, then it's determined. if let Some(binding) = binding { - let accessible = self.is_accessible_from(binding.vis, parent_scope.module); + let accessible = self.is_accessible_from(binding.vis(), parent_scope.module); return if accessible { Ok(binding) } else { Err(ControlFlow::Break(Determined)) }; } @@ -1103,7 +1103,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { // shadowing is enabled, see `macro_expanded_macro_export_errors`). if let Some(binding) = binding { return if binding.determined() || ns == MacroNS || shadowing == Shadowing::Restricted { - let accessible = self.is_accessible_from(binding.vis, parent_scope.module); + let accessible = self.is_accessible_from(binding.vis(), parent_scope.module); if accessible { Ok(binding) } else { Err(ControlFlow::Break(Determined)) } } else { Err(ControlFlow::Break(Undetermined)) @@ -1160,7 +1160,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { match result { Err(Determined) => continue, Ok(binding) - if !self.is_accessible_from(binding.vis, glob_import.parent_scope.module) => + if !self.is_accessible_from(binding.vis(), glob_import.parent_scope.module) => { continue; } @@ -1187,7 +1187,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { return Err(ControlFlow::Continue(Determined)); }; - if !self.is_accessible_from(binding.vis, parent_scope.module) { + if !self.is_accessible_from(binding.vis(), parent_scope.module) { if report_private { self.privacy_errors.push(PrivacyError { ident, @@ -1304,7 +1304,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ) { Err(Determined) => continue, Ok(binding) - if !self.is_accessible_from(binding.vis, single_import.parent_scope.module) => + if !self + .is_accessible_from(binding.vis(), single_import.parent_scope.module) => { continue; } diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index 7f28ac29ac1d..5b0755dbde25 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -306,7 +306,7 @@ fn remove_same_import<'ra>(d1: Decl<'ra>, d2: Decl<'ra>) -> (Decl<'ra>, Decl<'ra assert_eq!(d1.warn_ambiguity.get(), d2.warn_ambiguity.get()); assert_eq!(d1.expansion, d2.expansion); assert_eq!(d1.span, d2.span); - assert_eq!(d1.vis, d2.vis); + assert_eq!(d1.vis(), d2.vis()); remove_same_import(d1_next, d2_next) } else { (d1, d2) @@ -318,12 +318,12 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { /// create the corresponding import declaration. pub(crate) fn new_import_decl(&self, decl: Decl<'ra>, import: Import<'ra>) -> Decl<'ra> { let import_vis = import.vis.to_def_id(); - let vis = if decl.vis.is_at_least(import_vis, self.tcx) + let vis = if decl.vis().is_at_least(import_vis, self.tcx) || pub_use_of_private_extern_crate_hack(import, decl).is_some() { import_vis } else { - decl.vis + decl.vis() }; if let ImportKind::Glob { ref max_vis, .. } = import.kind @@ -338,7 +338,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ambiguity: None, warn_ambiguity: CmCell::new(false), span: import.span, - vis, + vis: CmCell::new(vis), expansion: import.parent_scope.expansion, }) } @@ -362,9 +362,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { // - A glob decl is overwritten by an ambiguous glob decl. // FIXME: avoid this by putting `DeclData::ambiguity` under a // cell and updating it in place. - // - A glob decl is overwritten by a glob decl with larger visibility. - // FIXME: avoid this by putting `DeclData::vis` under a cell - // and updating it in place. // - A glob decl is overwritten by a glob decl re-fetching an // overwritten decl from other module (the recursive case). // Here we are detecting all such re-fetches and overwrite old decls @@ -383,9 +380,10 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { glob_decl } else if glob_decl.res() != old_glob_decl.res() { self.new_decl_with_ambiguity(old_glob_decl, glob_decl, warn_ambiguity) - } else if !old_glob_decl.vis.is_at_least(glob_decl.vis, self.tcx) { + } else if !old_glob_decl.vis().is_at_least(glob_decl.vis(), self.tcx) { // We are glob-importing the same item but with greater visibility. - glob_decl + old_glob_decl.vis.set_unchecked(glob_decl.vis()); + old_glob_decl } else if glob_decl.is_ambiguity_recursive() { // Overwriting with an ambiguous glob import. glob_decl.warn_ambiguity.set_unchecked(true); @@ -464,7 +462,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ) -> Decl<'ra> { let ambiguity = Some(secondary_decl); let warn_ambiguity = CmCell::new(warn_ambiguity); - let data = DeclData { ambiguity, warn_ambiguity, ..*primary_decl }; + let vis = primary_decl.vis.clone(); + let data = DeclData { ambiguity, warn_ambiguity, vis, ..*primary_decl }; self.arenas.alloc_decl(data) } @@ -509,7 +508,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { Some(None) => import.parent_scope.module, None => continue, }; - if self.is_accessible_from(binding.vis, scope) { + if self.is_accessible_from(binding.vis(), scope) { let import_decl = self.new_import_decl(binding, *import); let _ = self.try_plant_decl_into_local_module( import.parent_scope.module, @@ -699,8 +698,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { && let Some(glob_import_id) = glob_import.id() && let glob_import_def_id = self.local_def_id(glob_import_id) && self.effective_visibilities.is_exported(glob_import_def_id) - && glob_decl.vis.is_public() - && !binding.vis.is_public() + && glob_decl.vis().is_public() + && !binding.vis().is_public() { let binding_id = match binding.kind { DeclKind::Def(res) => { @@ -1330,9 +1329,9 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { return; }; - if !binding.vis.is_at_least(import.vis, this.tcx) { + if !binding.vis().is_at_least(import.vis, this.tcx) { reexport_error = Some((ns, binding)); - if let Visibility::Restricted(binding_def_id) = binding.vis + if let Visibility::Restricted(binding_def_id) = binding.vis() && binding_def_id.is_top_level_module() { crate_private_reexport = true; @@ -1535,7 +1534,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { Some(None) => import.parent_scope.module, None => continue, }; - if self.is_accessible_from(binding.vis, scope) { + if self.is_accessible_from(binding.vis(), scope) { let import_decl = self.new_import_decl(binding, import); let warn_ambiguity = self .resolution(import.parent_scope.module, key) @@ -1577,7 +1576,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { let child = |reexport_chain| ModChild { ident: ident.0, res, - vis: binding.vis, + vis: binding.vis(), reexport_chain, }; if let Some((ambig_binding1, ambig_binding2)) = binding.descent_to_ambiguity() { @@ -1585,7 +1584,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { let second = ModChild { ident: ident.0, res: ambig_binding2.res().expect_non_local(), - vis: ambig_binding2.vis, + vis: ambig_binding2.vis(), reexport_chain: ambig_binding2.reexport_chain(this), }; ambig_children.push(AmbigModChild { main, second }) diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 73e1a8f0c3bc..a2805497e796 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -2793,7 +2793,7 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> { in_module.for_each_child(self.r, |r, ident, _, name_binding| { // abort if the module is already found or if name_binding is private external - if result.is_some() || !name_binding.vis.is_visible_locally() { + if result.is_some() || !name_binding.vis().is_visible_locally() { return; } if let Some(module_def_id) = name_binding.res().module_like_def_id() { diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index cf8d1952d4a0..bce14f5376d7 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -811,7 +811,7 @@ struct DeclData<'ra> { warn_ambiguity: CmCell, expansion: LocalExpnId, span: Span, - vis: Visibility, + vis: CmCell>, } /// All name declarations are unique and allocated on a same arena, @@ -923,6 +923,10 @@ struct AmbiguityError<'ra> { } impl<'ra> DeclData<'ra> { + fn vis(&self) -> Visibility { + self.vis.get() + } + fn res(&self) -> Res { match self.kind { DeclKind::Def(res) => res, @@ -1344,7 +1348,7 @@ impl<'ra> ResolverArenas<'ra> { kind: DeclKind::Def(res), ambiguity: None, warn_ambiguity: CmCell::new(false), - vis, + vis: CmCell::new(vis), span, expansion, })