make svh independent of item ordering

Author: nikomatsakis

src/librustc_incremental/calculate_svh/def_path_hash.rs

@@ -0,0 +1,36 @@
+// Copyright 2012-2014 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.
+
+use rustc::hir::def_id::DefId;
+use rustc::ty::TyCtxt;
+use rustc::util::nodemap::DefIdMap;
+
+pub struct DefPathHashes<'a, 'tcx: 'a> {
+    tcx: TyCtxt<'a, 'tcx, 'tcx>,
+    data: DefIdMap<u64>,
+}
+
+impl<'a, 'tcx> DefPathHashes<'a, 'tcx> {
+    pub fn new(tcx: TyCtxt<'a, 'tcx, 'tcx>) -> Self {
+        DefPathHashes {
+            tcx: tcx,
+            data: DefIdMap()
+        }
+    }
+
+    pub fn hash(&mut self, def_id: DefId) -> u64 {
+        let tcx = self.tcx;
+        *self.data.entry(def_id)
+                  .or_insert_with(|| {
+                      let def_path = tcx.def_path(def_id);
+                      def_path.deterministic_hash(tcx)
+                  })
+    }
+}

src/librustc_incremental/calculate_svh/mod.rs

@@ -35,11 +35,12 @@ use rustc::hir;
 use rustc::hir::def_id::{CRATE_DEF_INDEX, DefId};
 use rustc::hir::intravisit as visit;
 use rustc::ty::TyCtxt;
-use rustc::util::nodemap::DefIdMap;
 use rustc_data_structures::fnv::FnvHashMap;
 
+use self::def_path_hash::DefPathHashes;
 use self::svh_visitor::StrictVersionHashVisitor;
 
+mod def_path_hash;
 mod svh_visitor;
 
 pub type IncrementalHashesMap = FnvHashMap<DepNode<DefId>, u64>;
@@ -50,7 +51,7 @@ pub fn compute_incremental_hashes_map<'a, 'tcx: 'a>(tcx: TyCtxt<'a, 'tcx, 'tcx>)
     let krate = tcx.map.krate();
     let mut visitor = HashItemsVisitor { tcx: tcx,
                                          hashes: FnvHashMap(),
-                                         def_path_hashes: DefIdMap() };
+                                         def_path_hashes: DefPathHashes::new(tcx) };
     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();
@@ -59,20 +60,20 @@ pub fn compute_incremental_hashes_map<'a, 'tcx: 'a>(tcx: TyCtxt<'a, 'tcx, 'tcx>)
 
 struct HashItemsVisitor<'a, 'tcx: 'a> {
     tcx: TyCtxt<'a, 'tcx, 'tcx>,
-    def_path_hashes: DefIdMap<u64>,
+    def_path_hashes: DefPathHashes<'a, 'tcx>,
     hashes: IncrementalHashesMap,
 }
 
 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>)
+        where W: for<'v> FnMut(&mut StrictVersionHashVisitor<'v, 'a, 'tcx>)
     {
         let def_id = self.tcx.map.local_def_id(id);
         self.calculate_def_id(def_id, walk_op)
     }
 
     fn calculate_def_id<W>(&mut self, def_id: DefId, mut walk_op: W)
-        where W: for<'v> FnMut(&mut StrictVersionHashVisitor<'v, 'tcx>)
+        where W: for<'v> FnMut(&mut StrictVersionHashVisitor<'v, 'a, 'tcx>)
     {
         assert!(def_id.is_local());
         debug!("HashItemsVisitor::calculate(def_id={:?})", def_id);
@@ -99,15 +100,22 @@ impl<'a, 'tcx> HashItemsVisitor<'a, 'tcx> {
 
         // 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 keys: Vec<_> = self.hashes.keys().collect();
-            keys.sort();
-            for key in keys {
-                self.hashes[key].hash(&mut crate_state);
-            }
+            let def_path_hashes = &mut self.def_path_hashes;
+            let mut item_hashes: Vec<_> =
+                self.hashes.iter()
+                           .map(|(item_dep_node, &item_hash)| {
+                               // convert from a DepNode<DefId> tp a
+                               // DepNode<u64> where the u64 is the
+                               // hash of the def-id's def-path:
+                               let item_dep_node =
+                                   item_dep_node.map_def(|&did| Some(def_path_hashes.hash(did)))
+                                                .unwrap();
+                               (item_dep_node, item_hash)
+                           })
+                           .collect();
+            item_hashes.sort(); // avoid artificial dependencies on item ordering
+            item_hashes.hash(&mut crate_state);
         }
 
         for attr in &krate.attrs {

src/librustc_incremental/calculate_svh/svh_visitor.rs

@@ -26,33 +26,29 @@ use rustc::hir::def_id::DefId;
 use rustc::hir::intravisit as visit;
 use rustc::hir::intravisit::{Visitor, FnKind};
 use rustc::ty::TyCtxt;
-use rustc::util::nodemap::DefIdMap;
 
 use std::hash::{Hash, SipHasher};
 
-pub struct StrictVersionHashVisitor<'a, 'tcx: 'a> {
-    pub tcx: TyCtxt<'a, 'tcx, 'tcx>,
+use super::def_path_hash::DefPathHashes;
+
+pub struct StrictVersionHashVisitor<'a, 'hash: 'a, 'tcx: 'hash> {
+    pub tcx: TyCtxt<'hash, 'tcx, 'tcx>,
     pub st: &'a mut SipHasher,
 
     // collect a deterministic hash of def-ids that we have seen
-    def_path_hashes: &'a mut DefIdMap<u64>,
+    def_path_hashes: &'a mut DefPathHashes<'hash, 'tcx>,
 }
 
-impl<'a, 'tcx> StrictVersionHashVisitor<'a, 'tcx> {
+impl<'a, 'hash, 'tcx> StrictVersionHashVisitor<'a, 'hash, 'tcx> {
     pub fn new(st: &'a mut SipHasher,
-               tcx: TyCtxt<'a, 'tcx, 'tcx>,
-               def_path_hashes: &'a mut DefIdMap<u64>)
+               tcx: TyCtxt<'hash, 'tcx, 'tcx>,
+               def_path_hashes: &'a mut DefPathHashes<'hash, 'tcx>)
                -> Self {
         StrictVersionHashVisitor { st: st, tcx: tcx, def_path_hashes: def_path_hashes }
     }
 
     fn compute_def_id_hash(&mut self, def_id: DefId) -> u64 {
-        let tcx = self.tcx;
-        *self.def_path_hashes.entry(def_id)
-                             .or_insert_with(|| {
-                                 let def_path = tcx.def_path(def_id);
-                                 def_path.deterministic_hash(tcx)
-                             })
+        self.def_path_hashes.hash(def_id)
     }
 }
 
@@ -196,7 +192,7 @@ pub enum SawStmtComponent {
     SawStmtSemi,
 }
 
-impl<'a, 'tcx> Visitor<'tcx> for StrictVersionHashVisitor<'a, 'tcx> {
+impl<'a, 'hash, 'tcx> Visitor<'tcx> for StrictVersionHashVisitor<'a, 'hash, 'tcx> {
     fn visit_nested_item(&mut self, _: ItemId) {
         // Each item is hashed independently; ignore nested items.
     }
@@ -370,7 +366,7 @@ pub enum DefHash {
     SawErr,
 }
 
-impl<'a, 'tcx> StrictVersionHashVisitor<'a, 'tcx> {
+impl<'a, 'hash, 'tcx> StrictVersionHashVisitor<'a, 'hash, 'tcx> {
     fn hash_resolve(&mut self, id: ast::NodeId) {
         // Because whether or not a given id has an entry is dependent
         // solely on expr variant etc, we don't need to hash whether