Skip to content

Implement new orphan rule that requires that impls of remote traits meet... #20594

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

Merged
merged 1 commit into from
Jan 6, 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
4 changes: 4 additions & 0 deletions src/libcore/borrow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,14 @@ use option::Option;
use self::Cow::*;

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

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

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

/// A generalization of Clone to borrowed data.
#[old_orphan_check]
pub trait ToOwned<Owned> for Sized?: BorrowFrom<Owned> {
/// Create owned data from borrowed data, usually by copying.
fn to_owned(&self) -> Owned;
Expand Down
1 change: 1 addition & 0 deletions src/libcore/cmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ use option::Option::{self, Some, None};
/// only if `a != b`.
#[lang="eq"]
#[stable]
#[old_orphan_check]
pub trait PartialEq<Sized? Rhs = Self> for Sized? {
/// This method tests for `self` and `other` values to be equal, and is used by `==`.
#[stable]
Expand Down
3 changes: 3 additions & 0 deletions src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -670,6 +670,9 @@ impl LintPass for UnusedAttributes {
"must_use",
"stable",
"unstable",

// FIXME: #19470 this shouldn't be needed forever
"old_orphan_check",
];

static CRATE_ATTRS: &'static [&'static str] = &[
Expand Down
56 changes: 23 additions & 33 deletions src/librustc/middle/traits/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use super::SelectionContext;
use super::{Obligation, ObligationCause};
use super::util;

use middle::subst::Subst;
use middle::subst::{Subst};
use middle::ty::{self, Ty};
use middle::infer::InferCtxt;
use std::collections::HashSet;
Expand Down Expand Up @@ -53,20 +53,20 @@ pub fn impl_can_satisfy(infcx: &InferCtxt,
}

#[allow(missing_copy_implementations)]
pub enum OrphanCheckErr {
pub enum OrphanCheckErr<'tcx> {
NoLocalInputType,
UncoveredTypeParameter(ty::ParamTy),
UncoveredTy(Ty<'tcx>),
}

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

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

// Check condition 1: at least one type must be local.
if !trait_ref.input_types().iter().any(|&t| ty_reaches_local(tcx, t)) {
return Err(OrphanCheckErr::NoLocalInputType);
// 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));
}

// Check condition 2: type parameters must be "covered" by a local type.
let covered_params: HashSet<_> =
trait_ref.input_types().iter()
.flat_map(|&t| type_parameters_covered_by_ty(tcx, t).into_iter())
.collect();
let all_params: HashSet<_> =
trait_ref.input_types().iter()
.flat_map(|&t| type_parameters_reachable_from_ty(t).into_iter())
.collect();
for &param in all_params.difference(&covered_params) {
return Err(OrphanCheckErr::UncoveredTypeParameter(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);
}

return Ok(());
}

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

fn ty_is_local_constructor<'tcx>(tcx: &ty::ctxt<'tcx>, ty: Ty<'tcx>) -> bool {
debug!("ty_is_local_constructor({})", ty.repr(tcx));

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

fn type_parameters_covered_by_ty<'tcx>(tcx: &ty::ctxt<'tcx>,
ty: Ty<'tcx>)
-> HashSet<ty::ParamTy>
ty: Ty<'tcx>)
-> HashSet<Ty<'tcx>>
{
if ty_is_local_constructor(tcx, ty) {
type_parameters_reachable_from_ty(ty)
Expand All @@ -165,14 +155,14 @@ 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::ParamTy> {
fn type_parameters_reachable_from_ty<'tcx>(ty: Ty<'tcx>) -> HashSet<Ty<'tcx>> {
ty.walk()
.filter_map(|t| {
.filter(|&t| {
match t.sty {
ty::ty_param(ref param_ty) => Some(param_ty.clone()),
_ => None,
// FIXME(#20590) straighten story about projection types
ty::ty_projection(..) | ty::ty_param(..) => true,
_ => false,
}
})
.collect()
}

2 changes: 1 addition & 1 deletion src/librustc_typeck/check/callee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use super::TupleArgumentsFlag;
use super::write_call;

use middle::infer;
use middle::ty::{mod, Ty};
use middle::ty::{self, Ty};
use syntax::ast;
use syntax::codemap::Span;
use syntax::parse::token;
Expand Down
26 changes: 18 additions & 8 deletions src/librustc_typeck/coherence/orphan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,20 +72,30 @@ impl<'cx, 'tcx,'v> visit::Visitor<'v> for OrphanChecker<'cx, 'tcx> {
ast::ItemImpl(_, _, Some(_), _, _) => {
// "Trait" impl
debug!("coherence2::orphan check: trait impl {}", item.repr(self.tcx));
let trait_def_id = ty::impl_trait_ref(self.tcx, def_id).unwrap().def_id;
match traits::orphan_check(self.tcx, def_id) {
Ok(()) => { }
Err(traits::OrphanCheckErr::NoLocalInputType) => {
span_err!(self.tcx.sess, item.span, E0117,
"cannot provide an extension implementation \
where both trait and type are not defined in this crate");
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 \
types defined in this crate; \
only traits defined in the current crate can be \
implemented for arbitrary types",
self_ty.user_string(self.tcx));
}
}
Err(traits::OrphanCheckErr::UncoveredTypeParameter(param_ty)) => {
if !self.tcx.sess.features.borrow().old_orphan_check {
Err(traits::OrphanCheckErr::UncoveredTy(param_ty)) => {
if !ty::has_attr(self.tcx, trait_def_id, "old_orphan_check") {
self.tcx.sess.span_err(
item.span,
format!("type parameter `{}` must also appear as a type parameter \
of some type defined within this crate",
param_ty.user_string(self.tcx)).as_slice());
format!(
"type parameter `{}` is not constrained by any local type; \
only traits defined in the current crate can be implemented \
for a type parameter",
param_ty.user_string(self.tcx)).as_slice());
self.tcx.sess.span_note(
item.span,
format!("for a limited time, you can add \
Expand Down
8 changes: 8 additions & 0 deletions src/libsyntax/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,14 @@ impl<'a, 'v> Visitor<'v> for PostExpansionVisitor<'a> {
removed in the future");
}

if attr::contains_name(i.attrs[],
"old_orphan_check") {
self.gate_feature(
"old_orphan_check",
i.span,
"the new orphan check rules will eventually be strictly enforced");
}

for item in items.iter() {
match *item {
ast::MethodImplItem(_) => {}
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::Remote;

impl<T> Remote for int { }
//~^ ERROR cannot provide an extension implementation
//~^ ERROR E0117

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 int { }
impl Remote1<BigInt> for int { } //~ ERROR E0117

fn main() { }
2 changes: 1 addition & 1 deletion src/test/compile-fail/coherence-bigint-param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,6 @@ use lib::Remote1;
pub struct BigInt;

impl<T> Remote1<BigInt> for T { }
//~^ ERROR type parameter `T` must also appear
//~^ 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 Vec<int> { }
impl Remote1<BigInt> for Vec<int> { } //~ ERROR E0117

fn main() { }
2 changes: 1 addition & 1 deletion src/test/compile-fail/coherence-cross-crate-conflict.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ extern crate trait_impl_conflict;
use trait_impl_conflict::Foo;

impl<A> Foo for A {
//~^ ERROR E0117
//~^ ERROR type parameter `A` is not constrained
//~^^ ERROR E0119
}

Expand Down
2 changes: 1 addition & 1 deletion src/test/compile-fail/coherence-lone-type-parameter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@
extern crate "coherence-lib" as lib;
use lib::Remote;

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

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 @@ -18,7 +18,7 @@ struct TheType;

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

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

impl TheTrait<int> for TheType { }

Expand Down
2 changes: 1 addition & 1 deletion src/test/compile-fail/coherence-overlapping-pairs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,6 @@ use lib::Remote;
struct Foo;

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

fn main() { }
2 changes: 1 addition & 1 deletion src/test/compile-fail/coherence-pair-covered-uncovered.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,6 @@ use lib::{Remote, Pair};
struct Local<T>(T);

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

fn main() { }
2 changes: 1 addition & 1 deletion src/test/compile-fail/drop-on-non-struct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

impl Drop for int {
//~^ ERROR the Drop trait may only be implemented on structures
//~^^ ERROR cannot provide an extension implementation
//~^^ ERROR E0117
fn drop(&mut self) {
println!("kaboom");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,5 @@ use lib::Remote1;
struct Foo<T>(T);

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

fn main() { }