Auto merge of #41709 - michaelwoerister:close-metadata-ich-holes, r=n…

…ikomatsakis

incr.comp.: Hash more pieces of crate metadata to detect changes there.

This PR adds incr. comp. hashes for non-`Entry` pieces of data in crate metadata.

The first part of it I like: `EntryBuilder` is refactored into the more generally applicable `IsolatedEncoder` which provides means of encoding something into metadata while also feeding the encoded data into an incr. comp. hash. We already did this for `Entry`, now we are doing it for various other pieces of data too, like the set of exported symbols and so on. The hashes generated there are persisted together with the per-`Entry` hashes and are also used for dep-graph dirtying the same way.

The second part of the PR I'm not entirely happy with: In order to make sure that we don't forget registering a read to the new `DepNodes` introduced here, I added the `Tracked<T>` struct. This struct wraps a value and requires a `DepNode` when accessing the wrapped value. This makes it harder to overlook adding read edges in the right places and works just fine.
However, crate metadata is already used in places where there is no `tcx` yet or even in places where no `cnum` has been assigned -- this makes it harder to apply this feature consistently or implement it ergonomically. The result is not too bad but there's a bit more code churn and a bit more opportunity to get something wrong than I would have liked. On the other hand, wrapping things in `Tracked<T>` already has revealed some bugs, so there's definitely some value in it.

This is still a work in progress:
- [x] I need to write some test cases.
- [x] Accessing the CodeMap should really be dependency tracked too, especially with the new path-remapping feature.

cc @nikomatsakis

src/librustc/dep_graph/dep_node.rs

@@ -51,6 +51,9 @@ pub enum DepNode<D: Clone + Debug> {
     // in an extern crate.
     MetaData(D),
 
+    // Represents some piece of metadata global to its crate.
+    GlobalMetaData(D, GlobalMetaDataKind),
+
     // Represents some artifact that we save to disk. Note that these
     // do not have a def-id as part of their identifier.
     WorkProduct(Arc<WorkProductId>),
@@ -79,7 +82,6 @@ pub enum DepNode<D: Clone + Debug> {
     MirKeys,
     LateLintCheck,
     TransCrateItem(D),
-    TransInlinedItem(D),
     TransWriteMetadata,
     CrateVariances,
 
@@ -157,6 +159,7 @@ pub enum DepNode<D: Clone + Debug> {
     DefSpan(D),
     Stability(D),
     Deprecation(D),
+    FileMap(D, Arc<String>),
 }
 
 impl<D: Clone + Debug> DepNode<D> {
@@ -234,7 +237,6 @@ impl<D: Clone + Debug> DepNode<D> {
             RegionMaps(ref d) => op(d).map(RegionMaps),
             RvalueCheck(ref d) => op(d).map(RvalueCheck),
             TransCrateItem(ref d) => op(d).map(TransCrateItem),
-            TransInlinedItem(ref d) => op(d).map(TransInlinedItem),
             AssociatedItems(ref d) => op(d).map(AssociatedItems),
             ItemSignature(ref d) => op(d).map(ItemSignature),
             ItemVariances(ref d) => op(d).map(ItemVariances),
@@ -271,6 +273,8 @@ impl<D: Clone + Debug> DepNode<D> {
             DefSpan(ref d) => op(d).map(DefSpan),
             Stability(ref d) => op(d).map(Stability),
             Deprecation(ref d) => op(d).map(Deprecation),
+            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())),
         }
     }
 }
@@ -282,3 +286,16 @@ impl<D: Clone + Debug> DepNode<D> {
 /// them even in the absence of a tcx.)
 #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, RustcEncodable, RustcDecodable)]
 pub struct WorkProductId(pub String);
+
+#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, RustcEncodable, RustcDecodable)]
+pub enum GlobalMetaDataKind {
+    Krate,
+    CrateDeps,
+    DylibDependencyFormats,
+    LangItems,
+    LangItemsMissing,
+    NativeLibraries,
+    CodeMap,
+    Impls,
+    ExportedSymbols,
+}

src/librustc/dep_graph/mod.rs

@@ -22,6 +22,7 @@ mod thread;
 pub use self::dep_tracking_map::{DepTrackingMap, DepTrackingMapConfig};
 pub use self::dep_node::DepNode;
 pub use self::dep_node::WorkProductId;
+pub use self::dep_node::GlobalMetaDataKind;
 pub use self::graph::DepGraph;
 pub use self::graph::WorkProduct;
 pub use self::query::DepGraphQuery;

src/librustc/hir/def_id.rs

@@ -36,7 +36,10 @@ pub const LOCAL_CRATE: CrateNum = CrateNum(0);
 
 /// Virtual crate for builtin macros
 // FIXME(jseyfried): this is also used for custom derives until proc-macro crates get `CrateNum`s.
-pub const BUILTIN_MACROS_CRATE: CrateNum = CrateNum(!0);
+pub const BUILTIN_MACROS_CRATE: CrateNum = CrateNum(u32::MAX);
+
+/// A CrateNum value that indicates that something is wrong.
+pub const INVALID_CRATE: CrateNum = CrateNum(u32::MAX - 1);
 
 impl CrateNum {
     pub fn new(x: usize) -> CrateNum {

src/librustc/hir/svh.rs

@@ -66,3 +66,7 @@ impl Decodable for Svh {
          .map(Svh::new)
     }
 }
+
+impl_stable_hash_for!(struct Svh {
+    hash
+});

src/librustc/ich/caching_codemap_view.rs

@@ -8,10 +8,14 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use ty::TyCtxt;
+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;
 
 #[derive(Clone)]
 struct CacheEntry {
@@ -20,30 +24,37 @@ struct CacheEntry {
     line_start: BytePos,
     line_end: BytePos,
     file: Rc<FileMap>,
+    file_index: usize,
 }
 
 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 first_file = codemap.files.borrow()[0].clone();
+        let files = codemap.files_untracked();
+        let first_file = files[0].clone();
         let entry = CacheEntry {
             time_stamp: 0,
             line_number: 0,
             line_start: BytePos(0),
             line_end: BytePos(0),
             file: first_file,
+            file_index: 0,
         };
 
         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()),
         }
     }
 
@@ -56,6 +67,10 @@ 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,
                              pos - cache_entry.line_start));
@@ -75,14 +90,15 @@ 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.borrow();
+            let files = self.codemap.files_untracked();
 
             if files.len() > 0 {
                 let file_index = self.codemap.lookup_filemap_idx(pos);
                 let file = files[file_index].clone();
 
                 if pos >= file.start_pos && pos < file.end_pos {
                     cache_entry.file = file;
+                    cache_entry.file_index = file_index;
                     file_valid = true;
                 } else {
                     file_valid = false;
@@ -104,8 +120,21 @@ 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/impls_cstore.rs

@@ -0,0 +1,40 @@
+// 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.
+
+//! This module contains `HashStable` implementations for various data types
+//! from rustc::middle::cstore in no particular order.
+
+use middle;
+
+impl_stable_hash_for!(enum middle::cstore::DepKind {
+    UnexportedMacrosOnly,
+    MacrosOnly,
+    Implicit,
+    Explicit
+});
+
+impl_stable_hash_for!(enum middle::cstore::NativeLibraryKind {
+    NativeStatic,
+    NativeStaticNobundle,
+    NativeFramework,
+    NativeUnknown
+});
+
+impl_stable_hash_for!(struct middle::cstore::NativeLibrary {
+    kind,
+    name,
+    cfg,
+    foreign_items
+});
+
+impl_stable_hash_for!(enum middle::cstore::LinkagePreference {
+    RequireDynamic,
+    RequireStatic
+});

src/librustc/ich/impls_hir.rs

@@ -1120,3 +1120,11 @@ impl_stable_hash_for!(struct hir::def::Export {
     def,
     span
 });
+
+impl<'a, 'tcx> HashStable<StableHashingContext<'a, 'tcx>> for ::middle::lang_items::LangItem {
+    fn hash_stable<W: StableHasherResult>(&self,
+                                          _: &mut StableHashingContext<'a, 'tcx>,
+                                          hasher: &mut StableHasher<W>) {
+        ::std::hash::Hash::hash(self, hasher);
+    }
+}

src/librustc/ich/impls_syntax.rs

@@ -19,7 +19,9 @@ use std::mem;
 use syntax::ast;
 use syntax::parse::token;
 use syntax::tokenstream;
-use syntax_pos::Span;
+use syntax_pos::{Span, FileMap};
+
+use hir::def_id::{DefId, CrateNum, CRATE_DEF_INDEX};
 
 use rustc_data_structures::stable_hasher::{HashStable, StableHasher,
                                            StableHasherResult};
@@ -299,3 +301,79 @@ fn hash_token<'a, 'tcx, W: StableHasherResult>(token: &token::Token,
         token::Token::Shebang(val) => val.hash_stable(hcx, hasher),
     }
 }
+
+impl_stable_hash_for_spanned!(::syntax::ast::NestedMetaItemKind);
+
+impl_stable_hash_for!(enum ::syntax::ast::NestedMetaItemKind {
+    MetaItem(meta_item),
+    Literal(lit)
+});
+
+impl_stable_hash_for!(struct ::syntax::ast::MetaItem {
+    name,
+    node,
+    span
+});
+
+impl_stable_hash_for!(enum ::syntax::ast::MetaItemKind {
+    Word,
+    List(nested_items),
+    NameValue(lit)
+});
+
+impl<'a, 'tcx> HashStable<StableHashingContext<'a, 'tcx>> for FileMap {
+    fn hash_stable<W: StableHasherResult>(&self,
+                                          hcx: &mut StableHashingContext<'a, 'tcx>,
+                                          hasher: &mut StableHasher<W>) {
+        let FileMap {
+            ref name,
+            name_was_remapped,
+            crate_of_origin,
+            // Do not hash the source as it is not encoded
+            src: _,
+            start_pos,
+            end_pos: _,
+            ref lines,
+            ref multibyte_chars,
+        } = *self;
+
+        name.hash_stable(hcx, hasher);
+        name_was_remapped.hash_stable(hcx, hasher);
+
+        DefId {
+            krate: CrateNum::from_u32(crate_of_origin),
+            index: CRATE_DEF_INDEX,
+        }.hash_stable(hcx, hasher);
+
+        // We only hash the relative position within this filemap
+        let lines = lines.borrow();
+        lines.len().hash_stable(hcx, hasher);
+        for &line in lines.iter() {
+            stable_byte_pos(line, start_pos).hash_stable(hcx, hasher);
+        }
+
+        // We only hash the relative position within this filemap
+        let multibyte_chars = multibyte_chars.borrow();
+        multibyte_chars.len().hash_stable(hcx, hasher);
+        for &char_pos in multibyte_chars.iter() {
+            stable_multibyte_char(char_pos, start_pos).hash_stable(hcx, hasher);
+        }
+    }
+}
+
+fn stable_byte_pos(pos: ::syntax_pos::BytePos,
+                   filemap_start: ::syntax_pos::BytePos)
+                   -> u32 {
+    pos.0 - filemap_start.0
+}
+
+fn stable_multibyte_char(mbc: ::syntax_pos::MultiByteChar,
+                         filemap_start: ::syntax_pos::BytePos)
+                         -> (u32, u32) {
+    let ::syntax_pos::MultiByteChar {
+        pos,
+        bytes,
+    } = mbc;
+
+    (pos.0 - filemap_start.0, bytes as u32)
+}

src/librustc/ich/mod.rs

@@ -19,6 +19,7 @@ mod caching_codemap_view;
 mod hcx;
 
 mod impls_const_math;
+mod impls_cstore;
 mod impls_hir;
 mod impls_mir;
 mod impls_ty;

src/librustc/middle/cstore.rs

@@ -23,6 +23,7 @@
 // probably get a better home if someone can find one.
 
 use hir::def;
+use dep_graph::DepNode;
 use hir::def_id::{CrateNum, DefId, DefIndex};
 use hir::map as hir_map;
 use hir::map::definitions::{Definitions, DefKey, DisambiguatedDefPathData};
@@ -161,7 +162,16 @@ pub struct ExternCrate {
 
 pub struct EncodedMetadata {
     pub raw_data: Vec<u8>,
-    pub hashes: Vec<EncodedMetadataHash>,
+    pub hashes: EncodedMetadataHashes,
+}
+
+impl EncodedMetadata {
+    pub fn new() -> EncodedMetadata {
+        EncodedMetadata {
+            raw_data: Vec::new(),
+            hashes: EncodedMetadataHashes::new(),
+        }
+    }
 }
 
 /// The hash for some metadata that (when saving) will be exported
@@ -173,6 +183,24 @@ pub struct EncodedMetadataHash {
     pub hash: ich::Fingerprint,
 }
 
+/// The hash for some metadata that (when saving) will be exported
+/// from this crate, or which (when importing) was exported by an
+/// upstream crate.
+#[derive(Debug, RustcEncodable, RustcDecodable, Clone)]
+pub struct EncodedMetadataHashes {
+    pub entry_hashes: Vec<EncodedMetadataHash>,
+    pub global_hashes: Vec<(DepNode<()>, ich::Fingerprint)>,
+}
+
+impl EncodedMetadataHashes {
+    pub fn new() -> EncodedMetadataHashes {
+        EncodedMetadataHashes {
+            entry_hashes: Vec::new(),
+            global_hashes: Vec::new(),
+        }
+    }
+}
+
 /// A store of Rust crates, through with their metadata
 /// can be accessed.
 pub trait CrateStore {