Auto merge of #30676 - nikomatsakis:issue-29857, r=arielb1

This is an alternative to #29954 for fixing #29857 that seems to me to be more inline with the general strategy around `TyError`. It also includes the fix for #30589 -- in fact, just the minimal change of making `ty_is_local` tolerate `TyError` avoids the ICE, but you get a lot of duplicate error reports, so in the case where the impl's trait reference already includes `TyError`, we just ignore the impl altogether.

cc @arielb1 @sanxiyn

Fixes #29857.
Fixes #30589.

Author: bors

src/librustc/middle/traits/coherence.rs

@@ -63,9 +63,9 @@ fn overlap<'cx, 'tcx>(selcx: &mut SelectionContext<'cx, 'tcx>,
                                                                 b_def_id,
                                                                 util::fresh_type_vars_for_impl);
 
-    debug!("overlap: a_trait_ref={:?}", a_trait_ref);
+    debug!("overlap: a_trait_ref={:?} a_obligations={:?}", a_trait_ref, a_obligations);
 
-    debug!("overlap: b_trait_ref={:?}", b_trait_ref);
+    debug!("overlap: b_trait_ref={:?} b_obligations={:?}", b_trait_ref, b_obligations);
 
     // Do `a` and `b` unify? If not, no overlap.
     if let Err(_) = infer::mk_eq_trait_refs(selcx.infcx(),
@@ -330,8 +330,11 @@ fn ty_is_local_constructor<'tcx>(tcx: &ty::ctxt<'tcx>,
             tt.principal_def_id().is_local()
         }
 
-        ty::TyClosure(..) |
         ty::TyError => {
+            true
+        }
+
+        ty::TyClosure(..) => {
             tcx.sess.bug(
                 &format!("ty_is_local invoked on unexpected type: {:?}",
                         ty))

src/librustc/middle/traits/project.rs

@@ -426,11 +426,25 @@ fn opt_normalize_projection_type<'a,'b,'tcx>(
     }
 }
 
-/// in various error cases, we just set TyError and return an obligation
-/// that, when fulfilled, will lead to an error.
+/// If we are projecting `<T as Trait>::Item`, but `T: Trait` does not
+/// hold. In various error cases, we cannot generate a valid
+/// normalized projection. Therefore, we create an inference variable
+/// return an associated obligation that, when fulfilled, will lead to
+/// an error.
 ///
-/// FIXME: the TyError created here can enter the obligation we create,
-/// leading to error messages involving TyError.
+/// Note that we used to return `TyError` here, but that was quite
+/// dubious -- the premise was that an error would *eventually* be
+/// reported, when the obligation was processed. But in general once
+/// you see a `TyError` you are supposed to be able to assume that an
+/// error *has been* reported, so that you can take whatever heuristic
+/// paths you want to take. To make things worse, it was possible for
+/// cycles to arise, where you basically had a setup like `<MyType<$0>
+/// as Trait>::Foo == $0`. Here, normalizing `<MyType<$0> as
+/// Trait>::Foo> to `[type error]` would lead to an obligation of
+/// `<MyType<[type error]> as Trait>::Foo`.  We are supposed to report
+/// an error for this obligation, but we legitimately should not,
+/// because it contains `[type error]`. Yuck! (See issue #29857 for
+/// one case where this arose.)
 fn normalize_to_error<'a,'tcx>(selcx: &mut SelectionContext<'a,'tcx>,
                                projection_ty: ty::ProjectionTy<'tcx>,
                                cause: ObligationCause<'tcx>,
@@ -441,8 +455,9 @@ fn normalize_to_error<'a,'tcx>(selcx: &mut SelectionContext<'a,'tcx>,
     let trait_obligation = Obligation { cause: cause,
                                         recursion_depth: depth,
                                         predicate: trait_ref.to_predicate() };
+    let new_value = selcx.infcx().next_ty_var();
     Normalized {
-        value: selcx.tcx().types.err,
+        value: new_value,
         obligations: vec!(trait_obligation)
     }
 }

src/librustc/middle/traits/select.rs

@@ -210,8 +210,6 @@ enum SelectionCandidate<'tcx> {
     BuiltinObjectCandidate,
 
     BuiltinUnsizeCandidate,
-
-    ErrorCandidate,
 }
 
 struct SelectionCandidateSet<'tcx> {
@@ -753,8 +751,15 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                                               stack: &TraitObligationStack<'o, 'tcx>)
                                               -> SelectionResult<'tcx, SelectionCandidate<'tcx>>
     {
-        if stack.obligation.predicate.0.self_ty().references_error() {
-            return Ok(Some(ErrorCandidate));
+        if stack.obligation.predicate.references_error() {
+            // If we encounter a `TyError`, we generally prefer the
+            // most "optimistic" result in response -- that is, the
+            // one least likely to report downstream errors. But
+            // because this routine is shared by coherence and by
+            // trait selection, there isn't an obvious "right" choice
+            // here in that respect, so we opt to just return
+            // ambiguity and let the upstream clients sort it out.
+            return Ok(None);
         }
 
         if !self.is_knowable(stack) {
@@ -1587,7 +1592,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                     true
                 },
                 &ParamCandidate(..) => false,
-                &ErrorCandidate => false // propagate errors
             },
             _ => false
         }
@@ -1998,10 +2002,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                     try!(self.confirm_builtin_candidate(obligation, builtin_bound))))
             }
 
-            ErrorCandidate => {
-                Ok(VtableBuiltin(VtableBuiltinData { nested: vec![] }))
-            }
-
             ParamCandidate(param) => {
                 let obligations = self.confirm_param_candidate(obligation, param);
                 Ok(VtableParam(obligations))

src/librustc_typeck/check/coercion.rs

@@ -69,6 +69,7 @@ use middle::ty::adjustment::{AutoAdjustment, AutoDerefRef, AdjustDerefRef};
 use middle::ty::adjustment::{AutoPtr, AutoUnsafe, AdjustReifyFnPointer};
 use middle::ty::adjustment::{AdjustUnsafeFnPointer};
 use middle::ty::{self, LvaluePreference, TypeAndMut, Ty};
+use middle::ty::fold::TypeFoldable;
 use middle::ty::error::TypeError;
 use middle::ty::relate::RelateResult;
 use util::common::indent;
@@ -110,10 +111,15 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
                a,
                b);
 
+        let a = self.fcx.infcx().shallow_resolve(a);
+
+        // Just ignore error types.
+        if a.references_error() || b.references_error() {
+            return Ok(None);
+        }
+
         // Consider coercing the subtype to a DST
-        let unsize = self.unpack_actual_value(a, |a| {
-            self.coerce_unsized(a, b)
-        });
+        let unsize = self.coerce_unsized(a, b);
         if unsize.is_ok() {
             return unsize;
         }
@@ -124,39 +130,33 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
         // See above for details.
         match b.sty {
             ty::TyRawPtr(mt_b) => {
-                return self.unpack_actual_value(a, |a| {
-                    self.coerce_unsafe_ptr(a, b, mt_b.mutbl)
-                });
+                return self.coerce_unsafe_ptr(a, b, mt_b.mutbl);
             }
 
             ty::TyRef(_, mt_b) => {
-                return self.unpack_actual_value(a, |a| {
-                    self.coerce_borrowed_pointer(expr_a, a, b, mt_b.mutbl)
-                });
+                return self.coerce_borrowed_pointer(expr_a, a, b, mt_b.mutbl);
             }
 
             _ => {}
         }
 
-        self.unpack_actual_value(a, |a| {
-            match a.sty {
-                ty::TyBareFn(Some(_), a_f) => {
-                    // Function items are coercible to any closure
-                    // type; function pointers are not (that would
-                    // require double indirection).
-                    self.coerce_from_fn_item(a, a_f, b)
-                }
-                ty::TyBareFn(None, a_f) => {
-                    // We permit coercion of fn pointers to drop the
-                    // unsafe qualifier.
-                    self.coerce_from_fn_pointer(a, a_f, b)
-                }
-                _ => {
-                    // Otherwise, just use subtyping rules.
-                    self.subtype(a, b)
-                }
+        match a.sty {
+            ty::TyBareFn(Some(_), a_f) => {
+                // Function items are coercible to any closure
+                // type; function pointers are not (that would
+                // require double indirection).
+                self.coerce_from_fn_item(a, a_f, b)
             }
-        })
+            ty::TyBareFn(None, a_f) => {
+                // We permit coercion of fn pointers to drop the
+                // unsafe qualifier.
+                self.coerce_from_fn_pointer(a, a_f, b)
+            }
+            _ => {
+                // Otherwise, just use subtyping rules.
+                self.subtype(a, b)
+            }
+        }
     }
 
     /// Reborrows `&mut A` to `&mut B` and `&(mut) A` to `&B`.

src/librustc_typeck/check/mod.rs

@@ -1038,6 +1038,9 @@ fn report_cast_to_unsized_type<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
                                          t_cast: Ty<'tcx>,
                                          t_expr: Ty<'tcx>,
                                          id: ast::NodeId) {
+    if t_cast.references_error() || t_expr.references_error() {
+        return;
+    }
     let tstr = fcx.infcx().ty_to_string(t_cast);
     let mut err = fcx.type_error_struct(span, |actual| {
         format!("cast to unsized type: `{}` as `{}`", actual, tstr)
@@ -3511,9 +3514,10 @@ fn check_expr_with_unifier<'a, 'tcx, F>(fcx: &FnCtxt<'a, 'tcx>,
         let t_cast = structurally_resolved_type(fcx, expr.span, t_cast);
         check_expr_with_expectation(fcx, e, ExpectCastableToType(t_cast));
         let t_expr = fcx.expr_ty(e);
+        let t_cast = fcx.infcx().resolve_type_vars_if_possible(&t_cast);
 
         // Eagerly check for some obvious errors.
-        if t_expr.references_error() {
+        if t_expr.references_error() || t_cast.references_error() {
             fcx.write_error(id);
         } else if !fcx.type_is_known_to_be_sized(t_cast, expr.span) {
             report_cast_to_unsized_type(fcx, expr.span, t.span, e.span, t_cast, t_expr, id);

src/librustc_typeck/coherence/mod.rs

@@ -149,11 +149,23 @@ impl<'a, 'tcx> CoherenceChecker<'a, 'tcx> {
                    trait_ref,
                    item.name);
 
+            // Skip impls where one of the self type is an error type.
+            // This occurs with e.g. resolve failures (#30589).
+            if trait_ref.references_error() {
+                return;
+            }
+
             enforce_trait_manually_implementable(self.crate_context.tcx,
                                                  item.span,
                                                  trait_ref.def_id);
             self.add_trait_impl(trait_ref, impl_did);
         } else {
+            // Skip inherent impls where the self type is an error
+            // type. This occurs with e.g. resolve failures (#30589).
+            if self_type.ty.references_error() {
+                return;
+            }
+
             // Add the implementation to the mapping from implementation to base
             // type def ID, if there is a base type for this implementation and
             // the implementation does not have any associated traits.

src/test/compile-fail/coherence-projection-conflict-orphan.rs

@@ -0,0 +1,28 @@
+// Copyright 2016 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.
+
+#![feature(rustc_attrs)]
+
+// Here we expect a coherence conflict because, even though `i32` does
+// not implement `Iterator`, we cannot rely on that negative reasoning
+// due to the orphan rules. Therefore, `A::Item` may yet turn out to
+// be `i32`.
+
+pub trait Foo<P> {}
+
+pub trait Bar {
+    type Output: 'static;
+}
+
+impl Foo<i32> for i32 { } //~ ERROR E0119
+
+impl<A:Iterator> Foo<A::Item> for A { }
+
+fn main() {}

src/test/compile-fail/coherence-projection-conflict-ty-param.rs

@@ -0,0 +1,22 @@
+// Copyright 2016 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.
+
+// Coherence error results because we do not know whether `T: Foo<P>` or not
+// for the second impl.
+
+use std::marker::PhantomData;
+
+pub trait Foo<P> {}
+
+impl <P, T: Foo<P>> Foo<P> for Option<T> {} //~ ERROR E0119
+
+impl<T, U> Foo<T> for Option<U> { }
+
+fn main() {}

src/test/compile-fail/coherence-projection-conflict.rs

@@ -0,0 +1,27 @@
+// Copyright 2016 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::marker::PhantomData;
+
+pub trait Foo<P> {}
+
+pub trait Bar {
+    type Output: 'static;
+}
+
+impl Foo<i32> for i32 { } //~ ERROR E0119
+
+impl<A:Bar> Foo<A::Output> for A { }
+
+impl Bar for i32 {
+    type Output = i32;
+}
+
+fn main() {}

src/test/compile-fail/coherence-projection-ok-orphan.rs

@@ -0,0 +1,29 @@
+// Copyright 2016 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.
+
+#![feature(rustc_attrs)]
+#![allow(dead_code)]
+
+// Here we do not get a coherence conflict because `Baz: Iterator`
+// does not hold and (due to the orphan rules), we can rely on that.
+
+pub trait Foo<P> {}
+
+pub trait Bar {
+    type Output: 'static;
+}
+
+struct Baz;
+impl Foo<i32> for Baz { }
+
+impl<A:Iterator> Foo<A::Item> for A { }
+
+#[rustc_error]
+fn main() {} //~ ERROR compilation successful

src/test/compile-fail/coherence-projection-ok.rs

@@ -0,0 +1,28 @@
+// Copyright 2016 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.
+
+#![feature(rustc_attrs)]
+
+pub trait Foo<P> {}
+
+pub trait Bar {
+    type Output: 'static;
+}
+
+impl Foo<i32> for i32 { }
+
+impl<A:Bar> Foo<A::Output> for A { }
+
+impl Bar for i32 {
+    type Output = u32;
+}
+
+#[rustc_error]
+fn main() {} //~ ERROR compilation successful

src/test/compile-fail/const-eval-overflow-4b.rs

@@ -22,7 +22,7 @@ use std::{u8, u16, u32, u64, usize};
 const A_I8_T
     : [u32; (i8::MAX as i8 + 1u8) as usize]
     //~^ ERROR mismatched types
-    //~| the trait `core::ops::Add<u8>` is not implemented for the type `i8`
+    //~| ERROR the trait `core::ops::Add<u8>` is not implemented for the type `i8`
     = [0; (i8::MAX as usize) + 1];
 
 fn main() {

src/test/compile-fail/issue-19692.rs

@@ -12,7 +12,7 @@ struct Homura;
 
 fn akemi(homura: Homura) {
     let Some(ref madoka) = Some(homura.kaname()); //~ ERROR no method named `kaname` found
-    madoka.clone(); //~ ERROR the type of this value must be known in this context
+    madoka.clone(); //~ ERROR the type of this value must be known
 }
 
 fn main() { }