diff --git a/src/librustc/dep_graph/graph.rs b/src/librustc/dep_graph/graph.rs index e6736ccafbad9..8be5d4327e72e 100644 --- a/src/librustc/dep_graph/graph.rs +++ b/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(&self, key: DepNode, 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(&self, key: DepNode, 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) { diff --git a/src/librustc/dep_graph/mod.rs b/src/librustc/dep_graph/mod.rs index 7331756f35b8e..a9f0a44e4208c 100644 --- a/src/librustc/dep_graph/mod.rs +++ b/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; diff --git a/src/librustc/dep_graph/safe.rs b/src/librustc/dep_graph/safe.rs new file mode 100644 index 0000000000000..f85f0338ed997 --- /dev/null +++ b/src/librustc/dep_graph/safe.rs @@ -0,0 +1,63 @@ +// 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 or the MIT license +// , 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; + +/// 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> { +} + +/// Tuples make it easy to build up state. +impl 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(pub T); + +impl DepGraphSafe for AssertDepGraphSafe { +} diff --git a/src/librustc_borrowck/borrowck/mod.rs b/src/librustc_borrowck/borrowck/mod.rs index 47b614a81ae25..b441a231874a6 100644 --- a/src/librustc_borrowck/borrowck/mod.rs +++ b/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. diff --git a/src/librustc_incremental/persist/load.rs b/src/librustc_incremental/persist/load.rs index 03411e01a5798..2789250649674 100644 --- a/src/librustc_incremental/persist/load.rs +++ b/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 + } } } diff --git a/src/librustc_mir/mir_map.rs b/src/librustc_mir/mir_map.rs index a41489ff89ff4..58f23a5c81bd7 100644 --- a/src/librustc_mir/mir_map.rs +++ b/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) { diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index 36f6fa7643909..1b43491e73c8f 100644 --- a/src/librustc_trans/base.rs +++ b/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>>) { + // 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>>) { + // 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); diff --git a/src/librustc_trans/context.rs b/src/librustc_trans/context.rs index d5f7549ece07b..52851ea995d4b 100644 --- a/src/librustc_trans/context.rs +++ b/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,9 @@ pub struct CrateContext<'a, 'tcx: 'a> { index: usize, } +impl<'a, 'tcx> DepGraphSafe for CrateContext<'a, 'tcx> { +} + pub struct CrateContextIterator<'a, 'tcx: 'a> { shared: &'a SharedCrateContext<'a, 'tcx>, local_ccxs: &'a [LocalCrateContext<'tcx>], diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 2b9ccf6d3e246..5a582a523ea1c 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/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) { diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index db7cf3c000ba4..2417745571910 100644 --- a/src/librustc_typeck/collect.rs +++ b/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(&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);