diff options
Diffstat (limited to 'compiler')
| -rw-r--r-- | compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs | 27 | ||||
| -rw-r--r-- | compiler/rustc_codegen_ssa/src/back/write.rs | 63 | ||||
| -rw-r--r-- | compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs | 11 | ||||
| -rw-r--r-- | compiler/rustc_feature/src/active.rs | 2 | ||||
| -rw-r--r-- | compiler/rustc_lint/src/builtin.rs | 8 | ||||
| -rw-r--r-- | compiler/rustc_lint_defs/src/builtin.rs | 10 | ||||
| -rw-r--r-- | compiler/rustc_middle/src/middle/privacy.rs | 35 | ||||
| -rw-r--r-- | compiler/rustc_monomorphize/src/partitioning.rs | 36 | ||||
| -rw-r--r-- | compiler/rustc_privacy/src/lib.rs | 173 | ||||
| -rw-r--r-- | compiler/rustc_span/src/symbol.rs | 1 |
10 files changed, 202 insertions, 164 deletions
diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 39b05829888..a068f2d6926 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -1730,18 +1730,19 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { ( Some(name), BorrowExplanation::UsedLater(LaterUseKind::ClosureCapture, var_or_use_span, _), - ) => self.report_escaping_closure_capture( - borrow_spans, - borrow_span, - &RegionName { - name: self.synthesize_region_name(), - source: RegionNameSource::Static, - }, - ConstraintCategory::CallArgument(None), - var_or_use_span, - &format!("`{}`", name), - "block", - ), + ) if borrow_spans.for_generator() || borrow_spans.for_closure() => self + .report_escaping_closure_capture( + borrow_spans, + borrow_span, + &RegionName { + name: self.synthesize_region_name(), + source: RegionNameSource::Static, + }, + ConstraintCategory::CallArgument(None), + var_or_use_span, + &format!("`{}`", name), + "block", + ), ( Some(name), BorrowExplanation::MustBeValidFor { @@ -1754,7 +1755,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { span, .. }, - ) if borrow_spans.for_generator() | borrow_spans.for_closure() => self + ) if borrow_spans.for_generator() || borrow_spans.for_closure() => self .report_escaping_closure_capture( borrow_spans, borrow_span, diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs index 51ac441a7a4..dbe6f466a2a 100644 --- a/compiler/rustc_codegen_ssa/src/back/write.rs +++ b/compiler/rustc_codegen_ssa/src/back/write.rs @@ -698,28 +698,49 @@ impl<B: WriteBackendMethods> WorkItem<B> { /// Generate a short description of this work item suitable for use as a thread name. fn short_description(&self) -> String { - // `pthread_setname()` on *nix is limited to 15 characters and longer names are ignored. - // Use very short descriptions in this case to maximize the space available for the module name. - // Windows does not have that limitation so use slightly more descriptive names there. + // `pthread_setname()` on *nix ignores anything beyond the first 15 + // bytes. Use short descriptions to maximize the space available for + // the module name. + #[cfg(not(windows))] + fn desc(short: &str, _long: &str, name: &str) -> String { + // The short label is three bytes, and is followed by a space. That + // leaves 11 bytes for the CGU name. How we obtain those 11 bytes + // depends on the the CGU name form. + // + // - Non-incremental, e.g. `regex.f10ba03eb5ec7975-cgu.0`: the part + // before the `-cgu.0` is the same for every CGU, so use the + // `cgu.0` part. The number suffix will be different for each + // CGU. + // + // - Incremental (normal), e.g. `2i52vvl2hco29us0`: use the whole + // name because each CGU will have a unique ASCII hash, and the + // first 11 bytes will be enough to identify it. + // + // - Incremental (with `-Zhuman-readable-cgu-names`), e.g. + // `regex.f10ba03eb5ec7975-re_builder.volatile`: use the whole + // name. The first 11 bytes won't be enough to uniquely identify + // it, but no obvious substring will, and this is a rarely used + // option so it doesn't matter much. + // + assert_eq!(short.len(), 3); + let name = if let Some(index) = name.find("-cgu.") { + &name[index + 1..] // +1 skips the leading '-'. + } else { + name + }; + format!("{short} {name}") + } + + // Windows has no thread name length limit, so use more descriptive names. + #[cfg(windows)] + fn desc(_short: &str, long: &str, name: &str) -> String { + format!("{long} {name}") + } + match self { - WorkItem::Optimize(m) => { - #[cfg(windows)] - return format!("optimize module {}", m.name); - #[cfg(not(windows))] - return format!("opt {}", m.name); - } - WorkItem::CopyPostLtoArtifacts(m) => { - #[cfg(windows)] - return format!("copy LTO artifacts for {}", m.name); - #[cfg(not(windows))] - return format!("copy {}", m.name); - } - WorkItem::LTO(m) => { - #[cfg(windows)] - return format!("LTO module {}", m.name()); - #[cfg(not(windows))] - return format!("LTO {}", m.name()); - } + WorkItem::Optimize(m) => desc("opt", "optimize module {}", &m.name), + WorkItem::CopyPostLtoArtifacts(m) => desc("cpy", "copy LTO artifacts for {}", &m.name), + WorkItem::LTO(m) => desc("lto", "LTO module {}", m.name()), } } } diff --git a/compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs b/compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs index a2d23425f3b..36c76e53231 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs @@ -344,15 +344,18 @@ where }; // Check the qualifs of the value of `const` items. - // FIXME(valtrees): check whether const qualifs should behave the same - // way for type and mir constants. let uneval = match constant.literal { ConstantKind::Ty(ct) - if matches!(ct.kind(), ty::ConstKind::Param(_) | ty::ConstKind::Error(_)) => + if matches!( + ct.kind(), + ty::ConstKind::Param(_) | ty::ConstKind::Error(_) | ty::ConstKind::Value(_) + ) => { None } - ConstantKind::Ty(c) => bug!("expected ConstKind::Param here, found {:?}", c), + ConstantKind::Ty(c) => { + bug!("expected ConstKind::Param or ConstKind::Value here, found {:?}", c) + } ConstantKind::Unevaluated(uv, _) => Some(uv), ConstantKind::Val(..) => None, }; diff --git a/compiler/rustc_feature/src/active.rs b/compiler/rustc_feature/src/active.rs index 93f968aa851..906c31c9a3d 100644 --- a/compiler/rustc_feature/src/active.rs +++ b/compiler/rustc_feature/src/active.rs @@ -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. diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index 53aad6d8b31..3a177038ca8 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -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 { @@ -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; } diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index cbb4458d1d4..ef82a6c17ee 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -4264,6 +4264,7 @@ declare_lint! { /// ### Example /// /// ```rust,compile_fail + /// # #![feature(type_privacy_lints)] /// # #![allow(unused)] /// # #![allow(private_in_public)] /// #![deny(private_interfaces)] @@ -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! { @@ -4298,6 +4300,7 @@ declare_lint! { /// ### Example /// /// ```rust,compile_fail + /// # #![feature(type_privacy_lints)] /// # #![allow(private_in_public)] /// # #![allow(unused)] /// #![deny(private_bounds)] @@ -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! { @@ -4327,6 +4331,7 @@ declare_lint! { /// ### Example /// /// ```rust,compile_fail + /// # #![feature(type_privacy_lints)] /// # #![allow(unused)] /// #![deny(unnameable_types)] /// mod m { @@ -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; } diff --git a/compiler/rustc_middle/src/middle/privacy.rs b/compiler/rustc_middle/src/middle/privacy.rs index f45cf788dd9..5baeb1ee0cf 100644 --- a/compiler/rustc_middle/src/middle/privacy.rs +++ b/compiler/rustc_middle/src/middle/privacy.rs @@ -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}; @@ -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()); @@ -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 + ); + } } } } @@ -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, @@ -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 } diff --git a/compiler/rustc_monomorphize/src/partitioning.rs b/compiler/rustc_monomorphize/src/partitioning.rs index 97e3748b8b8..e663f4486f7 100644 --- a/compiler/rustc_monomorphize/src/partitioning.rs +++ b/compiler/rustc_monomorphize/src/partitioning.rs @@ -368,6 +368,7 @@ fn merge_codegen_units<'tcx>( let cgu_name_builder = &mut CodegenUnitNameBuilder::new(cx.tcx); + // Rename the newly merged CGUs. if cx.tcx.sess.opts.incremental.is_some() { // If we are doing incremental compilation, we want CGU names to // reflect the path of the source level module they correspond to. @@ -404,18 +405,41 @@ fn merge_codegen_units<'tcx>( } } } + + // A sorted order here ensures what follows can be deterministic. + codegen_units.sort_by(|a, b| a.name().as_str().cmp(b.name().as_str())); } else { - // If we are compiling non-incrementally we just generate simple CGU - // names containing an index. + // When compiling non-incrementally, we rename the CGUS so they have + // identical names except for the numeric suffix, something like + // `regex.f10ba03eb5ec7975-cgu.N`, where `N` varies. + // + // It is useful for debugging and profiling purposes if the resulting + // CGUs are sorted by name *and* reverse sorted by size. (CGU 0 is the + // biggest, CGU 1 is the second biggest, etc.) + // + // So first we reverse sort by size. Then we generate the names with + // zero-padded suffixes, which means they are automatically sorted by + // names. The numeric suffix width depends on the number of CGUs, which + // is always greater than zero: + // - [1,9] CGUS: `0`, `1`, `2`, ... + // - [10,99] CGUS: `00`, `01`, `02`, ... + // - [100,999] CGUS: `000`, `001`, `002`, ... + // - etc. + // + // If we didn't zero-pad the sorted-by-name order would be `XYZ-cgu.0`, + // `XYZ-cgu.1`, `XYZ-cgu.10`, `XYZ-cgu.11`, ..., `XYZ-cgu.2`, etc. + codegen_units.sort_by_key(|cgu| cmp::Reverse(cgu.size_estimate())); + let num_digits = codegen_units.len().ilog10() as usize + 1; for (index, cgu) in codegen_units.iter_mut().enumerate() { + // Note: `WorkItem::short_description` depends on this name ending + // with `-cgu.` followed by a numeric suffix. Please keep it in + // sync with this code. + let suffix = format!("{index:0num_digits$}"); let numbered_codegen_unit_name = - cgu_name_builder.build_cgu_name_no_mangle(LOCAL_CRATE, &["cgu"], Some(index)); + cgu_name_builder.build_cgu_name_no_mangle(LOCAL_CRATE, &["cgu"], Some(suffix)); cgu.set_name(numbered_codegen_unit_name); } } - - // A sorted order here ensures what follows can be deterministic. - codegen_units.sort_by(|a, b| a.name().as_str().cmp(b.name().as_str())); } fn internalize_symbols<'tcx>( diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index 5f7313dcca6..3841beb733f 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -73,14 +73,10 @@ impl<'tcx> fmt::Display for LazyDefPathStr<'tcx> { /// in `impl Trait`, see individual comments in `DefIdVisitorSkeleton::visit_ty`. trait DefIdVisitor<'tcx> { type BreakTy = (); + const SHALLOW: bool = false; + const SKIP_ASSOC_TYS: bool = false; fn tcx(&self) -> TyCtxt<'tcx>; - fn shallow(&self) -> bool { - false - } - fn skip_assoc_tys(&self) -> bool { - false - } fn visit_def_id( &mut self, def_id: DefId, @@ -135,11 +131,7 @@ where fn visit_trait(&mut self, trait_ref: TraitRef<'tcx>) -> ControlFlow<V::BreakTy> { let TraitRef { def_id, substs, .. } = trait_ref; self.def_id_visitor.visit_def_id(def_id, "trait", &trait_ref.print_only_trait_path())?; - if self.def_id_visitor.shallow() { - ControlFlow::Continue(()) - } else { - substs.visit_with(self) - } + if V::SHALLOW { ControlFlow::Continue(()) } else { substs.visit_with(self) } } fn visit_projection_ty(&mut self, projection: ty::AliasTy<'tcx>) -> ControlFlow<V::BreakTy> { @@ -158,7 +150,7 @@ where ) }; self.visit_trait(trait_ref)?; - if self.def_id_visitor.shallow() { + if V::SHALLOW { ControlFlow::Continue(()) } else { assoc_substs.iter().try_for_each(|subst| subst.visit_with(self)) @@ -208,7 +200,7 @@ where | ty::Closure(def_id, ..) | ty::Generator(def_id, ..) => { self.def_id_visitor.visit_def_id(def_id, "type", &ty)?; - if self.def_id_visitor.shallow() { + if V::SHALLOW { return ControlFlow::Continue(()); } // Default type visitor doesn't visit signatures of fn types. @@ -232,7 +224,7 @@ where self.def_id_visitor.visit_def_id(alias.def_id, "type alias", &ty); } ty::Alias(ty::Projection, proj) => { - if self.def_id_visitor.skip_assoc_tys() { + if V::SKIP_ASSOC_TYS { // Visitors searching for minimal visibility/reachability want to // conservatively approximate associated types like `<Type as Trait>::Alias` // as visible/reachable even if both `Type` and `Trait` are private. @@ -244,7 +236,7 @@ where return self.visit_projection_ty(proj); } ty::Alias(ty::Inherent, data) => { - if self.def_id_visitor.skip_assoc_tys() { + if V::SKIP_ASSOC_TYS { // Visitors searching for minimal visibility/reachability want to // conservatively approximate associated types like `Type::Alias` // as visible/reachable even if `Type` is private. @@ -260,7 +252,7 @@ where )?; // This will also visit substs if necessary, so we don't need to recurse. - return if self.def_id_visitor.shallow() { + return if V::SHALLOW { ControlFlow::Continue(()) } else { data.substs.iter().try_for_each(|subst| subst.visit_with(self)) @@ -319,11 +311,7 @@ where } } - if self.def_id_visitor.shallow() { - ControlFlow::Continue(()) - } else { - ty.super_visit_with(self) - } + if V::SHALLOW { ControlFlow::Continue(()) } else { ty.super_visit_with(self) } } fn visit_const(&mut self, c: Const<'tcx>) -> ControlFlow<Self::BreakTy> { @@ -340,22 +328,20 @@ fn min(vis1: ty::Visibility, vis2: ty::Visibility, tcx: TyCtxt<'_>) -> ty::Visib /// Visitor used to determine impl visibility and reachability. //////////////////////////////////////////////////////////////////////////////// -struct FindMin<'a, 'tcx, VL: VisibilityLike> { +struct FindMin<'a, 'tcx, VL: VisibilityLike, const SHALLOW: bool> { tcx: TyCtxt<'tcx>, effective_visibilities: &'a EffectiveVisibilities, min: VL, } -impl<'a, 'tcx, VL: VisibilityLike> DefIdVisitor<'tcx> for FindMin<'a, 'tcx, VL> { +impl<'a, 'tcx, VL: VisibilityLike, const SHALLOW: bool> DefIdVisitor<'tcx> + for FindMin<'a, 'tcx, VL, SHALLOW> +{ + const SHALLOW: bool = SHALLOW; + const SKIP_ASSOC_TYS: bool = true; fn tcx(&self) -> TyCtxt<'tcx> { self.tcx } - fn shallow(&self) -> bool { - VL::SHALLOW - } - fn skip_assoc_tys(&self) -> bool { - true - } fn visit_def_id( &mut self, def_id: DefId, @@ -371,17 +357,19 @@ impl<'a, 'tcx, VL: VisibilityLike> DefIdVisitor<'tcx> for FindMin<'a, 'tcx, VL> trait VisibilityLike: Sized { const MAX: Self; - const SHALLOW: bool = false; - fn new_min(find: &FindMin<'_, '_, Self>, def_id: LocalDefId) -> Self; + fn new_min<const SHALLOW: bool>( + find: &FindMin<'_, '_, Self, SHALLOW>, + def_id: LocalDefId, + ) -> Self; - // Returns an over-approximation (`skip_assoc_tys` = true) of visibility due to + // Returns an over-approximation (`SKIP_ASSOC_TYS` = true) of visibility due to // associated types for which we can't determine visibility precisely. - fn of_impl( + fn of_impl<const SHALLOW: bool>( def_id: LocalDefId, tcx: TyCtxt<'_>, effective_visibilities: &EffectiveVisibilities, ) -> Self { - let mut find = FindMin { tcx, effective_visibilities, min: Self::MAX }; + let mut find = FindMin::<_, SHALLOW> { tcx, effective_visibilities, min: Self::MAX }; find.visit(tcx.type_of(def_id).subst_identity()); if let Some(trait_ref) = tcx.impl_trait_ref(def_id) { find.visit_trait(trait_ref.subst_identity()); @@ -391,49 +379,28 @@ trait VisibilityLike: Sized { } impl VisibilityLike for ty::Visibility { const MAX: Self = ty::Visibility::Public; - fn new_min(find: &FindMin<'_, '_, Self>, def_id: LocalDefId) -> Self { + fn new_min<const SHALLOW: bool>( + find: &FindMin<'_, '_, Self, SHALLOW>, + def_id: LocalDefId, + ) -> Self { min(find.tcx.local_visibility(def_id), find.min, find.tcx) } } -struct NonShallowEffectiveVis(EffectiveVisibility); - -impl VisibilityLike for NonShallowEffectiveVis { - const MAX: Self = NonShallowEffectiveVis(EffectiveVisibility::from_vis(ty::Visibility::Public)); - const SHALLOW: bool = false; - - fn new_min(find: &FindMin<'_, '_, Self>, def_id: LocalDefId) -> Self { - let find = FindMin { - tcx: find.tcx, - effective_visibilities: find.effective_visibilities, - min: ShallowEffectiveVis(find.min.0), - }; - NonShallowEffectiveVis(VisibilityLike::new_min(&find, def_id).0) - } -} - -struct ShallowEffectiveVis(EffectiveVisibility); -impl VisibilityLike for ShallowEffectiveVis { - const MAX: Self = ShallowEffectiveVis(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> { ... }` - // can be usable from other crates (#57264). So we skip substs when calculating reachability - // and consider an impl reachable if its "shallow" type and trait are reachable. - // - // The assumption we make here is that type-inference won't let you use an impl without knowing - // both "shallow" version of its self type and "shallow" version of its trait if it exists - // (which require reaching the `DefId`s in them). - const SHALLOW: bool = true; - fn new_min(find: &FindMin<'_, '_, Self>, def_id: LocalDefId) -> Self { +impl VisibilityLike for EffectiveVisibility { + const MAX: Self = EffectiveVisibility::from_vis(ty::Visibility::Public); + fn new_min<const SHALLOW: bool>( + find: &FindMin<'_, '_, Self, SHALLOW>, + def_id: LocalDefId, + ) -> Self { let effective_vis = - find.effective_visibilities.effective_vis(def_id).cloned().unwrap_or_else(|| { + find.effective_visibilities.effective_vis(def_id).copied().unwrap_or_else(|| { let private_vis = ty::Visibility::Restricted(find.tcx.parent_module_from_def_id(def_id)); EffectiveVisibility::from_vis(private_vis) }); - ShallowEffectiveVis(effective_vis.min(find.min.0, find.tcx)) + effective_vis.min(find.min, find.tcx) } } @@ -492,14 +459,14 @@ impl<'tcx> EmbargoVisitor<'tcx> { &mut self, def_id: LocalDefId, inherited_effective_vis: EffectiveVisibility, - nominal_vis: Option<ty::Visibility>, + max_vis: Option<ty::Visibility>, level: Level, ) { let private_vis = ty::Visibility::Restricted(self.tcx.parent_module_from_def_id(def_id)); - if Some(private_vis) != nominal_vis { + if max_vis != Some(private_vis) { self.changed |= self.effective_visibilities.update( def_id, - nominal_vis, + max_vis, || private_vis, inherited_effective_vis, level, @@ -771,12 +738,21 @@ impl<'tcx> Visitor<'tcx> for EmbargoVisitor<'tcx> { } } hir::ItemKind::Impl(ref impl_) => { - let item_ev = ShallowEffectiveVis::of_impl( + // 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> { ... }` + // can be usable from other crates (#57264). So we skip substs when calculating + // reachability and consider an impl reachable if its "shallow" type and trait are + // reachable. + // + // The assumption we make here is that type-inference won't let you use an impl + // without knowing both "shallow" version of its self type and "shallow" version of + // its trait if it exists (which require reaching the `DefId`s in them). + let item_ev = EffectiveVisibility::of_impl::<true>( item.owner_id.def_id, self.tcx, &self.effective_visibilities, - ) - .0; + ); self.update_eff_vis(item.owner_id.def_id, item_ev, None, Level::Direct); @@ -784,9 +760,9 @@ impl<'tcx> Visitor<'tcx> for EmbargoVisitor<'tcx> { for impl_item_ref in impl_.items { let def_id = impl_item_ref.id.owner_id.def_id; - let nominal_vis = + let max_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); + self.update_eff_vis(def_id, item_ev, max_vis, Level::Direct); if let Some(impl_item_ev) = self.get(def_id) { self.reach(def_id, impl_item_ev).generics().predicates().ty(); @@ -904,7 +880,12 @@ impl<'tcx> DefIdVisitor<'tcx> for ReachEverythingInTheInterfaceVisitor<'_, 'tcx> _descr: &dyn fmt::Display, ) -> ControlFlow<Self::BreakTy> { 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 max_vis = (self.level != Level::ReachableThroughImplTrait) + .then(|| self.ev.tcx.local_visibility(def_id)); + self.ev.update_eff_vis(def_id, self.effective_vis, max_vis, self.level); } ControlFlow::Continue(()) } @@ -1876,10 +1857,9 @@ impl SearchInterfaceForPrivateItemsVisitor<'_> { return false; }; - // FIXME: `Level::Reachable` should be taken instead of `Level::Reexported` - let reexported_at_vis = *effective_vis.at_level(Level::Reexported); + let reachable_at_vis = *effective_vis.at_level(Level::Reachable); - 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 { lint::builtin::PRIVATE_INTERFACES } else { @@ -1896,7 +1876,7 @@ impl SearchInterfaceForPrivateItemsVisitor<'_> { tcx: self.tcx, }) .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_kind: kind, ty_descr: descr.into(), @@ -2137,27 +2117,28 @@ impl<'tcx> PrivateItemsInPublicInterfacesChecker<'tcx, '_> { DefKind::Impl { .. } => { let item = tcx.hir().item(id); if let hir::ItemKind::Impl(ref impl_) = item.kind { - let impl_vis = - ty::Visibility::of_impl(item.owner_id.def_id, tcx, &Default::default()); + let impl_vis = ty::Visibility::of_impl::<false>( + item.owner_id.def_id, + tcx, + &Default::default(), + ); - // we are using the non-shallow version here, unlike when building the + // We are using the non-shallow version here, unlike when building the // effective visisibilities table to avoid large number of false positives. - // For example: + // For example in // // impl From<Priv> for Pub { // fn from(_: Priv) -> Pub {...} // } // - // lints shouldn't be emmited even `from` effective visibility - // is larger then `Priv` nominal visibility. - let impl_ev = Some( - NonShallowEffectiveVis::of_impl( - item.owner_id.def_id, - tcx, - self.effective_visibilities, - ) - .0, - ); + // lints shouldn't be emmited even if `from` effective visibility + // is larger than `Priv` nominal visibility and if `Priv` can leak + // in some scenarios due to type inference. + let impl_ev = Some(EffectiveVisibility::of_impl::<false>( + item.owner_id.def_id, + tcx, + self.effective_visibilities, + )); // check that private components do not appear in the generics or predicates of inherent impls // this check is intentionally NOT performed for impls of traits, per #90586 @@ -2284,7 +2265,7 @@ fn effective_visibilities(tcx: TyCtxt<'_>, (): ()) -> &EffectiveVisibilities { changed: false, }; - visitor.effective_visibilities.check_invariants(tcx, true); + visitor.effective_visibilities.check_invariants(tcx); if visitor.impl_trait_pass { // 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. @@ -2301,7 +2282,7 @@ fn effective_visibilities(tcx: TyCtxt<'_>, (): ()) -> &EffectiveVisibilities { break; } } - visitor.effective_visibilities.check_invariants(tcx, false); + visitor.effective_visibilities.check_invariants(tcx); let mut check_visitor = TestReachabilityVisitor { tcx, effective_visibilities: &visitor.effective_visibilities }; diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index c71ed2097b8..c58d85b99f7 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1558,6 +1558,7 @@ symbols! { type_length_limit, type_macros, type_name, + type_privacy_lints, u128, u16, u32, |