Instrument the AST map so that it registers reads when data is

acccessed.

8 files changed, 118 insertions, 18 deletions

diff --git a/src/librustc/dep_graph/mod.rs b/src/librustc/dep_graph/mod.rs
index 3bb73ecca2c..d37a4acbcfb 100644
--- a/src/librustc/dep_graph/mod.rs
+++ b/src/librustc/dep_graph/mod.rs
@@ -116,6 +116,13 @@ impl DepGraph {
         }
     }
 
+    /// True if we are actually building a dep-graph. If this returns false,
+    /// then the other methods on this `DepGraph` will have no net effect.
+    #[inline]
+    pub fn enabled(&self) -> bool {
+        self.data.enabled()
+    }
+
     pub fn query(&self) -> DepGraphQuery {
         self.data.query()
     }
diff --git a/src/librustc/front/map/collector.rs b/src/librustc/front/map/collector.rs
index e85b0ec77cb..8ffc343c8f4 100644
--- a/src/librustc/front/map/collector.rs
+++ b/src/librustc/front/map/collector.rs
@@ -79,6 +79,7 @@ impl<'ast> NodeCollector<'ast> {
 
     fn create_def(&mut self, node_id: NodeId, data: DefPathData) -> DefIndex {
         let parent_def = self.parent_def();
+        debug!("create_def(node_id={:?}, data={:?}, parent_def={:?})", node_id, data, parent_def);
         self.definitions.create_def_with_parent(parent_def, node_id, data)
     }
 
@@ -115,10 +116,13 @@ impl<'ast> Visitor<'ast> for NodeCollector<'ast> {
     /// deep walking so that we walk nested items in the context of
     /// their outer items.
     fn visit_nested_item(&mut self, item: ItemId) {
+        debug!("visit_nested_item: {:?}", item);
         self.visit_item(self.krate.item(item.id))
     }
 
     fn visit_item(&mut self, i: &'ast Item) {
+        debug!("visit_item: {:?}", i);
+
         // Pick the def data. This need not be unique, but the more
         // information we encapsulate into
         let def_data = match i.node {
diff --git a/src/librustc/front/map/mod.rs b/src/librustc/front/map/mod.rs
index 7de60995445..8f3760c175f 100644
--- a/src/librustc/front/map/mod.rs
+++ b/src/librustc/front/map/mod.rs
@@ -14,6 +14,8 @@ use self::MapEntry::*;
 use self::collector::NodeCollector;
 pub use self::definitions::{Definitions, DefKey, DefPath, DefPathData, DisambiguatedDefPathData};
 
+use dep_graph::{DepGraph, DepNode};
+
 use middle::cstore::InlinedItem;
 use middle::cstore::InlinedItem as II;
 use middle::def_id::DefId;
@@ -228,19 +230,22 @@ impl<'ast> MapEntry<'ast> {
 
 /// Stores a crate and any number of inlined items from other crates.
 pub struct Forest {
-    pub krate: Crate,
+    krate: Crate,
+    pub dep_graph: DepGraph,
     inlined_items: TypedArena<InlinedParent>
 }
 
 impl Forest {
-    pub fn new(krate: Crate) -> Forest {
+    pub fn new(krate: Crate, dep_graph: DepGraph) -> Forest {
         Forest {
             krate: krate,
+            dep_graph: dep_graph,
             inlined_items: TypedArena::new()
         }
     }
 
     pub fn krate<'ast>(&'ast self) -> &'ast Crate {
+        self.dep_graph.read(DepNode::Krate);
         &self.krate
     }
 }
@@ -252,6 +257,10 @@ pub struct Map<'ast> {
     /// The backing storage for all the AST nodes.
     pub forest: &'ast Forest,
 
+    /// Same as the dep_graph in forest, just available with one fewer
+    /// deref. This is a gratuitious micro-optimization.
+    pub dep_graph: DepGraph,
+
     /// NodeIds are sequential integers from 0, so we can be
     /// super-compact by storing them in a vector. Not everything with
     /// a NodeId is in the map, but empirically the occupancy is about
@@ -267,6 +276,60 @@ pub struct Map<'ast> {
 }
 
 impl<'ast> Map<'ast> {
+    /// Registers a read in the dependency graph of the AST node with
+    /// the given `id`. This needs to be called each time a public
+    /// function returns the HIR for a node -- in other words, when it
+    /// "reveals" the content of a node to the caller (who might not
+    /// otherwise have had access to those contents, and hence needs a
+    /// read recorded). If the function just returns a DefId or
+    /// NodeId, no actual content was returned, so no read is needed.
+    fn read(&self, id: NodeId) {
+        self.dep_graph.read(self.dep_node(id));
+    }
+
+    fn dep_node(&self, id0: NodeId) -> DepNode {
+        let map = self.map.borrow();
+        let mut id = id0;
+        loop {
+            match map[id as usize] {
+                EntryItem(_, item) => {
+                    let def_id = self.local_def_id(item.id);
+                    // NB                          ^~~~~~~
+                    //
+                    // You would expect that `item.id == id`, but this
+                    // is not always the case. In particular, for
+                    // ViewPath like `use self::{mem, foo}`, we record
+                    // map the ids for `mem` and `foo` to the
+                    // enclosing view path item. This seems mega super
+                    // ultra wrong, but then who am I to
+                    // judge. -nmatsakis
+                    return DepNode::Hir(def_id);
+                }
+
+                EntryForeignItem(p, _) |
+                EntryTraitItem(p, _) |
+                EntryImplItem(p, _) |
+                EntryVariant(p, _) |
+                EntryExpr(p, _) |
+                EntryStmt(p, _) |
+                EntryLocal(p, _) |
+                EntryPat(p, _) |
+                EntryBlock(p, _) |
+                EntryStructCtor(p, _) |
+                EntryLifetime(p, _) |
+                EntryTyParam(p, _) =>
+                    id = p,
+
+                RootCrate |
+                RootInlinedParent(_) => // FIXME(#2369) clarify story about cross-crate dep tracking
+                    return DepNode::Krate,
+
+                NotPresent =>
+                    panic!("Walking parents from `{}` led to `NotPresent` at `{}`", id0, id),
+            }
+        }
+    }
+
     pub fn num_local_def_ids(&self) -> usize {
         self.definitions.borrow().len()
     }
@@ -309,26 +372,30 @@ impl<'ast> Map<'ast> {
     }
 
     pub fn krate(&self) -> &'ast Crate {
-        &self.forest.krate
+        self.forest.krate()
     }
 
     /// Retrieve the Node corresponding to `id`, panicking if it cannot
     /// be found.
     pub fn get(&self, id: NodeId) -> Node<'ast> {
         match self.find(id) {
-            Some(node) => node,
+            Some(node) => node, // read recorded by `find`
             None => panic!("couldn't find node id {} in the AST map", id)
         }
     }
 
     pub fn get_if_local(&self, id: DefId) -> Option<Node<'ast>> {
-        self.as_local_node_id(id).map(|id| self.get(id))
+        self.as_local_node_id(id).map(|id| self.get(id)) // read recorded by `get`
     }
 
     /// Retrieve the Node corresponding to `id`, returning None if
     /// cannot be found.
     pub fn find(&self, id: NodeId) -> Option<Node<'ast>> {
-        self.find_entry(id).and_then(|x| x.to_node())
+        let result = self.find_entry(id).and_then(|x| x.to_node());
+        if result.is_some() {
+            self.read(id);
+        }
+        result
     }
 
     /// Similar to get_parent, returns the parent node id or id if there is no
@@ -459,22 +526,25 @@ impl<'ast> Map<'ast> {
             _ => None
         };
         match abi {
-            Some(abi) => abi,
+            Some(abi) => {
+                self.read(id); // reveals some of the content of a node
+                abi
+            }
             None => panic!("expected foreign mod or inlined parent, found {}",
                           self.node_to_string(parent))
         }
     }
 
     pub fn get_foreign_vis(&self, id: NodeId) -> Visibility {
-        let vis = self.expect_foreign_item(id).vis;
-        match self.find(self.get_parent(id)) {
+        let vis = self.expect_foreign_item(id).vis; // read recorded by `expect_foreign_item`
+        match self.find(self.get_parent(id)) { // read recorded by `find`
             Some(NodeItem(i)) => vis.inherit_from(i.vis),
             _ => vis
         }
     }
 
     pub fn expect_item(&self, id: NodeId) -> &'ast Item {
-        match self.find(id) {
+        match self.find(id) { // read recorded by `id`
             Some(NodeItem(item)) => item,
             _ => panic!("expected item, found {}", self.node_to_string(id))
         }
@@ -521,7 +591,7 @@ impl<'ast> Map<'ast> {
     }
 
     pub fn expect_expr(&self, id: NodeId) -> &'ast Expr {
-        match self.find(id) {
+        match self.find(id) { // read recorded by find
             Some(NodeExpr(expr)) => expr,
             _ => panic!("expected expr, found {}", self.node_to_string(id))
         }
@@ -571,6 +641,11 @@ impl<'ast> Map<'ast> {
     fn with_path_next<T, F>(&self, id: NodeId, next: LinkedPath, f: F) -> T where
         F: FnOnce(PathElems) -> T,
     {
+        // This function reveals the name of the item and hence is a
+        // kind of read. This is inefficient, since it walks ancestors
+        // and we are walking them anyhow, but whatever.
+        self.read(id);
+
         let parent = self.get_parent(id);
         let parent = match self.find_entry(id) {
             Some(EntryForeignItem(..)) => {
@@ -602,6 +677,7 @@ impl<'ast> Map<'ast> {
     /// Given a node ID, get a list of attributes associated with the AST
     /// corresponding to the Node ID
     pub fn attrs(&self, id: NodeId) -> &'ast [ast::Attribute] {
+        self.read(id); // reveals attributes on the node
         let attrs = match self.find(id) {
             Some(NodeItem(i)) => Some(&i.attrs[..]),
             Some(NodeForeignItem(fi)) => Some(&fi.attrs[..]),
@@ -655,6 +731,7 @@ impl<'ast> Map<'ast> {
     }
 
     pub fn span(&self, id: NodeId) -> Span {
+        self.read(id); // reveals span from node
         self.opt_span(id)
             .unwrap_or_else(|| panic!("AstMap.span: could not find span for id {:?}", id))
     }
@@ -833,6 +910,7 @@ pub fn map_crate<'ast>(forest: &'ast mut Forest) -> Map<'ast> {
 
     Map {
         forest: forest,
+        dep_graph: forest.dep_graph.clone(),
         map: RefCell::new(map),
         definitions: RefCell::new(definitions),
     }
diff --git a/src/librustc/middle/ty/context.rs b/src/librustc/middle/ty/context.rs
index 0fd6b933ed5..071cc0cbe3d 100644
--- a/src/librustc/middle/ty/context.rs
+++ b/src/librustc/middle/ty/context.rs
@@ -509,7 +509,7 @@ impl<'tcx> ctxt<'tcx> {
     {
         let interner = RefCell::new(FnvHashMap());
         let common_types = CommonTypes::new(&arenas.type_, &interner);
-        let dep_graph = DepGraph::new(s.opts.incremental_compilation);
+        let dep_graph = map.dep_graph.clone();
         let fulfilled_predicates = traits::GlobalFulfilledPredicates::new(dep_graph.clone());
         tls::enter(ctxt {
             arenas: arenas,
diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs
index de128d839d9..68949863bfc 100644
--- a/src/librustc/session/config.rs
+++ b/src/librustc/session/config.rs
@@ -137,8 +137,13 @@ pub struct Options {
     pub no_trans: bool,
     pub error_format: ErrorOutputType,
     pub treat_err_as_bug: bool,
-    pub incremental_compilation: bool,
+
+    /// if true, build up the dep-graph
+    pub build_dep_graph: bool,
+
+    /// if true, -Z dump-dep-graph was passed to dump out the dep-graph
     pub dump_dep_graph: bool,
+
     pub no_analysis: bool,
     pub debugging_opts: DebuggingOptions,
     pub prints: Vec<PrintRequest>,
@@ -246,7 +251,7 @@ pub fn basic_options() -> Options {
         parse_only: false,
         no_trans: false,
         treat_err_as_bug: false,
-        incremental_compilation: false,
+        build_dep_graph: false,
         dump_dep_graph: false,
         no_analysis: false,
         debugging_opts: basic_debugging_options(),
@@ -1145,7 +1150,7 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options {
         parse_only: parse_only,
         no_trans: no_trans,
         treat_err_as_bug: treat_err_as_bug,
-        incremental_compilation: incremental_compilation || dump_dep_graph,
+        build_dep_graph: incremental_compilation || dump_dep_graph,
         dump_dep_graph: dump_dep_graph,
         no_analysis: no_analysis,
         debugging_opts: debugging_opts,
diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs
index 3e8457069d2..f0bd4dd83cb 100644
--- a/src/librustdoc/core.rs
+++ b/src/librustdoc/core.rs
@@ -11,6 +11,7 @@ pub use self::MaybeTyped::*;
 
 use rustc_lint;
 use rustc_driver::{driver, target_features, abort_on_err};
+use rustc::dep_graph::DepGraph;
 use rustc::session::{self, config};
 use rustc::middle::def_id::DefId;
 use rustc::middle::privacy::AccessLevels;
@@ -143,7 +144,7 @@ pub fn run_core(search_paths: SearchPaths, cfgs: Vec<String>, externs: Externs,
     let krate = driver::assign_node_ids(&sess, krate);
     // Lower ast -> hir.
     let lcx = LoweringContext::new(&sess, Some(&krate));
-    let mut hir_forest = hir_map::Forest::new(lower_crate(&lcx, &krate));
+    let mut hir_forest = hir_map::Forest::new(lower_crate(&lcx, &krate), DepGraph::new(false));
     let arenas = ty::CtxtArenas::new();
     let hir_map = driver::make_map(&sess, &mut hir_forest);
 
diff --git a/src/librustdoc/test.rs b/src/librustdoc/test.rs
index 81f984a5927..490e54f4e3d 100644
--- a/src/librustdoc/test.rs
+++ b/src/librustdoc/test.rs
@@ -25,6 +25,7 @@ use std::sync::{Arc, Mutex};
 
 use testing;
 use rustc_lint;
+use rustc::dep_graph::DepGraph;
 use rustc::front::map as hir_map;
 use rustc::session::{self, config};
 use rustc::session::config::{get_unstable_features_setting, OutputType};
@@ -99,7 +100,9 @@ pub fn run(input: &str,
 
     let opts = scrape_test_config(&krate);
 
-    let mut forest = hir_map::Forest::new(krate);
+    let dep_graph = DepGraph::new(false);
+    let _ignore = dep_graph.in_ignore();
+    let mut forest = hir_map::Forest::new(krate, dep_graph.clone());
     let map = hir_map::map_crate(&mut forest);
 
     let ctx = core::DocContext {
diff --git a/src/test/run-make/execution-engine/test.rs b/src/test/run-make/execution-engine/test.rs
index 3f9c81eac0b..13cbdfe24d6 100644
--- a/src/test/run-make/execution-engine/test.rs
+++ b/src/test/run-make/execution-engine/test.rs
@@ -26,6 +26,7 @@ use std::path::PathBuf;
 use std::rc::Rc;
 use std::thread::Builder;
 
+use rustc::dep_graph::DepGraph;
 use rustc::front::map as ast_map;
 use rustc::llvm;
 use rustc::middle::cstore::{CrateStore, LinkagePreference};
@@ -236,7 +237,8 @@ fn compile_program(input: &str, sysroot: PathBuf)
 
         let krate = driver::assign_node_ids(&sess, krate);
         let lcx = LoweringContext::new(&sess, Some(&krate));
-        let mut hir_forest = ast_map::Forest::new(lower_crate(&lcx, &krate));
+        let dep_graph = DepGraph::new(sess.opts.build_dep_graph);
+        let mut hir_forest = ast_map::Forest::new(lower_crate(&lcx, &krate), dep_graph);
         let arenas = ty::CtxtArenas::new();
         let ast_map = driver::make_map(&sess, &mut hir_forest);