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

Type privacy polishing #46083

Merged
merged 5 commits into from
Dec 21, 2017
Merged

Type privacy polishing #46083

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8dcd26a
Add tests for associated item privacy
petrochenkov Sep 23, 2017
53779ed
Check associated type bindings for privacy and stability
petrochenkov Nov 18, 2017
190adc0
Properly check traits in type privacy
petrochenkov Nov 18, 2017
020961d
Prohibit access to private statics from other crates through macros 2.0
petrochenkov Nov 18, 2017
c6209a3
Always report private-in-public in associated types as hard errors
petrochenkov Nov 18, 2017
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
9 changes: 7 additions & 2 deletions src/librustc/hir/mod.rs
View file
Open in desktop
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
Open in desktop
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