use the evaluation cache instead of the global fulfillment cache

arielb1 · nikomatsakis · commit 87a11812e14c · 2017-07-07T14:05:02.000-04:00
The evaluation cache already exists, and it can handle differing parameter environments natively. Fixes rust-lang#39970. Fixes rust-lang#42796.
diff --git a/src/librustc/traits/fulfill.rs b/src/librustc/traits/fulfill.rs
@@ -8,15 +8,14 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use dep_graph::DepGraph;
 use infer::{InferCtxt, InferOk};
-use ty::{self, Ty, TypeFoldable, ToPolyTraitRef, TyCtxt, ToPredicate};
+use ty::{self, Ty, TypeFoldable, ToPolyTraitRef, ToPredicate};
 use ty::error::ExpectedFound;
 use rustc_data_structures::obligation_forest::{ObligationForest, Error};
 use rustc_data_structures::obligation_forest::{ForestObligation, ObligationProcessor};
 use std::marker::PhantomData;
 use syntax::ast;
-use util::nodemap::{FxHashSet, NodeMap};
+use util::nodemap::NodeMap;
 use hir::def_id::DefId;
 
 use super::CodeAmbiguity;
@@ -34,11 +33,6 @@ impl<'tcx> ForestObligation for PendingPredicateObligation<'tcx> {
     fn as_predicate(&self) -> &Self::Predicate { &self.obligation.predicate }
 }
 
-pub struct GlobalFulfilledPredicates<'tcx> {
-    set: FxHashSet<ty::PolyTraitPredicate<'tcx>>,
-    dep_graph: DepGraph,
-}
-
 /// The fulfillment context is used to drive trait resolution.  It
 /// consists of a list of obligations that must be (eventually)
 /// satisfied. The job is to track which are satisfied, which yielded
@@ -183,13 +177,6 @@ impl<'a, 'gcx, 'tcx> FulfillmentContext<'tcx> {
 
         assert!(!infcx.is_in_snapshot());
 
-        let tcx = infcx.tcx;
-
-        if tcx.fulfilled_predicates.borrow().check_duplicate(tcx, &obligation.predicate) {
-            debug!("register_predicate_obligation: duplicate");
-            return
-        }
-
         self.predicates.register_obligation(PendingPredicateObligation {
             obligation,
             stalled_on: vec![]
@@ -264,13 +251,6 @@ impl<'a, 'gcx, 'tcx> FulfillmentContext<'tcx> {
             });
             debug!("select: outcome={:?}", outcome);
 
-            // these are obligations that were proven to be true.
-            for pending_obligation in outcome.completed {
-                let predicate = &pending_obligation.obligation.predicate;
-                selcx.tcx().fulfilled_predicates.borrow_mut()
-                           .add_if_global(selcx.tcx(), predicate);
-            }
-
             errors.extend(
                 outcome.errors.into_iter()
                               .map(|e| to_fulfillment_error(e)));
@@ -375,21 +355,31 @@ fn process_predicate<'a, 'gcx, 'tcx>(
 
     match obligation.predicate {
         ty::Predicate::Trait(ref data) => {
-            let tcx = selcx.tcx();
-            if tcx.fulfilled_predicates.borrow().check_duplicate_trait(tcx, data) {
-                return Ok(Some(vec![]));
+            let trait_obligation = obligation.with(data.clone());
+
+            if data.is_global() {
+                // no type variables present, can use evaluation for better caching.
+                // FIXME: consider caching errors too.
+                if
+                    // make defaulted unit go through the slow path for better warnings,
+                    // please remove this when the warnings are removed.
+                    !trait_obligation.predicate.skip_binder().self_ty().is_defaulted_unit() &&
+                    selcx.evaluate_obligation_conservatively(&obligation) {
+                    debug!("selecting trait `{:?}` at depth {} evaluated to holds",
+                           data, obligation.recursion_depth);
+                    return Ok(Some(vec![]))
+                }
             }
 
-            let trait_obligation = obligation.with(data.clone());
             match selcx.select(&trait_obligation) {
                 Ok(Some(vtable)) => {
                     debug!("selecting trait `{:?}` at depth {} yielded Ok(Some)",
-                          data, obligation.recursion_depth);
+                           data, obligation.recursion_depth);
                     Ok(Some(vtable.nested_obligations()))
                 }
                 Ok(None) => {
                     debug!("selecting trait `{:?}` at depth {} yielded Ok(None)",
-                          data, obligation.recursion_depth);
+                           data, obligation.recursion_depth);
 
                     // This is a bit subtle: for the most part, the
                     // only reason we can fail to make progress on
@@ -568,55 +558,6 @@ fn register_region_obligation<'tcx>(t_a: Ty<'tcx>,
 
 }
 
-impl<'a, 'gcx, 'tcx> GlobalFulfilledPredicates<'gcx> {
-    pub fn new(dep_graph: DepGraph) -> GlobalFulfilledPredicates<'gcx> {
-        GlobalFulfilledPredicates {
-            set: FxHashSet(),
-            dep_graph,
-        }
-    }
-
-    pub fn check_duplicate(&self, tcx: TyCtxt, key: &ty::Predicate<'tcx>) -> bool {
-        if let ty::Predicate::Trait(ref data) = *key {
-            self.check_duplicate_trait(tcx, data)
-        } else {
-            false
-        }
-    }
-
-    pub fn check_duplicate_trait(&self, tcx: TyCtxt, data: &ty::PolyTraitPredicate<'tcx>) -> bool {
-        // 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(tcx));
-            debug!("check_duplicate: global predicate `{:?}` already proved elsewhere", data);
-
-            true
-        } else {
-            false
-        }
-    }
-
-    fn add_if_global(&mut self, tcx: TyCtxt<'a, 'gcx, 'tcx>, 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() {
-                if let Some(data) = tcx.lift_to_global(data) {
-                    if self.set.insert(data.clone()) {
-                        debug!("add_if_global: global predicate `{:?}` added", data);
-                    }
-                }
-            }
-        }
-    }
-}
-
 fn to_fulfillment_error<'tcx>(
     error: Error<PendingPredicateObligation<'tcx>, FulfillmentErrorCode<'tcx>>)
     -> FulfillmentError<'tcx>
diff --git a/src/librustc/traits/mod.rs b/src/librustc/traits/mod.rs
@@ -31,7 +31,7 @@ use syntax_pos::{Span, DUMMY_SP};
 pub use self::coherence::orphan_check;
 pub use self::coherence::overlapping_impls;
 pub use self::coherence::OrphanCheckErr;
-pub use self::fulfill::{FulfillmentContext, GlobalFulfilledPredicates, RegionObligation};
+pub use self::fulfill::{FulfillmentContext, RegionObligation};
 pub use self::project::MismatchedProjectionTypes;
 pub use self::project::{normalize, normalize_projection_type, Normalized};
 pub use self::project::{ProjectionCache, ProjectionCacheSnapshot, Reveal};
diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs
@@ -514,21 +514,11 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
         debug!("evaluate_predicate_recursively({:?})",
                obligation);
 
-        let tcx = self.tcx();
-
-        // Check the cache from the tcx of predicates that we know
-        // have been proven elsewhere. This cache only contains
-        // predicates that are global in scope and hence unaffected by
-        // the current environment.
-        if tcx.fulfilled_predicates.borrow().check_duplicate(tcx, &obligation.predicate) {
-            return EvaluatedToOk;
-        }
-
         match obligation.predicate {
             ty::Predicate::Trait(ref t) => {
                 assert!(!t.has_escaping_regions());
                 let obligation = obligation.with(t.clone());
-                self.evaluate_obligation_recursively(previous_stack, &obligation)
+                self.evaluate_trait_predicate_recursively(previous_stack, obligation)
             }
 
             ty::Predicate::Equate(ref p) => {
@@ -613,15 +603,23 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
         }
     }
 
-    fn evaluate_obligation_recursively<'o>(&mut self,
-                                           previous_stack: TraitObligationStackList<'o, 'tcx>,
-                                           obligation: &TraitObligation<'tcx>)
-                                           -> EvaluationResult
+    fn evaluate_trait_predicate_recursively<'o>(&mut self,
+                                                previous_stack: TraitObligationStackList<'o, 'tcx>,
+                                                mut obligation: TraitObligation<'tcx>)
+                                                -> EvaluationResult
     {
-        debug!("evaluate_obligation_recursively({:?})",
+        debug!("evaluate_trait_predicate_recursively({:?})",
                obligation);
 
-        let stack = self.push_stack(previous_stack, obligation);
+        if !self.intercrate && obligation.is_global() {
+            // If a param env is consistent, global obligations do not depend on its particular
+            // value in order to work, so we can clear out the param env and get better
+            // caching. (If the current param env is inconsistent, we don't care what happens).
+            debug!("evaluate_trait_predicate_recursively({:?}) - in global", obligation);
+            obligation.param_env = ty::ParamEnv::empty(obligation.param_env.reveal);
+        }
+
+        let stack = self.push_stack(previous_stack, &obligation);
         let fresh_trait_ref = stack.fresh_trait_ref;
         if let Some(result) = self.check_evaluation_cache(obligation.param_env, fresh_trait_ref) {
             debug!("CACHE HIT: EVAL({:?})={:?}",
@@ -676,8 +674,9 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
         }
         if unbound_input_types &&
               stack.iter().skip(1).any(
-                  |prev| self.match_fresh_trait_refs(&stack.fresh_trait_ref,
-                                                     &prev.fresh_trait_ref))
+                  |prev| stack.obligation.param_env == prev.obligation.param_env &&
+                      self.match_fresh_trait_refs(&stack.fresh_trait_ref,
+                                                  &prev.fresh_trait_ref))
         {
             debug!("evaluate_stack({:?}) --> unbound argument, recursive --> giving up",
                    stack.fresh_trait_ref);
@@ -706,7 +705,8 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
         if let Some(rec_index) =
             stack.iter()
             .skip(1) // skip top-most frame
-            .position(|prev| stack.fresh_trait_ref == prev.fresh_trait_ref)
+            .position(|prev| stack.obligation.param_env == prev.obligation.param_env &&
+                      stack.fresh_trait_ref == prev.fresh_trait_ref)
         {
             debug!("evaluate_stack({:?}) --> recursive",
                    stack.fresh_trait_ref);
diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs
@@ -507,12 +507,6 @@ pub struct GlobalCtxt<'tcx> {
     /// Merge this with `selection_cache`?
     pub evaluation_cache: traits::EvaluationCache<'tcx>,
 
-    /// A set of predicates that have been fulfilled *somewhere*.
-    /// 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<traits::GlobalFulfilledPredicates<'tcx>>,
-
     /// Maps Expr NodeId's to `true` iff `&expr` can have 'static lifetime.
     pub rvalue_promotable_to_static: RefCell<NodeMap<bool>>,
 
@@ -686,7 +680,6 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
         let interners = CtxtInterners::new(arena);
         let common_types = CommonTypes::new(&interners);
         let dep_graph = hir.dep_graph.clone();
-        let fulfilled_predicates = traits::GlobalFulfilledPredicates::new(dep_graph.clone());
         let max_cnum = s.cstore.crates().iter().map(|c| c.as_usize()).max().unwrap_or(0);
         let mut providers = IndexVec::from_elem_n(extern_providers, max_cnum + 1);
         providers[LOCAL_CRATE] = local_providers;
@@ -735,7 +728,6 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
             named_region_map,
             trait_map: resolutions.trait_map,
             export_map: resolutions.export_map,
-            fulfilled_predicates: RefCell::new(fulfilled_predicates),
             hir,
             def_path_hash_to_def_id,
             maps: maps::Maps::new(providers),
diff --git a/src/test/compile-fail/issue-39970.rs b/src/test/compile-fail/issue-39970.rs
@@ -0,0 +1,31 @@
+// Copyright 2017 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.
+
+trait Array<'a> {
+    type Element: 'a;
+}
+
+trait Visit {
+    fn visit() {}
+}
+
+impl<'a> Array<'a> for () {
+    type Element = &'a ();
+}
+
+impl Visit for () where
+    //(): for<'a> Array<'a, Element=&'a ()>, // No ICE
+    (): for<'a> Array<'a, Element=()>, // ICE
+{}
+
+fn main() {
+    <() as Visit>::visit();
+    //~^ ERROR type mismatch resolving `for<'a> <() as Array<'a>>::Element == ()`
+}
diff --git a/src/test/compile-fail/issue-42796.rs b/src/test/compile-fail/issue-42796.rs
@@ -0,0 +1,29 @@
+// Copyright 2017 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.
+
+pub trait Mirror<Smoke> {
+    type Image;
+}
+
+impl<T, Smoke> Mirror<Smoke> for T {
+    type Image = T;
+}
+
+pub fn poison<S>(victim: String) where <String as Mirror<S>>::Image: Copy {
+    loop { drop(victim); } //~ ERROR use of moved value
+}
+
+fn main() {
+    let s = "Hello!".to_owned();
+    let mut s_copy = s;
+    s_copy.push_str("World!");
+    "0wned!".to_owned();
+    println!("{}", s); //~ ERROR use of moved value
+}
