Skip to content

Commit 17edd1a

Browse files
committed
privacy: Remove (Non)ShallowEffectiveVis
Use a boolean constant parameter instead. Also turn some methods on `DefIdVisitor` into associated constants.
1 parent d326aed commit 17edd1a

File tree

1 file changed

+61
-84
lines changed
  • compiler/rustc_privacy/src

1 file changed

+61
-84
lines changed

compiler/rustc_privacy/src/lib.rs

+61-84
Original file line numberDiff line numberDiff line change
@@ -73,14 +73,10 @@ impl<'tcx> fmt::Display for LazyDefPathStr<'tcx> {
7373
/// in `impl Trait`, see individual comments in `DefIdVisitorSkeleton::visit_ty`.
7474
trait DefIdVisitor<'tcx> {
7575
type BreakTy = ();
76+
const SHALLOW: bool = false;
77+
const SKIP_ASSOC_TYS: bool = false;
7678

7779
fn tcx(&self) -> TyCtxt<'tcx>;
78-
fn shallow(&self) -> bool {
79-
false
80-
}
81-
fn skip_assoc_tys(&self) -> bool {
82-
false
83-
}
8480
fn visit_def_id(
8581
&mut self,
8682
def_id: DefId,
@@ -129,11 +125,7 @@ where
129125
fn visit_trait(&mut self, trait_ref: TraitRef<'tcx>) -> ControlFlow<V::BreakTy> {
130126
let TraitRef { def_id, substs, .. } = trait_ref;
131127
self.def_id_visitor.visit_def_id(def_id, "trait", &trait_ref.print_only_trait_path())?;
132-
if self.def_id_visitor.shallow() {
133-
ControlFlow::Continue(())
134-
} else {
135-
substs.visit_with(self)
136-
}
128+
if V::SHALLOW { ControlFlow::Continue(()) } else { substs.visit_with(self) }
137129
}
138130

139131
fn visit_projection_ty(&mut self, projection: ty::AliasTy<'tcx>) -> ControlFlow<V::BreakTy> {
@@ -152,7 +144,7 @@ where
152144
)
153145
};
154146
self.visit_trait(trait_ref)?;
155-
if self.def_id_visitor.shallow() {
147+
if V::SHALLOW {
156148
ControlFlow::Continue(())
157149
} else {
158150
assoc_substs.iter().try_for_each(|subst| subst.visit_with(self))
@@ -222,7 +214,7 @@ where
222214
| ty::Closure(def_id, ..)
223215
| ty::Generator(def_id, ..) => {
224216
self.def_id_visitor.visit_def_id(def_id, "type", &ty)?;
225-
if self.def_id_visitor.shallow() {
217+
if V::SHALLOW {
226218
return ControlFlow::Continue(());
227219
}
228220
// Default type visitor doesn't visit signatures of fn types.
@@ -243,7 +235,7 @@ where
243235
}
244236
}
245237
ty::Alias(ty::Projection, proj) => {
246-
if self.def_id_visitor.skip_assoc_tys() {
238+
if V::SKIP_ASSOC_TYS {
247239
// Visitors searching for minimal visibility/reachability want to
248240
// conservatively approximate associated types like `<Type as Trait>::Alias`
249241
// as visible/reachable even if both `Type` and `Trait` are private.
@@ -255,7 +247,7 @@ where
255247
return self.visit_projection_ty(proj);
256248
}
257249
ty::Alias(ty::Inherent, data) => {
258-
if self.def_id_visitor.skip_assoc_tys() {
250+
if V::SKIP_ASSOC_TYS {
259251
// Visitors searching for minimal visibility/reachability want to
260252
// conservatively approximate associated types like `Type::Alias`
261253
// as visible/reachable even if `Type` is private.
@@ -271,7 +263,7 @@ where
271263
)?;
272264

273265
// This will also visit substs if necessary, so we don't need to recurse.
274-
return if self.def_id_visitor.shallow() {
266+
return if V::SHALLOW {
275267
ControlFlow::Continue(())
276268
} else {
277269
data.substs.iter().try_for_each(|subst| subst.visit_with(self))
@@ -333,11 +325,7 @@ where
333325
}
334326
}
335327

336-
if self.def_id_visitor.shallow() {
337-
ControlFlow::Continue(())
338-
} else {
339-
ty.super_visit_with(self)
340-
}
328+
if V::SHALLOW { ControlFlow::Continue(()) } else { ty.super_visit_with(self) }
341329
}
342330

343331
fn visit_const(&mut self, c: Const<'tcx>) -> ControlFlow<Self::BreakTy> {
@@ -354,22 +342,20 @@ fn min(vis1: ty::Visibility, vis2: ty::Visibility, tcx: TyCtxt<'_>) -> ty::Visib
354342
/// Visitor used to determine impl visibility and reachability.
355343
////////////////////////////////////////////////////////////////////////////////
356344

357-
struct FindMin<'a, 'tcx, VL: VisibilityLike> {
345+
struct FindMin<'a, 'tcx, VL: VisibilityLike, const SHALLOW: bool> {
358346
tcx: TyCtxt<'tcx>,
359347
effective_visibilities: &'a EffectiveVisibilities,
360348
min: VL,
361349
}
362350

363-
impl<'a, 'tcx, VL: VisibilityLike> DefIdVisitor<'tcx> for FindMin<'a, 'tcx, VL> {
351+
impl<'a, 'tcx, VL: VisibilityLike, const SHALLOW: bool> DefIdVisitor<'tcx>
352+
for FindMin<'a, 'tcx, VL, SHALLOW>
353+
{
354+
const SHALLOW: bool = SHALLOW;
355+
const SKIP_ASSOC_TYS: bool = true;
364356
fn tcx(&self) -> TyCtxt<'tcx> {
365357
self.tcx
366358
}
367-
fn shallow(&self) -> bool {
368-
VL::SHALLOW
369-
}
370-
fn skip_assoc_tys(&self) -> bool {
371-
true
372-
}
373359
fn visit_def_id(
374360
&mut self,
375361
def_id: DefId,
@@ -385,17 +371,19 @@ impl<'a, 'tcx, VL: VisibilityLike> DefIdVisitor<'tcx> for FindMin<'a, 'tcx, VL>
385371

386372
trait VisibilityLike: Sized {
387373
const MAX: Self;
388-
const SHALLOW: bool = false;
389-
fn new_min(find: &FindMin<'_, '_, Self>, def_id: LocalDefId) -> Self;
374+
fn new_min<const SHALLOW: bool>(
375+
find: &FindMin<'_, '_, Self, SHALLOW>,
376+
def_id: LocalDefId,
377+
) -> Self;
390378

391-
// Returns an over-approximation (`skip_assoc_tys` = true) of visibility due to
379+
// Returns an over-approximation (`SKIP_ASSOC_TYS` = true) of visibility due to
392380
// associated types for which we can't determine visibility precisely.
393-
fn of_impl(
381+
fn of_impl<const SHALLOW: bool>(
394382
def_id: LocalDefId,
395383
tcx: TyCtxt<'_>,
396384
effective_visibilities: &EffectiveVisibilities,
397385
) -> Self {
398-
let mut find = FindMin { tcx, effective_visibilities, min: Self::MAX };
386+
let mut find = FindMin::<_, SHALLOW> { tcx, effective_visibilities, min: Self::MAX };
399387
find.visit(tcx.type_of(def_id).subst_identity());
400388
if let Some(trait_ref) = tcx.impl_trait_ref(def_id) {
401389
find.visit_trait(trait_ref.subst_identity());
@@ -405,49 +393,28 @@ trait VisibilityLike: Sized {
405393
}
406394
impl VisibilityLike for ty::Visibility {
407395
const MAX: Self = ty::Visibility::Public;
408-
fn new_min(find: &FindMin<'_, '_, Self>, def_id: LocalDefId) -> Self {
396+
fn new_min<const SHALLOW: bool>(
397+
find: &FindMin<'_, '_, Self, SHALLOW>,
398+
def_id: LocalDefId,
399+
) -> Self {
409400
min(find.tcx.local_visibility(def_id), find.min, find.tcx)
410401
}
411402
}
412403

413-
struct NonShallowEffectiveVis(EffectiveVisibility);
414-
415-
impl VisibilityLike for NonShallowEffectiveVis {
416-
const MAX: Self = NonShallowEffectiveVis(EffectiveVisibility::from_vis(ty::Visibility::Public));
417-
const SHALLOW: bool = false;
418-
419-
fn new_min(find: &FindMin<'_, '_, Self>, def_id: LocalDefId) -> Self {
420-
let find = FindMin {
421-
tcx: find.tcx,
422-
effective_visibilities: find.effective_visibilities,
423-
min: ShallowEffectiveVis(find.min.0),
424-
};
425-
NonShallowEffectiveVis(VisibilityLike::new_min(&find, def_id).0)
426-
}
427-
}
428-
429-
struct ShallowEffectiveVis(EffectiveVisibility);
430-
impl VisibilityLike for ShallowEffectiveVis {
431-
const MAX: Self = ShallowEffectiveVis(EffectiveVisibility::from_vis(ty::Visibility::Public));
432-
// Type inference is very smart sometimes.
433-
// It can make an impl reachable even some components of its type or trait are unreachable.
434-
// E.g. methods of `impl ReachableTrait<UnreachableTy> for ReachableTy<UnreachableTy> { ... }`
435-
// can be usable from other crates (#57264). So we skip substs when calculating reachability
436-
// and consider an impl reachable if its "shallow" type and trait are reachable.
437-
//
438-
// The assumption we make here is that type-inference won't let you use an impl without knowing
439-
// both "shallow" version of its self type and "shallow" version of its trait if it exists
440-
// (which require reaching the `DefId`s in them).
441-
const SHALLOW: bool = true;
442-
fn new_min(find: &FindMin<'_, '_, Self>, def_id: LocalDefId) -> Self {
404+
impl VisibilityLike for EffectiveVisibility {
405+
const MAX: Self = EffectiveVisibility::from_vis(ty::Visibility::Public);
406+
fn new_min<const SHALLOW: bool>(
407+
find: &FindMin<'_, '_, Self, SHALLOW>,
408+
def_id: LocalDefId,
409+
) -> Self {
443410
let effective_vis =
444-
find.effective_visibilities.effective_vis(def_id).cloned().unwrap_or_else(|| {
411+
find.effective_visibilities.effective_vis(def_id).copied().unwrap_or_else(|| {
445412
let private_vis =
446413
ty::Visibility::Restricted(find.tcx.parent_module_from_def_id(def_id));
447414
EffectiveVisibility::from_vis(private_vis)
448415
});
449416

450-
ShallowEffectiveVis(effective_vis.min(find.min.0, find.tcx))
417+
effective_vis.min(find.min, find.tcx)
451418
}
452419
}
453420

@@ -785,12 +752,21 @@ impl<'tcx> Visitor<'tcx> for EmbargoVisitor<'tcx> {
785752
}
786753
}
787754
hir::ItemKind::Impl(ref impl_) => {
788-
let item_ev = ShallowEffectiveVis::of_impl(
755+
// Type inference is very smart sometimes. It can make an impl reachable even some
756+
// components of its type or trait are unreachable. E.g. methods of
757+
// `impl ReachableTrait<UnreachableTy> for ReachableTy<UnreachableTy> { ... }`
758+
// can be usable from other crates (#57264). So we skip substs when calculating
759+
// reachability and consider an impl reachable if its "shallow" type and trait are
760+
// reachable.
761+
//
762+
// The assumption we make here is that type-inference won't let you use an impl
763+
// without knowing both "shallow" version of its self type and "shallow" version of
764+
// its trait if it exists (which require reaching the `DefId`s in them).
765+
let item_ev = EffectiveVisibility::of_impl::<true>(
789766
item.owner_id.def_id,
790767
self.tcx,
791768
&self.effective_visibilities,
792-
)
793-
.0;
769+
);
794770

795771
self.update_eff_vis(item.owner_id.def_id, item_ev, None, Level::Direct);
796772

@@ -2154,27 +2130,28 @@ impl<'tcx> PrivateItemsInPublicInterfacesChecker<'tcx, '_> {
21542130
DefKind::Impl { .. } => {
21552131
let item = tcx.hir().item(id);
21562132
if let hir::ItemKind::Impl(ref impl_) = item.kind {
2157-
let impl_vis =
2158-
ty::Visibility::of_impl(item.owner_id.def_id, tcx, &Default::default());
2133+
let impl_vis = ty::Visibility::of_impl::<false>(
2134+
item.owner_id.def_id,
2135+
tcx,
2136+
&Default::default(),
2137+
);
21592138

2160-
// we are using the non-shallow version here, unlike when building the
2139+
// We are using the non-shallow version here, unlike when building the
21612140
// effective visisibilities table to avoid large number of false positives.
2162-
// For example:
2141+
// For example in
21632142
//
21642143
// impl From<Priv> for Pub {
21652144
// fn from(_: Priv) -> Pub {...}
21662145
// }
21672146
//
2168-
// lints shouldn't be emmited even `from` effective visibility
2169-
// is larger then `Priv` nominal visibility.
2170-
let impl_ev = Some(
2171-
NonShallowEffectiveVis::of_impl(
2172-
item.owner_id.def_id,
2173-
tcx,
2174-
self.effective_visibilities,
2175-
)
2176-
.0,
2177-
);
2147+
// lints shouldn't be emmited even if `from` effective visibility
2148+
// is larger than `Priv` nominal visibility and if `Priv` can leak
2149+
// in some scenarios due to type inference.
2150+
let impl_ev = Some(EffectiveVisibility::of_impl::<false>(
2151+
item.owner_id.def_id,
2152+
tcx,
2153+
self.effective_visibilities,
2154+
));
21782155

21792156
// check that private components do not appear in the generics or predicates of inherent impls
21802157
// this check is intentionally NOT performed for impls of traits, per #90586

0 commit comments

Comments
 (0)