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

1 files changed, 39 insertions, 4 deletions

diff --git a/src/libsyntax/codemap.rs b/src/libsyntax/codemap.rs
index 8a88ec3a672..0c8be1d4f24 100644
--- a/src/libsyntax/codemap.rs
+++ b/src/libsyntax/codemap.rs
@@ -21,8 +21,8 @@ pub use syntax_pos::*;
 pub use syntax_pos::hygiene::{ExpnFormat, ExpnInfo, NameAndSpan};
 pub use self::ExpnFormat::*;
 
-use std::cell::RefCell;
-use std::path::{Path,PathBuf};
+use std::cell::{RefCell, Ref};
+use std::path::{Path, PathBuf};
 use std::rc::Rc;
 
 use std::env;
@@ -103,11 +103,18 @@ impl FileLoader for RealFileLoader {
 //
 
 pub struct CodeMap {
-    pub files: RefCell<Vec<Rc<FileMap>>>,
+    // The `files` field should not be visible outside of libsyntax so that we
+    // can do proper dependency tracking.
+    pub(super) files: RefCell<Vec<Rc<FileMap>>>,
     file_loader: Box<FileLoader>,
     // This is used to apply the file path remapping as specified via
     // -Zremap-path-prefix to all FileMaps allocated within this CodeMap.
     path_mapping: FilePathMapping,
+    // The CodeMap will invoke this callback whenever a specific FileMap is
+    // accessed. The callback starts out as a no-op but when the dependency
+    // graph becomes available later during the compilation process, it is
+    // be replaced with something that notifies the dep-tracking system.
+    dep_tracking_callback: RefCell<Box<Fn(&FileMap)>>,
 }
 
 impl CodeMap {
@@ -116,6 +123,7 @@ impl CodeMap {
             files: RefCell::new(Vec::new()),
             file_loader: Box::new(RealFileLoader),
             path_mapping: path_mapping,
+            dep_tracking_callback: RefCell::new(Box::new(|_| {})),
         }
     }
 
@@ -126,6 +134,7 @@ impl CodeMap {
             files: RefCell::new(Vec::new()),
             file_loader: file_loader,
             path_mapping: path_mapping,
+            dep_tracking_callback: RefCell::new(Box::new(|_| {})),
         }
     }
 
@@ -133,6 +142,10 @@ impl CodeMap {
         &self.path_mapping
     }
 
+    pub fn set_dep_tracking_callback(&self, cb: Box<Fn(&FileMap)>) {
+        *self.dep_tracking_callback.borrow_mut() = cb;
+    }
+
     pub fn file_exists(&self, path: &Path) -> bool {
         self.file_loader.file_exists(path)
     }
@@ -142,6 +155,19 @@ impl CodeMap {
         Ok(self.new_filemap(path.to_str().unwrap().to_string(), src))
     }
 
+    pub fn files(&self) -> Ref<Vec<Rc<FileMap>>> {
+        let files = self.files.borrow();
+        for file in files.iter() {
+            (self.dep_tracking_callback.borrow())(file);
+        }
+        files
+    }
+
+    /// Only use this if you do your own dependency tracking!
+    pub fn files_untracked(&self) -> Ref<Vec<Rc<FileMap>>> {
+        self.files.borrow()
+    }
+
     fn next_start_pos(&self) -> usize {
         let files = self.files.borrow();
         match files.last() {
@@ -170,6 +196,7 @@ impl CodeMap {
         let filemap = Rc::new(FileMap {
             name: filename,
             name_was_remapped: was_remapped,
+            crate_of_origin: 0,
             src: Some(Rc::new(src)),
             start_pos: Pos::from_usize(start_pos),
             end_pos: Pos::from_usize(end_pos),
@@ -204,6 +231,7 @@ impl CodeMap {
     pub fn new_imported_filemap(&self,
                                 filename: FileName,
                                 name_was_remapped: bool,
+                                crate_of_origin: u32,
                                 source_len: usize,
                                 mut file_local_lines: Vec<BytePos>,
                                 mut file_local_multibyte_chars: Vec<MultiByteChar>)
@@ -225,6 +253,7 @@ impl CodeMap {
         let filemap = Rc::new(FileMap {
             name: filename,
             name_was_remapped: name_was_remapped,
+            crate_of_origin: crate_of_origin,
             src: None,
             start_pos: start_pos,
             end_pos: end_pos,
@@ -282,6 +311,8 @@ impl CodeMap {
         let files = self.files.borrow();
         let f = (*files)[idx].clone();
 
+        (self.dep_tracking_callback.borrow())(&f);
+
         match f.lookup_line(pos) {
             Some(line) => Ok(FileMapAndLine { fm: f, line: line }),
             None => Err(f)
@@ -471,6 +502,7 @@ impl CodeMap {
     pub fn get_filemap(&self, filename: &str) -> Option<Rc<FileMap>> {
         for fm in self.files.borrow().iter() {
             if filename == fm.name {
+               (self.dep_tracking_callback.borrow())(&fm);
                 return Some(fm.clone());
             }
         }
@@ -481,6 +513,7 @@ impl CodeMap {
     pub fn lookup_byte_offset(&self, bpos: BytePos) -> FileMapAndBytePos {
         let idx = self.lookup_filemap_idx(bpos);
         let fm = (*self.files.borrow())[idx].clone();
+        (self.dep_tracking_callback.borrow())(&fm);
         let offset = bpos - fm.start_pos;
         FileMapAndBytePos {fm: fm, pos: offset}
     }
@@ -491,6 +524,8 @@ impl CodeMap {
         let files = self.files.borrow();
         let map = &(*files)[idx];
 
+        (self.dep_tracking_callback.borrow())(map);
+
         // The number of extra bytes due to multibyte chars in the FileMap
         let mut total_extra_bytes = 0;
 
@@ -536,7 +571,7 @@ impl CodeMap {
     }
 
     pub fn count_lines(&self) -> usize {
-        self.files.borrow().iter().fold(0, |a, f| a + f.count_lines())
+        self.files().iter().fold(0, |a, f| a + f.count_lines())
     }
 }