Skip to content

privacy: Assert that compared visibilities are (usually) ordered #155257

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
petrochenkov wants to merge 4 commits into
base: main
Choose a base branch
from
Open

privacy: Assert that compared visibilities are (usually) ordered #155257

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8a6617a
privacy: Assert that compared visibilities are (usually) ordered
petrochenkov Apr 13, 2026
39982c1
privacy: Use `greater_than` instead of `is_at_least` for comparing vi…
petrochenkov Apr 13, 2026
bd09b85
privacy: Make "greater than" the new base operation for comparing vis…
petrochenkov Apr 13, 2026
8528576
fixes
petrochenkov Apr 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/coherence/builtin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
30 changes: 17 additions & 13 deletions compiler/rustc_middle/src/middle/privacy.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.greater_than_ext(lhs_vis, tcx, false) {
*rhs_vis = lhs_vis;
};
}
Expand Down Expand Up @@ -139,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
}
}
Expand All @@ -160,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 {:?}",
Expand All @@ -183,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 {:?}",
Expand Down Expand Up @@ -243,7 +243,7 @@ impl<Id: Eq + Hash> EffectiveVisibilities<Id> {
&& 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_ext(max_vis, tcx, false)
{
max_vis
} else {
Expand All @@ -252,9 +252,13 @@ impl<Id: Eq + Hash> EffectiveVisibilities<Id> {
}
// effective visibility can't be decreased at next update call for the
// same id
if *current_effective_vis_at_level != calculated_effective_vis
&& calculated_effective_vis.is_at_least(*current_effective_vis_at_level, tcx)
{
// FIXME: figure out why unordered visibilities occur here,
// and what the behavior for them should be.
if calculated_effective_vis.greater_than_ext(
*current_effective_vis_at_level,
tcx,
false,
) {
changed = true;
*current_effective_vis_at_level = calculated_effective_vis;
}
Expand Down
32 changes: 26 additions & 6 deletions compiler/rustc_middle/src/ty/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,18 +375,38 @@ impl<Id: Into<DefId>> Visibility<Id> {
}
}

/// Returns `true` if this visibility is at least as accessible as the given visibility
pub fn is_at_least(self, vis: Visibility<impl Into<DefId>>, tcx: TyCtxt<'_>) -> bool {
match vis {
Visibility::Public => self.is_public(),
Visibility::Restricted(id) => self.is_accessible_from(id, tcx),
/// Returns `true` if this visibility is strictly larger than the given visibility.
#[track_caller]
pub fn greater_than_ext(
self,
vis: Visibility<impl Into<DefId>>,
tcx: TyCtxt<'_>,
assert_ordered: bool,
) -> bool {
match (self, vis) {
(_, 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 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:?}");
}
gt
}
}
}

#[track_caller]
pub fn greater_than(self, vis: Visibility<impl Into<DefId>>, tcx: TyCtxt<'_>) -> bool {
self.greater_than_ext(vis, tcx, true)
}
}

impl<Id: Into<DefId> + Copy> Visibility<Id> {
pub fn min(self, vis: Visibility<Id>, tcx: TyCtxt<'_>) -> Visibility<Id> {
if self.is_at_least(vis, tcx) { vis } else { self }
if self.greater_than(vis, tcx) { vis } else { self }
}
}

Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_privacy/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/build_reduced_graph.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
13 changes: 7 additions & 6 deletions compiler/rustc_resolve/src/imports.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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().greater_than_ext(import_vis, self.tcx, false)
|| pub_use_of_private_extern_crate_hack(import, decl).is_some()
{
import_vis
Expand All @@ -376,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()))
}
Expand Down Expand Up @@ -448,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).
Expand Down Expand Up @@ -1214,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(
Expand Down Expand Up @@ -1449,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()
Expand Down
Loading