Auto merge of #42175 - michaelwoerister:filemap-hashing-fix-1, r=nikomatsakis

incr.comp.: Track expanded spans instead of FileMaps.

This PR removes explicit tracking of FileMaps in response to #42101. The reasoning behind being able to just *not* track access to FileMaps is similar to why we don't track access to the `DefId->DefPath` map:
1. One can only get ahold of a `Span` value by accessing the HIR (for local things) or a `metadata::schema::Entry` (for things from external crates).
2. For both of these things we compute a hash that incorporates the *expanded spans*, that is, what we hash is in the (FileMap independent) format `filename:line:col`.
3. Consequently, everything that emits a span should already be tracked via its dependency to something that has the span included in its hash and changes would be detected via that hash.

One caveat here is that we have to be conservative when exporting things in metadata. A crate can be built without debuginfo and would thus by default not incorporate most spans into the metadata hashes. However, a downstream crate can make an inline copy of things in the upstream crate and span changes in the upstream crate would then go undetected, even if the downstream uses them (e.g. by emitting debuginfo for an inlined function). For this reason, we always incorporate spans into metadata hashes for now (there might be more efficient ways to handle this safely when red-green tracking is implemented).

r? @nikomatsakis

Author: bors

src/librustc/dep_graph/dep_node.rs

@@ -176,7 +176,6 @@ pub enum DepNode<D: Clone + Debug> {
     IsMirAvailable(D),
     ItemAttrs(D),
     FnArgNames(D),
-    FileMap(D, Arc<String>),
 }
 
 impl<D: Clone + Debug> DepNode<D> {
@@ -307,7 +306,6 @@ impl<D: Clone + Debug> DepNode<D> {
             ConstIsRvaluePromotableToStatic(ref d) => op(d).map(ConstIsRvaluePromotableToStatic),
             IsMirAvailable(ref d) => op(d).map(IsMirAvailable),
             GlobalMetaData(ref d, kind) => op(d).map(|d| GlobalMetaData(d, kind)),
-            FileMap(ref d, ref file_name) => op(d).map(|d| FileMap(d, file_name.clone())),
         }
     }
 }

src/librustc/ich/caching_codemap_view.rs

@@ -8,11 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use dep_graph::{DepGraph, DepNode};
-use hir::def_id::{DefId, CrateNum, CRATE_DEF_INDEX};
-use rustc_data_structures::bitvec::BitVector;
 use std::rc::Rc;
-use std::sync::Arc;
 use syntax::codemap::CodeMap;
 use syntax_pos::{BytePos, FileMap};
 use ty::TyCtxt;
@@ -31,14 +27,12 @@ pub struct CachingCodemapView<'tcx> {
     codemap: &'tcx CodeMap,
     line_cache: [CacheEntry; 3],
     time_stamp: usize,
-    dep_graph: DepGraph,
-    dep_tracking_reads: BitVector,
 }
 
 impl<'tcx> CachingCodemapView<'tcx> {
     pub fn new<'a>(tcx: TyCtxt<'a, 'tcx, 'tcx>) -> CachingCodemapView<'tcx> {
         let codemap = tcx.sess.codemap();
-        let files = codemap.files_untracked();
+        let files = codemap.files();
         let first_file = files[0].clone();
         let entry = CacheEntry {
             time_stamp: 0,
@@ -50,11 +44,9 @@ impl<'tcx> CachingCodemapView<'tcx> {
         };
 
         CachingCodemapView {
-            dep_graph: tcx.dep_graph.clone(),
             codemap: codemap,
             line_cache: [entry.clone(), entry.clone(), entry.clone()],
             time_stamp: 0,
-            dep_tracking_reads: BitVector::new(files.len()),
         }
     }
 
@@ -67,9 +59,6 @@ impl<'tcx> CachingCodemapView<'tcx> {
         for cache_entry in self.line_cache.iter_mut() {
             if pos >= cache_entry.line_start && pos < cache_entry.line_end {
                 cache_entry.time_stamp = self.time_stamp;
-                if self.dep_tracking_reads.insert(cache_entry.file_index) {
-                    self.dep_graph.read(dep_node(cache_entry));
-                }
 
                 return Some((cache_entry.file.clone(),
                              cache_entry.line_number,
@@ -90,7 +79,7 @@ impl<'tcx> CachingCodemapView<'tcx> {
         // If the entry doesn't point to the correct file, fix it up
         if pos < cache_entry.file.start_pos || pos >= cache_entry.file.end_pos {
             let file_valid;
-            let files = self.codemap.files_untracked();
+            let files = self.codemap.files();
 
             if files.len() > 0 {
                 let file_index = self.codemap.lookup_filemap_idx(pos);
@@ -120,21 +109,8 @@ impl<'tcx> CachingCodemapView<'tcx> {
         cache_entry.line_end = line_bounds.1;
         cache_entry.time_stamp = self.time_stamp;
 
-        if self.dep_tracking_reads.insert(cache_entry.file_index) {
-            self.dep_graph.read(dep_node(cache_entry));
-        }
-
         return Some((cache_entry.file.clone(),
                      cache_entry.line_number,
                      pos - cache_entry.line_start));
     }
 }
-
-fn dep_node(cache_entry: &CacheEntry) -> DepNode<DefId> {
-    let def_id = DefId {
-        krate: CrateNum::from_u32(cache_entry.file.crate_of_origin),
-        index: CRATE_DEF_INDEX,
-    };
-    let name = Arc::new(cache_entry.file.name.clone());
-    DepNode::FileMap(def_id, name)
-}

src/librustc/ich/hcx.rs

@@ -74,6 +74,11 @@ impl<'a, 'tcx: 'a> StableHashingContext<'a, 'tcx> {
         }
     }
 
+    pub fn force_span_hashing(mut self) -> Self {
+        self.hash_spans = true;
+        self
+    }
+
     #[inline]
     pub fn while_hashing_hir_bodies<F: FnOnce(&mut Self)>(&mut self,
                                                           hash_bodies: bool,

src/librustc/ich/impls_mir.rs

@@ -22,11 +22,55 @@ impl_stable_hash_for!(struct mir::SourceInfo { span, scope });
 impl_stable_hash_for!(enum mir::Mutability { Mut, Not });
 impl_stable_hash_for!(enum mir::BorrowKind { Shared, Unique, Mut });
 impl_stable_hash_for!(enum mir::LocalKind { Var, Temp, Arg, ReturnPointer });
-impl_stable_hash_for!(struct mir::LocalDecl<'tcx> { mutability, ty, name, source_info,
-is_user_variable});
+impl_stable_hash_for!(struct mir::LocalDecl<'tcx> {
+    mutability,
+    ty,
+    name,
+    source_info,
+    is_user_variable
+});
 impl_stable_hash_for!(struct mir::UpvarDecl { debug_name, by_ref });
 impl_stable_hash_for!(struct mir::BasicBlockData<'tcx> { statements, terminator, is_cleanup });
-impl_stable_hash_for!(struct mir::Terminator<'tcx> { source_info, kind });
+
+impl<'a, 'tcx> HashStable<StableHashingContext<'a, 'tcx>> for mir::Terminator<'tcx> {
+    #[inline]
+    fn hash_stable<W: StableHasherResult>(&self,
+                                          hcx: &mut StableHashingContext<'a, 'tcx>,
+                                          hasher: &mut StableHasher<W>) {
+        let mir::Terminator {
+            ref kind,
+            ref source_info,
+        } = *self;
+
+        let hash_spans_unconditionally = match *kind {
+            mir::TerminatorKind::Assert { .. } => {
+                // Assert terminators generate a panic message that contains the
+                // source location, so we always have to feed its span into the
+                // ICH.
+                true
+            }
+            mir::TerminatorKind::Goto { .. } |
+            mir::TerminatorKind::SwitchInt { .. } |
+            mir::TerminatorKind::Resume |
+            mir::TerminatorKind::Return |
+            mir::TerminatorKind::Unreachable |
+            mir::TerminatorKind::Drop { .. } |
+            mir::TerminatorKind::DropAndReplace { .. } |
+            mir::TerminatorKind::Call { .. } => false,
+        };
+
+        if hash_spans_unconditionally {
+            hcx.while_hashing_spans(true, |hcx| {
+                source_info.hash_stable(hcx, hasher);
+            })
+        } else {
+            source_info.hash_stable(hcx, hasher);
+        }
+
+        kind.hash_stable(hcx, hasher);
+    }
+}
+
 
 impl<'a, 'tcx> HashStable<StableHashingContext<'a, 'tcx>> for mir::Local {
     #[inline]

src/librustc/session/mod.rs

@@ -11,8 +11,9 @@
 pub use self::code_stats::{CodeStats, DataTypeKind, FieldInfo};
 pub use self::code_stats::{SizeKind, TypeSizeInfo, VariantInfo};
 
-use dep_graph::{DepGraph, DepNode};
-use hir::def_id::{DefId, CrateNum, DefIndex, CRATE_DEF_INDEX};
+use dep_graph::DepGraph;
+use hir::def_id::{CrateNum, DefIndex};
+
 use lint;
 use middle::cstore::CrateStore;
 use middle::dependency_format;
@@ -32,7 +33,7 @@ use syntax::parse::ParseSess;
 use syntax::symbol::Symbol;
 use syntax::{ast, codemap};
 use syntax::feature_gate::AttributeType;
-use syntax_pos::{Span, MultiSpan, FileMap};
+use syntax_pos::{Span, MultiSpan};
 
 use rustc_back::{LinkerFlavor, PanicStrategy};
 use rustc_back::target::Target;
@@ -46,7 +47,6 @@ use std::io::Write;
 use std::rc::Rc;
 use std::fmt;
 use std::time::Duration;
-use std::sync::Arc;
 
 mod code_stats;
 pub mod config;
@@ -626,21 +626,6 @@ pub fn build_session_(sopts: config::Options,
     };
     let target_cfg = config::build_target_config(&sopts, &span_diagnostic);
 
-    // Hook up the codemap with a callback that allows it to register FileMap
-    // accesses with the dependency graph.
-    let cm_depgraph = dep_graph.clone();
-    let codemap_dep_tracking_callback = Box::new(move |filemap: &FileMap| {
-        let def_id = DefId {
-            krate: CrateNum::from_u32(filemap.crate_of_origin),
-            index: CRATE_DEF_INDEX,
-        };
-        let name = Arc::new(filemap.name.clone());
-        let dep_node = DepNode::FileMap(def_id, name);
-
-        cm_depgraph.read(dep_node);
-    });
-    codemap.set_dep_tracking_callback(codemap_dep_tracking_callback);
-
     let p_s = parse::ParseSess::with_span_handler(span_diagnostic, codemap);
     let default_sysroot = match sopts.maybe_sysroot {
         Some(_) => None,

src/librustc_incremental/calculate_svh/mod.rs

@@ -29,10 +29,9 @@
 
 use std::cell::RefCell;
 use std::hash::Hash;
-use std::sync::Arc;
 use rustc::dep_graph::DepNode;
 use rustc::hir;
-use rustc::hir::def_id::{LOCAL_CRATE, CRATE_DEF_INDEX, DefId};
+use rustc::hir::def_id::{CRATE_DEF_INDEX, DefId};
 use rustc::hir::itemlikevisit::ItemLikeVisitor;
 use rustc::ich::{Fingerprint, StableHashingContext};
 use rustc::ty::TyCtxt;
@@ -155,11 +154,6 @@ impl<'a, 'tcx: 'a> ComputeItemHashesVisitor<'a, 'tcx> {
                                         // We want to incoporate these into the
                                         // SVH.
                                     }
-                                    DepNode::FileMap(..) => {
-                                        // These don't make a semantic
-                                        // difference, filter them out.
-                                        return None
-                                    }
                                     DepNode::AllLocalTraitImpls => {
                                         // These are already covered by hashing
                                         // the HIR.
@@ -306,24 +300,6 @@ pub fn compute_incremental_hashes_map<'a, 'tcx: 'a>(tcx: TyCtxt<'a, 'tcx, 'tcx>)
             visitor.compute_and_store_ich_for_item_like(DepNode::HirBody(def_id), true, macro_def);
         }
 
-        for filemap in tcx.sess
-                          .codemap()
-                          .files_untracked()
-                          .iter()
-                          .filter(|fm| !fm.is_imported()) {
-            assert_eq!(LOCAL_CRATE.as_u32(), filemap.crate_of_origin);
-            let def_id = DefId {
-                krate: LOCAL_CRATE,
-                index: CRATE_DEF_INDEX,
-            };
-            let name = Arc::new(filemap.name.clone());
-            let dep_node = DepNode::FileMap(def_id, name);
-            let mut hasher = IchHasher::new();
-            filemap.hash_stable(&mut visitor.hcx, &mut hasher);
-            let fingerprint = hasher.finish();
-            visitor.hashes.insert(dep_node, fingerprint);
-        }
-
         visitor.compute_and_store_ich_for_trait_impls(krate);
     });
 

src/librustc_incremental/persist/hash.rs

@@ -51,8 +51,7 @@ impl<'a, 'tcx> HashContext<'a, 'tcx> {
         match *dep_node {
             DepNode::Krate |
             DepNode::Hir(_) |
-            DepNode::HirBody(_) |
-            DepNode::FileMap(..) =>
+            DepNode::HirBody(_) =>
                 true,
             DepNode::MetaData(def_id) |
             DepNode::GlobalMetaData(def_id, _) => !def_id.is_local(),
@@ -77,20 +76,6 @@ impl<'a, 'tcx> HashContext<'a, 'tcx> {
                 Some(self.incremental_hashes_map[dep_node])
             }
 
-            DepNode::FileMap(def_id, ref name) => {
-                if def_id.is_local() {
-                    // We will have been able to retrace the DefId (which is
-                    // always the local CRATE_DEF_INDEX), but the file with the
-                    // given name might have been removed, so we use get() in
-                    // order to allow for that case.
-                    self.incremental_hashes_map.get(dep_node).map(|x| *x)
-                } else {
-                    Some(self.metadata_hash(DepNode::FileMap(def_id, name.clone()),
-                                            def_id.krate,
-                                            |this| &mut this.global_metadata_hashes))
-                }
-            }
-
             // MetaData from other crates is an *input* to us.
             // MetaData nodes from *our* crates are an *output*; we
             // don't hash them, but we do compute a hash for them and
@@ -242,7 +227,6 @@ impl<'a, 'tcx> HashContext<'a, 'tcx> {
             let def_id = DefId { krate: cnum, index: CRATE_DEF_INDEX };
             let dep_node = match dep_node {
                 DepNode::GlobalMetaData(_, kind) => DepNode::GlobalMetaData(def_id, kind),
-                DepNode::FileMap(_, name) => DepNode::FileMap(def_id, name),
                 other => {
                     bug!("unexpected DepNode variant: {:?}", other)
                 }

src/librustc_metadata/encoder.rs

@@ -18,7 +18,7 @@ use rustc::middle::cstore::{LinkMeta, LinkagePreference, NativeLibrary,
 use rustc::hir::def_id::{CrateNum, CRATE_DEF_INDEX, DefIndex, DefId, LOCAL_CRATE};
 use rustc::hir::map::definitions::DefPathTable;
 use rustc::dep_graph::{DepNode, GlobalMetaDataKind};
-use rustc::ich::{StableHashingContext, Fingerprint};
+use rustc::ich::Fingerprint;
 use rustc::middle::dependency_format::Linkage;
 use rustc::middle::lang_items;
 use rustc::mir;
@@ -29,15 +29,13 @@ use rustc::session::config::{self, CrateTypeProcMacro};
 use rustc::util::nodemap::{FxHashMap, NodeSet};
 
 use rustc_serialize::{Encodable, Encoder, SpecializedEncoder, opaque};
-use rustc_data_structures::stable_hasher::{StableHasher, HashStable};
 
 use std::hash::Hash;
 use std::intrinsics;
 use std::io::prelude::*;
 use std::io::Cursor;
 use std::path::Path;
 use std::rc::Rc;
-use std::sync::Arc;
 use std::u32;
 use syntax::ast::{self, CRATE_NODE_ID};
 use syntax::codemap::Spanned;
@@ -284,7 +282,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
         let codemap = self.tcx.sess.codemap();
         let all_filemaps = codemap.files();
 
-        let hcx = &mut StableHashingContext::new(self.tcx);
         let (working_dir, working_dir_was_remapped) = self.tcx.sess.working_dir.clone();
 
         let adapted = all_filemaps.iter()
@@ -316,21 +313,10 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
                     adapted.name = abs_path;
                     Rc::new(adapted)
                 }
-            });
-
-        let filemaps: Vec<_> = if self.compute_ich {
-            adapted.inspect(|filemap| {
-                let mut hasher = StableHasher::new();
-                filemap.hash_stable(hcx, &mut hasher);
-                let fingerprint = hasher.finish();
-                let dep_node = DepNode::FileMap((), Arc::new(filemap.name.clone()));
-                self.metadata_hashes.global_hashes.push((dep_node, fingerprint));
-            }).collect()
-        } else {
-            adapted.collect()
-        };
+            })
+            .collect::<Vec<_>>();
 
-        self.lazy_seq_ref(filemaps.iter().map(|fm| &**fm))
+        self.lazy_seq_ref(adapted.iter().map(|rc| &**rc))
     }
 
     fn encode_crate_root(&mut self) -> Lazy<CrateRoot> {

src/librustc_metadata/isolated_encoder.rs

@@ -35,7 +35,17 @@ impl<'a, 'b: 'a, 'tcx: 'b> IsolatedEncoder<'a, 'b, 'tcx> {
             tcx: tcx,
             ecx: ecx,
             hcx: if compute_ich {
-                Some((StableHashingContext::new(tcx), StableHasher::new()))
+                // We are always hashing spans for things in metadata because
+                // don't know if a downstream crate will use them or not.
+                // Except when -Zquery-dep-graph is specified because we don't
+                // want to mess up our tests.
+                let hcx = if tcx.sess.opts.debugging_opts.query_dep_graph {
+                    StableHashingContext::new(tcx)
+                } else {
+                    StableHashingContext::new(tcx).force_span_hashing()
+                };
+
+                Some((hcx, StableHasher::new()))
             } else {
                 None
             }