From ba75970473ea93b720deddf29201f65086b51095 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Wed, 24 Jan 2024 00:07:00 +0300 Subject: [PATCH] privacy: Refactor top-level visiting in `TypePrivacyVisitor` --- compiler/rustc_privacy/src/lib.rs | 90 ++++++------------- tests/ui/privacy/private-type-in-interface.rs | 3 +- .../privacy/private-type-in-interface.stderr | 16 +++- 3 files changed, 43 insertions(+), 66 deletions(-) diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index 0d4ee1e8dcaad..ae10695fae453 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -24,13 +24,13 @@ use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::{DefId, LocalDefId, LocalModDefId, CRATE_DEF_ID}; use rustc_hir::intravisit::{self, Visitor}; use rustc_hir::{AssocItemKind, ForeignItemKind, ItemId, PatKind}; -use rustc_middle::bug; use rustc_middle::hir::nested_filter; use rustc_middle::middle::privacy::{EffectiveVisibilities, EffectiveVisibility, Level}; use rustc_middle::query::Providers; use rustc_middle::ty::GenericArgs; use rustc_middle::ty::{self, Const, GenericParamDefKind}; use rustc_middle::ty::{TraitRef, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitor}; +use rustc_middle::{bug, span_bug}; use rustc_session::lint; use rustc_span::hygiene::Transparency; use rustc_span::symbol::{kw, sym, Ident}; @@ -1064,29 +1064,22 @@ impl<'tcx> Visitor<'tcx> for NamePrivacyVisitor<'tcx> { struct TypePrivacyVisitor<'tcx> { tcx: TyCtxt<'tcx>, + module_def_id: LocalModDefId, maybe_typeck_results: Option<&'tcx ty::TypeckResults<'tcx>>, - current_item: LocalDefId, span: Span, } impl<'tcx> TypePrivacyVisitor<'tcx> { - /// Gets the type-checking results for the current body. - /// As this will ICE if called outside bodies, only call when working with - /// `Expr` or `Pat` nodes (they are guaranteed to be found only in bodies). - #[track_caller] - fn typeck_results(&self) -> &'tcx ty::TypeckResults<'tcx> { - self.maybe_typeck_results - .expect("`TypePrivacyVisitor::typeck_results` called outside of body") - } - fn item_is_accessible(&self, did: DefId) -> bool { - self.tcx.visibility(did).is_accessible_from(self.current_item, self.tcx) + self.tcx.visibility(did).is_accessible_from(self.module_def_id, self.tcx) } // Take node-id of an expression or pattern and check its type for privacy. fn check_expr_pat_type(&mut self, id: hir::HirId, span: Span) -> bool { self.span = span; - let typeck_results = self.typeck_results(); + let typeck_results = self + .maybe_typeck_results + .unwrap_or_else(|| span_bug!(span, "`hir::Expr` or `hir::Pat` outside of a body")); let result: ControlFlow<()> = try { self.visit(typeck_results.node_type(id))?; self.visit(typeck_results.node_args(id))?; @@ -1107,35 +1100,13 @@ impl<'tcx> TypePrivacyVisitor<'tcx> { } impl<'tcx> Visitor<'tcx> for TypePrivacyVisitor<'tcx> { - type NestedFilter = nested_filter::All; - - /// We want to visit items in the context of their containing - /// module and so forth, so supply a crate for doing a deep walk. - fn nested_visit_map(&mut self) -> Self::Map { - self.tcx.hir() - } - - fn visit_mod(&mut self, _m: &'tcx hir::Mod<'tcx>, _s: Span, _n: hir::HirId) { - // Don't visit nested modules, since we run a separate visitor walk - // for each module in `effective_visibilities` - } - - fn visit_nested_body(&mut self, body: hir::BodyId) { + fn visit_nested_body(&mut self, body_id: hir::BodyId) { let old_maybe_typeck_results = - self.maybe_typeck_results.replace(self.tcx.typeck_body(body)); - let body = self.tcx.hir().body(body); - self.visit_body(body); + self.maybe_typeck_results.replace(self.tcx.typeck_body(body_id)); + self.visit_body(self.tcx.hir().body(body_id)); self.maybe_typeck_results = old_maybe_typeck_results; } - fn visit_generic_arg(&mut self, generic_arg: &'tcx hir::GenericArg<'tcx>) { - match generic_arg { - hir::GenericArg::Type(t) => self.visit_ty(t), - hir::GenericArg::Infer(inf) => self.visit_infer(inf), - hir::GenericArg::Lifetime(_) | hir::GenericArg::Const(_) => {} - } - } - fn visit_ty(&mut self, hir_ty: &'tcx hir::Ty<'tcx>) { self.span = hir_ty.span; if let Some(typeck_results) = self.maybe_typeck_results { @@ -1163,19 +1134,19 @@ impl<'tcx> Visitor<'tcx> for TypePrivacyVisitor<'tcx> { return; } } else { - // We don't do anything for const infers here. + // FIXME: check types of const infers here. } } else { - bug!("visit_infer without typeck_results"); + span_bug!(self.span, "`hir::InferArg` outside of a body"); } intravisit::walk_inf(self, inf); } fn visit_trait_ref(&mut self, trait_ref: &'tcx hir::TraitRef<'tcx>) { self.span = trait_ref.path.span; - if self.maybe_typeck_results.is_none() { - // Avoid calling `hir_trait_to_predicates` in bodies, it will ICE. - // The traits' privacy in bodies is already checked as a part of trait object types. + if self.maybe_typeck_results.is_some() { + // Privacy of traits in bodies is checked as a part of trait object types. + } else { let bounds = rustc_hir_analysis::hir_trait_to_predicates( self.tcx, trait_ref, @@ -1223,7 +1194,10 @@ impl<'tcx> Visitor<'tcx> for TypePrivacyVisitor<'tcx> { hir::ExprKind::MethodCall(segment, ..) => { // Method calls have to be checked specially. self.span = segment.ident.span; - if let Some(def_id) = self.typeck_results().type_dependent_def_id(expr.hir_id) { + let typeck_results = self + .maybe_typeck_results + .unwrap_or_else(|| span_bug!(self.span, "`hir::Expr` outside of a body")); + if let Some(def_id) = typeck_results.type_dependent_def_id(expr.hir_id) { if self.visit(self.tcx.type_of(def_id).instantiate_identity()).is_break() { return; } @@ -1251,9 +1225,13 @@ impl<'tcx> Visitor<'tcx> for TypePrivacyVisitor<'tcx> { Res::Def(kind, def_id) => Some((kind, def_id)), _ => None, }, - hir::QPath::TypeRelative(..) | hir::QPath::LangItem(..) => self - .maybe_typeck_results - .and_then(|typeck_results| typeck_results.type_dependent_def(id)), + hir::QPath::TypeRelative(..) | hir::QPath::LangItem(..) => { + match self.maybe_typeck_results { + Some(typeck_results) => typeck_results.type_dependent_def(id), + // FIXME: Check type-relative associated types in signatures. + None => None, + } + } }; let def = def.filter(|(kind, _)| { matches!( @@ -1307,15 +1285,6 @@ impl<'tcx> Visitor<'tcx> for TypePrivacyVisitor<'tcx> { intravisit::walk_local(self, local); } - - // Check types in item interfaces. - fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) { - let orig_current_item = mem::replace(&mut self.current_item, item.owner_id.def_id); - let old_maybe_typeck_results = self.maybe_typeck_results.take(); - intravisit::walk_item(self, item); - self.maybe_typeck_results = old_maybe_typeck_results; - self.current_item = orig_current_item; - } } impl<'tcx> DefIdVisitor<'tcx> for TypePrivacyVisitor<'tcx> { @@ -1785,13 +1754,8 @@ fn check_mod_privacy(tcx: TyCtxt<'_>, module_def_id: LocalModDefId) { // Check privacy of explicitly written types and traits as well as // inferred types of expressions and patterns. - let mut visitor = TypePrivacyVisitor { - tcx, - maybe_typeck_results: None, - current_item: module_def_id.to_local_def_id(), - span, - }; - intravisit::walk_mod(&mut visitor, module, hir_id); + let mut visitor = TypePrivacyVisitor { tcx, module_def_id, maybe_typeck_results: None, span }; + tcx.hir().visit_item_likes_in_module(module_def_id, &mut visitor); } fn effective_visibilities(tcx: TyCtxt<'_>, (): ()) -> &EffectiveVisibilities { diff --git a/tests/ui/privacy/private-type-in-interface.rs b/tests/ui/privacy/private-type-in-interface.rs index 39e0bf23cacb2..9f55127fd168e 100644 --- a/tests/ui/privacy/private-type-in-interface.rs +++ b/tests/ui/privacy/private-type-in-interface.rs @@ -26,6 +26,7 @@ type A = ::X; //~ ERROR type `Priv` is private trait Tr2 {} impl Tr2 for u8 {} fn g() -> impl Tr2 { 0 } //~ ERROR type `Priv` is private + //~| ERROR type `Priv` is private fn g_ext() -> impl Tr2 { 0 } //~ ERROR type `ext::Priv` is private - + //~| ERROR type `ext::Priv` is private fn main() {} diff --git a/tests/ui/privacy/private-type-in-interface.stderr b/tests/ui/privacy/private-type-in-interface.stderr index 03225d84fdb34..a5e80d6962dba 100644 --- a/tests/ui/privacy/private-type-in-interface.stderr +++ b/tests/ui/privacy/private-type-in-interface.stderr @@ -46,11 +46,23 @@ error: type `Priv` is private LL | fn g() -> impl Tr2 { 0 } | ^^^^^^^^^^^^^^^^^^ private type +error: type `Priv` is private + --> $DIR/private-type-in-interface.rs:28:16 + | +LL | fn g() -> impl Tr2 { 0 } + | ^^^^^^^^^^^^^ private type + error: type `ext::Priv` is private - --> $DIR/private-type-in-interface.rs:29:15 + --> $DIR/private-type-in-interface.rs:30:15 | LL | fn g_ext() -> impl Tr2 { 0 } | ^^^^^^^^^^^^^^^^^^^^ private type -error: aborting due to 9 previous errors +error: type `ext::Priv` is private + --> $DIR/private-type-in-interface.rs:30:20 + | +LL | fn g_ext() -> impl Tr2 { 0 } + | ^^^^^^^^^^^^^^^ private type + +error: aborting due to 11 previous errors