Remove the coherence impls pass that was specialized to builtin bounds,

nikomatsakis · nikomatsakis · commit 4e789e03be33 · 2015-03-06T18:27:50.000-05:00
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.
diff --git a/src/librustc_typeck/coherence/impls.rs b/src/librustc_typeck/coherence/impls.rs
diff --git 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
@@ -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<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),
@@ -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
@@ -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
@@ -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
@@ -15,3 +15,5 @@ use std::marker::MarkerTrait;
 
 pub trait DefaultedTrait : MarkerTrait { }
 impl DefaultedTrait for .. { }
+
+pub struct Something<T> { t: T }
diff --git a/src/test/compile-fail/coherence-impls-copy.rs 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 <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() {
+}
diff --git a/src/test/compile-fail/coherence-impls-send.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<T: 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
@@ -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() {
+}
diff --git 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<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() { }
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
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
