fix corner case around top of stack

When deciding on a coinductive match, we were examining the new
obligation and the backtrace, but not the *current* obligation that goes
in between the two.  Refactoring the code to just have the cycle given
as input also made things a lot simpler.

2 files changed, 64 insertions, 39 deletions

diff --git a/src/librustc/traits/fulfill.rs b/src/librustc/traits/fulfill.rs
index 6f3cc49daf2..810dfb960c6 100644
--- a/src/librustc/traits/fulfill.rs
+++ b/src/librustc/traits/fulfill.rs
@@ -407,17 +407,17 @@ fn process_child_obligations<'a,'tcx>(
                 //                            ~~~ (*) see above
                 debug!("process_child_obligations: cycle index = {}", index);
 
-                if coinductive_match(selcx, &obligation, &backtrace) {
+                let backtrace = backtrace.clone();
+                let cycle: Vec<_> =
+                    iter::once(&obligation)
+                    .chain(Some(pending_obligation))
+                    .chain(backtrace.take(index + 1).map(|p| &p.obligation))
+                    .cloned()
+                    .collect();
+                if coinductive_match(selcx, &cycle) {
                     debug!("process_child_obligations: coinductive match");
                     None
                 } else {
-                    let backtrace = backtrace.clone();
-                    let cycle: Vec<_> =
-                        iter::once(&obligation)
-                        .chain(Some(pending_obligation))
-                        .chain(backtrace.take(index + 1).map(|p| &p.obligation))
-                        .cloned()
-                        .collect();
                     report_overflow_error_cycle(selcx.infcx(), &cycle);
                 }
             } else {
@@ -663,47 +663,40 @@ fn process_predicate1<'a,'tcx>(selcx: &mut SelectionContext<'a,'tcx>,
 
 /// For defaulted traits, we use a co-inductive strategy to solve, so
 /// that recursion is ok. This routine returns true if the top of the
-/// stack (`top_obligation` and `top_data`):
+/// stack (`cycle[0]`):
 /// - is a defaulted trait, and
 /// - it also appears in the backtrace at some position `X`; and,
 /// - all the predicates at positions `X..` between `X` an the top are
 ///   also defaulted traits.
 fn coinductive_match<'a,'tcx>(selcx: &mut SelectionContext<'a,'tcx>,
-                              top_obligation: &PredicateObligation<'tcx>,
-                              backtrace: &Backtrace<PendingPredicateObligation<'tcx>>)
+                              cycle: &[PredicateObligation<'tcx>])
                               -> bool
 {
-    // only trait predicates can be coinductive matches
-    let top_data = match top_obligation.predicate {
-        ty::Predicate::Trait(ref data) => data,
-        _ => return false
-    };
-
-    if selcx.tcx().trait_has_default_impl(top_data.def_id()) {
-        debug!("coinductive_match: top_data={:?}", top_data);
-        for bt_obligation in backtrace.clone() {
-            debug!("coinductive_match: bt_obligation={:?}", bt_obligation);
-
-            // *Everything* in the backtrace must be a defaulted trait.
-            match bt_obligation.obligation.predicate {
-                ty::Predicate::Trait(ref data) => {
-                    if !selcx.tcx().trait_has_default_impl(data.def_id()) {
-                        debug!("coinductive_match: trait does not have default impl");
-                        break;
-                    }
-                }
-                _ => { break; }
-            }
+    let len = cycle.len();
 
-            // And we must find a recursive match.
-            if bt_obligation.obligation.predicate == top_obligation.predicate {
-                debug!("coinductive_match: found a match in the backtrace");
-                return true;
-            }
+    assert_eq!(cycle[0].predicate, cycle[len - 1].predicate);
+
+    cycle[0..len-1]
+        .iter()
+        .all(|bt_obligation| {
+            let result = coinductive_obligation(selcx, bt_obligation);
+            debug!("coinductive_match: bt_obligation={:?} coinductive={}",
+                   bt_obligation, result);
+            result
+        })
+}
+
+fn coinductive_obligation<'a, 'tcx>(selcx: &SelectionContext<'a, 'tcx>,
+                                    obligation: &PredicateObligation<'tcx>)
+                                    -> bool {
+    match obligation.predicate {
+        ty::Predicate::Trait(ref data) => {
+            selcx.tcx().trait_has_default_impl(data.def_id())
+        }
+        _ => {
+            false
         }
     }
-
-    false
 }
 
 fn register_region_obligation<'tcx>(t_a: Ty<'tcx>,
diff --git a/src/test/compile-fail/traits-inductive-overflow-auto-normal-auto.rs b/src/test/compile-fail/traits-inductive-overflow-auto-normal-auto.rs
new file mode 100644
index 00000000000..6015c5669cd
--- /dev/null
+++ b/src/test/compile-fail/traits-inductive-overflow-auto-normal-auto.rs
@@ -0,0 +1,32 @@
+// Copyright 2015 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.
+
+// Test for a potential corner case in current impl where you have an
+// auto trait (Magic1) that depends on a normal trait (Magic2) which
+// in turn depends on the auto trait (Magic1). This was incorrectly
+// being considered coinductive, but because of the normal trait
+// interfering, it should not be.
+
+#![feature(optin_builtin_traits)]
+
+trait Magic1: Magic2 { }
+impl Magic1 for .. {}
+
+trait Magic2 { }
+impl<T: Magic1> Magic2 for T { }
+
+fn is_magic1<T: Magic1>() { }
+
+#[derive(Debug)]
+struct NoClone;
+
+fn main() {
+    is_magic1::<NoClone>();
+}