From 5e917a6039fb7894273eccf453c34ad907635920 Mon Sep 17 00:00:00 2001
From: Bryanskiy <ivakin.kir@gmail.com>
Date: Tue, 6 Jun 2023 18:26:38 +0300
Subject: [PATCH] increase the accuracy of effective visibilities calculation
---
compiler/rustc_privacy/src/lib.rs | 46 +++++++++----------
.../src/effective_visibilities.rs | 2 +-
.../effective_visibilities_full_priv.rs | 21 +++++++++
.../effective_visibilities_full_priv.stderr | 26 +++++++++++
4 files changed, 69 insertions(+), 26 deletions(-)
create mode 100644 tests/ui/privacy/effective_visibilities_full_priv.rs
create mode 100644 tests/ui/privacy/effective_visibilities_full_priv.stderr
diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs
index 0197b99c7db..afd32e38d5b 100644
--- a/compiler/rustc_privacy/src/lib.rs
+++ b/compiler/rustc_privacy/src/lib.rs
@@ -409,8 +409,8 @@ impl VisibilityLike for ty::Visibility {
}
}
-impl VisibilityLike for Option<EffectiveVisibility> {
- const MAX: Self = Some(EffectiveVisibility::from_vis(ty::Visibility::Public));
+impl VisibilityLike for EffectiveVisibility {
+ const MAX: Self = EffectiveVisibility::from_vis(ty::Visibility::Public);
// Type inference is very smart sometimes.
// It can make an impl reachable even some components of its type or trait are unreachable.
// E.g. methods of `impl ReachableTrait<UnreachableTy> for ReachableTy<UnreachableTy> { ... }`
@@ -422,13 +422,14 @@ impl VisibilityLike for Option<EffectiveVisibility> {
// (which require reaching the `DefId`s in them).
const SHALLOW: bool = true;
fn new_min(find: &FindMin<'_, '_, Self>, def_id: LocalDefId) -> Self {
- if let Some(min) = find.min {
- return find
- .effective_visibilities
- .effective_vis(def_id)
- .map(|eff_vis| min.min(*eff_vis, find.tcx));
- }
- None
+ let effective_vis =
+ find.effective_visibilities.effective_vis(def_id).cloned().unwrap_or_else(|| {
+ let private_vis =
+ ty::Visibility::Restricted(find.tcx.parent_module_from_def_id(def_id));
+ EffectiveVisibility::from_vis(private_vis)
+ });
+
+ effective_vis.min(find.min, find.tcx)
}
}
@@ -766,28 +767,23 @@ impl<'tcx> Visitor<'tcx> for EmbargoVisitor<'tcx> {
}
}
hir::ItemKind::Impl(ref impl_) => {
- if let Some(item_ev) = Option::<EffectiveVisibility>::of_impl(
+ let item_ev = EffectiveVisibility::of_impl(
item.owner_id.def_id,
self.tcx,
&self.effective_visibilities,
- ) {
- self.update_eff_vis(item.owner_id.def_id, item_ev, None, Level::Direct);
+ );
+ self.update_eff_vis(item.owner_id.def_id, item_ev, None, Level::Direct);
- self.reach(item.owner_id.def_id, item_ev)
- .generics()
- .predicates()
- .ty()
- .trait_ref();
+ self.reach(item.owner_id.def_id, item_ev).generics().predicates().ty().trait_ref();
- for impl_item_ref in impl_.items {
- let def_id = impl_item_ref.id.owner_id.def_id;
- let nominal_vis =
- impl_.of_trait.is_none().then(|| self.tcx.local_visibility(def_id));
- self.update_eff_vis(def_id, item_ev, nominal_vis, Level::Direct);
+ for impl_item_ref in impl_.items {
+ let def_id = impl_item_ref.id.owner_id.def_id;
+ let nominal_vis =
+ impl_.of_trait.is_none().then(|| self.tcx.local_visibility(def_id));
+ self.update_eff_vis(def_id, item_ev, nominal_vis, Level::Direct);
- if let Some(impl_item_ev) = self.get(def_id) {
- self.reach(def_id, impl_item_ev).generics().predicates().ty();
- }
+ if let Some(impl_item_ev) = self.get(def_id) {
+ self.reach(def_id, impl_item_ev).generics().predicates().ty();
}
}
}
diff --git a/compiler/rustc_resolve/src/effective_visibilities.rs b/compiler/rustc_resolve/src/effective_visibilities.rs
index 7393bdb388a..4863c9f4790 100644
--- a/compiler/rustc_resolve/src/effective_visibilities.rs
+++ b/compiler/rustc_resolve/src/effective_visibilities.rs
@@ -81,7 +81,7 @@ impl<'r, 'a, 'tcx> EffectiveVisibilitiesVisitor<'r, 'a, 'tcx> {
def_effective_visibilities: Default::default(),
import_effective_visibilities: Default::default(),
current_private_vis: Visibility::Restricted(CRATE_DEF_ID),
- changed: false,
+ changed: true,
};
visitor.def_effective_visibilities.update_root();
diff --git a/tests/ui/privacy/effective_visibilities_full_priv.rs b/tests/ui/privacy/effective_visibilities_full_priv.rs
new file mode 100644
index 00000000000..cc708917586
--- /dev/null
+++ b/tests/ui/privacy/effective_visibilities_full_priv.rs
@@ -0,0 +1,21 @@
+#![feature(rustc_attrs)]
+#![allow(private_in_public)]
+
+struct SemiPriv;
+
+mod m {
+ #[rustc_effective_visibility]
+ struct Priv;
+ //~^ ERROR Direct: pub(self), Reexported: pub(self), Reachable: pub(crate), ReachableThroughImplTrait: pub(crate)
+ //~| ERROR not in the table
+
+ #[rustc_effective_visibility]
+ pub fn foo() {} //~ ERROR Direct: pub(crate), Reexported: pub(crate), Reachable: pub(crate), ReachableThroughImplTrait: pub(crate)
+
+ #[rustc_effective_visibility]
+ impl crate::SemiPriv { //~ ERROR Direct: pub(crate), Reexported: pub(crate), Reachable: pub(crate), ReachableThroughImplTrait: pub(crate)
+ pub fn f(_: Priv) {}
+ }
+}
+
+fn main() {}
diff --git a/tests/ui/privacy/effective_visibilities_full_priv.stderr b/tests/ui/privacy/effective_visibilities_full_priv.stderr
new file mode 100644
index 00000000000..a856aa20d92
--- /dev/null
+++ b/tests/ui/privacy/effective_visibilities_full_priv.stderr
@@ -0,0 +1,26 @@
+error: Direct: pub(self), Reexported: pub(self), Reachable: pub(crate), ReachableThroughImplTrait: pub(crate)
+ --> $DIR/effective_visibilities_full_priv.rs:8:5
+ |
+LL | struct Priv;
+ | ^^^^^^^^^^^
+
+error: not in the table
+ --> $DIR/effective_visibilities_full_priv.rs:8:5
+ |
+LL | struct Priv;
+ | ^^^^^^^^^^^
+
+error: Direct: pub(crate), Reexported: pub(crate), Reachable: pub(crate), ReachableThroughImplTrait: pub(crate)
+ --> $DIR/effective_visibilities_full_priv.rs:13:5
+ |
+LL | pub fn foo() {}
+ | ^^^^^^^^^^^^
+
+error: Direct: pub(crate), Reexported: pub(crate), Reachable: pub(crate), ReachableThroughImplTrait: pub(crate)
+ --> $DIR/effective_visibilities_full_priv.rs:16:5
+ |
+LL | impl crate::SemiPriv {
+ | ^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 4 previous errors
+