Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

privacy: Refactor top-level visiting in TypePrivacyVisitor #120284

Merged
merged 1 commit into from
Jan 24, 2024
Merged

privacy: Refactor top-level visiting in TypePrivacyVisitor #120284

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 27 additions & 63 deletions compiler/rustc_privacy/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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))?;
Expand All @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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!(
Expand Down Expand Up @@ -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> {
Expand Down Expand Up @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion tests/ui/privacy/private-type-in-interface.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ type A = <m::Alias as m::Trait>::X; //~ ERROR type `Priv` is private
trait Tr2<T> {}
impl<T> Tr2<T> for u8 {}
fn g() -> impl Tr2<m::Alias> { 0 } //~ ERROR type `Priv` is private
//~| ERROR type `Priv` is private
fn g_ext() -> impl Tr2<ext::Alias> { 0 } //~ ERROR type `ext::Priv` is private

//~| ERROR type `ext::Priv` is private
fn main() {}
16 changes: 14 additions & 2 deletions tests/ui/privacy/private-type-in-interface.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,23 @@ error: type `Priv` is private
LL | fn g() -> impl Tr2<m::Alias> { 0 }
| ^^^^^^^^^^^^^^^^^^ private type

error: type `Priv` is private
--> $DIR/private-type-in-interface.rs:28:16
|
LL | fn g() -> impl Tr2<m::Alias> { 0 }
| ^^^^^^^^^^^^^ private type
Comment on lines +49 to +53
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a weird almost-duplication and not even the right span, either before or after this change.

The only functional change should have been the generic arg one, and I don't see why checking consts and lifetimes would have an effect here, as there are only traits and types involved.

Any ideas?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like visit_item_likes_in_module includes impl Trait items, and intravisit::walk_mod does not.


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<ext::Alias> { 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<ext::Alias> { 0 }
| ^^^^^^^^^^^^^^^ private type

error: aborting due to 11 previous errors

Loading