diff --git a/compiler/rustc_hir_analysis/src/coherence/builtin.rs b/compiler/rustc_hir_analysis/src/coherence/builtin.rs index d1ce29b703ee..7c3511e91d79 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 0be4c8243d63..c24e433073cc 100644 --- a/compiler/rustc_middle/src/middle/privacy.rs +++ b/compiler/rustc_middle/src/middle/privacy.rs @@ -2,6 +2,7 @@ //! outside their scopes. This pass will also generate a set of exported items //! which are available for use externally when compiled as a library. +use std::cmp::Ordering; use std::hash::Hash; use rustc_data_structures::fx::{FxIndexMap, IndexEntry}; @@ -82,7 +83,9 @@ pub fn min(mut self, lhs: EffectiveVisibility, tcx: TyCtxt<'_>) -> Self { 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.partial_cmp(lhs_vis, tcx) == Some(Ordering::Greater) { *rhs_vis = lhs_vis; }; } @@ -139,9 +142,7 @@ pub fn update_eff_vis( 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 +161,16 @@ pub fn check_invariants(&self, tcx: TyCtxt<'_>) { // 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 +184,7 @@ pub fn check_invariants(&self, tcx: TyCtxt<'_>) { 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 {:?}", @@ -242,8 +243,11 @@ pub fn update( if !(inherited_effective_vis_at_prev_level == inherited_effective_vis_at_level && level != l) { + // FIXME: figure out why unordered visibilities occur here, + // and what the behavior for them should be. 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.partial_cmp(max_vis, tcx) + == Some(Ordering::Greater) { max_vis } else { @@ -252,8 +256,10 @@ pub fn update( } // 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.partial_cmp(*current_effective_vis_at_level, tcx) + == Some(Ordering::Greater) { 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 59cc6a601bfa..1ec269571007 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -11,6 +11,7 @@ #![allow(rustc::usage_of_ty_tykind)] +use std::cmp::Ordering; use std::fmt::Debug; use std::hash::{Hash, Hasher}; use std::marker::PhantomData; @@ -379,18 +380,46 @@ pub fn is_accessible_from(self, module: impl Into, tcx: TyCtxt<'_>) -> bo } } - /// 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), + pub fn partial_cmp( + self, + vis: Visibility>, + tcx: TyCtxt<'_>, + ) -> Option { + match (self, vis) { + (Visibility::Public, Visibility::Public) => Some(Ordering::Equal), + (Visibility::Public, Visibility::Restricted(_)) => Some(Ordering::Greater), + (Visibility::Restricted(_), Visibility::Public) => Some(Ordering::Less), + (Visibility::Restricted(lhs_id), Visibility::Restricted(rhs_id)) => { + let (lhs_id, rhs_id) = (lhs_id.into(), rhs_id.into()); + if lhs_id == rhs_id { + Some(Ordering::Equal) + } else if tcx.is_descendant_of(rhs_id, lhs_id) { + Some(Ordering::Greater) + } else if tcx.is_descendant_of(lhs_id, rhs_id) { + Some(Ordering::Less) + } else { + None + } + } } } } -impl + Copy> Visibility { - pub fn min(self, vis: Visibility, tcx: TyCtxt<'_>) -> Visibility { - if self.is_at_least(vis, tcx) { vis } else { self } +impl + Debug + Copy> Visibility { + /// Returns `true` if this visibility is strictly larger than the given visibility. + #[track_caller] + pub fn greater_than( + self, + vis: Visibility + Debug + Copy>, + tcx: TyCtxt<'_>, + ) -> bool { + match self.partial_cmp(vis, tcx) { + Some(ord) => ord.is_gt(), + None => { + tcx.dcx().delayed_bug(format!("unordered visibilities: {self:?} and {vis:?}")); + false + } + } } } diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index 58121b8123c3..ba30dfccc33c 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -337,7 +337,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. @@ -1465,7 +1465,7 @@ fn check_def_id(&self, def_id: DefId, kind: &str, descr: &dyn fmt::Display) -> b }; 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) => { @@ -1499,7 +1499,7 @@ fn check_def_id(&self, def_id: DefId, kind: &str, descr: &dyn fmt::Display) -> b 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 7482bd3283c8..cf234080e100 100644 --- a/compiler/rustc_resolve/src/build_reduced_graph.rs +++ b/compiler/rustc_resolve/src/build_reduced_graph.rs @@ -924,7 +924,7 @@ fn build_reduced_graph_for_item(&mut self, item: &'a Item, feed: TyCtxtFeed<'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; } field_visibilities.push(field_vis.to_def_id()); diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index 4cb9a653e010..2aaab33283d3 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -1,5 +1,6 @@ //! A bunch of methods and structures more or less related to resolving imports. +use std::cmp::Ordering; use std::mem; use rustc_ast::{Item, NodeId}; @@ -373,24 +374,21 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { pub(crate) fn import_decl_vis(&self, decl: Decl<'ra>, import: ImportSummary) -> Visibility { assert!(import.vis.is_accessible_from(import.nearest_parent_mod, self.tcx)); let decl_vis = decl.vis(); - if decl_vis.is_at_least(import.vis, self.tcx) { - // Ordered, import is less visible than the imported declaration, or the same, - // use the import's visibility. - import.vis - } else if decl_vis.is_accessible_from(import.nearest_parent_mod, self.tcx) { - // Ordered, imported declaration is less visible than the import, but is still visible + if decl_vis.partial_cmp(import.vis, self.tcx) == Some(Ordering::Less) + && decl_vis.is_accessible_from(import.nearest_parent_mod, self.tcx) + && pub_use_of_private_extern_crate_hack(import, decl).is_none() + { + // Imported declaration is less visible than the import, but is still visible // from the current module, use the declaration's visibility. - assert!(import.vis.is_at_least(decl_vis, self.tcx)); - if pub_use_of_private_extern_crate_hack(import, decl).is_some() { - import.vis - } else { - decl_vis.expect_local() - } + decl_vis.expect_local() } else { - // Ordered or not, the imported declaration is too private for the current module. + // Good case - imported declaration is more visible than the import, or the same, + // use the import's visibility. + // Bad case - imported declaration is too private for the current module. // It doesn't matter what visibility we choose here (except in the `PRIVATE_MACRO_USE` - // case), because either some error will be reported, or the import declaration - // will be thrown away (unfortunately cannot use delayed bug here for this reason). + // and `PUB_USE_OF_PRIVATE_EXTERN_CRATE` cases), because either some error will be + // reported, or the import declaration will be thrown away (unfortunately cannot use + // delayed bug here for this reason). // Use import visibility to keep the all declaration visibilities in a module ordered. import.vis } @@ -403,7 +401,7 @@ pub(crate) fn new_import_decl(&self, decl: Decl<'ra>, import: Import<'ra>) -> De 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)) } @@ -474,7 +472,7 @@ fn select_glob_decl( // 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. // All visibilities here are ordered because all of them are ancestors of `module`. // FIXME: Update visibility in place, but without regressions @@ -1244,7 +1242,7 @@ fn finalize_import(&mut self, import: Import<'ra>) -> Option) -> Option