diff options
Diffstat (limited to 'compiler/rustc_privacy/src')
| -rw-r--r-- | compiler/rustc_privacy/src/errors.rs | 29 | ||||
| -rw-r--r-- | compiler/rustc_privacy/src/lib.rs | 479 |
2 files changed, 336 insertions, 172 deletions
diff --git a/compiler/rustc_privacy/src/errors.rs b/compiler/rustc_privacy/src/errors.rs index 72b53eefa..da18f0c82 100644 --- a/compiler/rustc_privacy/src/errors.rs +++ b/compiler/rustc_privacy/src/errors.rs @@ -98,3 +98,32 @@ pub struct PrivateInPublicLint<'a> { pub kind: &'a str, pub descr: DiagnosticArgFromDisplay<'a>, } + +#[derive(LintDiagnostic)] +#[diag(privacy_unnameable_types_lint)] +pub struct UnnameableTypesLint<'a> { + #[label] + pub span: Span, + pub kind: &'a str, + pub descr: DiagnosticArgFromDisplay<'a>, + pub reachable_vis: &'a str, + pub reexported_vis: &'a str, +} + +// Used for `private_interfaces` and `private_bounds` lints. +// They will replace private-in-public errors and compatibility lints in future. +// See https://rust-lang.github.io/rfcs/2145-type-privacy.html for more details. +#[derive(LintDiagnostic)] +#[diag(privacy_private_interface_or_bounds_lint)] +pub struct PrivateInterfacesOrBoundsLint<'a> { + #[label(privacy_item_label)] + pub item_span: Span, + pub item_kind: &'a str, + pub item_descr: DiagnosticArgFromDisplay<'a>, + pub item_vis_descr: &'a str, + #[note(privacy_ty_note)] + pub ty_span: Span, + pub ty_kind: &'a str, + pub ty_descr: DiagnosticArgFromDisplay<'a>, + pub ty_vis_descr: &'a str, +} diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index 65dfdf31e..4fcee9396 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -22,7 +22,7 @@ use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::{DefId, LocalDefId, CRATE_DEF_ID}; use rustc_hir::intravisit::{self, Visitor}; -use rustc_hir::{AssocItemKind, HirIdSet, ItemId, Node, PatKind}; +use rustc_hir::{AssocItemKind, ForeignItemKind, HirIdSet, ItemId, Node, PatKind}; use rustc_middle::bug; use rustc_middle::hir::nested_filter; use rustc_middle::middle::privacy::{EffectiveVisibilities, EffectiveVisibility, Level}; @@ -42,8 +42,8 @@ use std::{fmt, mem}; use errors::{ FieldIsPrivate, FieldIsPrivateLabel, FromPrivateDependencyInPublicInterface, InPublicInterface, - InPublicInterfaceTraits, ItemIsPrivate, PrivateInPublicLint, ReportEffectiveVisibility, - UnnamedItemIsPrivate, + InPublicInterfaceTraits, ItemIsPrivate, PrivateInPublicLint, PrivateInterfacesOrBoundsLint, + ReportEffectiveVisibility, UnnameableTypesLint, UnnamedItemIsPrivate, }; fluent_messages! { "../messages.ftl" } @@ -52,6 +52,17 @@ fluent_messages! { "../messages.ftl" } /// Generic infrastructure used to implement specific visitors below. //////////////////////////////////////////////////////////////////////////////// +struct LazyDefPathStr<'tcx> { + def_id: DefId, + tcx: TyCtxt<'tcx>, +} + +impl<'tcx> fmt::Display for LazyDefPathStr<'tcx> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.tcx.def_path_str(self.def_id)) + } +} + /// Implemented to visit all `DefId`s in a type. /// Visiting `DefId`s is useful because visibilities and reachabilities are attached to them. /// The idea is to visit "all components of a type", as documented in @@ -62,14 +73,10 @@ fluent_messages! { "../messages.ftl" } /// 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, @@ -101,7 +108,13 @@ trait DefIdVisitor<'tcx> { &mut self, predicates: ty::GenericPredicates<'tcx>, ) -> ControlFlow<Self::BreakTy> { - self.skeleton().visit_predicates(predicates) + self.skeleton().visit_clauses(predicates.predicates) + } + fn visit_clauses( + &mut self, + clauses: &[(ty::Clause<'tcx>, Span)], + ) -> ControlFlow<Self::BreakTy> { + self.skeleton().visit_clauses(clauses) } } @@ -118,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> { @@ -141,56 +150,35 @@ 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)) } } - fn visit_predicate(&mut self, predicate: ty::Predicate<'tcx>) -> ControlFlow<V::BreakTy> { - match predicate.kind().skip_binder() { - ty::PredicateKind::Clause(ty::Clause::Trait(ty::TraitPredicate { - trait_ref, - constness: _, - polarity: _, - })) => self.visit_trait(trait_ref), - ty::PredicateKind::Clause(ty::Clause::Projection(ty::ProjectionPredicate { - projection_ty, - term, - })) => { + fn visit_clause(&mut self, clause: ty::Clause<'tcx>) -> ControlFlow<V::BreakTy> { + match clause.kind().skip_binder() { + ty::ClauseKind::Trait(ty::TraitPredicate { trait_ref, constness: _, polarity: _ }) => { + self.visit_trait(trait_ref) + } + ty::ClauseKind::Projection(ty::ProjectionPredicate { projection_ty, term }) => { term.visit_with(self)?; self.visit_projection_ty(projection_ty) } - ty::PredicateKind::Clause(ty::Clause::TypeOutlives(ty::OutlivesPredicate( - ty, - _region, - ))) => ty.visit_with(self), - ty::PredicateKind::Clause(ty::Clause::RegionOutlives(..)) => ControlFlow::Continue(()), - ty::PredicateKind::Clause(ty::Clause::ConstArgHasType(ct, ty)) => { + ty::ClauseKind::TypeOutlives(ty::OutlivesPredicate(ty, _region)) => ty.visit_with(self), + ty::ClauseKind::RegionOutlives(..) => ControlFlow::Continue(()), + ty::ClauseKind::ConstArgHasType(ct, ty) => { ct.visit_with(self)?; ty.visit_with(self) } - ty::PredicateKind::ConstEvaluatable(ct) => ct.visit_with(self), - ty::PredicateKind::WellFormed(arg) => arg.visit_with(self), - - ty::PredicateKind::ObjectSafe(_) - | ty::PredicateKind::ClosureKind(_, _, _) - | ty::PredicateKind::Subtype(_) - | ty::PredicateKind::Coerce(_) - | ty::PredicateKind::ConstEquate(_, _) - | ty::PredicateKind::TypeWellFormedFromEnv(_) - | ty::PredicateKind::Ambiguous - | ty::PredicateKind::AliasRelate(..) => bug!("unexpected predicate: {:?}", predicate), + ty::ClauseKind::ConstEvaluatable(ct) => ct.visit_with(self), + ty::ClauseKind::WellFormed(arg) => arg.visit_with(self), } } - fn visit_predicates( - &mut self, - predicates: ty::GenericPredicates<'tcx>, - ) -> ControlFlow<V::BreakTy> { - let ty::GenericPredicates { parent: _, predicates } = predicates; - predicates.iter().try_for_each(|&(predicate, _span)| self.visit_predicate(predicate)) + fn visit_clauses(&mut self, clauses: &[(ty::Clause<'tcx>, Span)]) -> ControlFlow<V::BreakTy> { + clauses.into_iter().try_for_each(|&(clause, _span)| self.visit_clause(clause)) } } @@ -211,7 +199,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. @@ -231,8 +219,11 @@ where } } } + ty::Alias(ty::Weak, alias) => { + 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 +235,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. @@ -259,18 +250,8 @@ where &LazyDefPathStr { def_id: data.def_id, tcx }, )?; - struct LazyDefPathStr<'tcx> { - def_id: DefId, - tcx: TyCtxt<'tcx>, - } - impl<'tcx> fmt::Display for LazyDefPathStr<'tcx> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.tcx.def_path_str(self.def_id)) - } - } - // 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)) @@ -301,10 +282,7 @@ where // through the trait list (default type visitor doesn't visit those traits). // All traits in the list are considered the "primary" part of the type // and are visited by shallow visitors. - self.visit_predicates(ty::GenericPredicates { - parent: None, - predicates: tcx.explicit_item_bounds(def_id).skip_binder(), - })?; + self.visit_clauses(tcx.explicit_item_bounds(def_id).skip_binder())?; } } // These types don't have their own def-ids (but may have subcomponents @@ -332,11 +310,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> { @@ -353,22 +327,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, @@ -384,17 +356,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()); @@ -404,31 +378,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) } } -impl VisibilityLike for Option<EffectiveVisibility> { - const MAX: Self = Some(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 { - if let Some(min) = find.min { - return find - .effective_visibilities - .effective_vis(def_id) - .map(|eff_vis| min.min(*eff_vis, find.tcx)); - } - None +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).copied().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) } } @@ -487,14 +458,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, @@ -751,7 +722,7 @@ impl<'tcx> Visitor<'tcx> for EmbargoVisitor<'tcx> { reach.generics().predicates(); if trait_item_ref.kind == AssocItemKind::Type - && !tcx.impl_defaultness(trait_item_ref.id.owner_id).has_value() + && !tcx.defaultness(trait_item_ref.id.owner_id).has_value() { // No type to visit. } else { @@ -766,28 +737,34 @@ impl<'tcx> Visitor<'tcx> for EmbargoVisitor<'tcx> { } } hir::ItemKind::Impl(ref impl_) => { - if let Some(item_ev) = Option::<EffectiveVisibility>::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, - ) { - 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.update_eff_vis(item.owner_id.def_id, item_ev, None, 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); + self.reach(item.owner_id.def_id, item_ev).generics().predicates().ty().trait_ref(); - if let Some(impl_item_ev) = self.get(def_id) { - self.reach(def_id, impl_item_ev).generics().predicates().ty(); - } + for impl_item_ref in impl_.items { + let def_id = impl_item_ref.id.owner_id.def_id; + let max_vis = + impl_.of_trait.is_none().then(|| self.tcx.local_visibility(def_id)); + 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(); } } } @@ -902,7 +879,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(()) } @@ -916,6 +898,21 @@ pub struct TestReachabilityVisitor<'tcx, 'a> { effective_visibilities: &'a EffectiveVisibilities, } +fn vis_to_string<'tcx>(def_id: LocalDefId, vis: ty::Visibility, tcx: TyCtxt<'tcx>) -> String { + match vis { + ty::Visibility::Restricted(restricted_id) => { + if restricted_id.is_top_level_module() { + "pub(crate)".to_string() + } else if restricted_id == tcx.parent_module_from_def_id(def_id) { + "pub(self)".to_string() + } else { + format!("pub({})", tcx.item_name(restricted_id.to_def_id())) + } + } + ty::Visibility::Public => "pub".to_string(), + } +} + impl<'tcx, 'a> TestReachabilityVisitor<'tcx, 'a> { fn effective_visibility_diagnostic(&mut self, def_id: LocalDefId) { if self.tcx.has_attr(def_id, sym::rustc_effective_visibility) { @@ -923,18 +920,7 @@ impl<'tcx, 'a> TestReachabilityVisitor<'tcx, 'a> { let span = self.tcx.def_span(def_id.to_def_id()); if let Some(effective_vis) = self.effective_visibilities.effective_vis(def_id) { for level in Level::all_levels() { - let vis_str = match effective_vis.at_level(level) { - ty::Visibility::Restricted(restricted_id) => { - if restricted_id.is_top_level_module() { - "pub(crate)".to_string() - } else if *restricted_id == self.tcx.parent_module_from_def_id(def_id) { - "pub(self)".to_string() - } else { - format!("pub({})", self.tcx.item_name(restricted_id.to_def_id())) - } - } - ty::Visibility::Public => "pub".to_string(), - }; + let vis_str = vis_to_string(def_id, *effective_vis.at_level(level), self.tcx); if level != Level::Direct { error_msg.push_str(", "); } @@ -1246,14 +1232,14 @@ impl<'tcx> Visitor<'tcx> for TypePrivacyVisitor<'tcx> { self.tcx.types.never, ); - for (pred, _) in bounds.predicates() { - match pred.kind().skip_binder() { - ty::PredicateKind::Clause(ty::Clause::Trait(trait_predicate)) => { + for (clause, _) in bounds.clauses() { + match clause.kind().skip_binder() { + ty::ClauseKind::Trait(trait_predicate) => { if self.visit_trait(trait_predicate.trait_ref).is_break() { return; } } - ty::PredicateKind::Clause(ty::Clause::Projection(proj_predicate)) => { + ty::ClauseKind::Projection(proj_predicate) => { let term = self.visit(proj_predicate.term); if term.is_break() || self.visit_projection_ty(proj_predicate.projection_ty).is_break() @@ -1749,12 +1735,15 @@ struct SearchInterfaceForPrivateItemsVisitor<'tcx> { item_def_id: LocalDefId, /// The visitor checks that each component type is at least this visible. required_visibility: ty::Visibility, + required_effective_vis: Option<EffectiveVisibility>, has_old_errors: bool, in_assoc_ty: bool, + in_primary_interface: bool, } impl SearchInterfaceForPrivateItemsVisitor<'_> { fn generics(&mut self) -> &mut Self { + self.in_primary_interface = true; for param in &self.tcx.generics_of(self.item_def_id).params { match param.kind { GenericParamDefKind::Lifetime => {} @@ -1773,6 +1762,7 @@ impl SearchInterfaceForPrivateItemsVisitor<'_> { } fn predicates(&mut self) -> &mut Self { + self.in_primary_interface = false; // N.B., we use `explicit_predicates_of` and not `predicates_of` // because we don't want to report privacy errors due to where // clauses that the compiler inferred. We only want to @@ -1784,14 +1774,13 @@ impl SearchInterfaceForPrivateItemsVisitor<'_> { } fn bounds(&mut self) -> &mut Self { - self.visit_predicates(ty::GenericPredicates { - parent: None, - predicates: self.tcx.explicit_item_bounds(self.item_def_id).skip_binder(), - }); + self.in_primary_interface = false; + self.visit_clauses(self.tcx.explicit_item_bounds(self.item_def_id).skip_binder()); self } fn ty(&mut self) -> &mut Self { + self.in_primary_interface = true; self.visit(self.tcx.type_of(self.item_def_id).subst_identity()); self } @@ -1815,8 +1804,10 @@ impl SearchInterfaceForPrivateItemsVisitor<'_> { }; let vis = self.tcx.local_visibility(local_def_id); + let hir_id = self.tcx.hir().local_def_id_to_hir_id(local_def_id); + let span = self.tcx.def_span(self.item_def_id.to_def_id()); + let vis_span = self.tcx.def_span(def_id); if !vis.is_at_least(self.required_visibility, self.tcx) { - let hir_id = self.tcx.hir().local_def_id_to_hir_id(local_def_id); let vis_descr = match vis { ty::Visibility::Public => "public", ty::Visibility::Restricted(vis_def_id) => { @@ -1829,12 +1820,11 @@ impl SearchInterfaceForPrivateItemsVisitor<'_> { } } }; - let span = self.tcx.def_span(self.item_def_id.to_def_id()); + if self.has_old_errors || self.in_assoc_ty || self.tcx.resolutions(()).has_pub_restricted { - let vis_span = self.tcx.def_span(def_id); if kind == "trait" { self.tcx.sess.emit_err(InPublicInterfaceTraits { span, @@ -1862,6 +1852,39 @@ impl SearchInterfaceForPrivateItemsVisitor<'_> { } } + let Some(effective_vis) = self.required_effective_vis else { + return false; + }; + + let reachable_at_vis = *effective_vis.at_level(Level::Reachable); + + if !vis.is_at_least(reachable_at_vis, self.tcx) { + let lint = if self.in_primary_interface { + lint::builtin::PRIVATE_INTERFACES + } else { + lint::builtin::PRIVATE_BOUNDS + }; + self.tcx.emit_spanned_lint( + lint, + hir_id, + span, + PrivateInterfacesOrBoundsLint { + item_span: span, + item_kind: self.tcx.def_descr(self.item_def_id.to_def_id()), + item_descr: (&LazyDefPathStr { + def_id: self.item_def_id.to_def_id(), + tcx: self.tcx, + }) + .into(), + 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(), + ty_vis_descr: &vis_to_string(local_def_id, vis, self.tcx), + }, + ); + } + false } @@ -1895,25 +1918,55 @@ impl<'tcx> DefIdVisitor<'tcx> for SearchInterfaceForPrivateItemsVisitor<'tcx> { } } -struct PrivateItemsInPublicInterfacesChecker<'tcx> { +struct PrivateItemsInPublicInterfacesChecker<'tcx, 'a> { tcx: TyCtxt<'tcx>, old_error_set_ancestry: HirIdSet, + effective_visibilities: &'a EffectiveVisibilities, } -impl<'tcx> PrivateItemsInPublicInterfacesChecker<'tcx> { +impl<'tcx> PrivateItemsInPublicInterfacesChecker<'tcx, '_> { fn check( &self, def_id: LocalDefId, required_visibility: ty::Visibility, + required_effective_vis: Option<EffectiveVisibility>, ) -> SearchInterfaceForPrivateItemsVisitor<'tcx> { SearchInterfaceForPrivateItemsVisitor { tcx: self.tcx, item_def_id: def_id, required_visibility, + required_effective_vis, has_old_errors: self .old_error_set_ancestry .contains(&self.tcx.hir().local_def_id_to_hir_id(def_id)), in_assoc_ty: false, + in_primary_interface: true, + } + } + + fn check_unnameable(&self, def_id: LocalDefId, effective_vis: Option<EffectiveVisibility>) { + let Some(effective_vis) = effective_vis else { + return; + }; + + let reexported_at_vis = effective_vis.at_level(Level::Reexported); + let reachable_at_vis = effective_vis.at_level(Level::Reachable); + + if reexported_at_vis != reachable_at_vis { + let hir_id = self.tcx.hir().local_def_id_to_hir_id(def_id); + let span = self.tcx.def_span(def_id.to_def_id()); + self.tcx.emit_spanned_lint( + lint::builtin::UNNAMEABLE_TYPES, + hir_id, + span, + UnnameableTypesLint { + span, + kind: self.tcx.def_descr(def_id.to_def_id()), + descr: (&LazyDefPathStr { def_id: def_id.to_def_id(), tcx: self.tcx }).into(), + reachable_vis: &vis_to_string(def_id, *reachable_at_vis, self.tcx), + reexported_vis: &vis_to_string(def_id, *reexported_at_vis, self.tcx), + }, + ); } } @@ -1922,13 +1975,19 @@ impl<'tcx> PrivateItemsInPublicInterfacesChecker<'tcx> { def_id: LocalDefId, assoc_item_kind: AssocItemKind, vis: ty::Visibility, + effective_vis: Option<EffectiveVisibility>, ) { - let mut check = self.check(def_id, vis); + let mut check = self.check(def_id, vis, effective_vis); let (check_ty, is_assoc_ty) = match assoc_item_kind { AssocItemKind::Const | AssocItemKind::Fn { .. } => (true, false), - AssocItemKind::Type => (self.tcx.impl_defaultness(def_id).has_value(), true), + AssocItemKind::Type => (self.tcx.defaultness(def_id).has_value(), true), }; + + if is_assoc_ty { + self.check_unnameable(def_id, self.get(def_id)); + } + check.in_assoc_ty = is_assoc_ty; check.generics().predicates(); if check_ty { @@ -1936,50 +1995,72 @@ impl<'tcx> PrivateItemsInPublicInterfacesChecker<'tcx> { } } + fn get(&self, def_id: LocalDefId) -> Option<EffectiveVisibility> { + self.effective_visibilities.effective_vis(def_id).copied() + } + pub fn check_item(&mut self, id: ItemId) { let tcx = self.tcx; let def_id = id.owner_id.def_id; let item_visibility = tcx.local_visibility(def_id); + let effective_vis = self.get(def_id); let def_kind = tcx.def_kind(def_id); match def_kind { DefKind::Const | DefKind::Static(_) | DefKind::Fn | DefKind::TyAlias => { - self.check(def_id, item_visibility).generics().predicates().ty(); + if let DefKind::TyAlias = def_kind { + self.check_unnameable(def_id, effective_vis); + } + self.check(def_id, item_visibility, effective_vis).generics().predicates().ty(); } DefKind::OpaqueTy => { // `ty()` for opaque types is the underlying type, // it's not a part of interface, so we skip it. - self.check(def_id, item_visibility).generics().bounds(); + self.check(def_id, item_visibility, effective_vis).generics().bounds(); } DefKind::Trait => { let item = tcx.hir().item(id); if let hir::ItemKind::Trait(.., trait_item_refs) = item.kind { - self.check(item.owner_id.def_id, item_visibility).generics().predicates(); + self.check_unnameable(item.owner_id.def_id, effective_vis); + + self.check(item.owner_id.def_id, item_visibility, effective_vis) + .generics() + .predicates(); for trait_item_ref in trait_item_refs { self.check_assoc_item( trait_item_ref.id.owner_id.def_id, trait_item_ref.kind, item_visibility, + effective_vis, ); if let AssocItemKind::Type = trait_item_ref.kind { - self.check(trait_item_ref.id.owner_id.def_id, item_visibility).bounds(); + self.check( + trait_item_ref.id.owner_id.def_id, + item_visibility, + effective_vis, + ) + .bounds(); } } } } DefKind::TraitAlias => { - self.check(def_id, item_visibility).generics().predicates(); + self.check(def_id, item_visibility, effective_vis).generics().predicates(); } DefKind::Enum => { let item = tcx.hir().item(id); if let hir::ItemKind::Enum(ref def, _) = item.kind { - self.check(item.owner_id.def_id, item_visibility).generics().predicates(); + self.check_unnameable(item.owner_id.def_id, effective_vis); + + self.check(item.owner_id.def_id, item_visibility, effective_vis) + .generics() + .predicates(); for variant in def.variants { for field in variant.data.fields() { - self.check(field.def_id, item_visibility).ty(); + self.check(field.def_id, item_visibility, effective_vis).ty(); } } } @@ -1989,8 +2070,16 @@ impl<'tcx> PrivateItemsInPublicInterfacesChecker<'tcx> { let item = tcx.hir().item(id); if let hir::ItemKind::ForeignMod { items, .. } = item.kind { for foreign_item in items { - let vis = tcx.local_visibility(foreign_item.id.owner_id.def_id); - self.check(foreign_item.id.owner_id.def_id, vis) + let foreign_item = tcx.hir().foreign_item(foreign_item.id); + + let ev = self.get(foreign_item.owner_id.def_id); + let vis = tcx.local_visibility(foreign_item.owner_id.def_id); + + if let ForeignItemKind::Type = foreign_item.kind { + self.check_unnameable(foreign_item.owner_id.def_id, ev); + } + + self.check(foreign_item.owner_id.def_id, vis, ev) .generics() .predicates() .ty(); @@ -2003,11 +2092,21 @@ impl<'tcx> PrivateItemsInPublicInterfacesChecker<'tcx> { if let hir::ItemKind::Struct(ref struct_def, _) | hir::ItemKind::Union(ref struct_def, _) = item.kind { - self.check(item.owner_id.def_id, item_visibility).generics().predicates(); + self.check_unnameable(item.owner_id.def_id, effective_vis); + self.check(item.owner_id.def_id, item_visibility, effective_vis) + .generics() + .predicates(); for field in struct_def.fields() { let field_visibility = tcx.local_visibility(field.def_id); - self.check(field.def_id, min(item_visibility, field_visibility, tcx)).ty(); + let field_ev = self.get(field.def_id); + + self.check( + field.def_id, + min(item_visibility, field_visibility, tcx), + field_ev, + ) + .ty(); } } } @@ -2018,12 +2117,35 @@ 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 + // effective visisibilities table to avoid large number of false positives. + // For example in + // + // impl From<Priv> for Pub { + // fn from(_: Priv) -> Pub {...} + // } + // + // 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 = 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 if impl_.of_trait.is_none() { - self.check(item.owner_id.def_id, impl_vis).generics().predicates(); + self.check(item.owner_id.def_id, impl_vis, Some(impl_ev)) + .generics() + .predicates(); } for impl_item_ref in impl_.items { let impl_item_vis = if impl_.of_trait.is_none() { @@ -2035,10 +2157,19 @@ impl<'tcx> PrivateItemsInPublicInterfacesChecker<'tcx> { } else { impl_vis }; + + let impl_item_ev = if impl_.of_trait.is_none() { + self.get(impl_item_ref.id.owner_id.def_id) + .map(|ev| ev.min(impl_ev, self.tcx)) + } else { + Some(impl_ev) + }; + self.check_assoc_item( impl_item_ref.id.owner_id.def_id, impl_item_ref.kind, impl_item_vis, + impl_item_ev, ); } } @@ -2137,7 +2268,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. @@ -2154,7 +2285,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 }; @@ -2190,7 +2321,11 @@ fn check_private_in_public(tcx: TyCtxt<'_>, (): ()) { } // Check for private types and traits in public interfaces. - let mut checker = PrivateItemsInPublicInterfacesChecker { tcx, old_error_set_ancestry }; + let mut checker = PrivateItemsInPublicInterfacesChecker { + tcx, + old_error_set_ancestry, + effective_visibilities, + }; for id in tcx.hir().items() { checker.check_item(id); |