From 8a6617ab991655f0c2b4340f58a6e0a975a4891c Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 13 Apr 2026 16:44:21 +0300 Subject: [PATCH 1/4] privacy: Assert that compared visibilities are (usually) ordered --- compiler/rustc_middle/src/middle/privacy.rs | 12 +++++++-- compiler/rustc_middle/src/ty/mod.rs | 28 ++++++++++++++++++--- compiler/rustc_resolve/src/imports.rs | 4 ++- 3 files changed, 37 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_middle/src/middle/privacy.rs b/compiler/rustc_middle/src/middle/privacy.rs index 0be4c8243d632..71b5ee9b8810b 100644 --- a/compiler/rustc_middle/src/middle/privacy.rs +++ b/compiler/rustc_middle/src/middle/privacy.rs @@ -82,7 +82,9 @@ impl EffectiveVisibility { for l in Level::all_levels() { let rhs_vis = self.at_level_mut(l); let lhs_vis = *lhs.at_level(l); - if rhs_vis.is_at_least(lhs_vis, tcx) { + // FIXME: figure out why unordered visibilities occur here, + // and what the behavior for them should be. + if rhs_vis.is_at_least_ext(lhs_vis, tcx, false) { *rhs_vis = lhs_vis; }; } @@ -252,8 +254,14 @@ impl EffectiveVisibilities { } // effective visibility can't be decreased at next update call for the // same id + // FIXME: figure out why unordered visibilities occur here, + // and what the behavior for them should be. if *current_effective_vis_at_level != calculated_effective_vis - && calculated_effective_vis.is_at_least(*current_effective_vis_at_level, tcx) + && calculated_effective_vis.is_at_least_ext( + *current_effective_vis_at_level, + tcx, + false, + ) { changed = true; *current_effective_vis_at_level = calculated_effective_vis; diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index a78e5096b2e5e..d5ec6d2d0ddc0 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -376,12 +376,32 @@ impl> Visibility { } /// Returns `true` if this visibility is at least as accessible as the given visibility - pub fn is_at_least(self, vis: Visibility>, tcx: TyCtxt<'_>) -> bool { - match vis { - Visibility::Public => self.is_public(), - Visibility::Restricted(id) => self.is_accessible_from(id, tcx), + #[track_caller] + pub fn is_at_least_ext( + self, + vis: Visibility>, + tcx: TyCtxt<'_>, + assert_ordered: bool, + ) -> bool { + match (self, vis) { + (Visibility::Public, _) => true, + (_, Visibility::Public) => false, + (Visibility::Restricted(lhs_id), Visibility::Restricted(rhs_id)) => { + let lhs_id = lhs_id.into(); + let rhs_id = rhs_id.into(); + let ge = tcx.is_descendant_of(rhs_id, lhs_id); + if !ge && assert_ordered && !tcx.is_descendant_of(lhs_id, rhs_id) { + bug!("unordered visibilities: {lhs_id:?} and {rhs_id:?}"); + } + ge + } } } + + #[track_caller] + pub fn is_at_least(self, vis: Visibility>, tcx: TyCtxt<'_>) -> bool { + self.is_at_least_ext(vis, tcx, true) + } } impl + Copy> Visibility { diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index a94f3ea435e2c..c81562deecfca 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -367,7 +367,9 @@ 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) + // FIXME: figure out why unordered visibilities occur here, + // and what the behavior for them should be. + let vis = if decl.vis().is_at_least_ext(import_vis, self.tcx, false) || pub_use_of_private_extern_crate_hack(import, decl).is_some() { import_vis From 39982c1823b3204cf3a1ae13942ed7443370b42c Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 13 Apr 2026 23:02:07 +0300 Subject: [PATCH 2/4] privacy: Use `greater_than` instead of `is_at_least` for comparing visibilities We can do it because the compared visibilities are guaranteed to be ordered --- .../rustc_hir_analysis/src/coherence/builtin.rs | 2 +- compiler/rustc_middle/src/middle/privacy.rs | 16 +++++++--------- compiler/rustc_middle/src/ty/mod.rs | 7 ++++--- compiler/rustc_privacy/src/lib.rs | 6 +++--- .../rustc_resolve/src/build_reduced_graph.rs | 2 +- compiler/rustc_resolve/src/imports.rs | 9 ++++----- 6 files changed, 20 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/coherence/builtin.rs b/compiler/rustc_hir_analysis/src/coherence/builtin.rs index 61453e5328d5f..0f5d2ba1d754a 100644 --- a/compiler/rustc_hir_analysis/src/coherence/builtin.rs +++ b/compiler/rustc_hir_analysis/src/coherence/builtin.rs @@ -190,7 +190,7 @@ fn visit_implementation_of_const_param_ty(checker: &Checker<'_>) -> Result<(), E let struct_vis = tcx.visibility(adt.did()); for variant in adt.variants() { for field in &variant.fields { - if !field.vis.is_at_least(struct_vis, tcx) { + if struct_vis.greater_than(field.vis, tcx) { let span = tcx.hir_expect_item(impl_did).expect_impl().self_ty.span; return Err(tcx .dcx() diff --git a/compiler/rustc_middle/src/middle/privacy.rs b/compiler/rustc_middle/src/middle/privacy.rs index 71b5ee9b8810b..4ff64e4238f31 100644 --- a/compiler/rustc_middle/src/middle/privacy.rs +++ b/compiler/rustc_middle/src/middle/privacy.rs @@ -141,9 +141,7 @@ impl EffectiveVisibilities { for l in Level::all_levels() { let vis_at_level = eff_vis.at_level(l); let old_vis_at_level = old_eff_vis.at_level_mut(l); - if vis_at_level != old_vis_at_level - && vis_at_level.is_at_least(*old_vis_at_level, tcx) - { + if vis_at_level.greater_than(*old_vis_at_level, tcx) { *old_vis_at_level = *vis_at_level } } @@ -162,16 +160,16 @@ impl EffectiveVisibilities { // and all effective visibilities are larger or equal than private visibility. let private_vis = Visibility::Restricted(tcx.parent_module_from_def_id(def_id)); let span = tcx.def_span(def_id.to_def_id()); - if !ev.direct.is_at_least(private_vis, tcx) { + if private_vis.greater_than(ev.direct, tcx) { span_bug!(span, "private {:?} > direct {:?}", private_vis, ev.direct); } - if !ev.reexported.is_at_least(ev.direct, tcx) { + if ev.direct.greater_than(ev.reexported, tcx) { span_bug!(span, "direct {:?} > reexported {:?}", ev.direct, ev.reexported); } - if !ev.reachable.is_at_least(ev.reexported, tcx) { + if ev.reexported.greater_than(ev.reachable, tcx) { span_bug!(span, "reexported {:?} > reachable {:?}", ev.reexported, ev.reachable); } - if !ev.reachable_through_impl_trait.is_at_least(ev.reachable, tcx) { + if ev.reachable.greater_than(ev.reachable_through_impl_trait, tcx) { span_bug!( span, "reachable {:?} > reachable_through_impl_trait {:?}", @@ -185,7 +183,7 @@ impl EffectiveVisibilities { let is_impl = matches!(tcx.def_kind(def_id), DefKind::Impl { .. }); if !is_impl && tcx.trait_impl_of_assoc(def_id.to_def_id()).is_none() { let nominal_vis = tcx.visibility(def_id); - if !nominal_vis.is_at_least(ev.reachable, tcx) { + if ev.reachable.greater_than(nominal_vis, tcx) { span_bug!( span, "{:?}: reachable {:?} > nominal {:?}", @@ -245,7 +243,7 @@ impl EffectiveVisibilities { && level != l) { calculated_effective_vis = if let Some(max_vis) = max_vis - && !max_vis.is_at_least(inherited_effective_vis_at_level, tcx) + && inherited_effective_vis_at_level.greater_than(max_vis, tcx) { max_vis } else { diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index d5ec6d2d0ddc0..76a0e2feacca8 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -399,14 +399,15 @@ impl> Visibility { } #[track_caller] - pub fn is_at_least(self, vis: Visibility>, tcx: TyCtxt<'_>) -> bool { - self.is_at_least_ext(vis, tcx, true) + pub fn greater_than(self, vis: Visibility>, tcx: TyCtxt<'_>) -> bool { + let (lhs, rhs) = (self.to_def_id(), vis.to_def_id()); + lhs != rhs && lhs.is_at_least_ext(rhs, tcx, true) } } impl + Copy> Visibility { pub fn min(self, vis: Visibility, tcx: TyCtxt<'_>) -> Visibility { - if self.is_at_least(vis, tcx) { vis } else { self } + if self.greater_than(vis, tcx) { vis } else { self } } } diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index 2147a160d3d6e..17c8e89ee08f2 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -330,7 +330,7 @@ fn assoc_has_type_of(tcx: TyCtxt<'_>, item: &ty::AssocItem) -> bool { } fn min(vis1: ty::Visibility, vis2: ty::Visibility, tcx: TyCtxt<'_>) -> ty::Visibility { - if vis1.is_at_least(vis2, tcx) { vis2 } else { vis1 } + if vis1.greater_than(vis2, tcx) { vis2 } else { vis1 } } /// Visitor used to determine impl visibility and reachability. @@ -1447,7 +1447,7 @@ impl SearchInterfaceForPrivateItemsVisitor<'_> { }; let vis = self.tcx.local_visibility(local_def_id); - if self.hard_error && !vis.is_at_least(self.required_visibility, self.tcx) { + if self.hard_error && self.required_visibility.greater_than(vis, self.tcx) { let vis_descr = match vis { ty::Visibility::Public => "public", ty::Visibility::Restricted(vis_def_id) => { @@ -1481,7 +1481,7 @@ impl SearchInterfaceForPrivateItemsVisitor<'_> { let reachable_at_vis = *effective_vis.at_level(Level::Reachable); - if !vis.is_at_least(reachable_at_vis, self.tcx) { + if reachable_at_vis.greater_than(vis, self.tcx) { let lint = if self.in_primary_interface { lint::builtin::PRIVATE_INTERFACES } else { diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs index 50977ba6cff5f..75d33f7ab3296 100644 --- a/compiler/rustc_resolve/src/build_reduced_graph.rs +++ b/compiler/rustc_resolve/src/build_reduced_graph.rs @@ -938,7 +938,7 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> { let field_vis = self .try_resolve_visibility(&field.vis, false) .unwrap_or(Visibility::Public); - if ctor_vis.is_at_least(field_vis, self.r.tcx) { + if ctor_vis.greater_than(field_vis, self.r.tcx) { ctor_vis = field_vis; } ret_fields.push(field_vis.to_def_id()); diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index c81562deecfca..6df50f99aaf1f 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -378,8 +378,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { }; if let ImportKind::Glob { ref max_vis, .. } = import.kind - && (vis == import_vis - || max_vis.get().is_none_or(|max_vis| vis.is_at_least(max_vis, self.tcx))) + && max_vis.get().is_none_or(|max_vis| vis.greater_than(max_vis, self.tcx)) { max_vis.set_unchecked(Some(vis.expect_local())) } @@ -450,7 +449,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { // FIXME: remove this when `warn_ambiguity` is removed (#149195). self.arenas.alloc_decl((*old_glob_decl).clone()) } - } else if !old_glob_decl.vis().is_at_least(glob_decl.vis(), self.tcx) { + } else if glob_decl.vis().greater_than(old_glob_decl.vis(), self.tcx) { // We are glob-importing the same item but with greater visibility. // FIXME: Update visibility in place, but without regressions // (#152004, #151124, #152347). @@ -1216,7 +1215,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { }); } if let Some(max_vis) = max_vis.get() - && !max_vis.is_at_least(import.vis, self.tcx) + && import.vis.greater_than(max_vis, self.tcx) { let def_id = self.local_def_id(id); self.lint_buffer.buffer_lint( @@ -1451,7 +1450,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { return; }; - if !binding.vis().is_at_least(import.vis, this.tcx) { + if import.vis.greater_than(binding.vis(), this.tcx) { reexport_error = Some((ns, binding)); if let Visibility::Restricted(binding_def_id) = binding.vis() && binding_def_id.is_top_level_module() From bd09b859cf0e1118f58323138a2c54efd9ee7241 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 13 Apr 2026 23:22:45 +0300 Subject: [PATCH 3/4] privacy: Make "greater than" the new base operation for comparing visibilities --- compiler/rustc_middle/src/middle/privacy.rs | 14 ++++++-------- compiler/rustc_middle/src/ty/mod.rs | 15 +++++++-------- compiler/rustc_resolve/src/imports.rs | 2 +- 3 files changed, 14 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_middle/src/middle/privacy.rs b/compiler/rustc_middle/src/middle/privacy.rs index 4ff64e4238f31..512d5784e889d 100644 --- a/compiler/rustc_middle/src/middle/privacy.rs +++ b/compiler/rustc_middle/src/middle/privacy.rs @@ -84,7 +84,7 @@ impl EffectiveVisibility { let lhs_vis = *lhs.at_level(l); // FIXME: figure out why unordered visibilities occur here, // and what the behavior for them should be. - if rhs_vis.is_at_least_ext(lhs_vis, tcx, false) { + if rhs_vis.greater_than_ext(lhs_vis, tcx, false) { *rhs_vis = lhs_vis; }; } @@ -254,13 +254,11 @@ impl EffectiveVisibilities { // same id // FIXME: figure out why unordered visibilities occur here, // and what the behavior for them should be. - if *current_effective_vis_at_level != calculated_effective_vis - && calculated_effective_vis.is_at_least_ext( - *current_effective_vis_at_level, - tcx, - false, - ) - { + if calculated_effective_vis.greater_than_ext( + *current_effective_vis_at_level, + tcx, + false, + ) { changed = true; *current_effective_vis_at_level = calculated_effective_vis; } diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index 76a0e2feacca8..03636e8a2604c 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -375,33 +375,32 @@ impl> Visibility { } } - /// Returns `true` if this visibility is at least as accessible as the given visibility + /// Returns `true` if this visibility is strictly larger than the given visibility. #[track_caller] - pub fn is_at_least_ext( + pub fn greater_than_ext( self, vis: Visibility>, tcx: TyCtxt<'_>, assert_ordered: bool, ) -> bool { match (self, vis) { - (Visibility::Public, _) => true, (_, Visibility::Public) => false, + (Visibility::Public, _) => true, (Visibility::Restricted(lhs_id), Visibility::Restricted(rhs_id)) => { let lhs_id = lhs_id.into(); let rhs_id = rhs_id.into(); - let ge = tcx.is_descendant_of(rhs_id, lhs_id); - if !ge && assert_ordered && !tcx.is_descendant_of(lhs_id, rhs_id) { + let gt = lhs_id != rhs_id && tcx.is_descendant_of(rhs_id, lhs_id); + if !gt && assert_ordered && !tcx.is_descendant_of(lhs_id, rhs_id) { bug!("unordered visibilities: {lhs_id:?} and {rhs_id:?}"); } - ge + gt } } } #[track_caller] pub fn greater_than(self, vis: Visibility>, tcx: TyCtxt<'_>) -> bool { - let (lhs, rhs) = (self.to_def_id(), vis.to_def_id()); - lhs != rhs && lhs.is_at_least_ext(rhs, tcx, true) + self.greater_than_ext(vis, tcx, true) } } diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index 6df50f99aaf1f..3df98ca7fa362 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -369,7 +369,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { let import_vis = import.vis.to_def_id(); // FIXME: figure out why unordered visibilities occur here, // and what the behavior for them should be. - let vis = if decl.vis().is_at_least_ext(import_vis, self.tcx, false) + let vis = if decl.vis().greater_than_ext(import_vis, self.tcx, false) || pub_use_of_private_extern_crate_hack(import, decl).is_some() { import_vis From 852857658f25b2adf746fb6fd6835df673abd0b3 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Tue, 14 Apr 2026 16:28:11 +0300 Subject: [PATCH 4/4] fixes --- compiler/rustc_middle/src/middle/privacy.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_middle/src/middle/privacy.rs b/compiler/rustc_middle/src/middle/privacy.rs index 512d5784e889d..73cb77cbf19b0 100644 --- a/compiler/rustc_middle/src/middle/privacy.rs +++ b/compiler/rustc_middle/src/middle/privacy.rs @@ -243,7 +243,7 @@ impl EffectiveVisibilities { && level != l) { calculated_effective_vis = if let Some(max_vis) = max_vis - && inherited_effective_vis_at_level.greater_than(max_vis, tcx) + && inherited_effective_vis_at_level.greater_than_ext(max_vis, tcx, false) { max_vis } else {