compute and cache HIR hashes at beginning

src/librustc/hir/mod.rs

@@ -1621,7 +1621,7 @@ pub type FreevarMap = NodeMap<Vec<Freevar>>;
 
 pub type CaptureModeMap = NodeMap<CaptureClause>;
 
-#[derive(Clone)]
+#[derive(Clone, Debug)]
 pub struct TraitCandidate {
     pub def_id: DefId,
     pub import_id: Option<NodeId>,

src/librustc_driver/driver.rs

@@ -26,7 +26,7 @@ use rustc::util::common::time;
 use rustc::util::nodemap::NodeSet;
 use rustc_back::sha2::{Sha256, Digest};
 use rustc_borrowck as borrowck;
-use rustc_incremental;
+use rustc_incremental::{self, HashesMap};
 use rustc_resolve::{MakeGlobMap, Resolver};
 use rustc_metadata::macro_import;
 use rustc_metadata::creader::read_local_crates;
@@ -172,7 +172,7 @@ pub fn compile_input(sess: &Session,
                                     resolutions,
                                     &arenas,
                                     &crate_name,
-                                    |tcx, mir_map, analysis, result| {
+                                    |tcx, mir_map, analysis, hashes_map, result| {
             {
                 // Eventually, we will want to track plugins.
                 let _ignore = tcx.dep_graph.in_ignore();
@@ -202,7 +202,8 @@ pub fn compile_input(sess: &Session,
             }
             let trans = phase_4_translate_to_llvm(tcx,
                                                   mir_map.unwrap(),
-                                                  analysis);
+                                                  analysis,
+                                                  &hashes_map);
 
             if log_enabled!(::log::INFO) {
                 println!("Post-trans");
@@ -797,14 +798,15 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session,
     where F: for<'a> FnOnce(TyCtxt<'a, 'tcx, 'tcx>,
                             Option<MirMap<'tcx>>,
                             ty::CrateAnalysis,
+                            HashesMap,
                             CompileResult) -> R
 {
     macro_rules! try_with_f {
-        ($e: expr, ($t: expr, $m: expr, $a: expr)) => {
+        ($e: expr, ($t: expr, $m: expr, $a: expr, $h: expr)) => {
             match $e {
                 Ok(x) => x,
                 Err(x) => {
-                    f($t, $m, $a, Err(x));
+                    f($t, $m, $a, $h, Err(x));
                     return Err(x);
                 }
             }
@@ -860,12 +862,16 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session,
                              index,
                              name,
                              |tcx| {
+        let hashes_map =
+            time(time_passes,
+                 "compute_hashes_map",
+                 || rustc_incremental::compute_hashes_map(tcx));
         time(time_passes,
              "load_dep_graph",
-             || rustc_incremental::load_dep_graph(tcx));
+             || rustc_incremental::load_dep_graph(tcx, &hashes_map));
 
         // passes are timed inside typeck
-        try_with_f!(typeck::check_crate(tcx), (tcx, None, analysis));
+        try_with_f!(typeck::check_crate(tcx), (tcx, None, analysis, hashes_map));
 
         time(time_passes,
              "const checking",
@@ -935,7 +941,7 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session,
         // lint warnings and so on -- kindck used to do this abort, but
         // kindck is gone now). -nmatsakis
         if sess.err_count() > 0 {
-            return Ok(f(tcx, Some(mir_map), analysis, Err(sess.err_count())));
+            return Ok(f(tcx, Some(mir_map), analysis, hashes_map, Err(sess.err_count())));
         }
 
         analysis.reachable =
@@ -963,17 +969,18 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session,
 
         // The above three passes generate errors w/o aborting
         if sess.err_count() > 0 {
-            return Ok(f(tcx, Some(mir_map), analysis, Err(sess.err_count())));
+            return Ok(f(tcx, Some(mir_map), analysis, hashes_map, Err(sess.err_count())));
         }
 
-        Ok(f(tcx, Some(mir_map), analysis, Ok(())))
+        Ok(f(tcx, Some(mir_map), analysis, hashes_map, Ok(())))
     })
 }
 
 /// Run the translation phase to LLVM, after which the AST and analysis can
 pub fn phase_4_translate_to_llvm<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
                                            mut mir_map: MirMap<'tcx>,
-                                           analysis: ty::CrateAnalysis)
+                                           analysis: ty::CrateAnalysis,
+                                           hashes_map: &HashesMap)
                                            -> trans::CrateTranslation {
     let time_passes = tcx.sess.time_passes();
 
@@ -1007,15 +1014,15 @@ pub fn phase_4_translate_to_llvm<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
     let translation =
         time(time_passes,
              "translation",
-             move || trans::trans_crate(tcx, &mir_map, analysis));
+             move || trans::trans_crate(tcx, &mir_map, analysis, &hashes_map));
 
     time(time_passes,
          "assert dep graph",
          move || rustc_incremental::assert_dep_graph(tcx));
 
     time(time_passes,
          "serialize dep graph",
-         move || rustc_incremental::save_dep_graph(tcx));
+         move || rustc_incremental::save_dep_graph(tcx, &hashes_map));
 
     translation
 }

src/librustc_driver/pretty.rs

@@ -234,7 +234,7 @@ impl PpSourceMode {
                                                                  resolutions.clone(),
                                                                  arenas,
                                                                  id,
-                                                                 |tcx, _, _, _| {
+                                                                 |tcx, _, _, _, _| {
                     let annotation = TypedAnnotation {
                         tcx: tcx,
                     };
@@ -951,7 +951,7 @@ fn print_with_analysis<'tcx, 'a: 'tcx>(sess: &'a Session,
                                                      resolutions.clone(),
                                                      arenas,
                                                      crate_name,
-                                                     |tcx, mir_map, _, _| {
+                                                     |tcx, mir_map, _, _, _| {
         match ppm {
             PpmMir | PpmMirCFG => {
                 if let Some(mir_map) = mir_map {

src/librustc_incremental/calculate_svh/mod.rs

@@ -8,106 +8,122 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-//! Calculation of a Strict Version Hash for crates.  For a length
-//! comment explaining the general idea, see `librustc/middle/svh.rs`.
-
+//! Calculation of the (misnamed) "strict version hash" for crates and
+//! items. This hash is used to tell when the HIR changed in such a
+//! way that results from previous compilations may no longer be
+//! applicable and hence must be recomputed. It should probably be
+//! renamed to the ICH (incremental compilation hash).
+//!
+//! The hashes for all items are computed once at the beginning of
+//! compilation and stored into a map. In addition, a hash is computed
+//! of the **entire crate**.
+//!
+//! Storing the hashes in a map avoids the need to compute them twice
+//! (once when loading prior incremental results and once when
+//! saving), but it is also important for correctness: at least as of
+//! the time of this writing, the typeck passes rewrites entries in
+//! the dep-map in-place to accommodate UFCS resolutions. Since name
+//! resolution is part of the hash, the result is that hashes computed
+//! at the end of compilation would be different from those computed
+//! at the beginning.
+
+use syntax::ast;
 use syntax::attr::AttributeMethods;
 use std::hash::{Hash, SipHasher, Hasher};
+use rustc::dep_graph::DepNode;
+use rustc::hir;
 use rustc::hir::def_id::{CRATE_DEF_INDEX, DefId};
-use rustc::hir::map::{NodeItem, NodeForeignItem};
-use rustc::hir::svh::Svh;
+use rustc::hir::intravisit as visit;
 use rustc::ty::TyCtxt;
-use rustc::hir::intravisit::{self, Visitor};
+use rustc_data_structures::fnv::FnvHashMap;
 
 use self::svh_visitor::StrictVersionHashVisitor;
 
 mod svh_visitor;
 
-pub trait SvhCalculate {
-    /// Calculate the SVH for an entire krate.
-    fn calculate_krate_hash(self) -> Svh;
+pub type HashesMap = FnvHashMap<DepNode<DefId>, u64>;
 
-    /// Calculate the SVH for a particular item.
-    fn calculate_item_hash(self, def_id: DefId) -> u64;
+pub fn compute_hashes_map<'a, 'tcx: 'a>(tcx: TyCtxt<'a, 'tcx, 'tcx>) -> HashesMap {
+    let _ignore = tcx.dep_graph.in_ignore();
+    let krate = tcx.map.krate();
+    let mut visitor = HashItemsVisitor { tcx: tcx, hashes: FnvHashMap() };
+    visitor.calculate_def_id(DefId::local(CRATE_DEF_INDEX), |v| visit::walk_crate(v, krate));
+    krate.visit_all_items(&mut visitor);
+    visitor.compute_crate_hash();
+    visitor.hashes
 }
 
-impl<'a, 'tcx> SvhCalculate for TyCtxt<'a, 'tcx, 'tcx> {
-    fn calculate_krate_hash(self) -> Svh {
-        // FIXME (#14132): This is better than it used to be, but it still not
-        // ideal. We now attempt to hash only the relevant portions of the
-        // Crate AST as well as the top-level crate attributes. (However,
-        // the hashing of the crate attributes should be double-checked
-        // to ensure it is not incorporating implementation artifacts into
-        // the hash that are not otherwise visible.)
+struct HashItemsVisitor<'a, 'tcx: 'a> {
+    tcx: TyCtxt<'a, 'tcx, 'tcx>,
+    hashes: HashesMap,
+}
 
-        let crate_disambiguator = self.sess.local_crate_disambiguator();
-        let krate = self.map.krate();
+impl<'a, 'tcx> HashItemsVisitor<'a, 'tcx> {
+    fn calculate_node_id<W>(&mut self, id: ast::NodeId, walk_op: W)
+        where W: for<'v> FnMut(&mut StrictVersionHashVisitor<'v, 'tcx>)
+    {
+        let def_id = self.tcx.map.local_def_id(id);
+        self.calculate_def_id(def_id, walk_op)
+    }
 
-        // FIXME: this should use SHA1, not SipHash. SipHash is not built to
-        //        avoid collisions.
+    fn calculate_def_id<W>(&mut self, def_id: DefId, mut walk_op: W)
+        where W: for<'v> FnMut(&mut StrictVersionHashVisitor<'v, 'tcx>)
+    {
+        assert!(def_id.is_local());
+        debug!("HashItemsVisitor::calculate(def_id={:?})", def_id);
+        // FIXME: this should use SHA1, not SipHash. SipHash is not
+        // built to avoid collisions.
         let mut state = SipHasher::new();
-        debug!("state: {:?}", state);
+        walk_op(&mut StrictVersionHashVisitor::new(&mut state, self.tcx));
+        let item_hash = state.finish();
+        self.hashes.insert(DepNode::Hir(def_id), item_hash);
+        debug!("calculate_item_hash: def_id={:?} hash={:?}", def_id, item_hash);
+    }
+
+    fn compute_crate_hash(&mut self) {
+        let krate = self.tcx.map.krate();
 
-        // FIXME(#32753) -- at (*) we `to_le` for endianness, but is
-        // this enough, and does it matter anyway?
-        "crate_disambiguator".hash(&mut state);
-        crate_disambiguator.len().to_le().hash(&mut state); // (*)
-        crate_disambiguator.hash(&mut state);
+        let mut crate_state = SipHasher::new();
 
-        debug!("crate_disambiguator: {:?}", crate_disambiguator);
-        debug!("state: {:?}", state);
+        let crate_disambiguator = self.tcx.sess.local_crate_disambiguator();
+        "crate_disambiguator".hash(&mut crate_state);
+        crate_disambiguator.len().hash(&mut crate_state);
+        crate_disambiguator.hash(&mut crate_state);
 
+        // add each item (in some deterministic order) to the overall
+        // crate hash.
+        //
+        // FIXME -- it'd be better to sort by the hash of the def-path,
+        // so that reordering items would not affect the crate hash.
         {
-            let mut visit = StrictVersionHashVisitor::new(&mut state, self);
-            krate.visit_all_items(&mut visit);
+            let mut keys: Vec<_> = self.hashes.keys().collect();
+            keys.sort();
+            for key in keys {
+                self.hashes[key].hash(&mut crate_state);
+            }
         }
 
-        // FIXME (#14132): This hash is still sensitive to e.g. the
-        // spans of the crate Attributes and their underlying
-        // MetaItems; we should make ContentHashable impl for those
-        // types and then use hash_content.  But, since all crate
-        // attributes should appear near beginning of the file, it is
-        // not such a big deal to be sensitive to their spans for now.
-        //
-        // We hash only the MetaItems instead of the entire Attribute
-        // to avoid hashing the AttrId
         for attr in &krate.attrs {
             debug!("krate attr {:?}", attr);
-            attr.meta().hash(&mut state);
+            attr.meta().hash(&mut crate_state);
         }
 
-        Svh::new(state.finish())
+        let crate_hash = crate_state.finish();
+        self.hashes.insert(DepNode::Krate, crate_hash);
+        debug!("calculate_crate_hash: crate_hash={:?}", crate_hash);
     }
+}
 
-    fn calculate_item_hash(self, def_id: DefId) -> u64 {
-        assert!(def_id.is_local());
-
-        debug!("calculate_item_hash(def_id={:?})", def_id);
-
-        let mut state = SipHasher::new();
-
-        {
-            let mut visit = StrictVersionHashVisitor::new(&mut state, self);
-            if def_id.index == CRATE_DEF_INDEX {
-                // the crate root itself is not registered in the map
-                // as an item, so we have to fetch it this way
-                let krate = self.map.krate();
-                intravisit::walk_crate(&mut visit, krate);
-            } else {
-                let node_id = self.map.as_local_node_id(def_id).unwrap();
-                match self.map.find(node_id) {
-                    Some(NodeItem(item)) => visit.visit_item(item),
-                    Some(NodeForeignItem(item)) => visit.visit_foreign_item(item),
-                    r => bug!("calculate_item_hash: expected an item for node {} not {:?}",
-                              node_id, r),
-                }
-            }
-        }
-
-        let hash = state.finish();
 
-        debug!("calculate_item_hash: def_id={:?} hash={:?}", def_id, hash);
+impl<'a, 'tcx> visit::Visitor<'tcx> for HashItemsVisitor<'a, 'tcx> {
+    fn visit_item(&mut self, item: &'tcx hir::Item) {
+        self.calculate_node_id(item.id, |v| v.visit_item(item));
+        visit::walk_item(self, item);
+    }
 
-        hash
+    fn visit_foreign_item(&mut self, item: &'tcx hir::ForeignItem) {
+        self.calculate_node_id(item.id, |v| v.visit_foreign_item(item));
+        visit::walk_foreign_item(self, item);
     }
 }
+