Add a per-tree error cache to the obligation forest

src/librustc/traits/auto_trait.rs

@@ -73,16 +73,16 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> {
     /// ```
     /// struct Foo<T> { data: Box<T> }
     /// ```
-
+    ///
     /// then this might return that Foo<T>: Send if T: Send (encoded in the AutoTraitResult type).
     /// The analysis attempts to account for custom impls as well as other complex cases. This
     /// result is intended for use by rustdoc and other such consumers.
-
+    ///
     /// (Note that due to the coinductive nature of Send, the full and correct result is actually
     /// quite simple to generate. That is, when a type has no custom impl, it is Send iff its field
     /// types are all Send. So, in our example, we might have that Foo<T>: Send if Box<T>: Send.
     /// But this is often not the best way to present to the user.)
-
+    ///
     /// Warning: The API should be considered highly unstable, and it may be refactored or removed
     /// in the future.
     pub fn find_auto_trait_generics<A>(
@@ -288,7 +288,7 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> {
     // hold.
     //
     // One additional consideration is supertrait bounds. Normally, a ParamEnv is only ever
-    // consutrcted once for a given type. As part of the construction process, the ParamEnv will
+    // constructed once for a given type. As part of the construction process, the ParamEnv will
     // have any supertrait bounds normalized - e.g. if we have a type 'struct Foo<T: Copy>', the
     // ParamEnv will contain 'T: Copy' and 'T: Clone', since 'Copy: Clone'. When we construct our
     // own ParamEnv, we need to do this ourselves, through traits::elaborate_predicates, or else

src/librustc/traits/fulfill.rs

@@ -45,7 +45,6 @@ impl<'tcx> ForestObligation for PendingPredicateObligation<'tcx> {
 /// along. Once all type inference constraints have been generated, the
 /// method `select_all_or_error` can be used to report any remaining
 /// ambiguous cases as errors.
-
 pub struct FulfillmentContext<'tcx> {
     // A list of all obligations that have been registered with this
     // fulfillment context.
@@ -89,7 +88,7 @@ impl<'a, 'gcx, 'tcx> FulfillmentContext<'tcx> {
 
     /// Attempts to select obligations using `selcx`.
     fn select(&mut self, selcx: &mut SelectionContext<'a, 'gcx, 'tcx>)
-              -> Result<(),Vec<FulfillmentError<'tcx>>> {
+              -> Result<(), Vec<FulfillmentError<'tcx>>> {
         debug!("select(obligation-forest-size={})", self.predicates.len());
 
         let mut errors = Vec::new();

src/librustc/traits/mod.rs

@@ -644,7 +644,7 @@ pub fn normalize_param_env_or_error<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
     // have the errors get reported at a defined place (e.g.,
     // during typeck). Instead I have all parameter
     // environments, in effect, going through this function
-    // and hence potentially reporting errors. This ensurse of
+    // and hence potentially reporting errors. This ensures of
     // course that we never forget to normalize (the
     // alternative seemed like it would involve a lot of
     // manual invocations of this fn -- and then we'd have to

src/librustc/traits/select.rs

@@ -582,7 +582,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
         match self.confirm_candidate(obligation, candidate) {
             Err(SelectionError::Overflow) => {
                 assert!(self.query_mode == TraitQueryMode::Canonical);
-                return Err(SelectionError::Overflow);
+                Err(SelectionError::Overflow)
             },
             Err(e) => Err(e),
             Ok(candidate) => Ok(Some(candidate))
@@ -879,7 +879,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
         // must be met of course). One obvious case this comes up is
         // marker traits like `Send`. Think of a linked list:
         //
-        //    struct List<T> { data: T, next: Option<Box<List<T>>> {
+        //    struct List<T> { data: T, next: Option<Box<List<T>>> }
         //
         // `Box<List<T>>` will be `Send` if `T` is `Send` and
         // `Option<Box<List<T>>>` is `Send`, and in turn
@@ -1407,7 +1407,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
                                stack: &TraitObligationStack<'o, 'tcx>)
                                -> Result<SelectionCandidateSet<'tcx>, SelectionError<'tcx>>
     {
-        let TraitObligationStack { obligation, .. } = *stack;
+        let obligation = stack.obligation;
         let ref obligation = Obligation {
             param_env: obligation.param_env,
             cause: obligation.cause.clone(),
@@ -1788,9 +1788,9 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
     }
 
     fn assemble_candidates_from_auto_impls(&mut self,
-                                              obligation: &TraitObligation<'tcx>,
-                                              candidates: &mut SelectionCandidateSet<'tcx>)
-                                              -> Result<(), SelectionError<'tcx>>
+                                           obligation: &TraitObligation<'tcx>,
+                                           candidates: &mut SelectionCandidateSet<'tcx>)
+                                           -> Result<(), SelectionError<'tcx>>
     {
         // OK to skip binder here because the tests we do below do not involve bound regions
         let self_ty = *obligation.self_ty().skip_binder();
@@ -2433,7 +2433,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
     fn confirm_candidate(&mut self,
                          obligation: &TraitObligation<'tcx>,
                          candidate: SelectionCandidate<'tcx>)
-                         -> Result<Selection<'tcx>,SelectionError<'tcx>>
+                         -> Result<Selection<'tcx>, SelectionError<'tcx>>
     {
         debug!("confirm_candidate({:?}, {:?})",
                obligation,
@@ -2612,11 +2612,11 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
         let mut obligations = self.collect_predicates_for_types(
             obligation.param_env,
             cause,
-            obligation.recursion_depth+1,
+            obligation.recursion_depth + 1,
             trait_def_id,
             nested);
 
-        let trait_obligations = self.in_snapshot(|this, snapshot| {
+        let trait_obligations: Vec<PredicateObligation<'_>> = self.in_snapshot(|this, snapshot| {
             let poly_trait_ref = obligation.predicate.to_poly_trait_ref();
             let (trait_ref, skol_map) =
                 this.infcx().skolemize_late_bound_regions(&poly_trait_ref);
@@ -2630,6 +2630,8 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
                                            snapshot)
         });
 
+        // Adds the predicates from the trait.  Note that this contains a `Self: Trait`
+        // predicate as usual.  It won't have any effect since auto traits are coinductive.
         obligations.extend(trait_obligations);
 
         debug!("vtable_auto_impl: obligations={:?}", obligations);

src/librustc_data_structures/obligation_forest/mod.rs

@@ -65,6 +65,12 @@ pub enum ProcessResult<O, E> {
     Error(E),
 }
 
+#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)]
+struct ObligationTreeId(usize);
+
+type ObligationTreeIdGenerator =
+    ::std::iter::Map<::std::ops::RangeFrom<usize>, fn(usize) -> ObligationTreeId>;
+
 pub struct ObligationForest<O: ForestObligation> {
     /// The list of obligations. In between calls to
     /// `process_obligations`, this list only contains nodes in the
@@ -79,11 +85,25 @@ pub struct ObligationForest<O: ForestObligation> {
     /// at a higher index than its parent. This is needed by the
     /// backtrace iterator (which uses `split_at`).
     nodes: Vec<Node<O>>,
+
     /// A cache of predicates that have been successfully completed.
     done_cache: FxHashSet<O::Predicate>,
+
     /// An cache of the nodes in `nodes`, indexed by predicate.
     waiting_cache: FxHashMap<O::Predicate, NodeIndex>,
+
     scratch: Option<Vec<usize>>,
+
+    obligation_tree_id_generator: ObligationTreeIdGenerator,
+
+    /// Per tree error cache.  This is used to deduplicate errors,
+    /// which is necessary to avoid trait resolution overflow in
+    /// some cases.
+    ///
+    /// See [this][details] for details.
+    ///
+    /// [details]: https://github.com/rust-lang/rust/pull/53255#issuecomment-421184780
+    error_cache: FxHashMap<ObligationTreeId, FxHashSet<O::Predicate>>,
 }
 
 #[derive(Debug)]
@@ -99,6 +119,9 @@ struct Node<O> {
     /// Obligations that depend on this obligation for their
     /// completion. They must all be in a non-pending state.
     dependents: Vec<NodeIndex>,
+
+    /// Identifier of the obligation tree to which this node belongs.
+    obligation_tree_id: ObligationTreeId,
 }
 
 /// The state of one node in some tree within the forest. This
@@ -165,6 +188,8 @@ impl<O: ForestObligation> ObligationForest<O> {
             done_cache: FxHashSet(),
             waiting_cache: FxHashMap(),
             scratch: Some(vec![]),
+            obligation_tree_id_generator: (0..).map(|i| ObligationTreeId(i)),
+            error_cache: FxHashMap(),
         }
     }
 
@@ -187,7 +212,7 @@ impl<O: ForestObligation> ObligationForest<O> {
                               -> Result<(), ()>
     {
         if self.done_cache.contains(obligation.as_predicate()) {
-            return Ok(())
+            return Ok(());
         }
 
         match self.waiting_cache.entry(obligation.as_predicate().clone()) {
@@ -214,9 +239,29 @@ impl<O: ForestObligation> ObligationForest<O> {
             Entry::Vacant(v) => {
                 debug!("register_obligation_at({:?}, {:?}) - ok, new index is {}",
                        obligation, parent, self.nodes.len());
-                v.insert(NodeIndex::new(self.nodes.len()));
-                self.nodes.push(Node::new(parent, obligation));
-                Ok(())
+
+                let obligation_tree_id = match parent {
+                    Some(p) => {
+                        let parent_node = &self.nodes[p.get()];
+                        parent_node.obligation_tree_id
+                    }
+                    None => self.obligation_tree_id_generator.next().unwrap()
+                };
+
+                let already_failed =
+                    parent.is_some()
+                        && self.error_cache
+                            .get(&obligation_tree_id)
+                            .map(|errors| errors.contains(obligation.as_predicate()))
+                            .unwrap_or(false);
+
+                if already_failed {
+                    Err(())
+                } else {
+                    v.insert(NodeIndex::new(self.nodes.len()));
+                    self.nodes.push(Node::new(parent, obligation, obligation_tree_id));
+                    Ok(())
+                }
             }
         }
     }
@@ -251,6 +296,15 @@ impl<O: ForestObligation> ObligationForest<O> {
             .collect()
     }
 
+    fn insert_into_error_cache(&mut self, node_index: usize) {
+        let node = &self.nodes[node_index];
+
+        self.error_cache
+            .entry(node.obligation_tree_id)
+            .or_insert_with(|| FxHashSet())
+            .insert(node.obligation.as_predicate().clone());
+    }
+
     /// Perform a pass through the obligation list. This must
     /// be called in a loop until `outcome.stalled` is false.
     ///
@@ -264,22 +318,15 @@ impl<O: ForestObligation> ObligationForest<O> {
         let mut stalled = true;
 
         for index in 0..self.nodes.len() {
-            debug!("process_obligations: node {} == {:?}",
-                   index,
-                   self.nodes[index]);
+            debug!("process_obligations: node {} == {:?}", index, self.nodes[index]);
 
             let result = match self.nodes[index] {
-                Node { state: ref _state, ref mut obligation, .. }
-                    if _state.get() == NodeState::Pending =>
-                {
-                    processor.process_obligation(obligation)
-                }
+                Node { ref state, ref mut obligation, .. } if state.get() == NodeState::Pending =>
+                    processor.process_obligation(obligation),
                 _ => continue
             };
 
-            debug!("process_obligations: node {} got result {:?}",
-                   index,
-                   result);
+            debug!("process_obligations: node {} got result {:?}", index, result);
 
             match result {
                 ProcessResult::Unchanged => {
@@ -420,13 +467,13 @@ impl<O: ForestObligation> ObligationForest<O> {
         }
 
         while let Some(i) = error_stack.pop() {
-            let node = &self.nodes[i];
-
-            match node.state.get() {
+            match self.nodes[i].state.get() {
                 NodeState::Error => continue,
-                _ => node.state.set(NodeState::Error)
+                _ => self.nodes[i].state.set(NodeState::Error),
             }
 
+            let node = &self.nodes[i];
+
             error_stack.extend(
                 node.parent.iter().chain(node.dependents.iter()).map(|x| x.get())
             );
@@ -514,6 +561,7 @@ impl<O: ForestObligation> ObligationForest<O> {
                     self.waiting_cache.remove(self.nodes[i].obligation.as_predicate());
                     node_rewrites[i] = nodes_len;
                     dead_nodes += 1;
+                    self.insert_into_error_cache(i);
                 }
                 NodeState::OnDfsStack | NodeState::Success => unreachable!()
             }
@@ -587,12 +635,17 @@ impl<O: ForestObligation> ObligationForest<O> {
 }
 
 impl<O> Node<O> {
-    fn new(parent: Option<NodeIndex>, obligation: O) -> Node<O> {
+    fn new(
+        parent: Option<NodeIndex>,
+        obligation: O,
+        obligation_tree_id: ObligationTreeId
+    ) -> Node<O> {
         Node {
             obligation,
             state: Cell::new(NodeState::Pending),
             parent,
             dependents: vec![],
+            obligation_tree_id,
         }
     }
 }

src/librustc_data_structures/obligation_forest/test.rs

@@ -59,8 +59,9 @@ impl<OF, BF, O, E> ObligationProcessor for ClosureObligationProcessor<OF, BF, O,
 
     fn process_backedge<'c, I>(&mut self, _cycle: I,
                                _marker: PhantomData<&'c Self::Obligation>)
-        where I: Clone + Iterator<Item=&'c Self::Obligation> {
-        }
+        where I: Clone + Iterator<Item=&'c Self::Obligation>
+    {
+    }
 }
 
 
@@ -350,11 +351,8 @@ fn done_dependency() {
         }, |_|{}));
     assert_eq!(ok, vec!["(A,B,C): Sized"]);
     assert_eq!(err.len(), 0);
-
-
 }
 
-
 #[test]
 fn orphan() {
     // check that orphaned nodes are handled correctly

src/test/ui/issue-40827.rs

@@ -0,0 +1,27 @@
+// Copyright 2018 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.
+
+use std::rc::Rc;
+use std::sync::Arc;
+
+struct Foo(Arc<Bar>);
+
+enum Bar {
+    A(Rc<Foo>),
+    B(Option<Foo>),
+}
+
+fn f<T: Send>(_: T) {}
+
+fn main() {
+    f(Foo(Arc::new(Bar::B(None))));
+    //~^ ERROR E0277
+    //~| ERROR E0277
+}