1
Fork 0

privacy: Do not mark items reachable farther than their nominal visibility

This commit reverts a change made in #111425.
It was believed that this change was necessary for implementing type privacy lints, but #111801 showed that it was not necessary.
Quite opposite, the revert fixes some issues.
This commit is contained in:
Vadim Petrochenkov 2023-06-15 19:07:51 +03:00
parent 17edd1a779
commit 95a24c6ed4
6 changed files with 37 additions and 34 deletions

View file

@ -665,9 +665,7 @@ declare_lint_pass!(MissingCopyImplementations => [MISSING_COPY_IMPLEMENTATIONS])
impl<'tcx> LateLintPass<'tcx> for MissingCopyImplementations { impl<'tcx> LateLintPass<'tcx> for MissingCopyImplementations {
fn check_item(&mut self, cx: &LateContext<'_>, item: &hir::Item<'_>) { fn check_item(&mut self, cx: &LateContext<'_>, item: &hir::Item<'_>) {
if !(cx.effective_visibilities.is_reachable(item.owner_id.def_id) if !cx.effective_visibilities.is_reachable(item.owner_id.def_id) {
&& cx.tcx.local_visibility(item.owner_id.def_id).is_public())
{
return; return;
} }
let (def, ty) = match item.kind { let (def, ty) = match item.kind {
@ -786,9 +784,7 @@ impl_lint_pass!(MissingDebugImplementations => [MISSING_DEBUG_IMPLEMENTATIONS]);
impl<'tcx> LateLintPass<'tcx> for MissingDebugImplementations { impl<'tcx> LateLintPass<'tcx> for MissingDebugImplementations {
fn check_item(&mut self, cx: &LateContext<'_>, item: &hir::Item<'_>) { fn check_item(&mut self, cx: &LateContext<'_>, item: &hir::Item<'_>) {
if !(cx.effective_visibilities.is_reachable(item.owner_id.def_id) if !cx.effective_visibilities.is_reachable(item.owner_id.def_id) {
&& cx.tcx.local_visibility(item.owner_id.def_id).is_public())
{
return; return;
} }

View file

@ -4,6 +4,7 @@
use crate::ty::{TyCtxt, Visibility}; use crate::ty::{TyCtxt, Visibility};
use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_hir::def::DefKind;
use rustc_macros::HashStable; use rustc_macros::HashStable;
use rustc_query_system::ich::StableHashingContext; use rustc_query_system::ich::StableHashingContext;
use rustc_span::def_id::{LocalDefId, CRATE_DEF_ID}; use rustc_span::def_id::{LocalDefId, CRATE_DEF_ID};
@ -148,13 +149,12 @@ impl EffectiveVisibilities {
}; };
} }
pub fn check_invariants(&self, tcx: TyCtxt<'_>, early: bool) { pub fn check_invariants(&self, tcx: TyCtxt<'_>) {
if !cfg!(debug_assertions) { if !cfg!(debug_assertions) {
return; return;
} }
for (&def_id, ev) in &self.map { for (&def_id, ev) in &self.map {
// More direct visibility levels can never go farther than less direct ones, // More direct visibility levels can never go farther than less direct ones,
// neither of effective visibilities can go farther than nominal visibility,
// and all effective visibilities are larger or equal than private visibility. // 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 private_vis = Visibility::Restricted(tcx.parent_module_from_def_id(def_id));
let span = tcx.def_span(def_id.to_def_id()); let span = tcx.def_span(def_id.to_def_id());
@ -175,17 +175,20 @@ impl EffectiveVisibilities {
ev.reachable_through_impl_trait ev.reachable_through_impl_trait
); );
} }
let nominal_vis = tcx.visibility(def_id); // All effective visibilities except `reachable_through_impl_trait` are limited to
// FIXME: `rustc_privacy` is not yet updated for the new logic and can set // nominal visibility. For some items nominal visibility doesn't make sense so we
// effective visibilities that are larger than the nominal one. // don't check this condition for them.
if !nominal_vis.is_at_least(ev.reachable_through_impl_trait, tcx) && early { if !matches!(tcx.def_kind(def_id), DefKind::Impl { .. }) {
span_bug!( let nominal_vis = tcx.visibility(def_id);
span, if !nominal_vis.is_at_least(ev.reachable, tcx) {
"{:?}: reachable_through_impl_trait {:?} > nominal {:?}", span_bug!(
def_id, span,
ev.reachable_through_impl_trait, "{:?}: reachable {:?} > nominal {:?}",
nominal_vis def_id,
); ev.reachable,
nominal_vis
);
}
} }
} }
} }

View file

@ -894,7 +894,12 @@ impl<'tcx> DefIdVisitor<'tcx> for ReachEverythingInTheInterfaceVisitor<'_, 'tcx>
_descr: &dyn fmt::Display, _descr: &dyn fmt::Display,
) -> ControlFlow<Self::BreakTy> { ) -> ControlFlow<Self::BreakTy> {
if let Some(def_id) = def_id.as_local() { if let Some(def_id) = def_id.as_local() {
self.ev.update_eff_vis(def_id, self.effective_vis, None, self.level); // All effective visibilities except `reachable_through_impl_trait` are limited to
// nominal visibility. If any type or trait is leaked farther than that, it will
// produce type privacy errors on any use, so we don't consider it leaked.
let nominal_vis = (self.level != Level::ReachableThroughImplTrait)
.then(|| self.ev.tcx.local_visibility(def_id));
self.ev.update_eff_vis(def_id, self.effective_vis, nominal_vis, self.level);
} }
ControlFlow::Continue(()) ControlFlow::Continue(())
} }
@ -1869,10 +1874,9 @@ impl SearchInterfaceForPrivateItemsVisitor<'_> {
return false; return false;
}; };
// FIXME: `Level::Reachable` should be taken instead of `Level::Reexported` let reachable_at_vis = *effective_vis.at_level(Level::Reachable);
let reexported_at_vis = *effective_vis.at_level(Level::Reexported);
if !vis.is_at_least(reexported_at_vis, self.tcx) { if !vis.is_at_least(reachable_at_vis, self.tcx) {
let lint = if self.in_primary_interface { let lint = if self.in_primary_interface {
lint::builtin::PRIVATE_INTERFACES lint::builtin::PRIVATE_INTERFACES
} else { } else {
@ -1889,7 +1893,7 @@ impl SearchInterfaceForPrivateItemsVisitor<'_> {
tcx: self.tcx, tcx: self.tcx,
}) })
.into(), .into(),
item_vis_descr: &vis_to_string(self.item_def_id, reexported_at_vis, self.tcx), item_vis_descr: &vis_to_string(self.item_def_id, reachable_at_vis, self.tcx),
ty_span: vis_span, ty_span: vis_span,
ty_kind: kind, ty_kind: kind,
ty_descr: descr.into(), ty_descr: descr.into(),
@ -2278,7 +2282,7 @@ fn effective_visibilities(tcx: TyCtxt<'_>, (): ()) -> &EffectiveVisibilities {
changed: false, changed: false,
}; };
visitor.effective_visibilities.check_invariants(tcx, true); visitor.effective_visibilities.check_invariants(tcx);
if visitor.impl_trait_pass { if visitor.impl_trait_pass {
// Underlying types of `impl Trait`s are marked as reachable unconditionally, // Underlying types of `impl Trait`s are marked as reachable unconditionally,
// so this pass doesn't need to be a part of the fixed point iteration below. // so this pass doesn't need to be a part of the fixed point iteration below.
@ -2295,7 +2299,7 @@ fn effective_visibilities(tcx: TyCtxt<'_>, (): ()) -> &EffectiveVisibilities {
break; break;
} }
} }
visitor.effective_visibilities.check_invariants(tcx, false); visitor.effective_visibilities.check_invariants(tcx);
let mut check_visitor = let mut check_visitor =
TestReachabilityVisitor { tcx, effective_visibilities: &visitor.effective_visibilities }; TestReachabilityVisitor { tcx, effective_visibilities: &visitor.effective_visibilities };

View file

@ -310,7 +310,7 @@ where
/// Real logic of both `Flatten` and `FlatMap` which simply delegate to /// Real logic of both `Flatten` and `FlatMap` which simply delegate to
/// this type. /// this type.
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
#[unstable(feature = "trusted_len", issue = "37572")] #[cfg_attr(bootstrap, unstable(feature = "trusted_len", issue = "37572"))]
struct FlattenCompat<I, U> { struct FlattenCompat<I, U> {
iter: Fuse<I>, iter: Fuse<I>,
frontiter: Option<U>, frontiter: Option<U>,
@ -464,7 +464,7 @@ where
} }
} }
#[unstable(feature = "trusted_len", issue = "37572")] #[cfg_attr(bootstrap, unstable(feature = "trusted_len", issue = "37572"))]
impl<I, U> Iterator for FlattenCompat<I, U> impl<I, U> Iterator for FlattenCompat<I, U>
where where
I: Iterator<Item: IntoIterator<IntoIter = U, Item = U::Item>>, I: Iterator<Item: IntoIterator<IntoIter = U, Item = U::Item>>,
@ -579,7 +579,7 @@ where
} }
} }
#[unstable(feature = "trusted_len", issue = "37572")] #[cfg_attr(bootstrap, unstable(feature = "trusted_len", issue = "37572"))]
impl<I, U> DoubleEndedIterator for FlattenCompat<I, U> impl<I, U> DoubleEndedIterator for FlattenCompat<I, U>
where where
I: DoubleEndedIterator<Item: IntoIterator<IntoIter = U, Item = U::Item>>, I: DoubleEndedIterator<Item: IntoIterator<IntoIter = U, Item = U::Item>>,
@ -649,7 +649,7 @@ where
} }
} }
#[unstable(feature = "trusted_len", issue = "37572")] #[cfg_attr(bootstrap, unstable(feature = "trusted_len", issue = "37572"))]
unsafe impl<const N: usize, I, T> TrustedLen unsafe impl<const N: usize, I, T> TrustedLen
for FlattenCompat<I, <[T; N] as IntoIterator>::IntoIter> for FlattenCompat<I, <[T; N] as IntoIterator>::IntoIter>
where where
@ -657,7 +657,7 @@ where
{ {
} }
#[unstable(feature = "trusted_len", issue = "37572")] #[cfg_attr(bootstrap, unstable(feature = "trusted_len", issue = "37572"))]
unsafe impl<'a, const N: usize, I, T> TrustedLen unsafe impl<'a, const N: usize, I, T> TrustedLen
for FlattenCompat<I, <&'a [T; N] as IntoIterator>::IntoIter> for FlattenCompat<I, <&'a [T; N] as IntoIterator>::IntoIter>
where where
@ -665,7 +665,7 @@ where
{ {
} }
#[unstable(feature = "trusted_len", issue = "37572")] #[cfg_attr(bootstrap, unstable(feature = "trusted_len", issue = "37572"))]
unsafe impl<'a, const N: usize, I, T> TrustedLen unsafe impl<'a, const N: usize, I, T> TrustedLen
for FlattenCompat<I, <&'a mut [T; N] as IntoIterator>::IntoIter> for FlattenCompat<I, <&'a mut [T; N] as IntoIterator>::IntoIter>
where where

View file

@ -6,7 +6,7 @@ struct SemiPriv;
mod m { mod m {
#[rustc_effective_visibility] #[rustc_effective_visibility]
struct Priv; struct Priv;
//~^ ERROR Direct: pub(self), Reexported: pub(self), Reachable: pub(crate), ReachableThroughImplTrait: pub(crate) //~^ ERROR not in the table
//~| ERROR not in the table //~| ERROR not in the table
#[rustc_effective_visibility] #[rustc_effective_visibility]

View file

@ -1,4 +1,4 @@
error: Direct: pub(self), Reexported: pub(self), Reachable: pub(crate), ReachableThroughImplTrait: pub(crate) error: not in the table
--> $DIR/effective_visibilities_full_priv.rs:8:5 --> $DIR/effective_visibilities_full_priv.rs:8:5
| |
LL | struct Priv; LL | struct Priv;