fix remaining bugs

6 files changed, 125 insertions, 59 deletions

diff --git a/src/librustc/middle/traits/mod.rs b/src/librustc/middle/traits/mod.rs
index 9d8138dc38a..691bac0cef8 100644
--- a/src/librustc/middle/traits/mod.rs
+++ b/src/librustc/middle/traits/mod.rs
@@ -340,47 +340,53 @@ pub fn type_known_to_meet_builtin_bound<'a,'tcx>(infcx: &InferCtxt<'a,'tcx>,
            ty,
            bound);
 
-    if !ty.has_infer_types() && !ty.has_closure_types() {
-        let cause = ObligationCause::misc(span, ast::DUMMY_NODE_ID);
-        let obligation =
-            util::predicate_for_builtin_bound(infcx.tcx, cause, bound, 0, ty);
-        let obligation = match obligation {
-            Ok(o) => o,
-            Err(..) => return false
-        };
-        let result = SelectionContext::new(infcx)
-            .evaluate_obligation_conservatively(&obligation);
-        debug!("type_known_to_meet_builtin_bound: ty={:?} bound={:?} => {:?}",
-               ty, bound, result);
-        return result;
-    }
-
-    let mut fulfill_cx = FulfillmentContext::new(false);
-
-    // We can use a dummy node-id here because we won't pay any mind
-    // to region obligations that arise (there shouldn't really be any
-    // anyhow).
     let cause = ObligationCause::misc(span, ast::DUMMY_NODE_ID);
+    let obligation =
+        util::predicate_for_builtin_bound(infcx.tcx, cause, bound, 0, ty);
+    let obligation = match obligation {
+        Ok(o) => o,
+        Err(..) => return false
+    };
+    let result = SelectionContext::new(infcx)
+        .evaluate_obligation_conservatively(&obligation);
+    debug!("type_known_to_meet_builtin_bound: ty={:?} bound={:?} => {:?}",
+           ty, bound, result);
+
+    if result && (ty.has_infer_types() || ty.has_closure_types()) {
+        // Because of inference "guessing", selection can sometimes claim
+        // to succeed while the success requires a guess. To ensure
+        // this function's result remains infallible, we must confirm
+        // that guess. While imperfect, I believe this is sound.
+
+        let mut fulfill_cx = FulfillmentContext::new(false);
+
+        // We can use a dummy node-id here because we won't pay any mind
+        // to region obligations that arise (there shouldn't really be any
+        // anyhow).
+        let cause = ObligationCause::misc(span, ast::DUMMY_NODE_ID);
 
-    fulfill_cx.register_builtin_bound(infcx, ty, bound, cause);
-
-    // Note: we only assume something is `Copy` if we can
-    // *definitively* show that it implements `Copy`. Otherwise,
-    // assume it is move; linear is always ok.
-    match fulfill_cx.select_all_or_error(infcx) {
-        Ok(()) => {
-            debug!("type_known_to_meet_builtin_bound: ty={:?} bound={:?} success",
-                   ty,
-                   bound);
-            true
-        }
-        Err(e) => {
-            debug!("type_known_to_meet_builtin_bound: ty={:?} bound={:?} errors={:?}",
-                   ty,
-                   bound,
-                   e);
-            false
+        fulfill_cx.register_builtin_bound(infcx, ty, bound, cause);
+
+        // Note: we only assume something is `Copy` if we can
+        // *definitively* show that it implements `Copy`. Otherwise,
+        // assume it is move; linear is always ok.
+        match fulfill_cx.select_all_or_error(infcx) {
+            Ok(()) => {
+                debug!("type_known_to_meet_builtin_bound: ty={:?} bound={:?} success",
+                       ty,
+                       bound);
+                true
+            }
+            Err(e) => {
+                debug!("type_known_to_meet_builtin_bound: ty={:?} bound={:?} errors={:?}",
+                       ty,
+                       bound,
+                       e);
+                false
+            }
         }
+    } else {
+        result
     }
 }
 
diff --git a/src/librustc/middle/traits/select.rs b/src/librustc/middle/traits/select.rs
index 2bc1e8faddb..98ee94fe328 100644
--- a/src/librustc/middle/traits/select.rs
+++ b/src/librustc/middle/traits/select.rs
@@ -2851,18 +2851,37 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                                  -> Vec<PredicateObligation<'tcx>>
     {
         debug!("impl_or_trait_obligations(def_id={:?})", def_id);
+        let tcx = self.tcx();
 
-        let predicates = self.tcx().lookup_predicates(def_id);
-        let predicates = predicates.instantiate(self.tcx(), substs);
-        let predicates = normalize_with_depth(self, cause.clone(), recursion_depth, &predicates);
-        let predicates = self.infcx().plug_leaks(skol_map, snapshot, &predicates);
-
-        let mut obligations = predicates.obligations;
-        obligations.append(
-            &mut util::predicates_for_generics(cause,
-                                               recursion_depth,
-                                               &predicates.value));
-        obligations
+        // To allow for one-pass evaluation of the nested obligation,
+        // each predicate must be preceded by the obligations required
+        // to normalize it.
+        // for example, if we have:
+        //    impl<U: Iterator, V: Iterator<Item=U>> Foo for V where U::Item: Copy
+        // the impl will have the following predicates:
+        //    <V as Iterator>::Item = U,
+        //    U: Iterator, U: Sized,
+        //    V: Iterator, V: Sized,
+        //    <U as Iterator>::Item: Copy
+        // When we substitute, say, `V => IntoIter<u32>, U => $0`, the last
+        // obligation will normalize to `<$0 as Iterator>::Item = $1` and
+        // `$1: Copy`, so we must ensure the obligations are emitted in
+        // that order.
+        let predicates = tcx
+            .lookup_predicates(def_id)
+            .predicates.iter()
+            .flat_map(|predicate| {
+                let predicate =
+                    normalize_with_depth(self, cause.clone(), recursion_depth,
+                                         &predicate.subst(tcx, substs));
+                predicate.obligations.into_iter().chain(
+                    Some(Obligation {
+                        cause: cause.clone(),
+                        recursion_depth: recursion_depth,
+                        predicate: predicate.value
+                    }))
+            }).collect();
+        self.infcx().plug_leaks(skol_map, snapshot, &predicates)
     }
 
     #[allow(unused_comparisons)]
diff --git a/src/test/compile-fail/cast-rfc0401.rs b/src/test/compile-fail/cast-rfc0401.rs
index 4603ed00347..d14b0fa9e66 100644
--- a/src/test/compile-fail/cast-rfc0401.rs
+++ b/src/test/compile-fail/cast-rfc0401.rs
@@ -37,6 +37,7 @@ fn main()
     let f: f32 = 1.2;
     let v = 0 as *const u8;
     let fat_v : *const [u8] = unsafe { &*(0 as *const [u8; 1])};
+    let fat_sv : *const [i8] = unsafe { &*(0 as *const [i8; 1])};
     let foo: &Foo = &f;
 
     let _ = v as &u8; //~ ERROR non-scalar
@@ -94,7 +95,7 @@ fn main()
     let _ = main as *mut str; //~ ERROR casting
     let _ = &f as *mut f32; //~ ERROR casting
     let _ = &f as *const f64; //~ ERROR casting
-    let _ = fat_v as usize;
+    let _ = fat_sv as usize;
     //~^ ERROR casting
     //~^^ HELP through a thin pointer first
 
@@ -106,7 +107,7 @@ fn main()
     let _ = main.f as *const u32; //~ ERROR attempted access of field
 
     let cf: *const Foo = &0;
-    let _ = cf as *const [u8];
+    let _ = cf as *const [u16];
     //~^ ERROR casting
     //~^^ NOTE vtable kinds
     let _ = cf as *const Bar;
diff --git a/src/test/compile-fail/infinite-instantiation.rs b/src/test/compile-fail/infinite-instantiation.rs
index 9b02bf4cb85..28806b6e2ab 100644
--- a/src/test/compile-fail/infinite-instantiation.rs
+++ b/src/test/compile-fail/infinite-instantiation.rs
@@ -8,7 +8,6 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-//~^^^^^^^^^^ ERROR overflow
 //
 // We get an error message at the top of file (dummy span).
 // This is not helpful, but also kind of annoying to prevent,
@@ -32,6 +31,7 @@ impl<T:Clone> ToOpt for Option<T> {
 }
 
 fn function<T:ToOpt + Clone>(counter: usize, t: T) {
+//~^ ERROR reached the recursion limit during monomorphization
     if counter > 0 {
         function(counter - 1, t.to_option());
         // FIXME(#4287) Error message should be here. It should be
diff --git a/src/test/compile-fail/recursion.rs b/src/test/compile-fail/recursion.rs
index 55f3b995336..b1d45a82276 100644
--- a/src/test/compile-fail/recursion.rs
+++ b/src/test/compile-fail/recursion.rs
@@ -8,12 +8,6 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-//~^^^^^^^^^^ ERROR overflow
-//
-// We get an error message at the top of file (dummy span).
-// This is not helpful, but also kind of annoying to prevent,
-// so for now just live with it.
-
 enum Nil {NilValue}
 struct Cons<T> {head:isize, tail:T}
 trait Dot {fn dot(&self, other:Self) -> isize;}
@@ -26,7 +20,7 @@ impl<T:Dot> Dot for Cons<T> {
   }
 }
 fn test<T:Dot> (n:isize, i:isize, first:T, second:T) ->isize {
-  match n {    0 => {first.dot(second)}
+  match n {    0 => {first.dot(second)} //~ ERROR overflow
       // FIXME(#4287) Error message should be here. It should be
       // a type error to instantiate `test` at a type other than T.
     _ => {test (n-1, i+1, Cons {head:2*i+1, tail:first}, Cons{head:i*i, tail:second})}
diff --git a/src/test/run-pass/trait-copy-guessing.rs b/src/test/run-pass/trait-copy-guessing.rs
new file mode 100644
index 00000000000..71cd3c9441e
--- /dev/null
+++ b/src/test/run-pass/trait-copy-guessing.rs
@@ -0,0 +1,46 @@
+// 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.
+
+// "guessing" in trait selection can affect `copy_or_move`. Check that this
+// is correctly handled. I am not sure what is the "correct" behaviour,
+// but we should at least not ICE.
+
+use std::mem;
+
+struct U([u8; 1337]);
+
+struct S<'a,T:'a>(&'a T);
+impl<'a, T> Clone for S<'a, T> { fn clone(&self) -> Self { S(self.0) } }
+/// This impl triggers inference "guessing" - S<_>: Copy => _ = U
+impl<'a> Copy for S<'a, Option<U>> {}
+
+fn assert_impls_fn<R,T: Fn()->R>(_: &T){}
+
+fn main() {
+    let n = None;
+    let e = S(&n);
+    let f = || {
+        // S being copy is critical for this to work
+        drop(e);
+        mem::size_of_val(e.0)
+    };
+    assert_impls_fn(&f);
+    assert_eq!(f(), 1337+1);
+
+    assert_eq!((|| {
+        // S being Copy is not critical here, but
+        // we check it anyway.
+        let n = None;
+        let e = S(&n);
+        let ret = mem::size_of_val(e.0);
+        drop(e);
+        ret
+    })(), 1337+1);
+}