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

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

diff --combined src/librustc_metadata/creader.rs
index 325511c9787e1b62655b9c47a0047bd4c1f7201c,58c2d3b2c124d073f3370eae457bc3e857674810..d2874f16289015afb8095c96111d203aac3745fa
--- 1/src/librustc_metadata/creader.rs
--- 2/src/librustc_metadata/creader.rs
+++ b/src/librustc_metadata/creader.rs
@@@ -12,9 -12,10 +12,10 @@@
  
  use cstore::{self, CStore, CrateSource, MetadataBlob};
  use locator::{self, CratePaths};
- use schema::CrateRoot;
+ use schema::{CrateRoot, Tracked};
  
- use rustc::hir::def_id::{CrateNum, DefIndex};
+ use rustc::dep_graph::{DepNode, GlobalMetaDataKind};
+ use rustc::hir::def_id::{DefId, CrateNum, DefIndex, CRATE_DEF_INDEX};
  use rustc::hir::svh::Svh;
  use rustc::middle::cstore::DepKind;
  use rustc::session::Session;
@@@ -88,7 -89,7 +89,7 @@@ fn register_native_lib(sess: &Session
              Some(span) => {
                  struct_span_err!(sess, span, E0454,
                                   "#[link(name = \"\")] given with empty name")
 -                    .span_label(span, &format!("empty name given"))
 +                    .span_label(span, "empty name given")
                      .emit();
              }
              None => {
@@@ -311,7 -312,8 +312,8 @@@ impl<'a> CrateLoader<'a> 
              crate_root.def_path_table.decode(&metadata)
          });
  
-         let exported_symbols = crate_root.exported_symbols.decode(&metadata).collect();
+         let exported_symbols = crate_root.exported_symbols
+                                          .map(|x| x.decode(&metadata).collect());
  
          let mut cmeta = cstore::CrateMetadata {
              name: name,
@@@ -333,16 -335,27 +335,27 @@@
                  rlib: rlib,
                  rmeta: rmeta,
              },
-             dllimport_foreign_items: FxHashSet(),
+             // Initialize this with an empty set. The field is populated below
+             // after we were able to deserialize its contents.
+             dllimport_foreign_items: Tracked::new(FxHashSet()),
          };
  
-         let dllimports: Vec<_> = cmeta.get_native_libraries().iter()
-                             .filter(|lib| relevant_lib(self.sess, lib) &&
-                                           lib.kind == cstore::NativeLibraryKind::NativeUnknown)
-                             .flat_map(|lib| &lib.foreign_items)
-                             .map(|id| *id)
-                             .collect();
-         cmeta.dllimport_foreign_items.extend(dllimports);
+         let dllimports: Tracked<FxHashSet<_>> = cmeta
+             .root
+             .native_libraries
+             .map(|native_libraries| {
+                 let native_libraries: Vec<_> = native_libraries.decode(&cmeta)
+                                                                .collect();
+                 native_libraries
+                     .iter()
+                     .filter(|lib| relevant_lib(self.sess, lib) &&
+                                   lib.kind == cstore::NativeLibraryKind::NativeUnknown)
+                     .flat_map(|lib| lib.foreign_items.iter())
+                     .map(|id| *id)
+                     .collect()
+             });
+ 
+         cmeta.dllimport_foreign_items = dllimports;
  
          let cmeta = Rc::new(cmeta);
          self.cstore.set_crate_data(cnum, cmeta.clone());
@@@ -493,10 -506,16 +506,16 @@@
              return cstore::CrateNumMap::new();
          }
  
+         let dep_node = DepNode::GlobalMetaData(DefId { krate, index: CRATE_DEF_INDEX },
+                                                GlobalMetaDataKind::CrateDeps);
+ 
          // The map from crate numbers in the crate we're resolving to local crate numbers.
          // We map 0 and all other holes in the map to our parent crate. The "additional"
          // self-dependencies should be harmless.
-         ::std::iter::once(krate).chain(crate_root.crate_deps.decode(metadata).map(|dep| {
+         ::std::iter::once(krate).chain(crate_root.crate_deps
+                                                  .get(&self.sess.dep_graph, dep_node)
+                                                  .decode(metadata)
+                                                  .map(|dep| {
              debug!("resolving dep crate {} hash: `{}`", dep.name, dep.hash);
              if dep.kind == DepKind::UnexportedMacrosOnly {
                  return krate;
@@@ -654,7 -673,9 +673,9 @@@
  
      /// Look for a plugin registrar. Returns library path, crate
      /// SVH and DefIndex of the registrar function.
-     pub fn find_plugin_registrar(&mut self, span: Span, name: &str)
+     pub fn find_plugin_registrar(&mut self,
+                                  span: Span,
+                                  name: &str)
                                   -> Option<(PathBuf, Symbol, DefIndex)> {
          let ekrate = self.read_extension_crate(span, &ExternCrateInfo {
               name: Symbol::intern(name),
@@@ -740,13 -761,17 +761,17 @@@
          let mut runtime_found = false;
          let mut needs_panic_runtime = attr::contains_name(&krate.attrs,
                                                            "needs_panic_runtime");
+ 
+         let dep_graph = &self.sess.dep_graph;
+ 
          self.cstore.iter_crate_data(|cnum, data| {
-             needs_panic_runtime = needs_panic_runtime || data.needs_panic_runtime();
-             if data.is_panic_runtime() {
+             needs_panic_runtime = needs_panic_runtime ||
+                                   data.needs_panic_runtime(dep_graph);
+             if data.is_panic_runtime(dep_graph) {
                  // Inject a dependency from all #![needs_panic_runtime] to this
                  // #![panic_runtime] crate.
                  self.inject_dependency_if(cnum, "a panic runtime",
-                                           &|data| data.needs_panic_runtime());
+                                           &|data| data.needs_panic_runtime(dep_graph));
                  runtime_found = runtime_found || data.dep_kind.get() == DepKind::Explicit;
              }
          });
@@@ -782,11 -807,11 +807,11 @@@
  
          // Sanity check the loaded crate to ensure it is indeed a panic runtime
          // and the panic strategy is indeed what we thought it was.
-         if !data.is_panic_runtime() {
+         if !data.is_panic_runtime(dep_graph) {
              self.sess.err(&format!("the crate `{}` is not a panic runtime",
                                     name));
          }
-         if data.panic_strategy() != desired_strategy {
+         if data.panic_strategy(dep_graph) != desired_strategy {
              self.sess.err(&format!("the crate `{}` does not have the panic \
                                      strategy `{}`",
                                     name, desired_strategy.desc()));
@@@ -794,7 -819,7 +819,7 @@@
  
          self.sess.injected_panic_runtime.set(Some(cnum));
          self.inject_dependency_if(cnum, "a panic runtime",
-                                   &|data| data.needs_panic_runtime());
+                                   &|data| data.needs_panic_runtime(dep_graph));
      }
  
      fn inject_sanitizer_runtime(&mut self) {
@@@ -862,7 -887,7 +887,7 @@@
                                         PathKind::Crate, dep_kind);
  
                  // Sanity check the loaded crate to ensure it is indeed a sanitizer runtime
-                 if !data.is_sanitizer_runtime() {
+                 if !data.is_sanitizer_runtime(&self.sess.dep_graph) {
                      self.sess.err(&format!("the crate `{}` is not a sanitizer runtime",
                                             name));
                  }
@@@ -878,12 -903,13 +903,13 @@@
          // also bail out as we don't need to implicitly inject one.
          let mut needs_allocator = false;
          let mut found_required_allocator = false;
+         let dep_graph = &self.sess.dep_graph;
          self.cstore.iter_crate_data(|cnum, data| {
-             needs_allocator = needs_allocator || data.needs_allocator();
-             if data.is_allocator() {
+             needs_allocator = needs_allocator || data.needs_allocator(dep_graph);
+             if data.is_allocator(dep_graph) {
                  info!("{} required by rlib and is an allocator", data.name());
                  self.inject_dependency_if(cnum, "an allocator",
-                                           &|data| data.needs_allocator());
+                                           &|data| data.needs_allocator(dep_graph));
                  found_required_allocator = found_required_allocator ||
                      data.dep_kind.get() == DepKind::Explicit;
              }
@@@ -937,14 -963,14 +963,14 @@@
  
          // Sanity check the crate we loaded to ensure that it is indeed an
          // allocator.
-         if !data.is_allocator() {
+         if !data.is_allocator(dep_graph) {
              self.sess.err(&format!("the allocator crate `{}` is not tagged \
                                      with #![allocator]", data.name()));
          }
  
          self.sess.injected_allocator.set(Some(cnum));
          self.inject_dependency_if(cnum, "an allocator",
-                                   &|data| data.needs_allocator());
+                                   &|data| data.needs_allocator(dep_graph));
      }
  
      fn inject_dependency_if(&self,
@@@ -1029,7 -1055,7 +1055,7 @@@ impl<'a> CrateLoader<'a> 
                  Some(k) => {
                      struct_span_err!(self.sess, m.span, E0458,
                                "unknown kind: `{}`", k)
 -                        .span_label(m.span, &format!("unknown kind")).emit();
 +                        .span_label(m.span, "unknown kind").emit();
                      cstore::NativeUnknown
                  }
                  None => cstore::NativeUnknown
@@@ -1042,7 -1068,7 +1068,7 @@@
                  None => {
                      struct_span_err!(self.sess, m.span, E0459,
                                       "#[link(...)] specified without `name = \"foo\"`")
 -                        .span_label(m.span, &format!("missing `name` argument")).emit();
 +                        .span_label(m.span, "missing `name` argument").emit();
                      Symbol::intern("foo")
                  }
              };