diff --git a/src/librustc_typeck/coherence/impls.rs b/src/librustc_typeck/coherence/impls.rs deleted file mode 100644 index e89c96b36e1..00000000000 --- a/src/librustc_typeck/coherence/impls.rs +++ /dev/null @@ -1,47 +0,0 @@ -// 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 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - -//! Implementations checker: builtin traits and default impls are allowed just -//! for structs and enums. - -use middle::ty; -use syntax::ast::{Item, ItemImpl}; -use syntax::ast; -use syntax::visit; - -pub fn check(tcx: &ty::ctxt) { - let mut impls = ImplsChecker { tcx: tcx }; - visit::walk_crate(&mut impls, tcx.map.krate()); -} - -struct ImplsChecker<'cx, 'tcx:'cx> { - tcx: &'cx ty::ctxt<'tcx> -} - -impl<'cx, 'tcx,'v> visit::Visitor<'v> for ImplsChecker<'cx, 'tcx> { - fn visit_item(&mut self, item: &'v ast::Item) { - match item.node { - ast::ItemImpl(_, _, _, Some(_), _, _) => { - let trait_ref = ty::impl_id_to_trait_ref(self.tcx, item.id); - if let Some(_) = self.tcx.lang_items.to_builtin_kind(trait_ref.def_id) { - match trait_ref.self_ty().sty { - ty::ty_struct(..) | ty::ty_enum(..) => {} - _ => { - span_err!(self.tcx.sess, item.span, E0209, - "builtin traits can only be \ - implemented on structs or enums"); - } - } - } - } - _ => {} - } - } -} diff --git a/src/librustc_typeck/coherence/mod.rs b/src/librustc_typeck/coherence/mod.rs index 9a8545f3dd5..a06dcbaf556 100644 --- a/src/librustc_typeck/coherence/mod.rs +++ b/src/librustc_typeck/coherence/mod.rs @@ -49,7 +49,6 @@ use syntax::visit; use util::nodemap::{DefIdMap, FnvHashMap}; use util::ppaux::Repr; -mod impls; mod orphan; mod overlap; mod unsafety; @@ -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); diff --git a/src/librustc_typeck/coherence/orphan.rs b/src/librustc_typeck/coherence/orphan.rs index fc49a555ad3..0857203b777 100644 --- a/src/librustc_typeck/coherence/orphan.rs +++ b/src/librustc_typeck/coherence/orphan.rs @@ -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, _, _) => { @@ -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; } } } @@ -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)) => { @@ -90,11 +95,44 @@ impl<'cx, 'tcx,'v> visit::Visitor<'v> for OrphanChecker<'cx, 'tcx> { some local type (e.g. `MyStruct`); 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(..) { + // one_foo::<(A,B)>(..) + // } + // fn one_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), @@ -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`, 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` or + // `unsafe impl !Send for Box`. + 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 @@ -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); } } diff --git a/src/librustc_typeck/diagnostics.rs b/src/librustc_typeck/diagnostics.rs index 3bd15fbc7db..03fa269ccf8 100644 --- a/src/librustc_typeck/diagnostics.rs +++ b/src/librustc_typeck/diagnostics.rs @@ -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 } diff --git a/src/test/auxiliary/typeck-default-trait-impl-cross-crate-coherence-lib.rs b/src/test/auxiliary/typeck-default-trait-impl-cross-crate-coherence-lib.rs index 579b9b01925..506e7a00c75 100644 --- a/src/test/auxiliary/typeck-default-trait-impl-cross-crate-coherence-lib.rs +++ b/src/test/auxiliary/typeck-default-trait-impl-cross-crate-coherence-lib.rs @@ -15,3 +15,5 @@ use std::marker::MarkerTrait; pub trait DefaultedTrait : MarkerTrait { } impl DefaultedTrait for .. { } + +pub struct Something { t: T } diff --git a/src/test/compile-fail/coherence-impls-copy.rs b/src/test/compile-fail/coherence-impls-copy.rs new file mode 100644 index 00000000000..3034be177ca --- /dev/null +++ b/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 or the MIT license +// , 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() { +} diff --git a/src/test/compile-fail/coherence-impls-builtin.rs b/src/test/compile-fail/coherence-impls-send.rs similarity index 65% rename from src/test/compile-fail/coherence-impls-builtin.rs rename to src/test/compile-fail/coherence-impls-send.rs index 3e132dcb11f..b05c1ff0f0b 100644 --- a/src/test/compile-fail/coherence-impls-builtin.rs +++ b/src/test/compile-fail/coherence-impls-send.rs @@ -10,7 +10,7 @@ #![feature(optin_builtin_traits)] -use std::marker::Send; +use std::marker::Copy; enum TestE { A @@ -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() {} +//~^ ERROR E0321 +//~| ERROR conflicting implementations fn main() { - is_send::<(MyType, TestE)>(); } diff --git a/src/test/compile-fail/coherence-impls-sized.rs b/src/test/compile-fail/coherence-impls-sized.rs new file mode 100644 index 00000000000..a9a3ebaffb7 --- /dev/null +++ b/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 or the MIT license +// , 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() { +} diff --git a/src/test/compile-fail/coherence-orphan.rs b/src/test/compile-fail/coherence-orphan.rs index d7cd68e73c3..97dffec2dd9 100644 --- a/src/test/compile-fail/coherence-orphan.rs +++ b/src/test/compile-fail/coherence-orphan.rs @@ -19,13 +19,15 @@ use lib::TheTrait; struct TheType; -impl TheTrait for isize { } //~ ERROR E0117 +impl TheTrait for isize { } +//~^ ERROR E0117 impl TheTrait for isize { } impl TheTrait for TheType { } -impl !Send for Vec { } //~ ERROR E0117 -//~^ ERROR conflicting +impl !Send for Vec { } +//~^ ERROR E0117 +//~| ERROR E0119 fn main() { } diff --git a/src/test/compile-fail/typeck-default-trait-impl-cross-crate-coherence.rs b/src/test/compile-fail/typeck-default-trait-impl-cross-crate-coherence.rs index bcc01d1a0a7..3a29bb9c227 100644 --- a/src/test/compile-fail/typeck-default-trait-impl-cross-crate-coherence.rs +++ b/src/test/compile-fail/typeck-default-trait-impl-cross-crate-coherence.rs @@ -20,15 +20,15 @@ extern crate "typeck-default-trait-impl-cross-crate-coherence-lib" as lib; use lib::DefaultedTrait; struct A; -impl DefaultedTrait for (A,) { } -//~^ ERROR can only be implemented for a struct or enum type +impl DefaultedTrait for (A,) { } //~ ERROR E0321 struct B; -impl !DefaultedTrait for (B,) { } -//~^ ERROR can only be implemented for a struct or enum type +impl !DefaultedTrait for (B,) { } //~ ERROR E0321 struct C; -impl DefaultedTrait for Box { } -//~^ ERROR can only be implemented for a struct or enum type +struct D(T); +impl DefaultedTrait for Box { } //~ ERROR E0321 +impl DefaultedTrait for lib::Something { } //~ ERROR E0321 +impl DefaultedTrait for D { } // OK fn main() { } diff --git a/src/test/compile-fail/typeck-default-trait-impl-outside-crate.rs b/src/test/compile-fail/typeck-default-trait-impl-outside-crate.rs index 10ba8c74164..a345bd1b65c 100644 --- a/src/test/compile-fail/typeck-default-trait-impl-outside-crate.rs +++ b/src/test/compile-fail/typeck-default-trait-impl-outside-crate.rs @@ -13,6 +13,6 @@ #![feature(optin_builtin_traits)] impl Copy for .. {} -//~^ ERROR cannot create default implementations for traits outside the crate they're defined in; define a new trait instead. +//~^ ERROR E0318 fn main() {}