diff --git a/compiler/rustc_hir_analysis/src/coherence/builtin.rs b/compiler/rustc_hir_analysis/src/coherence/builtin.rs index d1ce29b703eea..7c3511e91d79d 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..53776441e1a58 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 @@ 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.partial_cmp(lhs_vis, tcx) == Ok(Ordering::Greater) { *rhs_vis = lhs_vis; }; } @@ -139,9 +142,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 +161,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 +184,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 {:?}", @@ -242,8 +243,11 @@ impl EffectiveVisibilities { 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) + == Ok(Ordering::Greater) { max_vis } else { @@ -252,8 +256,10 @@ 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.partial_cmp(*current_effective_vis_at_level, tcx) + == Ok(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 56a7abfac6635..a09b209ee9602 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,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), + pub fn partial_cmp( + self, + vis: Visibility>, + tcx: TyCtxt<'_>, + ) -> Result { + match (self, vis) { + (Visibility::Public, Visibility::Public) => Ok(Ordering::Equal), + (Visibility::Public, Visibility::Restricted(_)) => Ok(Ordering::Greater), + (Visibility::Restricted(_), Visibility::Public) => Ok(Ordering::Less), + (Visibility::Restricted(lhs_id), Visibility::Restricted(rhs_id)) => { + let lhs_id = lhs_id.into(); + let rhs_id = rhs_id.into(); + if lhs_id == rhs_id { + Ok(Ordering::Equal) + } else if tcx.is_descendant_of(rhs_id, lhs_id) { + Ok(Ordering::Greater) + } else if tcx.is_descendant_of(lhs_id, rhs_id) { + Ok(Ordering::Less) + } else { + Err((lhs_id, rhs_id)) + } + } } } -} -impl + Copy> Visibility { - pub fn min(self, vis: Visibility, tcx: TyCtxt<'_>) -> Visibility { - if self.is_at_least(vis, tcx) { vis } else { self } + /// Returns `true` if this visibility is strictly larger than the given visibility. + #[track_caller] + pub fn greater_than(self, vis: Visibility>, tcx: TyCtxt<'_>) -> bool { + match self.partial_cmp(vis, tcx) { + Ok(ord) => ord.is_gt(), + Err((lhs_id, rhs_id)) => bug!("unordered visibilities: {lhs_id:?} and {rhs_id:?}"), + } } } diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index 58121b8123c35..ba30dfccc33cc 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 @@ 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) => { @@ -1499,7 +1499,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 4148ec1b7abd7..25f798eddd7a7 100644 --- a/compiler/rustc_resolve/src/build_reduced_graph.rs +++ b/compiler/rustc_resolve/src/build_reduced_graph.rs @@ -922,7 +922,7 @@ impl<'a, 'ra, 'tcx> DefCollector<'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; } 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 9df075561b3f2..f37b23c18eac4 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) == Ok(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 @@ 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)) } @@ -474,7 +472,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. // All visibilities here are ordered because all of them are ancestors of `module`. // FIXME: Update visibility in place, but without regressions @@ -1242,7 +1240,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( @@ -1477,7 +1475,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()