Auto merge of #46083 - petrochenkov:morepriv, r=nikomatsakis

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

Author: bors

src/librustc/hir/mod.rs

@@ -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)))
     }
 }
 

src/librustc_privacy/lib.rs

@@ -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>,
 }
@@ -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;
@@ -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> {
@@ -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 {
@@ -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);
@@ -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;
             }
         }
 
@@ -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;
     }
 
@@ -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;
                 }
             }
@@ -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> {
@@ -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,
@@ -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");
@@ -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,
         }
     }
 }
@@ -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 &&
@@ -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;
@@ -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;
@@ -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,
     };