Skip to content

Commit

Permalink
Rollup merge of #112670 - petrochenkov:typriv, r=eholk
Browse files Browse the repository at this point in the history
privacy: Type privacy lints fixes and cleanups

See individual commits.
Follow up to #111801.
  • Loading branch information
matthiaskrgr authored Jun 29, 2023
2 parents 75726ca + 98a86ff commit 42a495d
Show file tree
Hide file tree
Showing 23 changed files with 255 additions and 158 deletions.
2 changes: 2 additions & 0 deletions compiler/rustc_feature/src/active.rs
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,8 @@ declare_features! (
/// Allows creation of instances of a struct by moving fields that have
/// not changed from prior instances of the same struct (RFC #2528)
(active, type_changing_struct_update, "1.58.0", Some(86555), None),
/// Allows using type privacy lints (`private_interfaces`, `private_bounds`, `unnameable_types`).
(active, type_privacy_lints, "CURRENT_RUSTC_VERSION", Some(48054), None),
/// Enables rustc to generate code that instructs libstd to NOT ignore SIGPIPE.
(active, unix_sigpipe, "1.65.0", Some(97889), None),
/// Allows unsized fn parameters.
Expand Down
8 changes: 2 additions & 6 deletions compiler/rustc_lint/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -665,9 +665,7 @@ declare_lint_pass!(MissingCopyImplementations => [MISSING_COPY_IMPLEMENTATIONS])

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

impl<'tcx> LateLintPass<'tcx> for MissingDebugImplementations {
fn check_item(&mut self, cx: &LateContext<'_>, item: &hir::Item<'_>) {
if !(cx.effective_visibilities.is_reachable(item.owner_id.def_id)
&& cx.tcx.local_visibility(item.owner_id.def_id).is_public())
{
if !cx.effective_visibilities.is_reachable(item.owner_id.def_id) {
return;
}

Expand Down
10 changes: 8 additions & 2 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4264,6 +4264,7 @@ declare_lint! {
/// ### Example
///
/// ```rust,compile_fail
/// # #![feature(type_privacy_lints)]
/// # #![allow(unused)]
/// # #![allow(private_in_public)]
/// #![deny(private_interfaces)]
Expand All @@ -4288,6 +4289,7 @@ declare_lint! {
pub PRIVATE_INTERFACES,
Allow,
"private type in primary interface of an item",
@feature_gate = sym::type_privacy_lints;
}

declare_lint! {
Expand All @@ -4298,6 +4300,7 @@ declare_lint! {
/// ### Example
///
/// ```rust,compile_fail
/// # #![feature(type_privacy_lints)]
/// # #![allow(private_in_public)]
/// # #![allow(unused)]
/// #![deny(private_bounds)]
Expand All @@ -4317,7 +4320,8 @@ declare_lint! {
/// the item actually provides.
pub PRIVATE_BOUNDS,
Allow,
"private type in secondary interface of an item"
"private type in secondary interface of an item",
@feature_gate = sym::type_privacy_lints;
}

declare_lint! {
Expand All @@ -4327,6 +4331,7 @@ declare_lint! {
/// ### Example
///
/// ```rust,compile_fail
/// # #![feature(type_privacy_lints)]
/// # #![allow(unused)]
/// #![deny(unnameable_types)]
/// mod m {
Expand All @@ -4345,5 +4350,6 @@ declare_lint! {
/// you can name the type `T` as well, this lint attempts to enforce this rule.
pub UNNAMEABLE_TYPES,
Allow,
"effective visibility of a type is larger than the area in which it can be named"
"effective visibility of a type is larger than the area in which it can be named",
@feature_gate = sym::type_privacy_lints;
}
35 changes: 19 additions & 16 deletions compiler/rustc_middle/src/middle/privacy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
use crate::ty::{TyCtxt, Visibility};
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_hir::def::DefKind;
use rustc_macros::HashStable;
use rustc_query_system::ich::StableHashingContext;
use rustc_span::def_id::{LocalDefId, CRATE_DEF_ID};
Expand Down Expand Up @@ -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) {
return;
}
for (&def_id, ev) in &self.map {
// 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.
let private_vis = Visibility::Restricted(tcx.parent_module_from_def_id(def_id));
let span = tcx.def_span(def_id.to_def_id());
Expand All @@ -175,17 +175,20 @@ impl EffectiveVisibilities {
ev.reachable_through_impl_trait
);
}
let nominal_vis = tcx.visibility(def_id);
// FIXME: `rustc_privacy` is not yet updated for the new logic and can set
// effective visibilities that are larger than the nominal one.
if !nominal_vis.is_at_least(ev.reachable_through_impl_trait, tcx) && early {
span_bug!(
span,
"{:?}: reachable_through_impl_trait {:?} > nominal {:?}",
def_id,
ev.reachable_through_impl_trait,
nominal_vis
);
// All effective visibilities except `reachable_through_impl_trait` are limited to
// nominal visibility. For some items nominal visibility doesn't make sense so we
// don't check this condition for them.
if !matches!(tcx.def_kind(def_id), DefKind::Impl { .. }) {
let nominal_vis = tcx.visibility(def_id);
if !nominal_vis.is_at_least(ev.reachable, tcx) {
span_bug!(
span,
"{:?}: reachable {:?} > nominal {:?}",
def_id,
ev.reachable,
nominal_vis
);
}
}
}
}
Expand All @@ -212,7 +215,7 @@ impl<Id: Eq + Hash> EffectiveVisibilities<Id> {
pub fn update(
&mut self,
id: Id,
nominal_vis: Option<Visibility>,
max_vis: Option<Visibility>,
lazy_private_vis: impl FnOnce() -> Visibility,
inherited_effective_vis: EffectiveVisibility,
level: Level,
Expand All @@ -236,8 +239,8 @@ impl<Id: Eq + Hash> EffectiveVisibilities<Id> {
if !(inherited_effective_vis_at_prev_level == inherited_effective_vis_at_level
&& level != l)
{
calculated_effective_vis = if let Some(nominal_vis) = nominal_vis && !nominal_vis.is_at_least(inherited_effective_vis_at_level, tcx) {
nominal_vis
calculated_effective_vis = if let Some(max_vis) = max_vis && !max_vis.is_at_least(inherited_effective_vis_at_level, tcx) {
max_vis
} else {
inherited_effective_vis_at_level
}
Expand Down
Loading

0 comments on commit 42a495d

Please sign in to comment.