From ecaa68768cdc1fa82a750998634e9cc3202975f9 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 30 Jan 2015 18:46:53 -0500 Subject: [PATCH 1/2] 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. --- src/librustc/middle/traits/coherence.rs | 56 ++++++++++++------- src/test/compile-fail/coherence-all-remote.rs | 2 +- .../compile-fail/coherence-cow-no-cover.rs | 23 ++++++++ src/test/compile-fail/coherence-orphan.rs | 2 +- .../coherence-pair-covered-uncovered-1.rs | 24 ++++++++ .../coherence-bigint-int.rs | 2 +- .../coherence-bigint-vecint.rs | 2 +- src/test/run-pass/coherence-cow-1.rs | 23 ++++++++ src/test/run-pass/coherence-cow-2.rs | 23 ++++++++ 9 files changed, 133 insertions(+), 24 deletions(-) create mode 100644 src/test/compile-fail/coherence-cow-no-cover.rs create mode 100644 src/test/compile-fail/coherence-pair-covered-uncovered-1.rs rename src/test/{compile-fail => run-pass}/coherence-bigint-int.rs (92%) rename src/test/{compile-fail => run-pass}/coherence-bigint-vecint.rs (91%) create mode 100644 src/test/run-pass/coherence-cow-1.rs create mode 100644 src/test/run-pass/coherence-cow-2.rs diff --git a/src/librustc/middle/traits/coherence.rs b/src/librustc/middle/traits/coherence.rs index e3363aa8fb70a..44cd17caaecbb 100644 --- a/src/librustc/middle/traits/coherence.rs +++ b/src/librustc/middle/traits/coherence.rs @@ -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; @@ -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 ¶m 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 ¶m in all_params.difference(&covered_params) { + return Err(OrphanCheckErr::UncoveredTy(param)); + } + } + None => { + return Err(OrphanCheckErr::NoLocalInputType); + } } return Ok(()); @@ -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.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, + } } diff --git a/src/test/compile-fail/coherence-all-remote.rs b/src/test/compile-fail/coherence-all-remote.rs index f8ddf83c9c637..1e3b7f6dbd22f 100644 --- a/src/test/compile-fail/coherence-all-remote.rs +++ b/src/test/compile-fail/coherence-all-remote.rs @@ -14,6 +14,6 @@ extern crate "coherence-lib" as lib; use lib::Remote1; impl Remote1 for isize { } -//~^ ERROR E0117 +//~^ ERROR E0210 fn main() { } diff --git a/src/test/compile-fail/coherence-cow-no-cover.rs b/src/test/compile-fail/coherence-cow-no-cover.rs new file mode 100644 index 0000000000000..8d14eb96c6170 --- /dev/null +++ b/src/test/compile-fail/coherence-cow-no-cover.rs @@ -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 or the MIT license +// , 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); + +impl Remote for Pair,U> { } +//~^ ERROR type parameter `U` is not constrained by any local type + +fn main() { } diff --git a/src/test/compile-fail/coherence-orphan.rs b/src/test/compile-fail/coherence-orphan.rs index f9f965e1ae39d..d7cd68e73c349 100644 --- a/src/test/compile-fail/coherence-orphan.rs +++ b/src/test/compile-fail/coherence-orphan.rs @@ -21,7 +21,7 @@ struct TheType; impl TheTrait for isize { } //~ ERROR E0117 -impl TheTrait for isize { } //~ ERROR E0117 +impl TheTrait for isize { } impl TheTrait for TheType { } diff --git a/src/test/compile-fail/coherence-pair-covered-uncovered-1.rs b/src/test/compile-fail/coherence-pair-covered-uncovered-1.rs new file mode 100644 index 0000000000000..3655bca6f1d8d --- /dev/null +++ b/src/test/compile-fail/coherence-pair-covered-uncovered-1.rs @@ -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 or the MIT license +// , 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); + +impl Remote1>> for i32 { } +//~^ ERROR type parameter `T` is not constrained + +fn main() { } diff --git a/src/test/compile-fail/coherence-bigint-int.rs b/src/test/run-pass/coherence-bigint-int.rs similarity index 92% rename from src/test/compile-fail/coherence-bigint-int.rs rename to src/test/run-pass/coherence-bigint-int.rs index 684773098cd95..baf2f57206d6a 100644 --- a/src/test/compile-fail/coherence-bigint-int.rs +++ b/src/test/run-pass/coherence-bigint-int.rs @@ -15,6 +15,6 @@ use lib::Remote1; pub struct BigInt; -impl Remote1 for isize { } //~ ERROR E0117 +impl Remote1 for isize { } fn main() { } diff --git a/src/test/compile-fail/coherence-bigint-vecint.rs b/src/test/run-pass/coherence-bigint-vecint.rs similarity index 91% rename from src/test/compile-fail/coherence-bigint-vecint.rs rename to src/test/run-pass/coherence-bigint-vecint.rs index 28747674b8b10..cdc5bc1171655 100644 --- a/src/test/compile-fail/coherence-bigint-vecint.rs +++ b/src/test/run-pass/coherence-bigint-vecint.rs @@ -15,6 +15,6 @@ use lib::Remote1; pub struct BigInt; -impl Remote1 for Vec { } //~ ERROR E0117 +impl Remote1 for Vec { } fn main() { } diff --git a/src/test/run-pass/coherence-cow-1.rs b/src/test/run-pass/coherence-cow-1.rs new file mode 100644 index 0000000000000..b380372b40125 --- /dev/null +++ b/src/test/run-pass/coherence-cow-1.rs @@ -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 or the MIT license +// , 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); + +impl Remote for Pair> { } + +fn main() { } diff --git a/src/test/run-pass/coherence-cow-2.rs b/src/test/run-pass/coherence-cow-2.rs new file mode 100644 index 0000000000000..93e507c0d639d --- /dev/null +++ b/src/test/run-pass/coherence-cow-2.rs @@ -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 or the MIT license +// , 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); + +impl Remote for Pair,T> { } + +fn main() { } From 2c2879bbce4177552ce26c6445dcb7027a1245c7 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sat, 31 Jan 2015 04:39:16 -0500 Subject: [PATCH 2/2] Adjust error message not to mention the self type --- src/librustc_typeck/coherence/orphan.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/librustc_typeck/coherence/orphan.rs b/src/librustc_typeck/coherence/orphan.rs index 60b1fa5f4cf5d..5b97175ab22fb 100644 --- a/src/librustc_typeck/coherence/orphan.rs +++ b/src/librustc_typeck/coherence/orphan.rs @@ -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)) => {