resolve: Update NameBindingData::vis in place

instead of overwriting bindings in modules.
This commit is contained in:
Vadim Petrochenkov 2025-12-30 01:47:50 +03:00
parent 32cf3a96d7
commit 227e7bd48b
7 changed files with 49 additions and 45 deletions

View file

@ -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);
}

View file

@ -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",

View file

@ -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;

View file

@ -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;
}

View file

@ -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 })

View file

@ -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() {

View file

@ -811,7 +811,7 @@ struct DeclData<'ra> {
warn_ambiguity: CmCell<bool>,
expansion: LocalExpnId,
span: Span,
vis: Visibility<DefId>,
vis: CmCell<Visibility<DefId>>,
}
/// 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<DefId> {
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,
})