Skip to content

Commit

Permalink
Auto merge of #21792 - nikomatsakis:orphan-ordered-first, r=aturon
Browse files Browse the repository at this point in the history
Update the coherence rules to "covered first" -- the first type parameter to contain either a local type or a type parameter must contain only covered type parameters.

cc #19470.
Fixes #20974.
Fixes #20749.

r? @aturon
  • Loading branch information
bors committed Feb 1, 2015
2 parents f1398d2 + 2c2879b commit e8489d3
Show file tree
Hide file tree
Showing 10 changed files with 135 additions and 28 deletions.
56 changes: 36 additions & 20 deletions src/librustc/middle/traits/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use super::{Obligation, ObligationCause};
use super::project;
use super::util;

use middle::subst::{Subst};
use middle::subst::{Subst, TypeSpace};
use middle::ty::{self, Ty};
use middle::infer::InferCtxt;
use std::collections::HashSet;
Expand Down Expand Up @@ -89,16 +89,28 @@ pub fn orphan_check<'tcx>(tcx: &ty::ctxt<'tcx>,
return Ok(());
}

// Otherwise, check that (1) all type parameters are covered.
let covered_params = type_parameters_covered_by_ty(tcx, trait_ref.self_ty());
let all_params = type_parameters_reachable_from_ty(trait_ref.self_ty());
for &param in all_params.difference(&covered_params) {
return Err(OrphanCheckErr::UncoveredTy(param));
}

// And (2) some local type appears.
if !trait_ref.self_ty().walk().any(|t| ty_is_local_constructor(tcx, t)) {
return Err(OrphanCheckErr::NoLocalInputType);
// First, create an ordered iterator over all the type parameters to the trait, with the self
// type appearing first.
let input_tys = Some(trait_ref.self_ty());
let input_tys = input_tys.iter().chain(trait_ref.substs.types.get_slice(TypeSpace).iter());
let mut input_tys = input_tys;

// Find the first input type that either references a type parameter OR
// some local type.
match input_tys.find(|&&input_ty| references_local_or_type_parameter(tcx, input_ty)) {
Some(&input_ty) => {
// Within this first type, check that all type parameters are covered by a local
// type constructor. Note that if there is no local type constructor, then any
// type parameter at all will be an error.
let covered_params = type_parameters_covered_by_ty(tcx, input_ty);
let all_params = type_parameters_reachable_from_ty(input_ty);
for &param in all_params.difference(&covered_params) {
return Err(OrphanCheckErr::UncoveredTy(param));
}
}
None => {
return Err(OrphanCheckErr::NoLocalInputType);
}
}

return Ok(());
Expand Down Expand Up @@ -162,13 +174,17 @@ fn type_parameters_covered_by_ty<'tcx>(tcx: &ty::ctxt<'tcx>,

/// All type parameters reachable from `ty`
fn type_parameters_reachable_from_ty<'tcx>(ty: Ty<'tcx>) -> HashSet<Ty<'tcx>> {
ty.walk()
.filter(|&t| {
match t.sty {
// FIXME(#20590) straighten story about projection types
ty::ty_projection(..) | ty::ty_param(..) => true,
_ => false,
}
})
.collect()
ty.walk().filter(|&t| is_type_parameter(t)).collect()
}

fn references_local_or_type_parameter<'tcx>(tcx: &ty::ctxt<'tcx>, ty: Ty<'tcx>) -> bool {
ty.walk().any(|ty| is_type_parameter(ty) || ty_is_local_constructor(tcx, ty))
}

fn is_type_parameter<'tcx>(ty: Ty<'tcx>) -> bool {
match ty.sty {
// FIXME(#20590) straighten story about projection types
ty::ty_projection(..) | ty::ty_param(..) => true,
_ => false,
}
}
6 changes: 2 additions & 4 deletions src/librustc_typeck/coherence/orphan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,12 @@ impl<'cx, 'tcx,'v> visit::Visitor<'v> for OrphanChecker<'cx, 'tcx> {
Ok(()) => { }
Err(traits::OrphanCheckErr::NoLocalInputType) => {
if !ty::has_attr(self.tcx, trait_def_id, "old_orphan_check") {
let self_ty = ty::lookup_item_type(self.tcx, def_id).ty;
span_err!(
self.tcx.sess, item.span, E0117,
"the type `{}` does not reference any \
"the impl does not reference any \
types defined in this crate; \
only traits defined in the current crate can be \
implemented for arbitrary types",
self_ty.user_string(self.tcx));
implemented for arbitrary types");
}
}
Err(traits::OrphanCheckErr::UncoveredTy(param_ty)) => {
Expand Down
2 changes: 1 addition & 1 deletion src/test/compile-fail/coherence-all-remote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@ extern crate "coherence-lib" as lib;
use lib::Remote1;

impl<T> Remote1<T> for isize { }
//~^ ERROR E0117
//~^ ERROR E0210

fn main() { }
23 changes: 23 additions & 0 deletions src/test/compile-fail/coherence-cow-no-cover.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright 2014 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.

// aux-build:coherence-lib.rs

// Test that it's not ok for U to appear uncovered

extern crate "coherence-lib" as lib;
use lib::{Remote,Pair};

pub struct Cover<T>(T);

impl<T,U> Remote for Pair<Cover<T>,U> { }
//~^ ERROR type parameter `U` is not constrained by any local type

fn main() { }
2 changes: 1 addition & 1 deletion src/test/compile-fail/coherence-orphan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ struct TheType;

impl TheTrait<usize> for isize { } //~ ERROR E0117

impl TheTrait<TheType> for isize { } //~ ERROR E0117
impl TheTrait<TheType> for isize { }

impl TheTrait<isize> for TheType { }

Expand Down
24 changes: 24 additions & 0 deletions src/test/compile-fail/coherence-pair-covered-uncovered-1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright 2014 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 that the same coverage rules apply even if the local type appears in the
// list of type parameters, not the self type.

// aux-build:coherence-lib.rs

extern crate "coherence-lib" as lib;
use lib::{Remote1, Pair};

pub struct Local<T>(T);

impl<T,U> Remote1<Pair<T,Local<U>>> for i32 { }
//~^ ERROR type parameter `T` is not constrained

fn main() { }
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@ use lib::Remote1;

pub struct BigInt;

impl Remote1<BigInt> for isize { } //~ ERROR E0117
impl Remote1<BigInt> for isize { }

fn main() { }
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@ use lib::Remote1;

pub struct BigInt;

impl Remote1<BigInt> for Vec<isize> { } //~ ERROR E0117
impl Remote1<BigInt> for Vec<isize> { }

fn main() { }
23 changes: 23 additions & 0 deletions src/test/run-pass/coherence-cow-1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright 2014 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.

// aux-build:coherence-lib.rs

// Test that it's ok for T to appear first in the self-type, as long
// as it's covered somewhere.

extern crate "coherence-lib" as lib;
use lib::{Remote,Pair};

pub struct Cover<T>(T);

impl<T> Remote for Pair<T,Cover<T>> { }

fn main() { }
23 changes: 23 additions & 0 deletions src/test/run-pass/coherence-cow-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright 2014 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.

// aux-build:coherence-lib.rs

// Test that it's ok for T to appear second in the self-type, as long
// as it's covered somewhere.

extern crate "coherence-lib" as lib;
use lib::{Remote,Pair};

pub struct Cover<T>(T);

impl<T> Remote for Pair<Cover<T>,T> { }

fn main() { }

0 comments on commit e8489d3

Please sign in to comment.