Fix stack overflow when finding blanket impls

src/librustc/traits/select.rs

@@ -42,7 +42,7 @@ use rustc_data_structures::bit_set::GrowableBitSet;
 use rustc_data_structures::sync::Lock;
 use rustc_target::spec::abi::Abi;
 use std::cmp;
-use std::fmt;
+use std::fmt::{self, Display};
 use std::iter;
 use std::rc::Rc;
 use util::nodemap::{FxHashMap, FxHashSet};
@@ -629,7 +629,8 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
         obligation: &PredicateObligation<'tcx>,
     ) -> Result<EvaluationResult, OverflowError> {
         self.evaluation_probe(|this| {
-            this.evaluate_predicate_recursively(TraitObligationStackList::empty(), obligation)
+            this.evaluate_predicate_recursively(TraitObligationStackList::empty(),
+                obligation.clone())
         })
     }
 
@@ -655,12 +656,12 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
         predicates: I,
     ) -> Result<EvaluationResult, OverflowError>
     where
-        I: IntoIterator<Item = &'a PredicateObligation<'tcx>>,
+        I: IntoIterator<Item = PredicateObligation<'tcx>>,
         'tcx: 'a,
     {
         let mut result = EvaluatedToOk;
         for obligation in predicates {
-            let eval = self.evaluate_predicate_recursively(stack, obligation)?;
+            let eval = self.evaluate_predicate_recursively(stack, obligation.clone())?;
             debug!(
                 "evaluate_predicate_recursively({:?}) = {:?}",
                 obligation, eval
@@ -679,9 +680,19 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
     fn evaluate_predicate_recursively<'o>(
         &mut self,
         previous_stack: TraitObligationStackList<'o, 'tcx>,
-        obligation: &PredicateObligation<'tcx>,
+        obligation: PredicateObligation<'tcx>,
     ) -> Result<EvaluationResult, OverflowError> {
-        debug!("evaluate_predicate_recursively({:?})", obligation);
+        debug!("evaluate_predicate_recursively(previous_stack={:?}, obligation={:?})",
+            previous_stack.head(), obligation);
+
+        // Previous_stack stores a TraitObligatiom, while 'obligation' is
+        // a PredicateObligation. These are distinct types, so we can't
+        // use any Option combinator method that would force them to be
+        // the same
+        match previous_stack.head() {
+            Some(h) => self.check_recursion_limit(&obligation, h.obligation)?,
+            None => self.check_recursion_limit(&obligation, &obligation)?
+        }
 
         match obligation.predicate {
             ty::Predicate::Trait(ref t) => {
@@ -695,8 +706,9 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
                 match self.infcx
                     .subtype_predicate(&obligation.cause, obligation.param_env, p)
                 {
-                    Some(Ok(InferOk { obligations, .. })) => {
-                        self.evaluate_predicates_recursively(previous_stack, &obligations)
+                    Some(Ok(InferOk { mut obligations, .. })) => {
+                        self.add_depth(obligations.iter_mut(), obligation.recursion_depth);
+                        self.evaluate_predicates_recursively(previous_stack,obligations.into_iter())
                     }
                     Some(Err(_)) => Ok(EvaluatedToErr),
                     None => Ok(EvaluatedToAmbig),
@@ -710,8 +722,9 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
                 ty,
                 obligation.cause.span,
             ) {
-                Some(obligations) => {
-                    self.evaluate_predicates_recursively(previous_stack, obligations.iter())
+                Some(mut obligations) => {
+                    self.add_depth(obligations.iter_mut(), obligation.recursion_depth);
+                    self.evaluate_predicates_recursively(previous_stack, obligations.into_iter())
                 }
                 None => Ok(EvaluatedToAmbig),
             },
@@ -733,10 +746,11 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
             ty::Predicate::Projection(ref data) => {
                 let project_obligation = obligation.with(data.clone());
                 match project::poly_project_and_unify_type(self, &project_obligation) {
-                    Ok(Some(subobligations)) => {
+                    Ok(Some(mut subobligations)) => {
+                        self.add_depth(subobligations.iter_mut(), obligation.recursion_depth);
                         let result = self.evaluate_predicates_recursively(
                             previous_stack,
-                            subobligations.iter(),
+                            subobligations.into_iter(),
                         );
                         if let Some(key) =
                             ProjectionCacheKey::from_poly_projection_predicate(self, data)
@@ -1005,7 +1019,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
             match this.confirm_candidate(stack.obligation, candidate) {
                 Ok(selection) => this.evaluate_predicates_recursively(
                     stack.list(),
-                    selection.nested_obligations().iter(),
+                    selection.nested_obligations().into_iter()
                 ),
                 Err(..) => Ok(EvaluatedToErr),
             }
@@ -1080,6 +1094,45 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
             .insert(trait_ref, WithDepNode::new(dep_node, result));
     }
 
+    // For various reasons, it's possible for a subobligation
+    // to have a *lower* recursion_depth than the obligation used to create it.
+    // Projection sub-obligations may be returned from the projection cache,
+    // which results in obligations with an 'old' recursion_depth.
+    // Additionally, methods like ty::wf::obligations and
+    // InferCtxt.subtype_predicate produce subobligations without
+    // taking in a 'parent' depth, causing the generated subobligations
+    // to have a recursion_depth of 0
+    //
+    // To ensure that obligation_depth never decreasees, we force all subobligations
+    // to have at least the depth of the original obligation.
+    fn add_depth<T: 'cx, I: Iterator<Item = &'cx mut Obligation<'tcx, T>>>(&self, it: I,
+                                                                           min_depth: usize) {
+        it.for_each(|o| o.recursion_depth = cmp::max(min_depth, o.recursion_depth) + 1);
+    }
+
+    // Check that the recursion limit has not been exceeded.
+    //
+    // The weird return type of this function allows it to be used with the 'try' (?)
+    // operator within certain functions
+    fn check_recursion_limit<T: Display + TypeFoldable<'tcx>, V: Display + TypeFoldable<'tcx>>(
+        &self,
+        obligation: &Obligation<'tcx, T>,
+        error_obligation: &Obligation<'tcx, V>
+    ) -> Result<(), OverflowError>  {
+        let recursion_limit = *self.infcx.tcx.sess.recursion_limit.get();
+        if obligation.recursion_depth >= recursion_limit {
+            match self.query_mode {
+                TraitQueryMode::Standard => {
+                    self.infcx().report_overflow_error(error_obligation, true);
+                }
+                TraitQueryMode::Canonical => {
+                    return Err(OverflowError);
+                }
+            }
+        }
+        Ok(())
+    }
+
     ///////////////////////////////////////////////////////////////////////////
     // CANDIDATE ASSEMBLY
     //
@@ -1096,17 +1149,8 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
     ) -> SelectionResult<'tcx, SelectionCandidate<'tcx>> {
         // Watch out for overflow. This intentionally bypasses (and does
         // not update) the cache.
-        let recursion_limit = *self.infcx.tcx.sess.recursion_limit.get();
-        if stack.obligation.recursion_depth >= recursion_limit {
-            match self.query_mode {
-                TraitQueryMode::Standard => {
-                    self.infcx().report_overflow_error(&stack.obligation, true);
-                }
-                TraitQueryMode::Canonical => {
-                    return Err(Overflow);
-                }
-            }
-        }
+        self.check_recursion_limit(&stack.obligation, &stack.obligation)?;
+
 
         // Check the cache. Note that we freshen the trait-ref
         // separately rather than using `stack.fresh_trait_ref` --
@@ -1774,7 +1818,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
         self.evaluation_probe(|this| {
             match this.match_where_clause_trait_ref(stack.obligation, where_clause_trait_ref) {
                 Ok(obligations) => {
-                    this.evaluate_predicates_recursively(stack.list(), obligations.iter())
+                    this.evaluate_predicates_recursively(stack.list(), obligations.into_iter())
                 }
                 Err(()) => Ok(EvaluatedToErr),
             }
@@ -3800,6 +3844,10 @@ impl<'o, 'tcx> TraitObligationStackList<'o, 'tcx> {
     fn with(r: &'o TraitObligationStack<'o, 'tcx>) -> TraitObligationStackList<'o, 'tcx> {
         TraitObligationStackList { head: Some(r) }
     }
+
+    fn head(&self) -> Option<&'o TraitObligationStack<'o, 'tcx>> {
+        self.head
+    }
 }
 
 impl<'o, 'tcx> Iterator for TraitObligationStackList<'o, 'tcx> {

src/librustc_typeck/diagnostics.rs

@@ -516,7 +516,7 @@ recursion limit (which can be set via the `recursion_limit` attribute).
 For a somewhat artificial example:
 
 ```compile_fail,E0055
-#![recursion_limit="2"]
+#![recursion_limit="5"]
 
 struct Foo;
 
@@ -526,9 +526,9 @@ impl Foo {
 
 fn main() {
     let foo = Foo;
-    let ref_foo = &&Foo;
+    let ref_foo = &&&&&Foo;
 
-    // error, reached the recursion limit while auto-dereferencing `&&Foo`
+    // error, reached the recursion limit while auto-dereferencing `&&&&&Foo`
     ref_foo.foo();
 }
 ```

src/test/run-pass/weird-exprs.rs

@@ -4,7 +4,7 @@
 #![allow(unused_parens)]
 // compile-flags: -Z borrowck=compare
 
-#![recursion_limit = "128"]
+#![recursion_limit = "256"]
 
 use std::cell::Cell;
 use std::mem::swap;

src/test/rustdoc/issue-56701.rs

@@ -0,0 +1,34 @@
+// This shouldn't cause a stack overflow when rustdoc is run
+
+use std::ops::Deref;
+use std::ops::DerefMut;
+
+pub trait SimpleTrait {
+    type SimpleT;
+}
+
+impl<Inner: SimpleTrait, Outer: Deref<Target = Inner>> SimpleTrait for Outer {
+    type SimpleT = Inner::SimpleT;
+}
+
+pub trait AnotherTrait {
+    type AnotherT;
+}
+
+impl<T, Simple: SimpleTrait<SimpleT = Vec<T>>> AnotherTrait for Simple {
+    type AnotherT = T;
+}
+
+pub struct Unrelated<Inner, UnrelatedT: DerefMut<Target = Vec<Inner>>>(UnrelatedT);
+
+impl<Inner, UnrelatedT: DerefMut<Target = Vec<Inner>>> Deref for Unrelated<Inner, UnrelatedT> {
+    type Target = Vec<Inner>;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+
+pub fn main() { }
+

src/test/ui/did_you_mean/recursion_limit.stderr

@@ -1,11 +1,10 @@
-error[E0275]: overflow evaluating the requirement `K: std::marker::Send`
+error[E0275]: overflow evaluating the requirement `J: std::marker::Send`
   --> $DIR/recursion_limit.rs:34:5
    |
 LL |     is_send::<A>(); //~ ERROR overflow evaluating the requirement
    |     ^^^^^^^^^^^^
    |
    = help: consider adding a `#![recursion_limit="20"]` attribute to your crate
-   = note: required because it appears within the type `J`
    = note: required because it appears within the type `I`
    = note: required because it appears within the type `H`
    = note: required because it appears within the type `G`

src/test/ui/error-codes/E0055.rs

@@ -1,4 +1,4 @@
-#![recursion_limit="2"]
+#![recursion_limit="5"]
 struct Foo;
 
 impl Foo {
@@ -7,7 +7,7 @@ impl Foo {
 
 fn main() {
     let foo = Foo;
-    let ref_foo = &&Foo;
+    let ref_foo = &&&&&Foo;
     ref_foo.foo();
     //~^ ERROR E0055
 }

src/test/ui/error-codes/E0055.stderr

@@ -4,7 +4,7 @@ error[E0055]: reached the recursion limit while auto-dereferencing `Foo`
 LL |     ref_foo.foo();
    |             ^^^ deref recursion limit reached
    |
-   = help: consider adding a `#![recursion_limit="4"]` attribute to your crate
+   = help: consider adding a `#![recursion_limit="10"]` attribute to your crate
 
 error: aborting due to previous error
 

src/test/ui/error-codes/E0275.stderr

@@ -1,11 +1,10 @@
-error[E0275]: overflow evaluating the requirement `Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<T>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>: std::marker::Sized`
+error[E0275]: overflow evaluating the requirement `Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<T>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>: Foo`
   --> $DIR/E0275.rs:5:1
    |
 LL | impl<T> Foo for T where Bar<T>: Foo {} //~ ERROR E0275
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider adding a `#![recursion_limit="128"]` attribute to your crate
-   = note: required because of the requirements on the impl of `Foo` for `Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<T>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>`
    = note: required because of the requirements on the impl of `Foo` for `Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<T>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>`
    = note: required because of the requirements on the impl of `Foo` for `Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<T>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>`
    = note: required because of the requirements on the impl of `Foo` for `Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<Bar<T>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>`

src/test/ui/issues/issue-20413.stderr

@@ -6,7 +6,7 @@ LL | struct NoData<T>;
    |
    = help: consider removing `T` or using a marker such as `std::marker::PhantomData`
 
-error[E0275]: overflow evaluating the requirement `NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<T>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>: std::marker::Sized`
+error[E0275]: overflow evaluating the requirement `NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<T>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>: Foo`
   --> $DIR/issue-20413.rs:8:1
    |
 LL | / impl<T> Foo for T where NoData<T>: Foo {
@@ -18,7 +18,6 @@ LL | | }
    | |_^
    |
    = help: consider adding a `#![recursion_limit="128"]` attribute to your crate
-   = note: required because of the requirements on the impl of `Foo` for `NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<T>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>`
    = note: required because of the requirements on the impl of `Foo` for `NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<T>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>`
    = note: required because of the requirements on the impl of `Foo` for `NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<T>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>`
    = note: required because of the requirements on the impl of `Foo` for `NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<NoData<T>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>`