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 0be4c8243d632..73cb77cbf19b0 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.greater_than_ext(lhs_vis, tcx, false) { *rhs_vis = lhs_vis; }; } @@ -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 } } @@ -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 {:?}", @@ -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 {:?}", @@ -243,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_ext(max_vis, tcx, false) { max_vis } else { @@ -252,9 +252,13 @@ impl EffectiveVisibilities { } // 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; } diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index a78e5096b2e5e..03636e8a2604c 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -375,18 +375,38 @@ 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), + /// Returns `true` if this visibility is strictly larger than the given visibility. + #[track_caller] + pub fn greater_than_ext( + self, + vis: Visibility>, + 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>, tcx: TyCtxt<'_>) -> bool { + self.greater_than_ext(vis, 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 a94f3ea435e2c..3df98ca7fa362 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().greater_than_ext(import_vis, self.tcx, false) || pub_use_of_private_extern_crate_hack(import, decl).is_some() { import_vis @@ -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())) } @@ -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). @@ -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( @@ -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()