Auto merge of #41733 - nikomatsakis:incr-comp-remove-ast-ty-to-ty-cache, r=eddyb

Remove ast-ty-to-ty cache

As discussed on IRC, this basically just removes the cache, and rewrites rustdoc and save-analysis so call into the astconv code. It *might* make sense for this to be a more fine-grained query, but that would (at least) require us to be using `HirId` and not `NodeId`.

(Perhaps I should open a FIXME?)

I didn't measure perf impact (yet?). I did observe that the cache seems to hit *rarely* -- and only in between items (I experimented with a cache "per def-id", but that had zero hits). In other words, every single hit on the cache is a dependency bug, since it is "shuttling" information between items without dependency edges.

r? @eddyb

Author: bors

src/Cargo.lock

@@ -566,9 +566,6 @@ pub struct GlobalCtxt<'tcx> {
     /// error reporting, and so is lazily initialised and generally
     /// shouldn't taint the common path (hence the RefCell).
     pub all_traits: RefCell<Option<Vec<DefId>>>,
-
-    /// HIR Ty -> Ty lowering cache.
-    pub ast_ty_to_ty_cache: RefCell<NodeMap<Ty<'tcx>>>,
 }
 
 impl<'tcx> GlobalCtxt<'tcx> {
@@ -786,7 +783,6 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
             derive_macros: RefCell::new(NodeMap()),
             stability_interner: RefCell::new(FxHashSet()),
             all_traits: RefCell::new(None),
-            ast_ty_to_ty_cache: RefCell::new(NodeMap()),
        }, f)
     }
 

src/librustc/ty/context.rs

@@ -11,6 +11,7 @@ crate-type = ["dylib"]
 [dependencies]
 log = "0.3"
 rustc = { path = "../librustc" }
+rustc_typeck = { path = "../librustc_typeck" }
 syntax = { path = "../libsyntax" }
 syntax_pos = { path = "../libsyntax_pos" }
 rls-data = "0.1"

src/librustc_save_analysis/Cargo.toml

@@ -122,7 +122,7 @@ impl<'l, 'tcx: 'l, 'll, D: Dump + 'll> DumpVisitor<'l, 'tcx, 'll, D> {
             f(self);
             self.save_ctxt.tables = old_tables;
         } else {
-            f(self)
+            f(self);
         }
     }
 

src/librustc_save_analysis/dump_visitor.rs

@@ -27,6 +27,7 @@
 #[macro_use] extern crate log;
 #[macro_use] extern crate syntax;
 extern crate rustc_serialize;
+extern crate rustc_typeck;
 extern crate syntax_pos;
 
 extern crate rls_data;
@@ -50,6 +51,7 @@ use rustc::hir::def_id::DefId;
 use rustc::session::config::CrateType::CrateTypeExecutable;
 use rustc::session::Session;
 use rustc::ty::{self, TyCtxt};
+use rustc_typeck::hir_ty_to_ty;
 
 use std::env;
 use std::fs::File;
@@ -606,11 +608,12 @@ impl<'l, 'tcx: 'l> SaveContext<'l, 'tcx> {
                 Def::Local(def_id)
             }
 
-            Node::NodeTy(&hir::Ty { node: hir::TyPath(ref qpath), .. }) => {
-                match *qpath {
-                    hir::QPath::Resolved(_, ref path) => path.def,
-                    hir::QPath::TypeRelative(..) => {
-                        if let Some(ty) = self.tcx.ast_ty_to_ty_cache.borrow().get(&id) {
+            Node::NodeTy(ty) => {
+                if let hir::Ty { node: hir::TyPath(ref qpath), .. } = *ty {
+                    match *qpath {
+                        hir::QPath::Resolved(_, ref path) => path.def,
+                        hir::QPath::TypeRelative(..) => {
+                            let ty = hir_ty_to_ty(self.tcx, ty);
                             if let ty::TyProjection(proj) = ty.sty {
                                 for item in self.tcx.associated_items(proj.trait_ref.def_id) {
                                     if item.kind == ty::AssociatedKind::Type {
@@ -620,9 +623,11 @@ impl<'l, 'tcx: 'l> SaveContext<'l, 'tcx> {
                                     }
                                 }
                             }
+                            Def::Err
                         }
-                        Def::Err
                     }
+                } else {
+                    Def::Err
                 }
             }
 

src/librustc_save_analysis/lib.rs

@@ -25,9 +25,8 @@ use rustc::ty::wf::object_region_bounds;
 use rustc_back::slice;
 use require_c_abi_if_variadic;
 use util::common::{ErrorReported, FN_OUTPUT_NAME};
-use util::nodemap::{NodeMap, FxHashSet};
+use util::nodemap::FxHashSet;
 
-use std::cell::RefCell;
 use std::iter;
 use syntax::{abi, ast};
 use syntax::feature_gate::{GateIssue, emit_feature_err};
@@ -37,9 +36,6 @@ use syntax_pos::Span;
 pub trait AstConv<'gcx, 'tcx> {
     fn tcx<'a>(&'a self) -> TyCtxt<'a, 'gcx, 'tcx>;
 
-    /// A cache used for the result of `ast_ty_to_ty_cache`
-    fn ast_ty_to_ty_cache(&self) -> &RefCell<NodeMap<Ty<'tcx>>>;
-
     /// Returns the set of bounds in scope for the type parameter with
     /// the given id.
     fn get_type_parameter_bounds(&self, span: Span, def_id: DefId)
@@ -1074,11 +1070,6 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o {
 
         let tcx = self.tcx();
 
-        let cache = self.ast_ty_to_ty_cache();
-        if let Some(ty) = cache.borrow().get(&ast_ty.id) {
-            return ty;
-        }
-
         let result_ty = match ast_ty.node {
             hir::TySlice(ref ty) => {
                 tcx.mk_slice(self.ast_ty_to_ty(&ty))
@@ -1240,8 +1231,6 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o {
             }
         };
 
-        cache.borrow_mut().insert(ast_ty.id, result_ty);
-
         result_ty
     }
 

src/librustc_typeck/astconv.rs

@@ -965,10 +965,6 @@ impl<'gcx, 'tcx, 'exprs, E> CoerceMany<'gcx, 'tcx, 'exprs, E>
         }
     }
 
-    pub fn is_empty(&self) -> bool {
-        self.pushed == 0
-    }
-
     /// Return the "expected type" with which this coercion was
     /// constructed.  This represents the "downward propagated" type
     /// that was given to us at the start of typing whatever construct

src/librustc_typeck/check/coercion.rs

@@ -451,8 +451,6 @@ impl<'gcx, 'tcx> EnclosingBreakables<'gcx, 'tcx> {
 }
 
 pub struct FnCtxt<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
-    ast_ty_to_ty_cache: RefCell<NodeMap<Ty<'tcx>>>,
-
     body_id: ast::NodeId,
 
     // Number of errors that had been reported when we started
@@ -1516,10 +1514,6 @@ pub fn check_enum<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
 impl<'a, 'gcx, 'tcx> AstConv<'gcx, 'tcx> for FnCtxt<'a, 'gcx, 'tcx> {
     fn tcx<'b>(&'b self) -> TyCtxt<'b, 'gcx, 'tcx> { self.tcx }
 
-    fn ast_ty_to_ty_cache(&self) -> &RefCell<NodeMap<Ty<'tcx>>> {
-        &self.ast_ty_to_ty_cache
-    }
-
     fn get_free_substs(&self) -> Option<&Substs<'tcx>> {
         Some(&self.parameter_environment.free_substs)
     }
@@ -1621,7 +1615,6 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                body_id: ast::NodeId)
                -> FnCtxt<'a, 'gcx, 'tcx> {
         FnCtxt {
-            ast_ty_to_ty_cache: RefCell::new(NodeMap()),
             body_id: body_id,
             err_count_on_creation: inh.tcx.sess.err_count(),
             ret_coercion: None,

src/librustc_typeck/check/mod.rs

@@ -43,7 +43,6 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
         wbcx.visit_liberated_fn_sigs();
         wbcx.visit_fru_field_types();
         wbcx.visit_anon_types();
-        wbcx.visit_type_nodes();
         wbcx.visit_cast_types();
         wbcx.visit_lints();
         wbcx.visit_free_region_map();
@@ -442,13 +441,6 @@ impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> {
         }
     }
 
-    fn visit_type_nodes(&self) {
-        for (&id, ty) in self.fcx.ast_ty_to_ty_cache.borrow().iter() {
-            let ty = self.resolve(ty, &id);
-            self.fcx.tcx.ast_ty_to_ty_cache.borrow_mut().insert(id, ty);
-        }
-    }
-
     fn resolve<T>(&self, x: &T, span: &Locatable) -> T::Lifted
         where T: TypeFoldable<'tcx> + ty::Lift<'gcx>
     {

src/librustc_typeck/check/writeback.rs

@@ -64,11 +64,10 @@ use rustc::ty::{ToPredicate, ReprOptions};
 use rustc::ty::{self, AdtKind, ToPolyTraitRef, Ty, TyCtxt};
 use rustc::ty::maps::Providers;
 use rustc::ty::util::IntTypeExt;
-use util::nodemap::{NodeMap, FxHashMap};
+use util::nodemap::FxHashMap;
 
 use rustc_const_math::ConstInt;
 
-use std::cell::RefCell;
 use std::collections::BTreeMap;
 
 use syntax::{abi, ast};
@@ -116,7 +115,7 @@ pub fn provide(providers: &mut Providers) {
 /// `ItemCtxt` is parameterized by a `DefId` that it uses to satisfy
 /// `get_type_parameter_bounds` requests, drawing the information from
 /// the AST (`hir::Generics`), recursively.
-struct ItemCtxt<'a,'tcx:'a> {
+pub struct ItemCtxt<'a,'tcx:'a> {
     tcx: TyCtxt<'a, 'tcx, 'tcx>,
     item_def_id: DefId,
 }
@@ -180,7 +179,7 @@ impl<'a, 'tcx> Visitor<'tcx> for CollectItemTypesVisitor<'a, 'tcx> {
 // Utility types and common code for the above passes.
 
 impl<'a, 'tcx> ItemCtxt<'a, 'tcx> {
-    fn new(tcx: TyCtxt<'a, 'tcx, 'tcx>, item_def_id: DefId)
+    pub fn new(tcx: TyCtxt<'a, 'tcx, 'tcx>, item_def_id: DefId)
            -> ItemCtxt<'a,'tcx> {
         ItemCtxt {
             tcx: tcx,
@@ -190,18 +189,14 @@ impl<'a, 'tcx> ItemCtxt<'a, 'tcx> {
 }
 
 impl<'a,'tcx> ItemCtxt<'a,'tcx> {
-    fn to_ty(&self, ast_ty: &hir::Ty) -> Ty<'tcx> {
+    pub fn to_ty(&self, ast_ty: &hir::Ty) -> Ty<'tcx> {
         AstConv::ast_ty_to_ty(self, ast_ty)
     }
 }
 
 impl<'a, 'tcx> AstConv<'tcx, 'tcx> for ItemCtxt<'a, 'tcx> {
     fn tcx<'b>(&'b self) -> TyCtxt<'b, 'tcx, 'tcx> { self.tcx }
 
-    fn ast_ty_to_ty_cache(&self) -> &RefCell<NodeMap<Ty<'tcx>>> {
-        &self.tcx.ast_ty_to_ty_cache
-    }
-
     fn get_type_parameter_bounds(&self,
                                  span: Span,
                                  def_id: DefId)

src/librustc_typeck/collect.rs

@@ -122,14 +122,14 @@ use std::iter;
 // registered before they are used.
 pub mod diagnostics;
 
-pub mod check;
-pub mod check_unused;
+mod check;
+mod check_unused;
 mod astconv;
-pub mod collect;
+mod collect;
 mod constrained_type_params;
 mod impl_wf_check;
-pub mod coherence;
-pub mod variance;
+mod coherence;
+mod variance;
 
 pub struct TypeAndSubsts<'tcx> {
     pub substs: &'tcx Substs<'tcx>,
@@ -337,4 +337,16 @@ pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>)
     }
 }
 
+/// A quasi-deprecated helper used in rustdoc and save-analysis to get
+/// the type from a HIR node.
+pub fn hir_ty_to_ty<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, hir_ty: &hir::Ty) -> Ty<'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_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)
+}
+
 __build_diagnostic_array! { librustc_typeck, DIAGNOSTICS }

src/librustc_typeck/lib.rs

@@ -21,6 +21,7 @@ rustc_errors = { path = "../librustc_errors" }
 rustc_lint = { path = "../librustc_lint" }
 rustc_metadata = { path = "../librustc_metadata" }
 rustc_resolve = { path = "../librustc_resolve" }
+rustc_typeck = { path = "../librustc_typeck" }
 rustc_trans = { path = "../librustc_trans" }
 serialize = { path = "../libserialize" }
 syntax = { path = "../libsyntax" }

src/librustdoc/Cargo.toml

@@ -36,6 +36,7 @@ use rustc::ty::subst::Substs;
 use rustc::ty::{self, AdtKind};
 use rustc::middle::stability;
 use rustc::util::nodemap::{FxHashMap, FxHashSet};
+use rustc_typeck::hir_ty_to_ty;
 
 use rustc::hir;
 
@@ -1779,10 +1780,9 @@ impl Clean<Type> for hir::Ty {
             }
             TyPath(hir::QPath::TypeRelative(ref qself, ref segment)) => {
                 let mut def = Def::Err;
-                if let Some(ty) = cx.tcx.ast_ty_to_ty_cache.borrow().get(&self.id) {
-                    if let ty::TyProjection(proj) = ty.sty {
-                        def = Def::Trait(proj.trait_ref.def_id);
-                    }
+                let ty = hir_ty_to_ty(cx.tcx, self);
+                if let ty::TyProjection(proj) = ty.sty {
+                    def = Def::Trait(proj.trait_ref.def_id);
                 }
                 let trait_path = hir::Path {
                     span: self.span,

src/librustdoc/clean/mod.rs

@@ -41,6 +41,7 @@ extern crate rustc_resolve;
 extern crate rustc_lint;
 extern crate rustc_back;
 extern crate rustc_metadata;
+extern crate rustc_typeck;
 extern crate serialize;
 #[macro_use] extern crate syntax;
 extern crate syntax_pos;