diff --git a/src/librustc/dep_graph/mod.rs b/src/librustc/dep_graph/mod.rs index 9bf0a79115e78..3bb73ecca2c43 100644 --- a/src/librustc/dep_graph/mod.rs +++ b/src/librustc/dep_graph/mod.rs @@ -11,7 +11,6 @@ use self::thread::{DepGraphThreadData, DepMessage}; use middle::def_id::DefId; use middle::ty; -use middle::ty::fast_reject::SimplifiedType; use rustc_front::hir; use rustc_front::intravisit::Visitor; use std::rc::Rc; @@ -102,7 +101,7 @@ pub enum DepNode { // which would yield an overly conservative dep-graph. TraitItems(DefId), ReprHints(DefId), - TraitSelect(DefId, Option), + TraitSelect(DefId), } #[derive(Clone)] diff --git a/src/librustc/middle/traits/fulfill.rs b/src/librustc/middle/traits/fulfill.rs index bdf1c4645c0ed..d28504db585c7 100644 --- a/src/librustc/middle/traits/fulfill.rs +++ b/src/librustc/middle/traits/fulfill.rs @@ -8,6 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +use dep_graph::DepGraph; use middle::infer::InferCtxt; use middle::ty::{self, Ty, TypeFoldable}; use rustc_data_structures::obligation_forest::{Backtrace, ObligationForest, Error}; @@ -30,7 +31,12 @@ use super::select::SelectionContext; use super::Unimplemented; use super::util::predicate_for_builtin_bound; -pub struct FulfilledPredicates<'tcx> { +pub struct GlobalFulfilledPredicates<'tcx> { + set: FnvHashSet>, + dep_graph: DepGraph, +} + +pub struct LocalFulfilledPredicates<'tcx> { set: FnvHashSet> } @@ -56,7 +62,7 @@ pub struct FulfillmentContext<'tcx> { // initially-distinct type variables are unified after being // inserted. Deduplicating the predicate set on selection had a // significant performance cost the last time I checked. - duplicate_set: FulfilledPredicates<'tcx>, + duplicate_set: LocalFulfilledPredicates<'tcx>, // A list of all obligations that have been registered with this // fulfillment context. @@ -106,7 +112,7 @@ impl<'tcx> FulfillmentContext<'tcx> { /// Creates a new fulfillment context. pub fn new() -> FulfillmentContext<'tcx> { FulfillmentContext { - duplicate_set: FulfilledPredicates::new(), + duplicate_set: LocalFulfilledPredicates::new(), predicates: ObligationForest::new(), region_obligations: NodeMap(), } @@ -240,7 +246,7 @@ impl<'tcx> FulfillmentContext<'tcx> { // local cache). This is because the tcx cache maintains the // invariant that it only contains things that have been // proven, and we have not yet proven that `predicate` holds. - if predicate.is_global() && tcx.fulfilled_predicates.borrow().is_duplicate(predicate) { + if tcx.fulfilled_predicates.borrow().check_duplicate(predicate) { return true; } @@ -283,10 +289,7 @@ impl<'tcx> FulfillmentContext<'tcx> { // these are obligations that were proven to be true. for pending_obligation in outcome.completed { let predicate = &pending_obligation.obligation.predicate; - if predicate.is_global() { - selcx.tcx().fulfilled_predicates.borrow_mut() - .is_duplicate_or_add(predicate); - } + selcx.tcx().fulfilled_predicates.borrow_mut().add_if_global(predicate); } errors.extend( @@ -329,6 +332,8 @@ fn process_predicate<'a,'tcx>(selcx: &mut SelectionContext<'a,'tcx>, // However, this is a touch tricky, so I'm doing something // a bit hackier for now so that the `huge-struct.rs` passes. + let tcx = selcx.tcx(); + let retain_vec: Vec<_> = { let mut dedup = FnvHashSet(); v.iter() @@ -336,10 +341,7 @@ fn process_predicate<'a,'tcx>(selcx: &mut SelectionContext<'a,'tcx>, // Screen out obligations that we know globally // are true. This should really be the DAG check // mentioned above. - if - o.predicate.is_global() && - selcx.tcx().fulfilled_predicates.borrow().is_duplicate(&o.predicate) - { + if tcx.fulfilled_predicates.borrow().check_duplicate(&o.predicate) { return false; } @@ -611,22 +613,62 @@ fn register_region_obligation<'tcx>(t_a: Ty<'tcx>, } -impl<'tcx> FulfilledPredicates<'tcx> { - pub fn new() -> FulfilledPredicates<'tcx> { - FulfilledPredicates { +impl<'tcx> LocalFulfilledPredicates<'tcx> { + pub fn new() -> LocalFulfilledPredicates<'tcx> { + LocalFulfilledPredicates { set: FnvHashSet() } } - pub fn is_duplicate(&self, key: &ty::Predicate<'tcx>) -> bool { - self.set.contains(key) - } - fn is_duplicate_or_add(&mut self, key: &ty::Predicate<'tcx>) -> bool { + // For a `LocalFulfilledPredicates`, if we find a match, we + // don't need to add a read edge to the dep-graph. This is + // because it means that the predicate has already been + // considered by this `FulfillmentContext`, and hence the + // containing task will already have an edge. (Here we are + // assuming each `FulfillmentContext` only gets used from one + // task; but to do otherwise makes no sense) !self.set.insert(key.clone()) } } +impl<'tcx> GlobalFulfilledPredicates<'tcx> { + pub fn new(dep_graph: DepGraph) -> GlobalFulfilledPredicates<'tcx> { + GlobalFulfilledPredicates { + set: FnvHashSet(), + dep_graph: dep_graph, + } + } + + pub fn check_duplicate(&self, key: &ty::Predicate<'tcx>) -> bool { + if let ty::Predicate::Trait(ref data) = *key { + // For the global predicate registry, when we find a match, it + // may have been computed by some other task, so we want to + // add a read from the node corresponding to the predicate + // processing to make sure we get the transitive dependencies. + if self.set.contains(data) { + debug_assert!(data.is_global()); + self.dep_graph.read(data.dep_node()); + return true; + } + } + + return false; + } + + fn add_if_global(&mut self, key: &ty::Predicate<'tcx>) { + if let ty::Predicate::Trait(ref data) = *key { + // We only add things to the global predicate registry + // after the current task has proved them, and hence + // already has the required read edges, so we don't need + // to add any more edges here. + if data.is_global() { + self.set.insert(data.clone()); + } + } + } +} + fn to_fulfillment_error<'tcx>( error: Error, FulfillmentErrorCode<'tcx>>) -> FulfillmentError<'tcx> diff --git a/src/librustc/middle/traits/mod.rs b/src/librustc/middle/traits/mod.rs index 8fecffcea9fe4..f0ff0380aaa31 100644 --- a/src/librustc/middle/traits/mod.rs +++ b/src/librustc/middle/traits/mod.rs @@ -15,12 +15,10 @@ pub use self::FulfillmentErrorCode::*; pub use self::Vtable::*; pub use self::ObligationCauseCode::*; -use dep_graph::DepNode; use middle::def_id::DefId; use middle::free_region::FreeRegionMap; use middle::subst; use middle::ty::{self, Ty, TypeFoldable}; -use middle::ty::fast_reject; use middle::infer::{self, fixup_err_to_string, InferCtxt}; use std::rc::Rc; @@ -37,7 +35,7 @@ pub use self::error_reporting::report_object_safety_error; pub use self::coherence::orphan_check; pub use self::coherence::overlapping_impls; pub use self::coherence::OrphanCheckErr; -pub use self::fulfill::{FulfillmentContext, FulfilledPredicates, RegionObligation}; +pub use self::fulfill::{FulfillmentContext, GlobalFulfilledPredicates, RegionObligation}; pub use self::project::MismatchedProjectionTypes; pub use self::project::normalize; pub use self::project::Normalized; @@ -617,18 +615,6 @@ impl<'tcx> FulfillmentError<'tcx> { } impl<'tcx> TraitObligation<'tcx> { - /// Creates the dep-node for selecting/evaluating this trait reference. - fn dep_node(&self, tcx: &ty::ctxt<'tcx>) -> DepNode { - let simplified_ty = - fast_reject::simplify_type(tcx, - self.predicate.skip_binder().self_ty(), // (*) - true); - - // (*) skip_binder is ok because `simplify_type` doesn't care about regions - - DepNode::TraitSelect(self.predicate.def_id(), simplified_ty) - } - fn self_ty(&self) -> ty::Binder> { ty::Binder(self.predicate.skip_binder().self_ty()) } diff --git a/src/librustc/middle/traits/select.rs b/src/librustc/middle/traits/select.rs index 75992b6849b01..61418734b674b 100644 --- a/src/librustc/middle/traits/select.rs +++ b/src/librustc/middle/traits/select.rs @@ -307,7 +307,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { debug!("select({:?})", obligation); assert!(!obligation.predicate.has_escaping_regions()); - let dep_node = obligation.dep_node(self.tcx()); + let dep_node = obligation.predicate.dep_node(); let _task = self.tcx().dep_graph.in_task(dep_node); let stack = self.push_stack(TraitObligationStackList::empty(), obligation); @@ -462,7 +462,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { // have been proven elsewhere. This cache only contains // predicates that are global in scope and hence unaffected by // the current environment. - if self.tcx().fulfilled_predicates.borrow().is_duplicate(&obligation.predicate) { + if self.tcx().fulfilled_predicates.borrow().check_duplicate(&obligation.predicate) { return EvaluatedToOk; } diff --git a/src/librustc/middle/ty/context.rs b/src/librustc/middle/ty/context.rs index d1504d25288a8..bf768a40999dc 100644 --- a/src/librustc/middle/ty/context.rs +++ b/src/librustc/middle/ty/context.rs @@ -367,7 +367,7 @@ pub struct ctxt<'tcx> { /// This is used to avoid duplicate work. Predicates are only /// added to this set when they mention only "global" names /// (i.e., no type or lifetime parameters). - pub fulfilled_predicates: RefCell>, + pub fulfilled_predicates: RefCell>, /// Caches the representation hints for struct definitions. repr_hint_cache: RefCell>>, @@ -510,6 +510,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 fulfilled_predicates = traits::GlobalFulfilledPredicates::new(dep_graph.clone()); tls::enter(ctxt { arenas: arenas, interner: interner, @@ -532,7 +533,7 @@ impl<'tcx> ctxt<'tcx> { adt_defs: RefCell::new(DepTrackingMap::new(dep_graph.clone())), predicates: RefCell::new(DepTrackingMap::new(dep_graph.clone())), super_predicates: RefCell::new(DepTrackingMap::new(dep_graph.clone())), - fulfilled_predicates: RefCell::new(traits::FulfilledPredicates::new()), + fulfilled_predicates: RefCell::new(fulfilled_predicates), map: map, freevars: RefCell::new(freevars), tcache: RefCell::new(DepTrackingMap::new(dep_graph.clone())), diff --git a/src/librustc/middle/ty/mod.rs b/src/librustc/middle/ty/mod.rs index b902a46fea314..e25c05f6272c6 100644 --- a/src/librustc/middle/ty/mod.rs +++ b/src/librustc/middle/ty/mod.rs @@ -838,6 +838,11 @@ impl<'tcx> TraitPredicate<'tcx> { self.trait_ref.def_id } + /// Creates the dep-node for selecting/evaluating this trait reference. + fn dep_node(&self) -> DepNode { + DepNode::TraitSelect(self.def_id()) + } + pub fn input_types(&self) -> &[Ty<'tcx>] { self.trait_ref.substs.types.as_slice() } @@ -849,8 +854,14 @@ impl<'tcx> TraitPredicate<'tcx> { impl<'tcx> PolyTraitPredicate<'tcx> { pub fn def_id(&self) -> DefId { + // ok to skip binder since trait def-id does not care about regions self.0.def_id() } + + pub fn dep_node(&self) -> DepNode { + // ok to skip binder since depnode does not care about regions + self.0.dep_node() + } } #[derive(Clone, PartialEq, Eq, Hash, Debug)] diff --git a/src/librustc_trans/trans/base.rs b/src/librustc_trans/trans/base.rs index 4c619f895de56..7460ef82ebee4 100644 --- a/src/librustc_trans/trans/base.rs +++ b/src/librustc_trans/trans/base.rs @@ -3323,6 +3323,14 @@ impl<'a, 'tcx, 'v> Visitor<'v> for TransItemsWithinModVisitor<'a, 'tcx> { // giving `trans_item` access to this item, so also record a read. tcx.dep_graph.with_task(DepNode::TransCrateItem(def_id), || { tcx.dep_graph.read(DepNode::Hir(def_id)); + + // We are going to be accessing various tables + // generated by TypeckItemBody; we also assume + // that the body passes type check. These tables + // are not individually tracked, so just register + // a read here. + tcx.dep_graph.read(DepNode::TypeckItemBody(def_id)); + trans_item(self.ccx, i); }); diff --git a/src/test/compile-fail/dep-graph-assoc-type-trans.rs b/src/test/compile-fail/dep-graph-assoc-type-trans.rs new file mode 100644 index 0000000000000..d1ecff5984ac5 --- /dev/null +++ b/src/test/compile-fail/dep-graph-assoc-type-trans.rs @@ -0,0 +1,48 @@ +// Copyright 2012-2014 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. + +// Test that when a trait impl changes, fns whose body uses that trait +// must also be recompiled. + +// compile-flags: -Z incr-comp + +#![feature(rustc_attrs)] +#![allow(warnings)] + +fn main() { } + +pub trait Foo: Sized { + type T; + fn method(self) { } +} + +mod x { + use Foo; + + #[rustc_if_this_changed] + impl Foo for char { type T = char; } + + impl Foo for u32 { type T = u32; } +} + +mod y { + use Foo; + + #[rustc_then_this_would_need(TypeckItemBody)] //~ ERROR OK + #[rustc_then_this_would_need(TransCrateItem)] //~ ERROR OK + pub fn use_char_assoc() { + // Careful here: in the representation, ::T gets + // normalized away, so at a certain point we had no edge to + // trans. (But now trans just depends on typeck.) + let x: ::T = 'a'; + } + + pub fn take_foo(t: T) { } +} diff --git a/src/test/compile-fail/dep-graph-trait-impl.rs b/src/test/compile-fail/dep-graph-trait-impl.rs index 83e924fe06d7d..b38fdad9809ed 100644 --- a/src/test/compile-fail/dep-graph-trait-impl.rs +++ b/src/test/compile-fail/dep-graph-trait-impl.rs @@ -40,9 +40,8 @@ mod y { char::method('a'); } - // FIXME(#30741) tcx fulfillment cache not tracked - #[rustc_then_this_would_need(TypeckItemBody)] //~ ERROR no path - #[rustc_then_this_would_need(TransCrateItem)] //~ ERROR no path + #[rustc_then_this_would_need(TypeckItemBody)] //~ ERROR OK + #[rustc_then_this_would_need(TransCrateItem)] //~ ERROR OK pub fn take_foo_with_char() { take_foo::('a'); } @@ -53,9 +52,8 @@ mod y { u32::method(22); } - // FIXME(#30741) tcx fulfillment cache not tracked - #[rustc_then_this_would_need(TypeckItemBody)] //~ ERROR no path - #[rustc_then_this_would_need(TransCrateItem)] //~ ERROR no path + #[rustc_then_this_would_need(TypeckItemBody)] //~ ERROR OK + #[rustc_then_this_would_need(TransCrateItem)] //~ ERROR OK pub fn take_foo_with_u32() { take_foo::(22); }