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

Update the coherence rules to "covered first" #21792

Merged
merged 2 commits into from
Feb 1, 2015
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
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() { }