Skip to content

Commit c61a009

Browse files
committed
Fix orphan checking (cc rust-lang#19470). (This is not a complete fix of rust-lang#19470 because of the backwards compatibility feature gate.)
This is a [breaking-change]. The new rules require that, for an impl of a trait defined in some other crate, two conditions must hold: 1. Some type must be local. 2. Every type parameter must appear "under" some local type. Here are some examples that are legal: ```rust struct MyStruct<T> { ... } // Here `T` appears "under' `MyStruct`. impl<T> Clone for MyStruct<T> { } // Here `T` appears "under' `MyStruct` as well. Note that it also appears // elsewhere. impl<T> Iterator<T> for MyStruct<T> { } ``` Here is an illegal example: ```rust // Here `U` does not appear "under" `MyStruct` or any other local type. // We call `U` "uncovered". impl<T,U> Iterator<U> for MyStruct<T> { } ``` There are a couple of ways to rewrite this last example so that it is legal: 1. In some cases, the uncovered type parameter (here, `U`) should be converted into an associated type. This is however a non-local change that requires access to the original trait. Also, associated types are not fully baked. 2. Add `U` as a type parameter of `MyStruct`: ```rust struct MyStruct<T,U> { ... } impl<T,U> Iterator<U> for MyStruct<T,U> { } ``` 3. Create a newtype wrapper for `U` ```rust impl<T,U> Iterator<Wrapper<U>> for MyStruct<T,U> { } ``` Because associated types are not fully baked, which in the case of the `Hash` trait makes adhering to this rule impossible, you can temporarily disable this rule in your crate by using `#![feature(old_orphan_check)]`. Note that the `old_orphan_check` feature will be removed before 1.0 is released.
1 parent 77723d2 commit c61a009

File tree

26 files changed

+171
-95
lines changed

26 files changed

+171
-95
lines changed

src/liballoc/lib.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,8 @@
6464
html_root_url = "http://doc.rust-lang.org/nightly/")]
6565

6666
#![no_std]
67-
#![feature(lang_items, phase, unsafe_destructor, default_type_params)]
67+
#![allow(unknown_features)]
68+
#![feature(lang_items, phase, unsafe_destructor, default_type_params, old_orphan_check)]
6869

6970
#[phase(plugin, link)]
7071
extern crate core;

src/libcollections/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#![feature(macro_rules, default_type_params, phase, globs)]
2626
#![feature(unsafe_destructor, slicing_syntax)]
2727
#![feature(unboxed_closures)]
28+
#![feature(old_orphan_check)]
2829
#![no_std]
2930

3031
#[phase(plugin, link)] extern crate core;

src/libgraphviz/maybe_owned_vec.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,9 @@ impl<'a, T: Ord> Ord for MaybeOwnedVector<'a, T> {
9696
}
9797

9898
#[allow(deprecated)]
99-
impl<'a, T: PartialEq, Sized? V: AsSlice<T>> Equiv<V> for MaybeOwnedVector<'a, T> {
100-
fn equiv(&self, other: &V) -> bool {
101-
self.as_slice() == other.as_slice()
99+
impl<'a, T: PartialEq> Equiv<[T]> for MaybeOwnedVector<'a, T> {
100+
fn equiv(&self, other: &[T]) -> bool {
101+
self.as_slice() == other
102102
}
103103
}
104104

src/librustc/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,12 @@
2222
html_favicon_url = "http://www.rust-lang.org/favicon.ico",
2323
html_root_url = "http://doc.rust-lang.org/nightly/")]
2424

25+
#![allow(unknown_features)]
2526
#![feature(default_type_params, globs, macro_rules, phase, quote)]
2627
#![feature(slicing_syntax, unsafe_destructor)]
2728
#![feature(rustc_diagnostic_macros)]
2829
#![feature(unboxed_closures)]
30+
#![feature(old_orphan_check)]
2931

3032
extern crate arena;
3133
extern crate flate;

src/librustc/middle/traits/coherence.rs

Lines changed: 82 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@ use super::SelectionContext;
1414
use super::{Obligation, ObligationCause};
1515
use super::util;
1616

17-
use middle::subst;
1817
use middle::subst::Subst;
1918
use middle::ty::{mod, Ty};
2019
use middle::infer::InferCtxt;
20+
use std::collections::HashSet;
2121
use std::rc::Rc;
2222
use syntax::ast;
2323
use syntax::codemap::DUMMY_SP;
@@ -52,9 +52,21 @@ pub fn impl_can_satisfy(infcx: &InferCtxt,
5252
selcx.evaluate_impl(impl2_def_id, &obligation)
5353
}
5454

55-
pub fn impl_is_local(tcx: &ty::ctxt,
56-
impl_def_id: ast::DefId)
57-
-> bool
55+
#[allow(missing_copy_implementations)]
56+
pub enum OrphanCheckErr {
57+
NoLocalInputType,
58+
UncoveredTypeParameter(ty::ParamTy),
59+
}
60+
61+
/// Checks the coherence orphan rules. `impl_def_id` should be the
62+
/// def-id of a trait impl. To pass, either the trait must be local, or else
63+
/// two conditions must be satisfied:
64+
///
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>
5870
{
5971
debug!("impl_is_local({})", impl_def_id.repr(tcx));
6072

@@ -63,99 +75,74 @@ pub fn impl_is_local(tcx: &ty::ctxt,
6375
let trait_ref = ty::impl_trait_ref(tcx, impl_def_id).unwrap();
6476
debug!("trait_ref={}", trait_ref.repr(tcx));
6577

66-
// If the trait is local to the crate, ok.
78+
// If the *trait* is local to the crate, ok.
6779
if trait_ref.def_id.krate == ast::LOCAL_CRATE {
6880
debug!("trait {} is local to current crate",
6981
trait_ref.def_id.repr(tcx));
70-
return true;
82+
return Ok(());
7183
}
7284

73-
// Otherwise, at least one of the input types must be local to the
74-
// crate.
75-
trait_ref.input_types().iter().any(|&t| ty_is_local(tcx, t))
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);
88+
}
89+
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));
101+
}
102+
103+
return Ok(());
104+
}
105+
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))
76108
}
77109

78-
pub fn ty_is_local<'tcx>(tcx: &ty::ctxt<'tcx>, ty: Ty<'tcx>) -> bool {
79-
debug!("ty_is_local({})", ty.repr(tcx));
110+
fn ty_is_local_constructor<'tcx>(tcx: &ty::ctxt<'tcx>, ty: Ty<'tcx>) -> bool {
111+
debug!("ty_is_local_constructor({})", ty.repr(tcx));
80112

81113
match ty.sty {
82114
ty::ty_bool |
83115
ty::ty_char |
84116
ty::ty_int(..) |
85117
ty::ty_uint(..) |
86118
ty::ty_float(..) |
87-
ty::ty_str(..) => {
88-
false
89-
}
90-
91-
ty::ty_unboxed_closure(..) => {
92-
// This routine is invoked on types specified by users as
93-
// part of an impl and hence an unboxed closure type
94-
// cannot appear.
95-
tcx.sess.bug("ty_is_local applied to unboxed closure type")
96-
}
97-
119+
ty::ty_str(..) |
98120
ty::ty_bare_fn(..) |
99-
ty::ty_closure(..) => {
121+
ty::ty_closure(..) |
122+
ty::ty_vec(..) |
123+
ty::ty_ptr(..) |
124+
ty::ty_rptr(..) |
125+
ty::ty_tup(..) |
126+
ty::ty_param(..) |
127+
ty::ty_projection(..) => {
100128
false
101129
}
102130

103-
ty::ty_uniq(t) => {
104-
let krate = tcx.lang_items.owned_box().map(|d| d.krate);
105-
krate == Some(ast::LOCAL_CRATE) || ty_is_local(tcx, t)
106-
}
107-
108-
ty::ty_vec(t, _) |
109-
ty::ty_ptr(ty::mt { ty: t, .. }) |
110-
ty::ty_rptr(_, ty::mt { ty: t, .. }) => {
111-
ty_is_local(tcx, t)
112-
}
113-
114-
ty::ty_tup(ref ts) => {
115-
ts.iter().any(|&t| ty_is_local(tcx, t))
131+
ty::ty_enum(def_id, _) |
132+
ty::ty_struct(def_id, _) => {
133+
def_id.krate == ast::LOCAL_CRATE
116134
}
117135

118-
ty::ty_enum(def_id, ref substs) |
119-
ty::ty_struct(def_id, ref substs) => {
120-
def_id.krate == ast::LOCAL_CRATE || {
121-
let variances = ty::item_variances(tcx, def_id);
122-
subst::ParamSpace::all().iter().any(|&space| {
123-
substs.types.get_slice(space).iter().enumerate().any(
124-
|(i, &t)| {
125-
match *variances.types.get(space, i) {
126-
ty::Bivariant => {
127-
// If Foo<T> is bivariant with respect to
128-
// T, then it doesn't matter whether T is
129-
// local or not, because `Foo<U>` for any
130-
// U will be a subtype of T.
131-
false
132-
}
133-
ty::Contravariant |
134-
ty::Covariant |
135-
ty::Invariant => {
136-
ty_is_local(tcx, t)
137-
}
138-
}
139-
})
140-
})
141-
}
136+
ty::ty_uniq(_) => { // treat ~T like Box<T>
137+
let krate = tcx.lang_items.owned_box().map(|d| d.krate);
138+
krate == Some(ast::LOCAL_CRATE)
142139
}
143140

144141
ty::ty_trait(ref tt) => {
145142
tt.principal_def_id().krate == ast::LOCAL_CRATE
146143
}
147144

148-
// Type parameters may be bound to types that are not local to
149-
// the crate.
150-
ty::ty_param(..) => {
151-
false
152-
}
153-
154-
// Associated types could be anything, I guess.
155-
ty::ty_projection(..) => {
156-
false
157-
}
158-
145+
ty::ty_unboxed_closure(..) |
159146
ty::ty_infer(..) |
160147
ty::ty_open(..) |
161148
ty::ty_err => {
@@ -165,3 +152,27 @@ pub fn ty_is_local<'tcx>(tcx: &ty::ctxt<'tcx>, ty: Ty<'tcx>) -> bool {
165152
}
166153
}
167154
}
155+
156+
fn type_parameters_covered_by_ty<'tcx>(tcx: &ty::ctxt<'tcx>,
157+
ty: Ty<'tcx>)
158+
-> HashSet<ty::ParamTy>
159+
{
160+
if ty_is_local_constructor(tcx, ty) {
161+
type_parameters_reachable_from_ty(ty)
162+
} else {
163+
ty.walk_children().flat_map(|t| type_parameters_covered_by_ty(tcx, t).into_iter()).collect()
164+
}
165+
}
166+
167+
/// All type parameters reachable from `ty`
168+
fn type_parameters_reachable_from_ty<'tcx>(ty: Ty<'tcx>) -> HashSet<ty::ParamTy> {
169+
ty.walk()
170+
.filter_map(|t| {
171+
match t.sty {
172+
ty::ty_param(ref param_ty) => Some(param_ty.clone()),
173+
_ => None,
174+
}
175+
})
176+
.collect()
177+
}
178+

src/librustc/middle/traits/mod.rs

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ use syntax::codemap::{Span, DUMMY_SP};
2525
use util::ppaux::Repr;
2626

2727
pub use self::error_reporting::report_fulfillment_errors;
28+
pub use self::coherence::orphan_check;
29+
pub use self::coherence::OrphanCheckErr;
2830
pub use self::fulfill::{FulfillmentContext, RegionObligation};
2931
pub use self::project::MismatchedProjectionTypes;
3032
pub use self::project::normalize;
@@ -245,15 +247,6 @@ pub struct VtableBuiltinData<N> {
245247
pub nested: subst::VecPerParamSpace<N>
246248
}
247249

248-
/// True if neither the trait nor self type is local. Note that `impl_def_id` must refer to an impl
249-
/// of a trait, not an inherent impl.
250-
pub fn is_orphan_impl(tcx: &ty::ctxt,
251-
impl_def_id: ast::DefId)
252-
-> bool
253-
{
254-
!coherence::impl_is_local(tcx, impl_def_id)
255-
}
256-
257250
/// True if there exist types that satisfy both of the two given impls.
258251
pub fn overlapping_impls(infcx: &InferCtxt,
259252
impl1_def_id: ast::DefId,

src/librustc_back/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
#![allow(unknown_features)]
3333
#![feature(globs, phase, macro_rules, slicing_syntax)]
3434
#![feature(unboxed_closures)]
35+
#![feature(old_orphan_check)]
3536

3637
#[phase(plugin, link)]
3738
extern crate log;

src/librustc_borrowck/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,12 @@
1616
html_favicon_url = "http://www.rust-lang.org/favicon.ico",
1717
html_root_url = "http://doc.rust-lang.org/nightly/")]
1818

19+
#![allow(unknown_features)]
1920
#![feature(default_type_params, globs, macro_rules, phase, quote)]
2021
#![feature(slicing_syntax, unsafe_destructor)]
2122
#![feature(rustc_diagnostic_macros)]
2223
#![feature(unboxed_closures)]
24+
#![feature(old_orphan_check)]
2325
#![allow(non_camel_case_types)]
2426

2527
#[phase(plugin, link)] extern crate log;

src/librustc_trans/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,12 @@
2222
html_favicon_url = "http://www.rust-lang.org/favicon.ico",
2323
html_root_url = "http://doc.rust-lang.org/nightly/")]
2424

25+
#![allow(unknown_features)]
2526
#![feature(default_type_params, globs, macro_rules, phase, quote)]
2627
#![feature(slicing_syntax, unsafe_destructor)]
2728
#![feature(rustc_diagnostic_macros)]
2829
#![feature(unboxed_closures)]
30+
#![feature(old_orphan_check)]
2931

3032
extern crate arena;
3133
extern crate flate;

src/librustc_typeck/coherence/orphan.rs

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ use syntax::ast;
1818
use syntax::ast_util;
1919
use syntax::codemap::Span;
2020
use syntax::visit;
21-
use util::ppaux::Repr;
21+
use util::ppaux::{Repr, UserString};
2222

2323
pub fn check(tcx: &ty::ctxt) {
2424
let mut orphan = OrphanChecker { tcx: tcx };
@@ -67,10 +67,27 @@ impl<'cx, 'tcx,'v> visit::Visitor<'v> for OrphanChecker<'cx, 'tcx> {
6767
ast::ItemImpl(_, _, Some(_), _, _) => {
6868
// "Trait" impl
6969
debug!("coherence2::orphan check: trait impl {}", item.repr(self.tcx));
70-
if traits::is_orphan_impl(self.tcx, def_id) {
71-
span_err!(self.tcx.sess, item.span, E0117,
72-
"cannot provide an extension implementation \
73-
where both trait and type are not defined in this crate");
70+
match traits::orphan_check(self.tcx, def_id) {
71+
Ok(()) => { }
72+
Err(traits::OrphanCheckErr::NoLocalInputType) => {
73+
span_err!(self.tcx.sess, item.span, E0117,
74+
"cannot provide an extension implementation \
75+
where both trait and type are not defined in this crate");
76+
}
77+
Err(traits::OrphanCheckErr::UncoveredTypeParameter(param_ty)) => {
78+
if !self.tcx.sess.features.borrow().old_orphan_check {
79+
self.tcx.sess.span_err(
80+
item.span,
81+
format!("type parameter `{}` must also appear as a type parameter \
82+
of some type defined within this crate",
83+
param_ty.user_string(self.tcx)).as_slice());
84+
self.tcx.sess.span_note(
85+
item.span,
86+
format!("for a limited time, you can add \
87+
`#![feature(old_orphan_check)]` to your crate \
88+
to disable this rule").as_slice());
89+
}
90+
}
7491
}
7592
}
7693
_ => {

src/librustdoc/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#![allow(unknown_features)]
2121
#![feature(globs, macro_rules, phase, slicing_syntax)]
2222
#![feature(unboxed_closures)]
23+
#![feature(old_orphan_check)]
2324

2425
extern crate arena;
2526
extern crate getopts;

src/libserialize/json.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@
8181
//! use serialize::json;
8282
//!
8383
//! // Automatically generate `Decodable` and `Encodable` trait implementations
84-
//! #[deriving(Decodable, Encodable)]
84+
//! #[deriving(RustcDecodable, RustcEncodable)]
8585
//! pub struct TestStruct {
8686
//! data_int: u8,
8787
//! data_str: String,

src/libstd/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@
107107
#![feature(macro_rules, globs, linkage, thread_local, asm)]
108108
#![feature(default_type_params, phase, lang_items, unsafe_destructor)]
109109
#![feature(slicing_syntax, unboxed_closures)]
110+
#![feature(old_orphan_check)]
110111

111112
// Don't link to std. We are std.
112113
#![no_std]

0 commit comments

Comments
 (0)