Convert Dependency Graph Tasks from in_tasks to with_tasks

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,38 @@ impl DepGraph {
         op()
     }
 
-    pub fn with_task<OP,R>(&self, key: DepNode<DefId>, op: OP) -> R
-        where OP: FnOnce() -> R
+    /// 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);
-        op()
+        task(cx, arg)
     }
 
     pub fn read(&self, v: DepNode<DefId>) {

src/librustc/dep_graph/mod.rs

@@ -15,6 +15,7 @@ mod edges;
 mod graph;
 mod query;
 mod raii;
+mod safe;
 mod shadow;
 mod thread;
 mod visit;
@@ -25,6 +26,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,92 @@
+// 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 hir::map::Map;
+use session::Session;
+use std::fmt::Debug;
+use syntax::ast::NodeId;
+use ty::TyCtxt;
+
+use super::{DepGraph, DepNode};
+
+/// 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 {
+}
+
+/// 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 {
+}
+
+/// 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 {
+}
+
+/// 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 {
+}
+
+/// 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> {
+}
+
+impl<'a, T> DepGraphSafe for &'a T
+    where T: DepGraphSafe
+{
+}
+
+/// The session gives access to lots of state, but it generates read events.
+impl DepGraphSafe for Session {
+}
+
+/// The map gives access to lots of state, but it generates read events.
+impl<'hir> DepGraphSafe for Map<'hir> {
+}
+
+/// The dep-graph better be safe to thread around =)
+impl DepGraphSafe for DepGraph {
+}
+
+/// DepNodes do not give access to anything in particular, other than
+/// def-ids.
+impl<D> DepGraphSafe for DepNode<D>
+    where D: DepGraphSafe + Clone + Debug
+{
+}
+
+/// Tuples make it easy to build up state.
+impl<A, B> DepGraphSafe for (A, B)
+    where A: DepGraphSafe, B: DepGraphSafe
+{
+}
+
+/// No data here! :)
+impl DepGraphSafe for () {
+}
+
+/// 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> {
+}

src/librustc/middle/dead.rs

@@ -12,7 +12,7 @@
 // closely. The idea is that all reachable symbols are live, codes called
 // from live codes are live, and everything else is dead.
 
-use dep_graph::DepNode;
+use dep_graph::{DepNode, AssertDepGraphSafe};
 use hir::map as hir_map;
 use hir::{self, PatKind};
 use hir::intravisit::{self, Visitor, NestedVisitorMap};
@@ -594,9 +594,15 @@ impl<'a, 'tcx> Visitor<'tcx> for DeadVisitor<'a, 'tcx> {
 
 pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
                              access_levels: &privacy::AccessLevels) {
-    let _task = tcx.dep_graph.in_task(DepNode::DeadCheck);
-    let krate = tcx.hir.krate();
-    let live_symbols = find_live(tcx, access_levels, krate);
-    let mut visitor = DeadVisitor { tcx: tcx, live_symbols: live_symbols };
-    intravisit::walk_crate(&mut visitor, krate);
+    tcx.dep_graph.with_task(DepNode::DeadCheck, tcx,
+                            AssertDepGraphSafe(access_levels), check_crate_task);
+
+    fn check_crate_task<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
+                                  AssertDepGraphSafe(access_levels):
+                                      AssertDepGraphSafe<&privacy::AccessLevels>) {
+        let krate = tcx.hir.krate();
+        let live_symbols = find_live(tcx, access_levels, krate);
+        let mut visitor = DeadVisitor { tcx: tcx, live_symbols: live_symbols };
+        intravisit::walk_crate(&mut visitor, krate);
+    }
 }

src/librustc/middle/effect.rs

@@ -241,13 +241,15 @@ impl<'a, 'tcx> Visitor<'tcx> for EffectCheckVisitor<'a, 'tcx> {
 }
 
 pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
-    let _task = tcx.dep_graph.in_task(DepNode::EffectCheck);
+    tcx.dep_graph.with_task(DepNode::EffectCheck, tcx, (), check_crate_task);
 
-    let mut visitor = EffectCheckVisitor {
-        tcx: tcx,
-        tables: &ty::TypeckTables::empty(),
-        unsafe_context: UnsafeContext::new(SafeContext),
-    };
+    fn check_crate_task<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, (): ()) {
+        let mut visitor = EffectCheckVisitor {
+            tcx: tcx,
+            tables: &ty::TypeckTables::empty(),
+            unsafe_context: UnsafeContext::new(SafeContext),
+        };
 
-    tcx.hir.krate().visit_all_item_likes(&mut visitor.as_deep_visitor());
+        tcx.hir.krate().visit_all_item_likes(&mut visitor.as_deep_visitor());
+    }
 }

src/librustc/middle/entry.rs

@@ -57,34 +57,36 @@ impl<'a, 'tcx> ItemLikeVisitor<'tcx> for EntryContext<'a, 'tcx> {
 }
 
 pub fn find_entry_point(session: &Session, hir_map: &hir_map::Map) {
-    let _task = hir_map.dep_graph.in_task(DepNode::EntryPoint);
-
-    let any_exe = session.crate_types.borrow().iter().any(|ty| {
-        *ty == config::CrateTypeExecutable
-    });
-    if !any_exe {
-        // No need to find a main function
-        return
-    }
+    hir_map.dep_graph.with_task(DepNode::EntryPoint, session, hir_map, find_entry_point_task);
+
+    fn find_entry_point_task(session: &Session, hir_map: &hir_map::Map) {
+        let any_exe = session.crate_types.borrow().iter().any(|ty| {
+            *ty == config::CrateTypeExecutable
+        });
+        if !any_exe {
+            // No need to find a main function
+            return
+        }
 
-    // If the user wants no main function at all, then stop here.
-    if attr::contains_name(&hir_map.krate().attrs, "no_main") {
-        session.entry_type.set(Some(config::EntryNone));
-        return
-    }
+        // If the user wants no main function at all, then stop here.
+        if attr::contains_name(&hir_map.krate().attrs, "no_main") {
+            session.entry_type.set(Some(config::EntryNone));
+            return
+        }
 
-    let mut ctxt = EntryContext {
-        session: session,
-        map: hir_map,
-        main_fn: None,
-        attr_main_fn: None,
-        start_fn: None,
-        non_main_fns: Vec::new(),
-    };
+        let mut ctxt = EntryContext {
+            session: session,
+            map: hir_map,
+            main_fn: None,
+            attr_main_fn: None,
+            start_fn: None,
+            non_main_fns: Vec::new(),
+        };
 
-    hir_map.krate().visit_all_item_likes(&mut ctxt);
+        hir_map.krate().visit_all_item_likes(&mut ctxt);
 
-    configure_main(&mut ctxt);
+        configure_main(&mut ctxt);
+    }
 }
 
 // Beware, this is duplicated in libsyntax/entry.rs, make sure to keep

src/librustc/middle/lang_items.rs

@@ -236,13 +236,16 @@ pub fn extract(attrs: &[ast::Attribute]) -> Option<Symbol> {
 pub fn collect_language_items(session: &Session,
                               map: &hir_map::Map)
                               -> LanguageItems {
-    let _task = map.dep_graph.in_task(DepNode::CollectLanguageItems);
-    let krate: &hir::Crate = map.krate();
-    let mut collector = LanguageItemCollector::new(session, map);
-    collector.collect(krate);
-    let LanguageItemCollector { mut items, .. } = collector;
-    weak_lang_items::check_crate(krate, session, &mut items);
-    items
+    return map.dep_graph.with_task(DepNode::CollectLanguageItems, session, map, collect_task);
+
+    fn collect_task(session: &Session, map: &hir_map::Map) -> LanguageItems {
+        let krate: &hir::Crate = map.krate();
+        let mut collector = LanguageItemCollector::new(session, map);
+        collector.collect(krate);
+        let LanguageItemCollector { mut items, .. } = collector;
+        weak_lang_items::check_crate(krate, session, &mut items);
+        items
+    }
 }
 
 // End of the macro

src/librustc/middle/liveness.rs

@@ -196,9 +196,11 @@ impl<'a, 'tcx> Visitor<'tcx> for IrMaps<'a, 'tcx> {
 }
 
 pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
-    let _task = tcx.dep_graph.in_task(DepNode::Liveness);
-    tcx.hir.krate().visit_all_item_likes(&mut IrMaps::new(tcx).as_deep_visitor());
-    tcx.sess.abort_if_errors();
+    tcx.dep_graph.with_task(DepNode::Liveness, tcx, (), check_crate_task);
+    fn check_crate_task<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, (): ()) {
+        tcx.hir.krate().visit_all_item_likes(&mut IrMaps::new(tcx).as_deep_visitor());
+        tcx.sess.abort_if_errors();
+    }
 }
 
 impl fmt::Debug for LiveNode {