Auto merge of #44633 - petrochenkov:priv2, r=nikomatsakis

Record semantic types for all syntactic types in bodies

... and use recorded types in type privacy checking (types are recorded after inference, so there are no `_`s left).
Also use `hir_ty_to_ty` for types in signatures in type privacy checking.

This could also be potentially useful for save-analysis and diagnostics.

Fixes https://github.com/rust-lang/rust/pull/42125#issuecomment-305987755
r? @eddyb

src/Cargo.lock

@@ -1715,6 +1715,7 @@ name = "rustc_privacy"
 version = "0.0.0"
 dependencies = [
  "rustc 0.0.0",
+ "rustc_typeck 0.0.0",
  "syntax 0.0.0",
  "syntax_pos 0.0.0",
 ]

src/librustc/hir/lowering.rs

@@ -683,7 +683,7 @@ impl<'a> LoweringContext<'a> {
                 return self.lower_ty(ty);
             }
             TyKind::Path(ref qself, ref path) => {
-                let id = self.lower_node_id(t.id).node_id;
+                let id = self.lower_node_id(t.id);
                 let qpath = self.lower_qpath(t.id, qself, path, ParamMode::Explicit);
                 return self.ty_path(id, t.span, qpath);
             }
@@ -732,10 +732,12 @@ impl<'a> LoweringContext<'a> {
             TyKind::Mac(_) => panic!("TyMac should have been expanded by now."),
         };
 
+        let LoweredNodeId { node_id, hir_id } = self.lower_node_id(t.id);
         P(hir::Ty {
-            id: self.lower_node_id(t.id).node_id,
+            id: node_id,
             node: kind,
             span: t.span,
+            hir_id,
         })
     }
 
@@ -861,7 +863,7 @@ impl<'a> LoweringContext<'a> {
             // Otherwise, the base path is an implicit `Self` type path,
             // e.g. `Vec` in `Vec::new` or `<I as Iterator>::Item` in
             // `<I as Iterator>::Item::default`.
-            let new_id = self.next_id().node_id;
+            let new_id = self.next_id();
             self.ty_path(new_id, p.span, hir::QPath::Resolved(qself, path))
         };
 
@@ -886,7 +888,7 @@ impl<'a> LoweringContext<'a> {
             }
 
             // Wrap the associated extension in another type node.
-            let new_id = self.next_id().node_id;
+            let new_id = self.next_id();
             ty = self.ty_path(new_id, p.span, qpath);
         }
 
@@ -994,7 +996,8 @@ impl<'a> LoweringContext<'a> {
         let &ParenthesizedParameterData { ref inputs, ref output, span } = data;
         let inputs = inputs.iter().map(|ty| self.lower_ty(ty)).collect();
         let mk_tup = |this: &mut Self, tys, span| {
-            P(hir::Ty { node: hir::TyTup(tys), id: this.next_id().node_id, span })
+            let LoweredNodeId { node_id, hir_id } = this.next_id();
+            P(hir::Ty { node: hir::TyTup(tys), id: node_id, hir_id, span })
         };
 
         hir::PathParameters {
@@ -2974,7 +2977,7 @@ impl<'a> LoweringContext<'a> {
         self.expr_block(block, attrs)
     }
 
-    fn ty_path(&mut self, id: NodeId, span: Span, qpath: hir::QPath) -> P<hir::Ty> {
+    fn ty_path(&mut self, id: LoweredNodeId, span: Span, qpath: hir::QPath) -> P<hir::Ty> {
         let mut id = id;
         let node = match qpath {
             hir::QPath::Resolved(None, path) => {
@@ -2984,14 +2987,14 @@ impl<'a> LoweringContext<'a> {
                         bound_lifetimes: hir_vec![],
                         trait_ref: hir::TraitRef {
                             path: path.and_then(|path| path),
-                            ref_id: id,
+                            ref_id: id.node_id,
                         },
                         span,
                     };
 
                     // The original ID is taken by the `PolyTraitRef`,
                     // so the `Ty` itself needs a different one.
-                    id = self.next_id().node_id;
+                    id = self.next_id();
 
                     hir::TyTraitObject(hir_vec![principal], self.elided_lifetime(span))
                 } else {
@@ -3000,7 +3003,7 @@ impl<'a> LoweringContext<'a> {
             }
             _ => hir::TyPath(qpath)
         };
-        P(hir::Ty { id, node, span })
+        P(hir::Ty { id: id.node_id, hir_id: id.hir_id, node, span })
     }
 
     fn elided_lifetime(&mut self, span: Span) -> hir::Lifetime {

src/librustc/hir/mod.rs

@@ -1354,6 +1354,7 @@ pub struct Ty {
     pub id: NodeId,
     pub node: Ty_,
     pub span: Span,
+    pub hir_id: HirId,
 }
 
 impl fmt::Debug for Ty {

src/librustc/ich/impls_hir.rs

@@ -245,6 +245,7 @@ impl<'gcx> HashStable<StableHashingContext<'gcx>> for hir::Ty {
         hcx.while_hashing_hir_bodies(true, |hcx| {
             let hir::Ty {
                 id: _,
+                hir_id: _,
                 ref node,
                 ref span,
             } = *self;

src/librustc_privacy/Cargo.toml

@@ -10,5 +10,6 @@ crate-type = ["dylib"]
 
 [dependencies]
 rustc = { path = "../librustc" }
+rustc_typeck = { path = "../librustc_typeck" }
 syntax = { path = "../libsyntax" }
 syntax_pos = { path = "../libsyntax_pos" }

src/librustc_privacy/lib.rs

@@ -17,6 +17,7 @@
 
 #[macro_use] extern crate rustc;
 #[macro_use] extern crate syntax;
+extern crate rustc_typeck;
 extern crate syntax_pos;
 
 use rustc::hir::{self, PatKind};
@@ -658,65 +659,6 @@ impl<'a, 'tcx> TypePrivacyVisitor<'a, 'tcx> {
         }
         false
     }
-
-    fn check_item(&mut self, item_id: ast::NodeId) -> &mut Self {
-        self.current_item = self.tcx.hir.local_def_id(item_id);
-        self.span = self.tcx.hir.span(item_id);
-        self
-    }
-
-    // Convenience methods for checking item interfaces
-    fn ty(&mut self) -> &mut Self {
-        self.tcx.type_of(self.current_item).visit_with(self);
-        self
-    }
-
-    fn generics(&mut self) -> &mut Self {
-        for def in &self.tcx.generics_of(self.current_item).types {
-            if def.has_default {
-                self.tcx.type_of(def.def_id).visit_with(self);
-            }
-        }
-        self
-    }
-
-    fn predicates(&mut self) -> &mut Self {
-        let predicates = self.tcx.predicates_of(self.current_item);
-        for predicate in &predicates.predicates {
-            predicate.visit_with(self);
-            match predicate {
-                &ty::Predicate::Trait(poly_predicate) => {
-                    self.check_trait_ref(poly_predicate.skip_binder().trait_ref);
-                },
-                &ty::Predicate::Projection(poly_predicate) => {
-                    let tcx = self.tcx;
-                    self.check_trait_ref(
-                        poly_predicate.skip_binder().projection_ty.trait_ref(tcx)
-                    );
-                },
-                _ => (),
-            };
-        }
-        self
-    }
-
-    fn impl_trait_ref(&mut self) -> &mut Self {
-        if let Some(impl_trait_ref) = self.tcx.impl_trait_ref(self.current_item) {
-            self.check_trait_ref(impl_trait_ref);
-        }
-        self.tcx.predicates_of(self.current_item).visit_with(self);
-        self
-    }
-
-    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> {
@@ -733,6 +675,35 @@ impl<'a, 'tcx> Visitor<'tcx> for TypePrivacyVisitor<'a, 'tcx> {
         self.tables = orig_tables;
     }
 
+    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) {
+            // Types in bodies.
+            if ty.visit_with(self) {
+                return;
+            }
+        } else {
+            // Types in signatures.
+            // FIXME: This is very ineffective. Ideally each HIR type should be converted
+            // into a semantic type only once and the result should be cached somehow.
+            if rustc_typeck::hir_ty_to_ty(self.tcx, hir_ty).visit_with(self) {
+                return;
+            }
+        }
+
+        intravisit::walk_ty(self, hir_ty);
+    }
+
+    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;
+        }
+
+        intravisit::walk_trait_ref(self, trait_ref);
+    }
+
     // Check types of expressions
     fn visit_expr(&mut self, expr: &'tcx hir::Expr) {
         if self.check_expr_pat_type(expr.hir_id, expr.span) {
@@ -807,63 +778,6 @@ impl<'a, 'tcx> Visitor<'tcx> for TypePrivacyVisitor<'a, 'tcx> {
                                         item.id,
                                         &mut self.tables,
                                         self.empty_tables);
-
-        match item.node {
-            hir::ItemExternCrate(..) | hir::ItemMod(..) |
-            hir::ItemUse(..) | hir::ItemGlobalAsm(..) => {}
-            hir::ItemConst(..) | hir::ItemStatic(..) |
-            hir::ItemTy(..) | hir::ItemFn(..) => {
-                self.check_item(item.id).generics().predicates().ty();
-            }
-            hir::ItemTrait(.., ref trait_item_refs) => {
-                self.check_item(item.id).generics().predicates();
-                for trait_item_ref in trait_item_refs {
-                    let check = self.check_item(trait_item_ref.id.node_id);
-                    check.generics().predicates();
-                    if trait_item_ref.kind != hir::AssociatedItemKind::Type ||
-                       trait_item_ref.defaultness.has_value() {
-                        check.ty();
-                    }
-                }
-            }
-            hir::ItemEnum(ref def, _) => {
-                self.check_item(item.id).generics().predicates();
-                for variant in &def.variants {
-                    for field in variant.node.data.fields() {
-                        self.check_item(field.id).ty();
-                    }
-                }
-            }
-            hir::ItemForeignMod(ref foreign_mod) => {
-                for foreign_item in &foreign_mod.items {
-                    self.check_item(foreign_item.id).generics().predicates().ty();
-                }
-            }
-            hir::ItemStruct(ref struct_def, _) |
-            hir::ItemUnion(ref struct_def, _) => {
-                self.check_item(item.id).generics().predicates();
-                for field in struct_def.fields() {
-                    self.check_item(field.id).ty();
-                }
-            }
-            hir::ItemDefaultImpl(..) => {
-                self.check_item(item.id).impl_trait_ref();
-            }
-            hir::ItemImpl(.., ref trait_ref, _, ref impl_item_refs) => {
-                {
-                    let check = self.check_item(item.id);
-                    check.ty().generics().predicates();
-                    if trait_ref.is_some() {
-                        check.impl_trait_ref();
-                    }
-                }
-                for impl_item_ref in impl_item_refs {
-                    let impl_item = self.tcx.hir.impl_item(impl_item_ref.id);
-                    self.check_item(impl_item.id).generics().predicates().ty();
-                }
-            }
-        }
-
         self.current_item = self.tcx.hir.local_def_id(item.id);
         intravisit::walk_item(self, item);
         self.tables = orig_tables;
@@ -924,8 +838,13 @@ impl<'a, 'tcx> TypeVisitor<'tcx> for TypePrivacyVisitor<'a, 'tcx> {
                 }
             }
             ty::TyProjection(ref proj) => {
-                let tcx = self.tcx;
-                if self.check_trait_ref(proj.trait_ref(tcx)) {
+                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) {
                     return true;
                 }
             }

src/librustc_typeck/astconv.rs

@@ -76,6 +76,8 @@ pub trait AstConv<'gcx, 'tcx> {
     /// used to help suppress derived errors typeck might otherwise
     /// report.
     fn set_tainted_by_errors(&self);
+
+    fn record_ty(&self, hir_id: hir::HirId, ty: Ty<'tcx>, span: Span);
 }
 
 struct ConvertedBinding<'tcx> {
@@ -975,6 +977,14 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o {
                 }
             }
             Def::Err => {
+                for segment in &path.segments {
+                    for ty in &segment.parameters.types {
+                        self.ast_ty_to_ty(ty);
+                    }
+                    for binding in &segment.parameters.bindings {
+                        self.ast_ty_to_ty(&binding.ty);
+                    }
+                }
                 self.set_tainted_by_errors();
                 return self.tcx().types.err;
             }
@@ -1115,6 +1125,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o {
             }
         };
 
+        self.record_ty(ast_ty.hir_id, result_ty, ast_ty.span);
         result_ty
     }
 
@@ -1124,8 +1135,10 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o {
                      -> Ty<'tcx>
     {
         match ty.node {
-            hir::TyInfer if expected_ty.is_some() => expected_ty.unwrap(),
-            hir::TyInfer => self.ty_infer(ty.span),
+            hir::TyInfer if expected_ty.is_some() => {
+                self.record_ty(ty.hir_id, expected_ty.unwrap(), ty.span);
+                expected_ty.unwrap()
+            }
             _ => self.ast_ty_to_ty(ty),
         }
     }
@@ -1214,19 +1227,22 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o {
 
         let expected_ret_ty = expected_sig.as_ref().map(|e| e.output());
 
-        let is_infer = match decl.output {
-            hir::Return(ref output) if output.node == hir::TyInfer => true,
-            hir::DefaultReturn(..) => true,
-            _ => false
-        };
-
         let output_ty = match decl.output {
-            _ if is_infer && expected_ret_ty.is_some() =>
-                expected_ret_ty.unwrap(),
-            _ if is_infer => self.ty_infer(decl.output.span()),
-            hir::Return(ref output) =>
-                self.ast_ty_to_ty(&output),
-            hir::DefaultReturn(..) => bug!(),
+            hir::Return(ref output) => {
+                if let (&hir::TyInfer, Some(expected_ret_ty)) = (&output.node, expected_ret_ty) {
+                    self.record_ty(output.hir_id, expected_ret_ty, output.span);
+                    expected_ret_ty
+                } else {
+                    self.ast_ty_to_ty(&output)
+                }
+            }
+            hir::DefaultReturn(span) => {
+                if let Some(expected_ret_ty) = expected_ret_ty {
+                    expected_ret_ty
+                } else {
+                    self.ty_infer(span)
+                }
+            }
         };
 
         debug!("ty_of_closure: output_ty={:?}", output_ty);

src/librustc_typeck/check/mod.rs

@@ -1665,6 +1665,10 @@ impl<'a, 'gcx, 'tcx> AstConv<'gcx, 'tcx> for FnCtxt<'a, 'gcx, 'tcx> {
     fn set_tainted_by_errors(&self) {
         self.infcx.set_tainted_by_errors()
     }
+
+    fn record_ty(&self, hir_id: hir::HirId, ty: Ty<'tcx>, _span: Span) {
+        self.write_ty(hir_id, ty)
+    }
 }
 
 /// Controls whether the arguments are tupled. This is used for the call

src/librustc_typeck/check/writeback.rs

@@ -207,6 +207,13 @@ impl<'cx, 'gcx, 'tcx> Visitor<'gcx> for WritebackCx<'cx, 'gcx, 'tcx> {
         let var_ty = self.resolve(&var_ty, &l.span);
         self.write_ty_to_tables(l.hir_id, var_ty);
     }
+
+    fn visit_ty(&mut self, hir_ty: &'gcx hir::Ty) {
+        intravisit::walk_ty(self, hir_ty);
+        let ty = self.fcx.node_ty(hir_ty.hir_id);
+        let ty = self.resolve(&ty, &hir_ty.span);
+        self.write_ty_to_tables(hir_ty.hir_id, ty);
+    }
 }
 
 impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> {

src/librustc_typeck/collect.rs

@@ -221,6 +221,10 @@ impl<'a, 'tcx> AstConv<'tcx, 'tcx> for ItemCtxt<'a, 'tcx> {
     fn set_tainted_by_errors(&self) {
         // no obvious place to track this, just let it go
     }
+
+    fn record_ty(&self, _hir_id: hir::HirId, _ty: Ty<'tcx>, _span: Span) {
+        // no place to record types from signatures?
+    }
 }
 
 fn type_param_predicates<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,

src/test/compile-fail/lint-stability-deprecated.rs

@@ -107,6 +107,7 @@ mod cross_crate {
         struct S1<T: TraitWithAssociatedTypes>(T::TypeUnstable);
         struct S2<T: TraitWithAssociatedTypes>(T::TypeDeprecated);
         //~^ WARN use of deprecated item
+        //~| WARN use of deprecated item
 
         let _ = DeprecatedStruct { //~ WARN use of deprecated item
             i: 0 //~ WARN use of deprecated item

src/test/compile-fail/private-inferred-type.rs

@@ -103,10 +103,11 @@ mod adjust {
 
 fn main() {
     let _: m::Alias; //~ ERROR type `m::Priv` is private
-    let _: <m::Alias as m::TraitWithAssocTy>::AssocTy; // FIXME
+                     //~^ ERROR type `m::Priv` is private
+    let _: <m::Alias as m::TraitWithAssocTy>::AssocTy; //~ ERROR type `m::Priv` is private
     m::Alias {}; //~ ERROR type `m::Priv` is private
     m::Pub { 0: m::Alias {} }; //~ ERROR type `m::Priv` is private
-    m::Pub { 0: loop {} }; // FIXME
+    m::Pub { 0: loop {} }; // OK, `m::Pub` is in value context, so it means Pub<_>, not Pub<Priv>
     m::Pub::static_method; //~ ERROR type `m::Priv` is private
     m::Pub::INHERENT_ASSOC_CONST; //~ ERROR type `m::Priv` is private
     m::Pub(0u8).method_with_substs::<m::Alias>(); //~ ERROR type `m::Priv` is private

src/test/compile-fail/private-type-in-interface.rs

@@ -31,7 +31,6 @@ fn f_ext(_: ext::Alias) {} //~ ERROR type `ext::Priv` is private
 trait Tr1 {}
 impl m::Alias {} //~ ERROR type `m::Priv` is private
 impl Tr1 for ext::Alias {} //~ ERROR type `ext::Priv` is private
-                           //~^ ERROR type `ext::Priv` is private
 type A = <m::Alias as m::Trait>::X; //~ ERROR type `m::Priv` is private
 
 trait Tr2<T> {}

src/test/compile-fail/type-path-err-node-types.rs

@@ -0,0 +1,17 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// Type arguments of unresolved types should have their types recorded
+
+fn main() {
+    let _: Nonexistent<u8, Assoc = u16>; //~ ERROR cannot find type `Nonexistent` in this scope
+
+    let _ = |a, b: _| -> _ { 0 };
+}