only insert global predicates into the global cache once we've

completely proven them to be true

Author: nikomatsakis

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);
                 }
             }
 

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,