add comments and remove unused code paths

Author: nikomatsakis

src/librustc/dep_graph/graph.rs

@@ -77,12 +77,37 @@ impl DepGraph {
         op()
     }
 
+    /// Starts a new dep-graph task. Dep-graph tasks are specified
+    /// using a free function (`task`) and **not** a closure -- this
+    /// is intentional because we want to exercise tight control over
+    /// what state they have access to. In particular, we want to
+    /// prevent implicit 'leaks' of tracked state into the task (which
+    /// could then be read without generating correct edges in the
+    /// dep-graph -- see the [README] for more details on the
+    /// dep-graph). To this end, the task function gets exactly two
+    /// pieces of state: the context `cx` and an argument `arg`. Both
+    /// of these bits of state must be of some type that implements
+    /// `DepGraphSafe` and hence does not leak.
+    ///
+    /// The choice of two arguments is not fundamental. One argument
+    /// would work just as well, since multiple values can be
+    /// collected using tuples. However, using two arguments works out
+    /// to be quite convenient, since it is common to need a context
+    /// (`cx`) and some argument (e.g., a `DefId` identifying what
+    /// item to process).
+    ///
+    /// For cases where you need some other number of arguments:
+    ///
+    /// - If you only need one argument, just use `()` for the `arg`
+    ///   parameter.
+    /// - If you need 3+ arguments, use a tuple for the
+    ///   `arg` parameter.
+    ///
+    /// [README]: README.md
     pub fn with_task<C, A, R>(&self, key: DepNode<DefId>, cx: C, arg: A, task: fn(C, A) -> R) -> R
         where C: DepGraphSafe, A: DepGraphSafe
     {
         let _task = self.in_task(key);
-        cx.read(self);
-        arg.read(self);
         task(cx, arg)
     }
 

src/librustc/dep_graph/mod.rs

@@ -15,7 +15,6 @@ mod edges;
 mod graph;
 mod query;
 mod raii;
-#[macro_use]
 mod safe;
 mod shadow;
 mod thread;

src/librustc/dep_graph/safe.rs

@@ -13,58 +13,51 @@ use hir::def_id::DefId;
 use syntax::ast::NodeId;
 use ty::TyCtxt;
 
-use super::graph::DepGraph;
-
-/// The `DepGraphSafe` auto trait is used to specify what kinds of
-/// values are safe to "leak" into a task.  The idea is that this
-/// should be only be implemented for things like the tcx, which will
-/// create reads in the dep-graph whenever the trait loads anything
-/// that might depend on the input program.
+/// The `DepGraphSafe` trait is used to specify what kinds of values
+/// are safe to "leak" into a task. The idea is that this should be
+/// only be implemented for things like the tcx as well as various id
+/// types, which will create reads in the dep-graph whenever the trait
+/// loads anything that might depend on the input program.
 pub trait DepGraphSafe {
-    fn read(&self, graph: &DepGraph);
 }
 
+/// A `BodyId` on its own doesn't give access to any particular state.
+/// You must fetch the state from the various maps or generate
+/// on-demand queries, all of which create reads.
 impl DepGraphSafe for BodyId {
-    fn read(&self, _graph: &DepGraph) {
-        // a BodyId on its own doesn't give access to any particular state
-    }
 }
 
+/// A `NodeId` on its own doesn't give access to any particular state.
+/// You must fetch the state from the various maps or generate
+/// on-demand queries, all of which create reads.
 impl DepGraphSafe for NodeId {
-    fn read(&self, _graph: &DepGraph) {
-        // a DefId doesn't give any particular state
-    }
 }
 
+/// A `DefId` on its own doesn't give access to any particular state.
+/// You must fetch the state from the various maps or generate
+/// on-demand queries, all of which create reads.
 impl DepGraphSafe for DefId {
-    fn read(&self, _graph: &DepGraph) {
-        // a DefId doesn't give any particular state
-    }
 }
 
+/// The type context itself can be used to access all kinds of tracked
+/// state, but those accesses should always generate read events.
 impl<'a, 'gcx, 'tcx> DepGraphSafe for TyCtxt<'a, 'gcx, 'tcx> {
-    fn read(&self, _graph: &DepGraph) {
-    }
 }
 
+/// Tuples make it easy to build up state.
 impl<A, B> DepGraphSafe for (A, B)
     where A: DepGraphSafe, B: DepGraphSafe
 {
-    fn read(&self, graph: &DepGraph) {
-        self.0.read(graph);
-        self.1.read(graph);
-    }
 }
 
+/// No data here! :)
 impl DepGraphSafe for () {
-    fn read(&self, _graph: &DepGraph) {
-    }
 }
 
-/// A convenient override. We should phase out usage of this over
-/// time.
+/// A convenient override that lets you pass arbitrary state into a
+/// task. Every use should be accompanied by a comment explaining why
+/// it makes sense (or how it could be refactored away in the future).
 pub struct AssertDepGraphSafe<T>(pub T);
+
 impl<T> DepGraphSafe for AssertDepGraphSafe<T> {
-    fn read(&self, _graph: &DepGraph) {
-    }
 }

src/librustc/lib.rs

@@ -70,7 +70,6 @@ mod macros;
 pub mod diagnostics;
 
 pub mod cfg;
-#[macro_use]
 pub mod dep_graph;
 pub mod hir;
 pub mod infer;

src/librustc_trans/context.rs

@@ -276,8 +276,6 @@ pub struct CrateContext<'a, 'tcx: 'a> {
 }
 
 impl<'a, 'tcx> DepGraphSafe for CrateContext<'a, 'tcx> {
-    fn read(&self, _graph: &DepGraph) {
-    }
 }
 
 pub struct CrateContextIterator<'a, 'tcx: 'a> {