Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make coherence more tolerant of error types. #30676

Merged
merged 6 commits into from
Jan 11, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/librustc/middle/traits/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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))
Expand Down
25 changes: 20 additions & 5 deletions src/librustc/middle/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand All @@ -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)
}
}
Expand Down
18 changes: 9 additions & 9 deletions src/librustc/middle/traits/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,6 @@ enum SelectionCandidate<'tcx> {
BuiltinObjectCandidate,

BuiltinUnsizeCandidate,

ErrorCandidate,
}

struct SelectionCandidateSet<'tcx> {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -1587,7 +1592,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
true
},
&ParamCandidate(..) => false,
&ErrorCandidate => false // propagate errors
},
_ => false
}
Expand Down Expand Up @@ -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))
Expand Down
54 changes: 27 additions & 27 deletions src/librustc_typeck/check/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand All @@ -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`.
Expand Down
6 changes: 5 additions & 1 deletion src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Die!

report_cast_to_unsized_type(fcx, expr.span, t.span, e.span, t_cast, t_expr, id);
Expand Down
12 changes: 12 additions & 0 deletions src/librustc_typeck/coherence/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
28 changes: 28 additions & 0 deletions src/test/compile-fail/coherence-projection-conflict-orphan.rs
Original file line number Diff line number Diff line change
@@ -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() {}
22 changes: 22 additions & 0 deletions src/test/compile-fail/coherence-projection-conflict-ty-param.rs
Original file line number Diff line number Diff line change
@@ -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() {}
27 changes: 27 additions & 0 deletions src/test/compile-fail/coherence-projection-conflict.rs
Original file line number Diff line number Diff line change
@@ -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() {}
29 changes: 29 additions & 0 deletions src/test/compile-fail/coherence-projection-ok-orphan.rs
Original file line number Diff line number Diff line change
@@ -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
28 changes: 28 additions & 0 deletions src/test/compile-fail/coherence-projection-ok.rs
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion src/test/compile-fail/const-eval-overflow-4b.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion src/test/compile-fail/issue-19692.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() { }
Loading