Skip to content

Commit

Permalink
Auto merge of #46083 - petrochenkov:morepriv, r=nikomatsakis
Browse files Browse the repository at this point in the history 
Type privacy polishing

Various preparations before implementing rust-lang/rfcs#2145 containing final minor breaking changes (mostly for unstable code or code using `allow(private_in_public)`).
(Continuation of #42125, #44633 and #41332.)

It would be good to run crater on this.
r? @eddyb
  • Loading branch information
@bors
bors committed Dec 21, 2017
2 parents fdfb007 + c6209a3 commit a12706c
Show file tree
Hide file tree
Showing 18 changed files with 656 additions and 136 deletions.
9 changes: 7 additions & 2 deletions src/librustc/hir/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,13 @@ impl Path {

impl fmt::Debug for Path {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "path({})",
print::to_string(print::NO_ANN, |s| s.print_path(self, false)))
write!(f, "path({})", print::to_string(print::NO_ANN, |s| s.print_path(self, false)))
}
}

impl fmt::Display for Path {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "{}", print::to_string(print::NO_ANN, |s| s.print_path(self, false)))
}
}

Expand Down
115 changes: 78 additions & 37 deletions src/librustc_privacy/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -614,6 +614,7 @@ struct TypePrivacyVisitor<'a, 'tcx: 'a> {
tcx: TyCtxt<'a, 'tcx, 'tcx>,
tables: &'a ty::TypeckTables<'tcx>,
current_item: DefId,
in_body: bool,
span: Span,
empty_tables: &'a ty::TypeckTables<'tcx>,
}
Expand Down Expand Up @@ -671,10 +672,8 @@ impl<'a, 'tcx> TypePrivacyVisitor<'a, '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;
if let Some(ty) = self.tables.node_id_to_type_opt(id) {
if ty.visit_with(self) {
return true;
}
if self.tables.node_id_to_type(id).visit_with(self) {
return true;
}
if self.tables.node_substs(id).visit_with(self) {
return true;
Expand All @@ -688,6 +687,16 @@ impl<'a, 'tcx> TypePrivacyVisitor<'a, 'tcx> {
}
false
}

fn check_trait_ref(&mut self, trait_ref: ty::TraitRef<'tcx>) -> bool {
if !self.item_is_accessible(trait_ref.def_id) {
let msg = format!("trait `{}` is private", trait_ref);
self.tcx.sess.span_err(self.span, &msg);
return true;
}

trait_ref.super_visit_with(self)
}
}

impl<'a, 'tcx> Visitor<'tcx> for TypePrivacyVisitor<'a, 'tcx> {
Expand All @@ -699,16 +708,18 @@ impl<'a, 'tcx> Visitor<'tcx> for TypePrivacyVisitor<'a, 'tcx> {

fn visit_nested_body(&mut self, body: hir::BodyId) {
let orig_tables = replace(&mut self.tables, self.tcx.body_tables(body));
let orig_in_body = replace(&mut self.in_body, true);
let body = self.tcx.hir.body(body);
self.visit_body(body);
self.tables = orig_tables;
self.in_body = orig_in_body;
}

fn visit_ty(&mut self, hir_ty: &'tcx hir::Ty) {
self.span = hir_ty.span;
if let Some(ty) = self.tables.node_id_to_type_opt(hir_ty.hir_id) {
if self.in_body {
// Types in bodies.
if ty.visit_with(self) {
if self.tables.node_id_to_type(hir_ty.hir_id).visit_with(self) {
return;
}
} else {
Expand All @@ -724,10 +735,21 @@ impl<'a, 'tcx> Visitor<'tcx> for TypePrivacyVisitor<'a, 'tcx> {
}

fn visit_trait_ref(&mut self, trait_ref: &'tcx hir::TraitRef) {
if !self.item_is_accessible(trait_ref.path.def.def_id()) {
let msg = format!("trait `{:?}` is private", trait_ref.path);
self.tcx.sess.span_err(self.span, &msg);
return;
self.span = trait_ref.path.span;
if !self.in_body {
// 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.
let (principal, projections) =
rustc_typeck::hir_trait_to_predicates(self.tcx, trait_ref);
if self.check_trait_ref(*principal.skip_binder()) {
return;
}
for poly_predicate in projections {
let tcx = self.tcx;
if self.check_trait_ref(poly_predicate.skip_binder().projection_ty.trait_ref(tcx)) {
return;
}
}
}

intravisit::walk_trait_ref(self, trait_ref);
Expand Down Expand Up @@ -760,19 +782,35 @@ impl<'a, 'tcx> Visitor<'tcx> for TypePrivacyVisitor<'a, 'tcx> {
intravisit::walk_expr(self, expr);
}

// Prohibit access to associated items with insufficient nominal visibility.
//
// Additionally, until better reachability analysis for macros 2.0 is available,
// we prohibit access to private statics from other crates, this allows to give
// more code internal visibility at link time. (Access to private functions
// is already prohibited by type privacy for funciton types.)
fn visit_qpath(&mut self, qpath: &'tcx hir::QPath, id: ast::NodeId, span: Span) {
// Inherent associated constants don't have self type in substs,
// we have to check it additionally.
if let hir::QPath::TypeRelative(..) = *qpath {
let hir_id = self.tcx.hir.node_to_hir_id(id);
if let Some(def) = self.tables.type_dependent_defs().get(hir_id).cloned() {
if let Some(assoc_item) = self.tcx.opt_associated_item(def.def_id()) {
if let ty::ImplContainer(impl_def_id) = assoc_item.container {
if self.tcx.type_of(impl_def_id).visit_with(self) {
return;
}
}
}
let def = match *qpath {
hir::QPath::Resolved(_, ref path) => match path.def {
Def::Method(..) | Def::AssociatedConst(..) |
Def::AssociatedTy(..) | Def::Static(..) => Some(path.def),
_ => None,
}
hir::QPath::TypeRelative(..) => {
let hir_id = self.tcx.hir.node_to_hir_id(id);
self.tables.type_dependent_defs().get(hir_id).cloned()
}
};
if let Some(def) = def {
let def_id = def.def_id();
let is_local_static = if let Def::Static(..) = def { def_id.is_local() } else { false };
if !self.item_is_accessible(def_id) && !is_local_static {
let name = match *qpath {
hir::QPath::Resolved(_, ref path) => format!("{}", path),
hir::QPath::TypeRelative(_, ref segment) => segment.name.to_string(),
};
let msg = format!("{} `{}` is private", def.kind_name(), name);
self.tcx.sess.span_err(span, &msg);
return;
}
}

Expand Down Expand Up @@ -807,9 +845,11 @@ impl<'a, 'tcx> Visitor<'tcx> for TypePrivacyVisitor<'a, 'tcx> {
item.id,
&mut self.tables,
self.empty_tables);
let orig_in_body = replace(&mut self.in_body, false);
self.current_item = self.tcx.hir.local_def_id(item.id);
intravisit::walk_item(self, item);
self.tables = orig_tables;
self.in_body = orig_in_body;
self.current_item = orig_current_item;
}

Expand Down Expand Up @@ -869,13 +909,8 @@ impl<'a, 'tcx> TypeVisitor<'tcx> for TypePrivacyVisitor<'a, 'tcx> {
}
}
ty::TyProjection(ref proj) => {
let trait_ref = proj.trait_ref(self.tcx);
if !self.item_is_accessible(trait_ref.def_id) {
let msg = format!("trait `{}` is private", trait_ref);
self.tcx.sess.span_err(self.span, &msg);
return true;
}
if trait_ref.super_visit_with(self) {
let tcx = self.tcx;
if self.check_trait_ref(proj.trait_ref(tcx)) {
return true;
}
}
Expand Down Expand Up @@ -1278,6 +1313,7 @@ struct SearchInterfaceForPrivateItemsVisitor<'a, 'tcx: 'a> {
min_visibility: ty::Visibility,
has_pub_restricted: bool,
has_old_errors: bool,
in_assoc_ty: bool,
}

impl<'a, 'tcx: 'a> SearchInterfaceForPrivateItemsVisitor<'a, 'tcx> {
Expand Down Expand Up @@ -1338,11 +1374,11 @@ impl<'a, 'tcx: 'a> SearchInterfaceForPrivateItemsVisitor<'a, 'tcx> {
self.min_visibility = vis;
}
if !vis.is_at_least(self.required_visibility, self.tcx) {
if self.has_pub_restricted || self.has_old_errors {
if self.has_pub_restricted || self.has_old_errors || self.in_assoc_ty {
struct_span_err!(self.tcx.sess, self.span, E0445,
"private trait `{}` in public interface", trait_ref)
.span_label(self.span, format!(
"private trait can't be public"))
"can't leak private trait"))
.emit();
} else {
self.tcx.lint_node(lint::builtin::PRIVATE_IN_PUBLIC,
Expand Down Expand Up @@ -1393,7 +1429,7 @@ impl<'a, 'tcx: 'a> TypeVisitor<'tcx> for SearchInterfaceForPrivateItemsVisitor<'
self.min_visibility = vis;
}
if !vis.is_at_least(self.required_visibility, self.tcx) {
if self.has_pub_restricted || self.has_old_errors {
if self.has_pub_restricted || self.has_old_errors || self.in_assoc_ty {
let mut err = struct_span_err!(self.tcx.sess, self.span, E0446,
"private type `{}` in public interface", ty);
err.span_label(self.span, "can't leak private type");
Expand Down Expand Up @@ -1454,6 +1490,7 @@ impl<'a, 'tcx> PrivateItemsInPublicInterfacesVisitor<'a, 'tcx> {
required_visibility,
has_pub_restricted: self.has_pub_restricted,
has_old_errors,
in_assoc_ty: false,
}
}
}
Expand Down Expand Up @@ -1494,6 +1531,7 @@ impl<'a, 'tcx> Visitor<'tcx> for PrivateItemsInPublicInterfacesVisitor<'a, 'tcx>

for trait_item_ref in trait_item_refs {
let mut check = self.check(trait_item_ref.id.node_id, item_visibility);
check.in_assoc_ty = trait_item_ref.kind == hir::AssociatedItemKind::Type;
check.generics().predicates();

if trait_item_ref.kind == hir::AssociatedItemKind::Type &&
Expand Down Expand Up @@ -1544,10 +1582,10 @@ impl<'a, 'tcx> Visitor<'tcx> for PrivateItemsInPublicInterfacesVisitor<'a, 'tcx>

for impl_item_ref in impl_item_refs {
let impl_item = self.tcx.hir.impl_item(impl_item_ref.id);
let impl_item_vis =
ty::Visibility::from_hir(&impl_item.vis, item.id, tcx);
self.check(impl_item.id, min(impl_item_vis, ty_vis))
.generics().predicates().ty();
let impl_item_vis = ty::Visibility::from_hir(&impl_item.vis, item.id, tcx);
let mut check = self.check(impl_item.id, min(impl_item_vis, ty_vis));
check.in_assoc_ty = impl_item_ref.kind == hir::AssociatedItemKind::Type;
check.generics().predicates().ty();

// Recurse for e.g. `impl Trait` (see `visit_ty`).
self.inner_visibility = impl_item_vis;
Expand All @@ -1562,7 +1600,9 @@ impl<'a, 'tcx> Visitor<'tcx> for PrivateItemsInPublicInterfacesVisitor<'a, 'tcx>
self.check(item.id, vis).generics().predicates();
for impl_item_ref in impl_item_refs {
let impl_item = self.tcx.hir.impl_item(impl_item_ref.id);
self.check(impl_item.id, vis).generics().predicates().ty();
let mut check = self.check(impl_item.id, vis);
check.in_assoc_ty = impl_item_ref.kind == hir::AssociatedItemKind::Type;
check.generics().predicates().ty();

// Recurse for e.g. `impl Trait` (see `visit_ty`).
self.inner_visibility = vis;
Expand Down Expand Up @@ -1629,6 +1669,7 @@ fn privacy_access_levels<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
tcx,
tables: &empty_tables,
current_item: DefId::local(CRATE_DEF_INDEX),
in_body: false,
span: krate.span,
empty_tables: &empty_tables,
};
Expand Down
Loading

0 comments on commit a12706c

Please sign in to comment.