From 2c3f0123e9e3da95cf32ba789b11c522ae3c9afc Mon Sep 17 00:00:00 2001 From: Niko Matsakis <niko@alum.mit.edu> Date: Tue, 22 Dec 2015 19:46:51 -0500 Subject: [PATCH] only insert global predicates into the global cache once we've completely proven them to be true --- src/librustc/middle/traits/fulfill.rs | 63 ++++++++++++--------------- src/librustc_driver/test.rs | 2 +- 2 files changed, 28 insertions(+), 37 deletions(-) diff --git a/src/librustc/middle/traits/fulfill.rs b/src/librustc/middle/traits/fulfill.rs index ca2a9f9ca0b..a13cc141608 100644 --- a/src/librustc/middle/traits/fulfill.rs +++ b/src/librustc/middle/traits/fulfill.rs @@ -86,8 +86,6 @@ pub struct FulfillmentContext<'tcx> { // obligations (otherwise, it's easy to fail to walk to a // particular node-id). region_obligations: NodeMap<Vec<RegionObligation<'tcx>>>, - - pub errors_will_be_reported: bool, } #[derive(Clone)] @@ -105,28 +103,11 @@ pub struct PendingPredicateObligation<'tcx> { impl<'tcx> FulfillmentContext<'tcx> { /// Creates a new fulfillment context. - /// - /// `errors_will_be_reported` indicates whether ALL errors that - /// are generated by this fulfillment context will be reported to - /// the end user. This is used to inform caching, because it - /// allows us to conclude that traits that resolve successfully - /// will in fact always resolve successfully (in particular, it - /// guarantees that if some dependent obligation encounters a - /// problem, compilation will be aborted). If you're not sure of - /// the right value here, pass `false`, as that is the more - /// conservative option. - /// - /// FIXME -- a better option would be to hold back on modifying - /// the global cache until we know that all dependent obligations - /// are also satisfied. In that case, we could actually remove - /// this boolean flag, and we'd also avoid the problem of squelching - /// duplicate errors that occur across fns. - pub fn new(errors_will_be_reported: bool) -> FulfillmentContext<'tcx> { + pub fn new() -> FulfillmentContext<'tcx> { FulfillmentContext { duplicate_set: FulfilledPredicates::new(), predicates: ObligationForest::new(), region_obligations: NodeMap(), - errors_will_be_reported: errors_will_be_reported, } } @@ -250,23 +231,27 @@ impl<'tcx> FulfillmentContext<'tcx> { tcx: &ty::ctxt<'tcx>, predicate: &ty::Predicate<'tcx>) -> bool { - // This is a kind of dirty hack to allow us to avoid "rederiving" - // things that we have already proven in other methods. - // - // The idea is that any predicate that doesn't involve type - // parameters and which only involves the 'static region (and - // no other regions) is universally solvable, since impls are global. - // - // This is particularly important since even if we have a - // cache hit in the selection context, we still wind up - // evaluating the 'nested obligations'. This cache lets us - // skip those. - - if self.errors_will_be_reported && predicate.is_global() { - tcx.fulfilled_predicates.borrow_mut().is_duplicate_or_add(predicate) - } else { - self.duplicate_set.is_duplicate_or_add(predicate) + // For "global" predicates -- that is, predicates that don't + // involve type parameters, inference variables, or regions + // other than 'static -- we can check the cache in the tcx, + // which allows us to leverage work from other threads. Note + // that we don't add anything to this cache yet (unlike the + // 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) { + return true; } + + // If `predicate` is not global, or not present in the tcx + // cache, we can still check for it in our local cache and add + // it if not present. Note that if we find this predicate in + // the local cache we can stop immediately, without reporting + // any errors, even though we don't know yet if it is + // true. This is because, while we don't yet know if the + // predicate holds, we know that this same fulfillment context + // already is in the process of finding out. + self.duplicate_set.is_duplicate_or_add(predicate) } /// Attempts to select obligations using `selcx`. If `only_new_obligations` is true, then it @@ -294,6 +279,12 @@ impl<'tcx> FulfillmentContext<'tcx> { debug!("select_where_possible: outcome={:?}", outcome); + // these are obligations that were proven to be true. + for pending_obligation in outcome.successful { + let predicate = &pending_obligation.obligation.predicate; + if predicate.is_global() { + selcx.tcx().fulfilled_predicates.borrow_mut() + .is_duplicate_or_add(predicate); } } diff --git a/src/librustc_driver/test.rs b/src/librustc_driver/test.rs index 8f3366eacb3..b19628baa88 100644 --- a/src/librustc_driver/test.rs +++ b/src/librustc_driver/test.rs @@ -139,7 +139,7 @@ fn test_env<F>(source_string: &str, lang_items, stability::Index::new(krate), |tcx| { - let infcx = infer::new_infer_ctxt(tcx, &tcx.tables, None, false); + let infcx = infer::new_infer_ctxt(tcx, &tcx.tables, None); body(Env { infcx: &infcx }); let free_regions = FreeRegionMap::new(); infcx.resolve_regions_and_report_errors(&free_regions,