Mirrors/rust
1
0
Fork 0
mirror of https://github.com/rust-lang/rust.git synced 2026-04-26 13:01:27 +03:00
Code Issues Packages Projects Releases Wiki Activity

privacy: Assert that compared visibilities are (usually) ordered

Browse Source 
Also use `greater_than` instead of `is_at_least` for comparing visibilities, which we can do because visibilities are asserted to be ordered now.
This commit is contained in:
Vadim Petrochenkov
2026-04-13 16:44:21 +03:00
parent 827651f220
commit 714df2bf00
6 changed files with 77 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files
compiler/rustc_hir_analysis/src/coherence/builtin.rs
+1 -1
View File
@@ -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()
compiler/rustc_middle/src/middle/privacy.rs
+18 -12
View File
@@ -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;
compiler/rustc_middle/src/ty/mod.rs
+37 -8
View File
@@ -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<DefId>, 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<impl Into<DefId>>, 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<impl Into<DefId>>,
tcx: TyCtxt<'_>,
) -> Option<Ordering> {
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<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 }
impl<Id: Into<DefId> + Debug + Copy> Visibility<Id> {
/// Returns `true` if this visibility is strictly larger than the given visibility.
#[track_caller]
pub fn greater_than(
self,
vis: Visibility<impl Into<DefId> + 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
}
}
}
}
compiler/rustc_privacy/src/lib.rs
+3 -3
View File
@@ -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 {
compiler/rustc_resolve/src/build_reduced_graph.rs
+1 -1
View File
@@ -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());
compiler/rustc_resolve/src/imports.rs
+17 -19
View File
@@ -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<UnresolvedImportErr
});
}
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(
@@ -1493,7 +1491,7 @@ fn finalize_import(&mut self, import: Import<'ra>) -> Option<UnresolvedImportErr
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()