isolate dep-graph tasks

A task function is now given as a `fn` pointer to ensure that it carries
no state. Each fn can take two arguments, because that worked out to be
convenient -- these two arguments must be of some type that is
`DepGraphSafe`, a new trait that is intended to prevent "leaking"
information into the task that was derived from tracked state.

This intentionally leaves `DepGraph::in_task()`, the more common form,
alone. Eventually all uses of `DepGraph::in_task()` should be ported
to `with_task()`, but I wanted to start with a smaller subset.

Originally I wanted to use closures bound by an auto trait, but that
approach has some limitations:

- the trait cannot have a `read()` method; since the current method
  is unused, that may not be a problem.
- more importantly, we would want the auto trait to be "undefined" for all types
  *by default* -- that is, this use case doesn't really fit the typical
  auto trait scenario. For example, imagine that there is a `u32` loaded
  out of a `hir::Node` -- we don't really want to be passing that
  `u32` into the task!

Author: nikomatsakis

src/librustc/dep_graph/graph.rs

@@ -18,6 +18,7 @@ use std::sync::Arc;
 use super::dep_node::{DepNode, WorkProductId};
 use super::query::DepGraphQuery;
 use super::raii;
+use super::safe::DepGraphSafe;
 use super::thread::{DepGraphThreadData, DepMessage};
 
 #[derive(Clone)]
@@ -76,11 +77,13 @@ impl DepGraph {
         op()
     }
 
-    pub fn with_task<OP,R>(&self, key: DepNode<DefId>, op: OP) -> R
-        where OP: FnOnce() -> R
+    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);
-        op()
+        cx.read(self);
+        arg.read(self);
+        task(cx, arg)
     }
 
     pub fn read(&self, v: DepNode<DefId>) {

src/librustc/dep_graph/mod.rs

@@ -15,6 +15,8 @@ mod edges;
 mod graph;
 mod query;
 mod raii;
+#[macro_use]
+mod safe;
 mod shadow;
 mod thread;
 mod visit;
@@ -25,6 +27,8 @@ pub use self::dep_node::WorkProductId;
 pub use self::graph::DepGraph;
 pub use self::graph::WorkProduct;
 pub use self::query::DepGraphQuery;
+pub use self::safe::AssertDepGraphSafe;
+pub use self::safe::DepGraphSafe;
 pub use self::visit::visit_all_bodies_in_krate;
 pub use self::visit::visit_all_item_likes_in_krate;
 pub use self::raii::DepTask;

src/librustc/dep_graph/safe.rs

@@ -0,0 +1,70 @@
+// Copyright 2012-2015 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.
+
+use hir::BodyId;
+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.
+pub trait DepGraphSafe {
+    fn read(&self, graph: &DepGraph);
+}
+
+impl DepGraphSafe for BodyId {
+    fn read(&self, _graph: &DepGraph) {
+        // a BodyId on its own doesn't give access to any particular state
+    }
+}
+
+impl DepGraphSafe for NodeId {
+    fn read(&self, _graph: &DepGraph) {
+        // a DefId doesn't give any particular state
+    }
+}
+
+impl DepGraphSafe for DefId {
+    fn read(&self, _graph: &DepGraph) {
+        // a DefId doesn't give any particular state
+    }
+}
+
+impl<'a, 'gcx, 'tcx> DepGraphSafe for TyCtxt<'a, 'gcx, 'tcx> {
+    fn read(&self, _graph: &DepGraph) {
+    }
+}
+
+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);
+    }
+}
+
+impl DepGraphSafe for () {
+    fn read(&self, _graph: &DepGraph) {
+    }
+}
+
+/// A convenient override. We should phase out usage of this over
+/// time.
+pub struct AssertDepGraphSafe<T>(pub T);
+impl<T> DepGraphSafe for AssertDepGraphSafe<T> {
+    fn read(&self, _graph: &DepGraph) {
+    }
+}

src/librustc/lib.rs

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

src/librustc_borrowck/borrowck/mod.rs

@@ -61,13 +61,16 @@ pub struct LoanDataFlowOperator;
 pub type LoanDataFlow<'a, 'tcx> = DataFlowContext<'a, 'tcx, LoanDataFlowOperator>;
 
 pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
-    tcx.dep_graph.with_task(DepNode::BorrowCheckKrate, || {
+    tcx.dep_graph.with_task(DepNode::BorrowCheckKrate, tcx, (), check_crate_task);
+
+    fn check_crate_task<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, (): ()) {
         tcx.visit_all_bodies_in_krate(|body_owner_def_id, body_id| {
-            tcx.dep_graph.with_task(DepNode::BorrowCheck(body_owner_def_id), || {
-                borrowck_fn(tcx, body_id);
-            });
+            tcx.dep_graph.with_task(DepNode::BorrowCheck(body_owner_def_id),
+                                    tcx,
+                                    body_id,
+                                    borrowck_fn);
         });
-    });
+    }
 }
 
 /// Collection of conclusions determined via borrow checker analyses.

src/librustc_incremental/persist/load.rs

@@ -192,7 +192,11 @@ pub fn decode_dep_graph<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
                 clean_work_products.insert(wp.clone());
             }
 
-            tcx.dep_graph.with_task(n, || ()); // create the node with no inputs
+            tcx.dep_graph.with_task(n, (), (), create_node);
+
+            fn create_node((): (), (): ()) {
+                // just create the node with no inputs
+            }
         }
     }
 

src/librustc_mir/mir_map.rs

@@ -38,11 +38,13 @@ use std::cell::RefCell;
 use std::mem;
 
 pub fn build_mir_for_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
-    tcx.dep_graph.with_task(DepNode::MirKrate, || {
+    tcx.dep_graph.with_task(DepNode::MirKrate, tcx, (), build_mir_for_crate_task);
+
+    fn build_mir_for_crate_task<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, (): ()) {
         tcx.visit_all_bodies_in_krate(|body_owner_def_id, _body_id| {
             tcx.item_mir(body_owner_def_id);
         });
-    });
+    }
 }
 
 pub fn provide(providers: &mut Providers) {

src/librustc_trans/base.rs

@@ -41,7 +41,7 @@ use rustc::mir::tcx::LvalueTy;
 use rustc::traits;
 use rustc::ty::{self, Ty, TyCtxt};
 use rustc::ty::adjustment::CustomCoerceUnsized;
-use rustc::dep_graph::{DepNode, WorkProduct};
+use rustc::dep_graph::{AssertDepGraphSafe, DepNode, WorkProduct};
 use rustc::hir::map as hir_map;
 use rustc::util::common::time;
 use session::config::{self, NoDebugInfo};
@@ -1211,21 +1211,40 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
 
     // Instantiate translation items without filling out definitions yet...
     for ccx in crate_context_list.iter_need_trans() {
-        let cgu = ccx.codegen_unit();
-        let trans_items = cgu.items_in_deterministic_order(tcx, &symbol_map);
-
-        tcx.dep_graph.with_task(cgu.work_product_dep_node(), || {
+        let dep_node = ccx.codegen_unit().work_product_dep_node();
+        tcx.dep_graph.with_task(dep_node,
+                                ccx,
+                                AssertDepGraphSafe(symbol_map.clone()),
+                                trans_decl_task);
+
+        fn trans_decl_task<'a, 'tcx>(ccx: CrateContext<'a, 'tcx>,
+                                     symbol_map: AssertDepGraphSafe<Rc<SymbolMap<'tcx>>>) {
+            // FIXME(#40304): Instead of this, the symbol-map should be an
+            // on-demand thing that we compute.
+            let AssertDepGraphSafe(symbol_map) = symbol_map;
+            let cgu = ccx.codegen_unit();
+            let trans_items = cgu.items_in_deterministic_order(ccx.tcx(), &symbol_map);
             for (trans_item, linkage) in trans_items {
                 trans_item.predefine(&ccx, linkage);
             }
-        });
+        }
     }
 
     // ... and now that we have everything pre-defined, fill out those definitions.
     for ccx in crate_context_list.iter_need_trans() {
-        let cgu = ccx.codegen_unit();
-        let trans_items = cgu.items_in_deterministic_order(tcx, &symbol_map);
-        tcx.dep_graph.with_task(cgu.work_product_dep_node(), || {
+        let dep_node = ccx.codegen_unit().work_product_dep_node();
+        tcx.dep_graph.with_task(dep_node,
+                                ccx,
+                                AssertDepGraphSafe(symbol_map.clone()),
+                                trans_def_task);
+
+        fn trans_def_task<'a, 'tcx>(ccx: CrateContext<'a, 'tcx>,
+                                    symbol_map: AssertDepGraphSafe<Rc<SymbolMap<'tcx>>>) {
+            // FIXME(#40304): Instead of this, the symbol-map should be an
+            // on-demand thing that we compute.
+            let AssertDepGraphSafe(symbol_map) = symbol_map;
+            let cgu = ccx.codegen_unit();
+            let trans_items = cgu.items_in_deterministic_order(ccx.tcx(), &symbol_map);
             for (trans_item, _) in trans_items {
                 trans_item.define(&ccx);
             }
@@ -1247,7 +1266,7 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
             if ccx.sess().opts.debuginfo != NoDebugInfo {
                 debuginfo::finalize(&ccx);
             }
-        });
+        }
     }
 
     symbol_names_test::report_symbol_names(&shared_ccx);

src/librustc_trans/context.rs

@@ -10,7 +10,8 @@
 
 use llvm;
 use llvm::{ContextRef, ModuleRef, ValueRef};
-use rustc::dep_graph::{DepGraph, DepNode, DepTrackingMap, DepTrackingMapConfig, WorkProduct};
+use rustc::dep_graph::{DepGraph, DepGraphSafe, DepNode, DepTrackingMap,
+                       DepTrackingMapConfig, WorkProduct};
 use middle::cstore::LinkMeta;
 use rustc::hir;
 use rustc::hir::def::ExportMap;
@@ -274,6 +275,11 @@ pub struct CrateContext<'a, 'tcx: 'a> {
     index: usize,
 }
 
+impl<'a, 'tcx> DepGraphSafe for CrateContext<'a, 'tcx> {
+    fn read(&self, _graph: &DepGraph) {
+    }
+}
+
 pub struct CrateContextIterator<'a, 'tcx: 'a> {
     shared: &'a SharedCrateContext<'a, 'tcx>,
     local_ccxs: &'a [LocalCrateContext<'tcx>],

src/librustc_typeck/check/mod.rs

@@ -539,13 +539,15 @@ pub fn check_item_types<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) -> CompileResult
 }
 
 pub fn check_item_bodies<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) -> CompileResult {
-    tcx.sess.track_errors(|| {
-        tcx.dep_graph.with_task(DepNode::TypeckBodiesKrate, || {
-            tcx.visit_all_bodies_in_krate(|body_owner_def_id, _body_id| {
-                tcx.item_tables(body_owner_def_id);
-            });
+    return tcx.sess.track_errors(|| {
+        tcx.dep_graph.with_task(DepNode::TypeckBodiesKrate, tcx, (), check_item_bodies_task);
+    });
+
+    fn check_item_bodies_task<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, (): ()) {
+        tcx.visit_all_bodies_in_krate(|body_owner_def_id, _body_id| {
+            tcx.item_tables(body_owner_def_id);
         });
-    })
+    }
 }
 
 pub fn provide(providers: &mut Providers) {

src/librustc_typeck/collect.rs

@@ -165,15 +165,10 @@ impl<'a, 'tcx> CollectItemTypesVisitor<'a, 'tcx> {
     /// 4. This is added by the code in `visit_expr` when we write to `item_types`.
     /// 5. This is added by the code in `convert_item` when we write to `item_types`;
     ///    note that this write occurs inside the `CollectItemSig` task.
-    /// 6. Added by explicit `read` below
-    fn with_collect_item_sig<OP>(&self, id: ast::NodeId, op: OP)
-        where OP: FnOnce()
-    {
+    /// 6. Added by reads from within `op`.
+    fn with_collect_item_sig(&self, id: ast::NodeId, op: fn(TyCtxt<'a, 'tcx, 'tcx>, ast::NodeId)) {
         let def_id = self.tcx.hir.local_def_id(id);
-        self.tcx.dep_graph.with_task(DepNode::CollectItemSig(def_id), || {
-            self.tcx.hir.read(id);
-            op();
-        });
+        self.tcx.dep_graph.with_task(DepNode::CollectItemSig(def_id), self.tcx, id, op);
     }
 }
 
@@ -183,7 +178,7 @@ impl<'a, 'tcx> Visitor<'tcx> for CollectItemTypesVisitor<'a, 'tcx> {
     }
 
     fn visit_item(&mut self, item: &'tcx hir::Item) {
-        self.with_collect_item_sig(item.id, || convert_item(self.tcx, item));
+        self.with_collect_item_sig(item.id, convert_item);
         intravisit::walk_item(self, item);
     }
 
@@ -216,16 +211,12 @@ impl<'a, 'tcx> Visitor<'tcx> for CollectItemTypesVisitor<'a, 'tcx> {
     }
 
     fn visit_trait_item(&mut self, trait_item: &'tcx hir::TraitItem) {
-        self.with_collect_item_sig(trait_item.id, || {
-            convert_trait_item(self.tcx, trait_item)
-        });
+        self.with_collect_item_sig(trait_item.id, convert_trait_item);
         intravisit::walk_trait_item(self, trait_item);
     }
 
     fn visit_impl_item(&mut self, impl_item: &'tcx hir::ImplItem) {
-        self.with_collect_item_sig(impl_item.id, || {
-            convert_impl_item(self.tcx, impl_item)
-        });
+        self.with_collect_item_sig(impl_item.id, convert_impl_item);
         intravisit::walk_impl_item(self, impl_item);
     }
 }
@@ -493,9 +484,10 @@ fn ensure_no_ty_param_bounds(tcx: TyCtxt,
     }
 }
 
-fn convert_item<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, it: &hir::Item) {
+fn convert_item<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, item_id: ast::NodeId) {
+    let it = tcx.hir.expect_item(item_id);
     debug!("convert: item {} with id {}", it.name, it.id);
-    let def_id = tcx.hir.local_def_id(it.id);
+    let def_id = tcx.hir.local_def_id(item_id);
     match it.node {
         // These don't define types.
         hir::ItemExternCrate(_) | hir::ItemUse(..) | hir::ItemMod(_) => {
@@ -560,7 +552,8 @@ fn convert_item<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, it: &hir::Item) {
     }
 }
 
-fn convert_trait_item<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, trait_item: &hir::TraitItem) {
+fn convert_trait_item<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, trait_item_id: ast::NodeId) {
+    let trait_item = tcx.hir.expect_trait_item(trait_item_id);
     let def_id = tcx.hir.local_def_id(trait_item.id);
     tcx.item_generics(def_id);
 
@@ -577,8 +570,8 @@ fn convert_trait_item<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, trait_item: &hir::T
     tcx.item_predicates(def_id);
 }
 
-fn convert_impl_item<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, impl_item: &hir::ImplItem) {
-    let def_id = tcx.hir.local_def_id(impl_item.id);
+fn convert_impl_item<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, impl_item_id: ast::NodeId) {
+    let def_id = tcx.hir.local_def_id(impl_item_id);
     tcx.item_generics(def_id);
     tcx.item_type(def_id);
     tcx.item_predicates(def_id);