Skip to content

Commit 6e68fd0

Browse files
committed
Implement new orphan rule that requires that impls of remote traits meet the following two criteria:
- the self type includes some local type; and, - type parameters in the self type must be constrained by a local type. A type parameter is called *constrained* if it appears in some type-parameter of a local type. Here are some examples that are accepted. In all of these examples, I assume that `Foo` is a trait defined in another crate. If `Foo` were defined in the local crate, then all the examples would be legal. - `impl Foo for LocalType` - `impl<T> Foo<T> for LocalType` -- T does not appear in Self, so it is OK - `impl<T> Foo<T> for LocalType<T>` -- T here is constrained by LocalType - `impl<T> Foo<T> for (LocalType<T>, T)` -- T here is constrained by LocalType Here are some illegal examples (again, these examples assume that `Foo` is not local to the current crate): - `impl Foo for int` -- the Self type is not local - `impl<T> Foo for T` -- T appears in Self unconstrained by a local type - `impl<T> Foo for (LocalType, T)` -- T appears in Self unconstrained by a local type This is a [breaking-change]. For the time being, you can opt out of the new rules by placing `#[old_orphan_check]` on the trait (and enabling the feature gate where the trait is defined). Longer term, you should restructure your traits to avoid the problem. Usually this means changing the order of parameters so that the "central" type parameter is in the `Self` position. As an example of that refactoring, consider the `BorrowFrom` trait: ```rust pub trait BorrowFrom<Sized? Owned> for Sized? { fn borrow_from(owned: &Owned) -> &Self; } ``` As defined, this trait is commonly implemented for custom pointer types, such as `Arc`. Those impls follow the pattern: ```rust impl<T> BorrowFrom<Arc<T>> for T {...} ``` Unfortunately, this impl is illegal because the self type `T` is not local to the current crate. Therefore, we are going to change the order of the parameters, so that `BorrowFrom` becomes `Borrow`: ```rust pub trait Borrow<Sized? Borrowed> for Sized? { fn borrow_from(owned: &Self) -> &Borrowed; } ``` Now the `Arc` impl is written: ```rust impl<T> Borrow<T> for Arc<T> { ... } ``` This impl is legal because the self type (`Arc<T>`) is local.
1 parent 260e461 commit 6e68fd0

18 files changed

+68
-53
lines changed

src/libcore/borrow.rs

+4
Original file line numberDiff line numberDiff line change
@@ -53,12 +53,14 @@ use option::Option;
5353
use self::Cow::*;
5454

5555
/// A trait for borrowing data.
56+
#[old_orphan_check]
5657
pub trait BorrowFrom<Sized? Owned> for Sized? {
5758
/// Immutably borrow from an owned value.
5859
fn borrow_from(owned: &Owned) -> &Self;
5960
}
6061

6162
/// A trait for mutably borrowing data.
63+
#[old_orphan_check]
6264
pub trait BorrowFromMut<Sized? Owned> for Sized? : BorrowFrom<Owned> {
6365
/// Mutably borrow from an owned value.
6466
fn borrow_from_mut(owned: &mut Owned) -> &mut Self;
@@ -91,6 +93,7 @@ impl<'a, T, Sized? B> BorrowFrom<Cow<'a, T, B>> for B where B: ToOwned<T> {
9193
}
9294

9395
/// Trait for moving into a `Cow`
96+
#[old_orphan_check]
9497
pub trait IntoCow<'a, T, Sized? B> {
9598
/// Moves `self` into `Cow`
9699
fn into_cow(self) -> Cow<'a, T, B>;
@@ -103,6 +106,7 @@ impl<'a, T, Sized? B> IntoCow<'a, T, B> for Cow<'a, T, B> where B: ToOwned<T> {
103106
}
104107

105108
/// A generalization of Clone to borrowed data.
109+
#[old_orphan_check]
106110
pub trait ToOwned<Owned> for Sized?: BorrowFrom<Owned> {
107111
/// Create owned data from borrowed data, usually by copying.
108112
fn to_owned(&self) -> Owned;

src/libcore/cmp.rs

+1
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ use option::Option::{self, Some, None};
6969
/// only if `a != b`.
7070
#[lang="eq"]
7171
#[stable]
72+
#[old_orphan_check]
7273
pub trait PartialEq<Sized? Rhs = Self> for Sized? {
7374
/// This method tests for `self` and `other` values to be equal, and is used by `==`.
7475
#[stable]

src/librustc/lint/builtin.rs

+3
Original file line numberDiff line numberDiff line change
@@ -670,6 +670,9 @@ impl LintPass for UnusedAttributes {
670670
"must_use",
671671
"stable",
672672
"unstable",
673+
674+
// FIXME: #19470 this shouldn't be needed forever
675+
"old_orphan_check",
673676
];
674677

675678
static CRATE_ATTRS: &'static [&'static str] = &[

src/librustc/middle/traits/coherence.rs

+23-33
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use super::SelectionContext;
1414
use super::{Obligation, ObligationCause};
1515
use super::util;
1616

17-
use middle::subst::Subst;
17+
use middle::subst::{Subst};
1818
use middle::ty::{self, Ty};
1919
use middle::infer::InferCtxt;
2020
use std::collections::HashSet;
@@ -53,20 +53,20 @@ pub fn impl_can_satisfy(infcx: &InferCtxt,
5353
}
5454

5555
#[allow(missing_copy_implementations)]
56-
pub enum OrphanCheckErr {
56+
pub enum OrphanCheckErr<'tcx> {
5757
NoLocalInputType,
58-
UncoveredTypeParameter(ty::ParamTy),
58+
UncoveredTy(Ty<'tcx>),
5959
}
6060

6161
/// Checks the coherence orphan rules. `impl_def_id` should be the
6262
/// def-id of a trait impl. To pass, either the trait must be local, or else
6363
/// two conditions must be satisfied:
6464
///
65-
/// 1. At least one of the input types must involve a local type.
66-
/// 2. All type parameters must be covered by a local type.
67-
pub fn orphan_check(tcx: &ty::ctxt,
68-
impl_def_id: ast::DefId)
69-
-> Result<(), OrphanCheckErr>
65+
/// 1. All type parameters in `Self` must be "covered" by some local type constructor.
66+
/// 2. Some local type must appear in `Self`.
67+
pub fn orphan_check<'tcx>(tcx: &ty::ctxt<'tcx>,
68+
impl_def_id: ast::DefId)
69+
-> Result<(), OrphanCheckErr<'tcx>>
7070
{
7171
debug!("impl_is_local({})", impl_def_id.repr(tcx));
7272

@@ -82,31 +82,21 @@ pub fn orphan_check(tcx: &ty::ctxt,
8282
return Ok(());
8383
}
8484

85-
// Check condition 1: at least one type must be local.
86-
if !trait_ref.input_types().iter().any(|&t| ty_reaches_local(tcx, t)) {
87-
return Err(OrphanCheckErr::NoLocalInputType);
85+
// Otherwise, check that (1) all type parameters are covered.
86+
let covered_params = type_parameters_covered_by_ty(tcx, trait_ref.self_ty());
87+
let all_params = type_parameters_reachable_from_ty(trait_ref.self_ty());
88+
for &param in all_params.difference(&covered_params) {
89+
return Err(OrphanCheckErr::UncoveredTy(param));
8890
}
8991

90-
// Check condition 2: type parameters must be "covered" by a local type.
91-
let covered_params: HashSet<_> =
92-
trait_ref.input_types().iter()
93-
.flat_map(|&t| type_parameters_covered_by_ty(tcx, t).into_iter())
94-
.collect();
95-
let all_params: HashSet<_> =
96-
trait_ref.input_types().iter()
97-
.flat_map(|&t| type_parameters_reachable_from_ty(t).into_iter())
98-
.collect();
99-
for &param in all_params.difference(&covered_params) {
100-
return Err(OrphanCheckErr::UncoveredTypeParameter(param));
92+
// And (2) some local type appears.
93+
if !trait_ref.self_ty().walk().any(|t| ty_is_local_constructor(tcx, t)) {
94+
return Err(OrphanCheckErr::NoLocalInputType);
10195
}
10296

10397
return Ok(());
10498
}
10599

106-
fn ty_reaches_local<'tcx>(tcx: &ty::ctxt<'tcx>, ty: Ty<'tcx>) -> bool {
107-
ty.walk().any(|t| ty_is_local_constructor(tcx, t))
108-
}
109-
110100
fn ty_is_local_constructor<'tcx>(tcx: &ty::ctxt<'tcx>, ty: Ty<'tcx>) -> bool {
111101
debug!("ty_is_local_constructor({})", ty.repr(tcx));
112102

@@ -154,8 +144,8 @@ fn ty_is_local_constructor<'tcx>(tcx: &ty::ctxt<'tcx>, ty: Ty<'tcx>) -> bool {
154144
}
155145

156146
fn type_parameters_covered_by_ty<'tcx>(tcx: &ty::ctxt<'tcx>,
157-
ty: Ty<'tcx>)
158-
-> HashSet<ty::ParamTy>
147+
ty: Ty<'tcx>)
148+
-> HashSet<Ty<'tcx>>
159149
{
160150
if ty_is_local_constructor(tcx, ty) {
161151
type_parameters_reachable_from_ty(ty)
@@ -165,14 +155,14 @@ fn type_parameters_covered_by_ty<'tcx>(tcx: &ty::ctxt<'tcx>,
165155
}
166156

167157
/// All type parameters reachable from `ty`
168-
fn type_parameters_reachable_from_ty<'tcx>(ty: Ty<'tcx>) -> HashSet<ty::ParamTy> {
158+
fn type_parameters_reachable_from_ty<'tcx>(ty: Ty<'tcx>) -> HashSet<Ty<'tcx>> {
169159
ty.walk()
170-
.filter_map(|t| {
160+
.filter(|&t| {
171161
match t.sty {
172-
ty::ty_param(ref param_ty) => Some(param_ty.clone()),
173-
_ => None,
162+
// FIXME(#20590) straighten story about projection types
163+
ty::ty_projection(..) | ty::ty_param(..) => true,
164+
_ => false,
174165
}
175166
})
176167
.collect()
177168
}
178-

src/librustc_typeck/check/callee.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use super::TupleArgumentsFlag;
2222
use super::write_call;
2323

2424
use middle::infer;
25-
use middle::ty::{mod, Ty};
25+
use middle::ty::{self, Ty};
2626
use syntax::ast;
2727
use syntax::codemap::Span;
2828
use syntax::parse::token;

src/librustc_typeck/coherence/orphan.rs

+18-8
Original file line numberDiff line numberDiff line change
@@ -72,20 +72,30 @@ impl<'cx, 'tcx,'v> visit::Visitor<'v> for OrphanChecker<'cx, 'tcx> {
7272
ast::ItemImpl(_, _, Some(_), _, _) => {
7373
// "Trait" impl
7474
debug!("coherence2::orphan check: trait impl {}", item.repr(self.tcx));
75+
let trait_def_id = ty::impl_trait_ref(self.tcx, def_id).unwrap().def_id;
7576
match traits::orphan_check(self.tcx, def_id) {
7677
Ok(()) => { }
7778
Err(traits::OrphanCheckErr::NoLocalInputType) => {
78-
span_err!(self.tcx.sess, item.span, E0117,
79-
"cannot provide an extension implementation \
80-
where both trait and type are not defined in this crate");
79+
if !ty::has_attr(self.tcx, trait_def_id, "old_orphan_check") {
80+
let self_ty = ty::lookup_item_type(self.tcx, def_id).ty;
81+
span_err!(
82+
self.tcx.sess, item.span, E0117,
83+
"the type `{}` does not reference any \
84+
types defined in this crate; \
85+
only traits defined in the current crate can be \
86+
implemented for arbitrary types",
87+
self_ty.user_string(self.tcx));
88+
}
8189
}
82-
Err(traits::OrphanCheckErr::UncoveredTypeParameter(param_ty)) => {
83-
if !self.tcx.sess.features.borrow().old_orphan_check {
90+
Err(traits::OrphanCheckErr::UncoveredTy(param_ty)) => {
91+
if !ty::has_attr(self.tcx, trait_def_id, "old_orphan_check") {
8492
self.tcx.sess.span_err(
8593
item.span,
86-
format!("type parameter `{}` must also appear as a type parameter \
87-
of some type defined within this crate",
88-
param_ty.user_string(self.tcx)).as_slice());
94+
format!(
95+
"type parameter `{}` is not constrained by any local type; \
96+
only traits defined in the current crate can be implemented \
97+
for a type parameter",
98+
param_ty.user_string(self.tcx)).as_slice());
8999
self.tcx.sess.span_note(
90100
item.span,
91101
format!("for a limited time, you can add \

src/libsyntax/feature_gate.rs

+8
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,14 @@ impl<'a, 'v> Visitor<'v> for PostExpansionVisitor<'a> {
301301
removed in the future");
302302
}
303303

304+
if attr::contains_name(i.attrs[],
305+
"old_orphan_check") {
306+
self.gate_feature(
307+
"old_orphan_check",
308+
i.span,
309+
"the new orphan check rules will eventually be strictly enforced");
310+
}
311+
304312
for item in items.iter() {
305313
match *item {
306314
ast::MethodImplItem(_) => {}

src/test/compile-fail/coherence-all-remote.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,6 @@ extern crate "coherence-lib" as lib;
1414
use lib::Remote;
1515

1616
impl<T> Remote for int { }
17-
//~^ ERROR cannot provide an extension implementation
17+
//~^ ERROR E0117
1818

1919
fn main() { }

src/test/run-pass/coherence-bigint-int.rs renamed to src/test/compile-fail/coherence-bigint-int.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,6 @@ use lib::Remote1;
1515

1616
pub struct BigInt;
1717

18-
impl Remote1<BigInt> for int { }
18+
impl Remote1<BigInt> for int { } //~ ERROR E0117
1919

2020
fn main() { }

src/test/compile-fail/coherence-bigint-param.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,6 @@ use lib::Remote1;
1616
pub struct BigInt;
1717

1818
impl<T> Remote1<BigInt> for T { }
19-
//~^ ERROR type parameter `T` must also appear
19+
//~^ ERROR type parameter `T` is not constrained
2020

2121
fn main() { }

src/test/run-pass/coherence-bigint-vecint.rs renamed to src/test/compile-fail/coherence-bigint-vecint.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,6 @@ use lib::Remote1;
1515

1616
pub struct BigInt;
1717

18-
impl Remote1<BigInt> for Vec<int> { }
18+
impl Remote1<BigInt> for Vec<int> { } //~ ERROR E0117
1919

2020
fn main() { }

src/test/compile-fail/coherence-cross-crate-conflict.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ extern crate trait_impl_conflict;
1616
use trait_impl_conflict::Foo;
1717

1818
impl<A> Foo for A {
19-
//~^ ERROR E0117
19+
//~^ ERROR type parameter `A` is not constrained
2020
//~^^ ERROR E0119
2121
}
2222

src/test/compile-fail/coherence-lone-type-parameter.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,6 @@
1313
extern crate "coherence-lib" as lib;
1414
use lib::Remote;
1515

16-
impl<T> Remote for T { } //~ ERROR E0117
16+
impl<T> Remote for T { } //~ ERROR type parameter `T` is not constrained
1717

1818
fn main() { }

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ struct TheType;
1818

1919
impl TheTrait<uint> for int { } //~ ERROR E0117
2020

21-
impl TheTrait<TheType> for int { }
21+
impl TheTrait<TheType> for int { } //~ ERROR E0117
2222

2323
impl TheTrait<int> for TheType { }
2424

src/test/compile-fail/coherence-overlapping-pairs.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,6 @@ use lib::Remote;
1616
struct Foo;
1717

1818
impl<T> Remote for lib::Pair<T,Foo> { }
19-
//~^ ERROR type parameter `T` must also appear
19+
//~^ ERROR type parameter `T` is not constrained
2020

2121
fn main() { }

src/test/compile-fail/coherence-pair-covered-uncovered.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,6 @@ use lib::{Remote, Pair};
1616
struct Local<T>(T);
1717

1818
impl<T,U> Remote for Pair<T,Local<U>> { }
19-
//~^ ERROR type parameter `T` must also appear
19+
//~^ ERROR type parameter `T` is not constrained
2020

2121
fn main() { }

src/test/compile-fail/drop-on-non-struct.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
impl Drop for int {
1212
//~^ ERROR the Drop trait may only be implemented on structures
13-
//~^^ ERROR cannot provide an extension implementation
13+
//~^^ ERROR E0117
1414
fn drop(&mut self) {
1515
println!("kaboom");
1616
}

src/test/compile-fail/coherence-iterator-vec-any-elem.rs renamed to src/test/run-pass/coherence-iterator-vec-any-elem.rs

-1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,5 @@ use lib::Remote1;
1616
struct Foo<T>(T);
1717

1818
impl<T,U> Remote1<U> for Foo<T> { }
19-
//~^ ERROR type parameter `U` must also appear
2019

2120
fn main() { }

0 commit comments

Comments
 (0)