Properly check traits in type privacy

Author: petrochenkov

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,29 @@ impl<'a, 'tcx> Visitor<'tcx> for TypePrivacyVisitor<'a, 'tcx> {
         intravisit::walk_expr(self, expr);
     }
 
+    // Prohibit access to associated items with insufficient nominal visibility.
     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(..) => 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();
+            if !self.item_is_accessible(def_id) {
+                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 +839,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 +903,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;
                 }
             }
@@ -1629,6 +1658,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,
     };

src/librustc_typeck/astconv.rs

@@ -347,13 +347,13 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o {
         }
     }
 
-    pub fn instantiate_poly_trait_ref(&self,
-        ast_trait_ref: &hir::PolyTraitRef,
+    pub(super) fn instantiate_poly_trait_ref_inner(&self,
+        trait_ref: &hir::TraitRef,
         self_ty: Ty<'tcx>,
-        poly_projections: &mut Vec<ty::PolyProjectionPredicate<'tcx>>)
+        poly_projections: &mut Vec<ty::PolyProjectionPredicate<'tcx>>,
+        speculative: bool)
         -> ty::PolyTraitRef<'tcx>
     {
-        let trait_ref = &ast_trait_ref.trait_ref;
         let trait_def_id = self.trait_def_id(trait_ref);
 
         debug!("ast_path_to_poly_trait_ref({:?}, def_id={:?})", trait_ref, trait_def_id);
@@ -371,7 +371,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o {
             // specify type to assert that error was already reported in Err case:
             let predicate: Result<_, ErrorReported> =
                 self.ast_type_binding_to_poly_projection_predicate(trait_ref.ref_id, poly_trait_ref,
-                                                                   binding);
+                                                                   binding, speculative);
             predicate.ok() // ok to ignore Err() because ErrorReported (see above)
         }));
 
@@ -380,6 +380,16 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o {
         poly_trait_ref
     }
 
+    pub fn instantiate_poly_trait_ref(&self,
+        poly_trait_ref: &hir::PolyTraitRef,
+        self_ty: Ty<'tcx>,
+        poly_projections: &mut Vec<ty::PolyProjectionPredicate<'tcx>>)
+        -> ty::PolyTraitRef<'tcx>
+    {
+        self.instantiate_poly_trait_ref_inner(&poly_trait_ref.trait_ref, self_ty,
+                                              poly_projections, false)
+    }
+
     fn ast_path_to_mono_trait_ref(&self,
                                   span: Span,
                                   trait_def_id: DefId,
@@ -445,55 +455,59 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o {
         &self,
         ref_id: ast::NodeId,
         trait_ref: ty::PolyTraitRef<'tcx>,
-        binding: &ConvertedBinding<'tcx>)
+        binding: &ConvertedBinding<'tcx>,
+        speculative: bool)
         -> Result<ty::PolyProjectionPredicate<'tcx>, ErrorReported>
     {
         let tcx = self.tcx();
 
-        // Given something like `U : SomeTrait<T=X>`, we want to produce a
-        // predicate like `<U as SomeTrait>::T = X`. This is somewhat
-        // subtle in the event that `T` is defined in a supertrait of
-        // `SomeTrait`, because in that case we need to upcast.
-        //
-        // That is, consider this case:
-        //
-        // ```
-        // trait SubTrait : SuperTrait<int> { }
-        // trait SuperTrait<A> { type T; }
-        //
-        // ... B : SubTrait<T=foo> ...
-        // ```
-        //
-        // We want to produce `<B as SuperTrait<int>>::T == foo`.
-
-        // Find any late-bound regions declared in `ty` that are not
-        // declared in the trait-ref. These are not wellformed.
-        //
-        // Example:
-        //
-        //     for<'a> <T as Iterator>::Item = &'a str // <-- 'a is bad
-        //     for<'a> <T as FnMut<(&'a u32,)>>::Output = &'a str // <-- 'a is ok
-        let late_bound_in_trait_ref = tcx.collect_constrained_late_bound_regions(&trait_ref);
-        let late_bound_in_ty = tcx.collect_referenced_late_bound_regions(&ty::Binder(binding.ty));
-        debug!("late_bound_in_trait_ref = {:?}", late_bound_in_trait_ref);
-        debug!("late_bound_in_ty = {:?}", late_bound_in_ty);
-        for br in late_bound_in_ty.difference(&late_bound_in_trait_ref) {
-            let br_name = match *br {
-                ty::BrNamed(_, name) => name,
-                _ => {
-                    span_bug!(
-                        binding.span,
-                        "anonymous bound region {:?} in binding but not trait ref",
-                        br);
-                }
-            };
-            struct_span_err!(tcx.sess,
-                             binding.span,
-                             E0582,
-                             "binding for associated type `{}` references lifetime `{}`, \
-                              which does not appear in the trait input types",
-                             binding.item_name, br_name)
-                .emit();
+        if !speculative {
+            // Given something like `U : SomeTrait<T=X>`, we want to produce a
+            // predicate like `<U as SomeTrait>::T = X`. This is somewhat
+            // subtle in the event that `T` is defined in a supertrait of
+            // `SomeTrait`, because in that case we need to upcast.
+            //
+            // That is, consider this case:
+            //
+            // ```
+            // trait SubTrait : SuperTrait<int> { }
+            // trait SuperTrait<A> { type T; }
+            //
+            // ... B : SubTrait<T=foo> ...
+            // ```
+            //
+            // We want to produce `<B as SuperTrait<int>>::T == foo`.
+
+            // Find any late-bound regions declared in `ty` that are not
+            // declared in the trait-ref. These are not wellformed.
+            //
+            // Example:
+            //
+            //     for<'a> <T as Iterator>::Item = &'a str // <-- 'a is bad
+            //     for<'a> <T as FnMut<(&'a u32,)>>::Output = &'a str // <-- 'a is ok
+            let late_bound_in_trait_ref = tcx.collect_constrained_late_bound_regions(&trait_ref);
+            let late_bound_in_ty =
+                tcx.collect_referenced_late_bound_regions(&ty::Binder(binding.ty));
+            debug!("late_bound_in_trait_ref = {:?}", late_bound_in_trait_ref);
+            debug!("late_bound_in_ty = {:?}", late_bound_in_ty);
+            for br in late_bound_in_ty.difference(&late_bound_in_trait_ref) {
+                let br_name = match *br {
+                    ty::BrNamed(_, name) => name,
+                    _ => {
+                        span_bug!(
+                            binding.span,
+                            "anonymous bound region {:?} in binding but not trait ref",
+                            br);
+                    }
+                };
+                struct_span_err!(tcx.sess,
+                                binding.span,
+                                E0582,
+                                "binding for associated type `{}` references lifetime `{}`, \
+                                which does not appear in the trait input types",
+                                binding.item_name, br_name)
+                    .emit();
+            }
         }
 
         let candidate = if self.trait_defines_associated_type_named(trait_ref.def_id(),

src/librustc_typeck/lib.rs

@@ -348,7 +348,22 @@ pub fn hir_ty_to_ty<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, hir_ty: &hir::Ty) ->
     let env_node_id = tcx.hir.get_parent(hir_ty.id);
     let env_def_id = tcx.hir.local_def_id(env_node_id);
     let item_cx = self::collect::ItemCtxt::new(tcx, env_def_id);
-    item_cx.to_ty(hir_ty)
+    astconv::AstConv::ast_ty_to_ty(&item_cx, hir_ty)
+}
+
+pub fn hir_trait_to_predicates<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, hir_trait: &hir::TraitRef)
+        -> (ty::PolyTraitRef<'tcx>, Vec<ty::PolyProjectionPredicate<'tcx>>) {
+    // In case there are any projections etc, find the "environment"
+    // def-id that will be used to determine the traits/predicates in
+    // scope.  This is derived from the enclosing item-like thing.
+    let env_node_id = tcx.hir.get_parent(hir_trait.ref_id);
+    let env_def_id = tcx.hir.local_def_id(env_node_id);
+    let item_cx = self::collect::ItemCtxt::new(tcx, env_def_id);
+    let mut projections = Vec::new();
+    let principal = astconv::AstConv::instantiate_poly_trait_ref_inner(
+        &item_cx, hir_trait, tcx.types.err, &mut projections, true
+    );
+    (principal, projections)
 }
 
 __build_diagnostic_array! { librustc_typeck, DIAGNOSTICS }

src/test/compile-fail/privacy/associated-item-privacy-inherent.rs

@@ -27,7 +27,7 @@ mod priv_nominal {
         Pub.method();
         //~^ ERROR type `for<'r> fn(&'r priv_nominal::Pub) {priv_nominal::Pub::method}` is private
         Pub::CONST;
-        //FIXME ERROR associated constant `CONST` is private
+        //~^ ERROR associated constant `CONST` is private
         // let _: Pub::AssocTy;
         // pub type InSignatureTy = Pub::AssocTy;
     }

src/test/compile-fail/privacy/associated-item-privacy-trait.rs

@@ -31,17 +31,16 @@ mod priv_trait {
         Pub.method();
         //~^ ERROR type `for<'r> fn(&'r Self) {<Self as priv_trait::PrivTr>::method}` is private
         <Pub as PrivTr>::CONST;
-        //FIXME ERROR associated constant `path(PrivTr::CONST)` is private
+        //~^ ERROR associated constant `PrivTr::CONST` is private
         let _: <Pub as PrivTr>::AssocTy;
         //~^ ERROR trait `priv_trait::PrivTr` is private
         //~| ERROR trait `priv_trait::PrivTr` is private
         pub type InSignatureTy = <Pub as PrivTr>::AssocTy;
         //~^ ERROR trait `priv_trait::PrivTr` is private
-        //~| ERROR trait `path(PrivTr)` is private
         pub trait InSignatureTr: PrivTr {}
-        //FIXME ERROR trait `priv_trait::PrivTr` is private
+        //~^ ERROR trait `priv_trait::PrivTr` is private
         impl PrivTr for u8 {}
-        //FIXME ERROR trait `priv_trait::PrivTr` is private
+        //~^ ERROR trait `priv_trait::PrivTr` is private
     }
 }
 fn priv_trait() {
@@ -142,7 +141,7 @@ mod priv_parent_substs {
         pub type InSignatureTy2 = <Priv as PubTr<Pub>>::AssocTy;
         //~^ ERROR type `priv_parent_substs::Priv` is private
         impl PubTr for u8 {}
-        //FIXME ERROR type `priv_parent_substs::Priv` is private
+        //~^ ERROR type `priv_parent_substs::Priv` is private
     }
 }
 fn priv_parent_substs() {