Skip to content

Commit

Permalink
Remove the coherence impls pass that was specialized to builtin bounds,
Browse files Browse the repository at this point in the history
since there are separate checks that apply to Copy (and Send uses the
generic defaulted trait rules). Also prohibit `Sized` from being
manually implemented for now.
  • Loading branch information
nikomatsakis committed Mar 6, 2015
1 parent 2e21689 commit 4e789e0
Show file tree
Hide file tree
Showing 11 changed files with 184 additions and 90 deletions.
47 changes: 0 additions & 47 deletions src/librustc_typeck/coherence/impls.rs

This file was deleted.

2 changes: 0 additions & 2 deletions src/librustc_typeck/coherence/mod.rs
Expand Up @@ -49,7 +49,6 @@ use syntax::visit;
use util::nodemap::{DefIdMap, FnvHashMap};
use util::ppaux::Repr;

mod impls;
mod orphan;
mod overlap;
mod unsafety;
Expand Down Expand Up @@ -583,7 +582,6 @@ pub fn check_coherence(crate_context: &CrateCtxt) {
inference_context: new_infer_ctxt(crate_context.tcx),
inherent_impls: RefCell::new(FnvHashMap()),
}.check(crate_context.tcx.map.krate());
impls::check(crate_context.tcx);
unsafety::check(crate_context.tcx);
orphan::check(crate_context.tcx);
overlap::check(crate_context.tcx);
Expand Down
110 changes: 89 additions & 21 deletions src/librustc_typeck/coherence/orphan.rs
Expand Up @@ -37,10 +37,13 @@ impl<'cx, 'tcx> OrphanChecker<'cx, 'tcx> {
a trait or new type instead");
}
}
}

impl<'cx, 'tcx,'v> visit::Visitor<'v> for OrphanChecker<'cx, 'tcx> {
fn visit_item(&mut self, item: &ast::Item) {
/// Checks exactly one impl for orphan rules and other such
/// restrictions. In this fn, it can happen that multiple errors
/// apply to a specific impl, so just return after reporting one
/// to prevent inundating the user with a bunch of similar error
/// reports.
fn check_item(&self, item: &ast::Item) {
let def_id = ast_util::local_def(item.id);
match item.node {
ast::ItemImpl(_, _, _, None, _, _) => {
Expand All @@ -63,6 +66,7 @@ impl<'cx, 'tcx,'v> visit::Visitor<'v> for OrphanChecker<'cx, 'tcx> {
span_err!(self.tcx.sess, item.span, E0118,
"no base type found for inherent implementation; \
implement a trait or new type instead");
return;
}
}
}
Expand All @@ -81,6 +85,7 @@ impl<'cx, 'tcx,'v> visit::Visitor<'v> for OrphanChecker<'cx, 'tcx> {
types defined in this crate; \
only traits defined in the current crate can be \
implemented for arbitrary types");
return;
}
}
Err(traits::OrphanCheckErr::UncoveredTy(param_ty)) => {
Expand All @@ -90,11 +95,44 @@ impl<'cx, 'tcx,'v> visit::Visitor<'v> for OrphanChecker<'cx, 'tcx> {
some local type (e.g. `MyStruct<T>`); only traits defined in \
the current crate can be implemented for a type parameter",
param_ty.user_string(self.tcx));
return;
}
}
}

// Impls of a defaulted trait face additional rules.
// In addition to the above rules, we restrict impls of defaulted traits
// so that they can only be implemented on structs/enums. To see why this
// restriction exists, consider the following example (#22978). Imagine
// that crate A defines a defaulted trait `Foo` and a fn that operates
// on pairs of types:
//
// ```
// // Crate A
// trait Foo { }
// impl Foo for .. { }
// fn two_foos<A:Foo,B:Foo>(..) {
// one_foo::<(A,B)>(..)
// }
// fn one_foo<T:Foo>(..) { .. }
// ```
//
// This type-checks fine; in particular the fn
// `two_foos` is able to conclude that `(A,B):Foo`
// because `A:Foo` and `B:Foo`.
//
// Now imagine that crate B comes along and does the following:
//
// ```
// struct A { }
// struct B { }
// impl Foo for A { }
// impl Foo for B { }
// impl !Send for (A, B) { }
// ```
//
// This final impl is legal according to the orpan
// rules, but it invalidates the reasoning from
// `two_foos` above.
debug!("trait_ref={} trait_def_id={} trait_has_default_impl={}",
trait_ref.repr(self.tcx),
trait_def_id.repr(self.tcx),
Expand All @@ -104,29 +142,53 @@ impl<'cx, 'tcx,'v> visit::Visitor<'v> for OrphanChecker<'cx, 'tcx> {
trait_def_id.krate != ast::LOCAL_CRATE
{
let self_ty = trait_ref.self_ty();
match self_ty.sty {
ty::ty_struct(self_def_id, _) | ty::ty_enum(self_def_id, _) => {
// The orphan check often rules this out,
// but not always. For example, the orphan
// check would accept `impl Send for
// Box<SomethingLocal>`, but we want to
// forbid that.
if self_def_id.krate != ast::LOCAL_CRATE {
self.tcx.sess.span_err(
item.span,
"cross-crate traits with a default impl \
let opt_self_def_id = match self_ty.sty {
ty::ty_struct(self_def_id, _) | ty::ty_enum(self_def_id, _) =>
Some(self_def_id),
ty::ty_uniq(..) =>
self.tcx.lang_items.owned_box(),
_ =>
None
};

let msg = match opt_self_def_id {
// We only want to permit structs/enums, but not *all* structs/enums.
// They must be local to the current crate, so that people
// can't do `unsafe impl Send for Rc<SomethingLocal>` or
// `unsafe impl !Send for Box<SomethingLocalAndSend>`.
Some(self_def_id) => {
if self_def_id.krate == ast::LOCAL_CRATE {
None
} else {
Some(format!(
"cross-crate traits with a default impl, like `{}`, \
can only be implemented for a struct/enum type \
defined in the current crate");
defined in the current crate",
ty::item_path_str(self.tcx, trait_def_id)))
}
}
_ => {
self.tcx.sess.span_err(
item.span,
"cross-crate traits with a default impl \
can only be implemented for a struct or enum type");
Some(format!(
"cross-crate traits with a default impl, like `{}`, \
can only be implemented for a struct/enum type, \
not `{}`",
ty::item_path_str(self.tcx, trait_def_id),
self_ty.user_string(self.tcx)))
}
};

if let Some(msg) = msg {
span_err!(self.tcx.sess, item.span, E0321, "{}", msg);
return;
}
}

// Disallow *all* explicit impls of `Sized` for now.
if Some(trait_def_id) == self.tcx.lang_items.sized_trait() {
span_err!(self.tcx.sess, item.span, E0322,
"explicit impls for the `Sized` trait are not permitted");
return;
}
}
ast::ItemDefaultImpl(..) => {
// "Trait" impl
Expand All @@ -135,14 +197,20 @@ impl<'cx, 'tcx,'v> visit::Visitor<'v> for OrphanChecker<'cx, 'tcx> {
if trait_ref.def_id.krate != ast::LOCAL_CRATE {
span_err!(self.tcx.sess, item.span, E0318,
"cannot create default implementations for traits outside the \
crate they're defined in; define a new trait instead.");
crate they're defined in; define a new trait instead");
return;
}
}
_ => {
// Not an impl
}
}
}
}

impl<'cx, 'tcx,'v> visit::Visitor<'v> for OrphanChecker<'cx, 'tcx> {
fn visit_item(&mut self, item: &ast::Item) {
self.check_item(item);
visit::walk_item(self, item);
}
}
4 changes: 3 additions & 1 deletion src/librustc_typeck/diagnostics.rs
Expand Up @@ -175,7 +175,9 @@ register_diagnostics! {
E0250, // expected constant expr for array length
E0318, // can't create default impls for traits outside their crates
E0319, // trait impls for defaulted traits allowed just for structs/enums
E0320 // recursive overflow during dropck
E0320, // recursive overflow during dropck
E0321, // extended coherence rules for defaulted traits violated
E0322 // cannot implement Sized explicitly
}

__build_diagnostic_array! { DIAGNOSTICS }
Expand Down
Expand Up @@ -15,3 +15,5 @@ use std::marker::MarkerTrait;

pub trait DefaultedTrait : MarkerTrait { }
impl DefaultedTrait for .. { }

pub struct Something<T> { t: T }
39 changes: 39 additions & 0 deletions src/test/compile-fail/coherence-impls-copy.rs
@@ -0,0 +1,39 @@
// Copyright 2015 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.

#![feature(optin_builtin_traits)]

use std::marker::Copy;

enum TestE {
A
}

struct MyType;

struct NotSync;
impl !Sync for NotSync {}

impl Copy for TestE {}
impl Copy for MyType {}
impl Copy for (MyType, MyType) {}
//~^ ERROR E0206

impl Copy for &'static NotSync {}
//~^ ERROR E0206

impl Copy for [MyType] {}
//~^ ERROR E0206

impl Copy for &'static [NotSync] {}
//~^ ERROR E0206

fn main() {
}
Expand Up @@ -10,7 +10,7 @@

#![feature(optin_builtin_traits)]

use std::marker::Send;
use std::marker::Copy;

enum TestE {
A
Expand All @@ -24,20 +24,17 @@ impl !Sync for NotSync {}
unsafe impl Send for TestE {}
unsafe impl Send for MyType {}
unsafe impl Send for (MyType, MyType) {}
//~^ ERROR builtin traits can only be implemented on structs or enums
//~^ ERROR E0321

unsafe impl Send for &'static NotSync {}
//~^ ERROR builtin traits can only be implemented on structs or enums
//~^ ERROR E0321

unsafe impl Send for [MyType] {}
//~^ ERROR builtin traits can only be implemented on structs or enums
//~^ ERROR E0321

unsafe impl Send for &'static [NotSync] {}
//~^ ERROR builtin traits can only be implemented on structs or enums
//~^^ ERROR conflicting implementations for trait `core::marker::Send`

fn is_send<T: Send>() {}
//~^ ERROR E0321
//~| ERROR conflicting implementations

fn main() {
is_send::<(MyType, TestE)>();
}
33 changes: 33 additions & 0 deletions src/test/compile-fail/coherence-impls-sized.rs
@@ -0,0 +1,33 @@
// Copyright 2015 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.

#![feature(optin_builtin_traits)]

use std::marker::Copy;

enum TestE {
A
}

struct MyType;

struct NotSync;
impl !Sync for NotSync {}

impl Sized for TestE {} //~ ERROR E0322
impl Sized for MyType {} //~ ERROR E0322
impl Sized for (MyType, MyType) {} //~ ERROR E0322
impl Sized for &'static NotSync {} //~ ERROR E0322
impl Sized for [MyType] {} //~ ERROR E0322
//~^ ERROR E0277
impl Sized for &'static [NotSync] {} //~ ERROR E0322

fn main() {
}
8 changes: 5 additions & 3 deletions src/test/compile-fail/coherence-orphan.rs
Expand Up @@ -19,13 +19,15 @@ use lib::TheTrait;

struct TheType;

impl TheTrait<usize> for isize { } //~ ERROR E0117
impl TheTrait<usize> for isize { }
//~^ ERROR E0117

impl TheTrait<TheType> for isize { }

impl TheTrait<isize> for TheType { }

impl !Send for Vec<isize> { } //~ ERROR E0117
//~^ ERROR conflicting
impl !Send for Vec<isize> { }
//~^ ERROR E0117
//~| ERROR E0119

fn main() { }

0 comments on commit 4e789e0

Please sign in to comment.