Auto merge of #48138 - estebank:issue-45092, r=nikomatsakis

Reword E0044 and message for `!Send` types

 - Reword E0044 help.
 - Change error message for types that don't implement `Send`

CC #45092, #46678, #24909, #33307.

src/libcore/marker.rs

@@ -343,8 +343,21 @@ pub trait Copy : Clone {
 /// [transmute]: ../../std/mem/fn.transmute.html
 #[stable(feature = "rust1", since = "1.0.0")]
 #[lang = "sync"]
-#[rustc_on_unimplemented = "`{Self}` cannot be shared between threads safely"]
+#[rustc_on_unimplemented(
+    message="`{Self}` cannot be shared between threads safely",
+    label="`{Self}` cannot be shared between threads safely"
+)]
 pub unsafe auto trait Sync {
+    // FIXME(estebank): once support to add notes in `rustc_on_unimplemented`
+    // lands in beta, and it has been extended to check whether a closure is
+    // anywhere in the requirement chain, extend it as such (#48534):
+    // ```
+    // on(
+    //     closure,
+    //     note="`{Self}` cannot be shared safely, consider marking the closure `move`"
+    // ),
+    // ```
+
     // Empty
 }
 

src/librustc/traits/error_reporting.rs

@@ -339,18 +339,15 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
             .unwrap_or(trait_ref.def_id());
         let trait_ref = *trait_ref.skip_binder();
 
-        let desugaring;
-        let method;
         let mut flags = vec![];
-        let direct = match obligation.cause.code {
+        match obligation.cause.code {
             ObligationCauseCode::BuiltinDerivedObligation(..) |
-            ObligationCauseCode::ImplDerivedObligation(..) => false,
-            _ => true
-        };
-        if direct {
-            // this is a "direct", user-specified, rather than derived,
-            // obligation.
-            flags.push(("direct".to_string(), None));
+            ObligationCauseCode::ImplDerivedObligation(..) => {}
+            _ => {
+                // this is a "direct", user-specified, rather than derived,
+                // obligation.
+                flags.push(("direct".to_string(), None));
+            }
         }
 
         if let ObligationCauseCode::ItemObligation(item) = obligation.cause.code {
@@ -360,21 +357,27 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
             //
             // Currently I'm leaving it for what I need for `try`.
             if self.tcx.trait_of_item(item) == Some(trait_ref.def_id) {
-                method = self.tcx.item_name(item);
+                let method = self.tcx.item_name(item);
                 flags.push(("from_method".to_string(), None));
                 flags.push(("from_method".to_string(), Some(method.to_string())));
             }
         }
 
         if let Some(k) = obligation.cause.span.compiler_desugaring_kind() {
-            desugaring = k.as_symbol().as_str();
+            let desugaring = k.as_symbol().as_str();
             flags.push(("from_desugaring".to_string(), None));
             flags.push(("from_desugaring".to_string(), Some(desugaring.to_string())));
         }
         let generics = self.tcx.generics_of(def_id);
         let self_ty = trait_ref.self_ty();
-        let self_ty_str = self_ty.to_string();
-        flags.push(("_Self".to_string(), Some(self_ty_str.clone())));
+        // This is also included through the generics list as `Self`,
+        // but the parser won't allow you to use it
+        flags.push(("_Self".to_string(), Some(self_ty.to_string())));
+        if let Some(def) = self_ty.ty_adt_def() {
+            // We also want to be able to select self's original
+            // signature with no type arguments resolved
+            flags.push(("_Self".to_string(), Some(self.tcx.type_of(def.did).to_string())));
+        }
 
         for param in generics.types.iter() {
             let name = param.name.as_str().to_string();

src/librustc_typeck/check/mod.rs

@@ -1223,9 +1223,11 @@ pub fn check_item_type<'a,'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, it: &'tcx hir::Item
                 if !generics.types.is_empty() {
                     let mut err = struct_span_err!(tcx.sess, item.span, E0044,
                         "foreign items may not have type parameters");
-                    span_help!(&mut err, item.span,
-                        "consider using specialization instead of \
-                        type parameters");
+                    err.span_label(item.span, "can't have type parameters");
+                    // FIXME: once we start storing spans for type arguments, turn this into a
+                    // suggestion.
+                    err.help("use specialization instead of type parameters by replacing them \
+                              with concrete types like `u32`");
                     err.emit();
                 }
 

src/test/compile-fail/builtin-superkinds-double-superkind.rs

@@ -13,9 +13,11 @@
 
 trait Foo : Send+Sync { }
 
-impl <T: Sync+'static> Foo for (T,) { } //~ ERROR `T: std::marker::Send` is not satisfied
+impl <T: Sync+'static> Foo for (T,) { }
+//~^ ERROR the trait bound `T: std::marker::Send` is not satisfied in `(T,)` [E0277]
 
-impl <T: Send> Foo for (T,T) { } //~ ERROR `T: std::marker::Sync` is not satisfied
+impl <T: Send> Foo for (T,T) { }
+//~^ ERROR `T` cannot be shared between threads safely [E0277]
 
 impl <T: Send+Sync> Foo for (T,T,T) { } // (ok)
 

src/test/compile-fail/closure-bounds-subtype.rs

@@ -21,7 +21,7 @@ fn give_any<F>(f: F) where F: FnOnce() {
 
 fn give_owned<F>(f: F) where F: FnOnce() + Send {
     take_any(f);
-    take_const_owned(f); //~ ERROR `F: std::marker::Sync` is not satisfied
+    take_const_owned(f); //~ ERROR `F` cannot be shared between threads safely [E0277]
 }
 
 fn main() {}

src/test/compile-fail/extern-types-not-sync-send.rs

@@ -21,7 +21,7 @@ fn assert_send<T: ?Sized + Send>() { }
 
 fn main() {
     assert_sync::<A>();
-    //~^ ERROR the trait bound `A: std::marker::Sync` is not satisfied
+    //~^ ERROR `A` cannot be shared between threads safely [E0277]
 
     assert_send::<A>();
     //~^ ERROR the trait bound `A: std::marker::Send` is not satisfied

src/test/compile-fail/issue-16538.rs

@@ -21,7 +21,7 @@ mod Y {
 }
 
 static foo: *const Y::X = Y::foo(Y::x as *const Y::X);
-//~^ ERROR `*const usize: std::marker::Sync` is not satisfied
+//~^ ERROR `*const usize` cannot be shared between threads safely [E0277]
 //~| ERROR cannot refer to other statics by value, use the address-of operator or a constant instead
 //~| ERROR E0015
 

src/test/compile-fail/issue-17718-static-sync.rs

@@ -17,6 +17,6 @@ impl !Sync for Foo {}
 
 static FOO: usize = 3;
 static BAR: Foo = Foo;
-//~^ ERROR: `Foo: std::marker::Sync` is not satisfied
+//~^ ERROR: `Foo` cannot be shared between threads safely [E0277]
 
 fn main() {}

src/test/compile-fail/issue-43733-2.rs

@@ -33,7 +33,7 @@ impl<T> Key<T> {
 use std::thread::__FastLocalKeyInner as Key;
 
 static __KEY: Key<()> = Key::new();
-//~^ ERROR `std::cell::UnsafeCell<std::option::Option<()>>: std::marker::Sync` is not satisfied
-//~| ERROR `std::cell::Cell<bool>: std::marker::Sync` is not satisfied
+//~^ ERROR `std::cell::UnsafeCell<std::option::Option<()>>` cannot be shared between threads
+//~| ERROR `std::cell::Cell<bool>` cannot be shared between threads safely [E0277]
 
 fn main() {}

src/test/compile-fail/issue-7364.rs

@@ -15,6 +15,6 @@ use std::cell::RefCell;
 // Regression test for issue 7364
 static boxed: Box<RefCell<isize>> = box RefCell::new(0);
 //~^ ERROR allocations are not allowed in statics
-//~| ERROR `std::cell::RefCell<isize>: std::marker::Sync` is not satisfied
+//~| ERROR `std::cell::RefCell<isize>` cannot be shared between threads safely [E0277]
 
 fn main() { }

src/test/compile-fail/kindck-send-object.rs

@@ -20,7 +20,7 @@ trait Message : Send { }
 
 fn object_ref_with_static_bound_not_ok() {
     assert_send::<&'static (Dummy+'static)>();
-    //~^ ERROR : std::marker::Sync` is not satisfied
+    //~^ ERROR `Dummy + 'static` cannot be shared between threads safely [E0277]
 }
 
 fn box_object_with_no_bound_not_ok<'a>() {

src/test/compile-fail/kindck-send-object1.rs

@@ -18,7 +18,7 @@ trait Dummy { }
 // careful with object types, who knows what they close over...
 fn test51<'a>() {
     assert_send::<&'a Dummy>();
-    //~^ ERROR : std::marker::Sync` is not satisfied
+    //~^ ERROR `Dummy + 'a` cannot be shared between threads safely [E0277]
 }
 fn test52<'a>() {
     assert_send::<&'a (Dummy+Sync)>();

src/test/compile-fail/kindck-send-object2.rs

@@ -14,7 +14,8 @@ fn assert_send<T:Send>() { }
 trait Dummy { }
 
 fn test50() {
-    assert_send::<&'static Dummy>(); //~ ERROR : std::marker::Sync` is not satisfied
+    assert_send::<&'static Dummy>();
+    //~^ ERROR `Dummy + 'static` cannot be shared between threads safely [E0277]
 }
 
 fn test53() {

src/test/compile-fail/mutable-enum-indirect.rs

@@ -24,5 +24,6 @@ fn bar<T: Sync>(_: T) {}
 
 fn main() {
     let x = Foo::A(NoSync);
-    bar(&x); //~ ERROR `NoSync: std::marker::Sync` is not satisfied
+    bar(&x);
+    //~^ ERROR `NoSync` cannot be shared between threads safely [E0277]
 }

src/test/compile-fail/mutexguard-sync.rs

@@ -18,5 +18,6 @@ fn main()
 {
     let m = Mutex::new(Cell::new(0i32));
     let guard = m.lock().unwrap();
-    test_sync(guard); //~ ERROR the trait bound
+    test_sync(guard);
+    //~^ ERROR `std::cell::Cell<i32>` cannot be shared between threads safely [E0277]
 }

src/test/compile-fail/no_share-enum.rs

@@ -22,5 +22,5 @@ fn bar<T: Sync>(_: T) {}
 fn main() {
     let x = Foo::A(NoSync);
     bar(x);
-    //~^ ERROR `NoSync: std::marker::Sync` is not satisfied
+    //~^ ERROR `NoSync` cannot be shared between threads safely [E0277]
 }

src/test/compile-fail/no_share-struct.rs

@@ -20,5 +20,5 @@ fn bar<T: Sync>(_: T) {}
 fn main() {
     let x = Foo { a: 5 };
     bar(x);
-    //~^ ERROR `Foo: std::marker::Sync` is not satisfied
+    //~^ ERROR `Foo` cannot be shared between threads safely [E0277]
 }

src/test/compile-fail/not-sync.rs

@@ -16,17 +16,17 @@ fn test<T: Sync>() {}
 
 fn main() {
     test::<Cell<i32>>();
-    //~^ ERROR `std::cell::Cell<i32>: std::marker::Sync` is not satisfied
+    //~^ ERROR `std::cell::Cell<i32>` cannot be shared between threads safely [E0277]
     test::<RefCell<i32>>();
-    //~^ ERROR `std::cell::RefCell<i32>: std::marker::Sync` is not satisfied
+    //~^ ERROR `std::cell::RefCell<i32>` cannot be shared between threads safely [E0277]
 
     test::<Rc<i32>>();
-    //~^ ERROR `std::rc::Rc<i32>: std::marker::Sync` is not satisfied
+    //~^ ERROR `std::rc::Rc<i32>` cannot be shared between threads safely [E0277]
     test::<Weak<i32>>();
-    //~^ ERROR `std::rc::Weak<i32>: std::marker::Sync` is not satisfied
+    //~^ ERROR `std::rc::Weak<i32>` cannot be shared between threads safely [E0277]
 
     test::<Receiver<i32>>();
-    //~^ ERROR `std::sync::mpsc::Receiver<i32>: std::marker::Sync` is not satisfied
+    //~^ ERROR `std::sync::mpsc::Receiver<i32>` cannot be shared between threads safely [E0277]
     test::<Sender<i32>>();
-    //~^ ERROR `std::sync::mpsc::Sender<i32>: std::marker::Sync` is not satisfied
+    //~^ ERROR `std::sync::mpsc::Sender<i32>` cannot be shared between threads safely [E0277]
 }

src/test/compile-fail/phantom-oibit.rs

@@ -28,11 +28,13 @@ struct Nested<T>(T);
 fn is_zen<T: Zen>(_: T) {}
 
 fn not_sync<T>(x: Guard<T>) {
-    is_zen(x)  //~ error: `T: std::marker::Sync` is not satisfied
+    is_zen(x)
+    //~^ ERROR `T` cannot be shared between threads safely [E0277]
 }
 
 fn nested_not_sync<T>(x: Nested<Guard<T>>) {
-    is_zen(x)  //~ error: `T: std::marker::Sync` is not satisfied
+    is_zen(x)
+    //~^ ERROR `T` cannot be shared between threads safely [E0277]
 }
 
 fn main() {}

src/test/compile-fail/typeck-default-trait-impl-negation-sync.rs

@@ -43,11 +43,11 @@ fn is_sync<T: Sync>() {}
 fn main() {
     is_sync::<MySync>();
     is_sync::<MyNotSync>();
-    //~^ ERROR `MyNotSync: std::marker::Sync` is not satisfied
+    //~^ ERROR `MyNotSync` cannot be shared between threads safely [E0277]
 
     is_sync::<MyTypeWUnsafe>();
-    //~^ ERROR `std::cell::UnsafeCell<u8>: std::marker::Sync` is not satisfied
+    //~^ ERROR `std::cell::UnsafeCell<u8>` cannot be shared between threads safely [E0277]
 
     is_sync::<MyTypeManaged>();
-    //~^ ERROR `Managed: std::marker::Sync` is not satisfied
+    //~^ ERROR `Managed` cannot be shared between threads safely [E0277]
 }

src/test/compile-fail/typeck-unsafe-always-share.rs

@@ -27,16 +27,16 @@ fn test<T: Sync>(s: T) {}
 fn main() {
     let us = UnsafeCell::new(MySync{u: UnsafeCell::new(0)});
     test(us);
-    //~^ ERROR `std::cell::UnsafeCell<MySync<{integer}>>: std::marker::Sync` is not satisfied
+    //~^ ERROR `std::cell::UnsafeCell<MySync<{integer}>>` cannot be shared between threads safely
 
     let uns = UnsafeCell::new(NoSync);
     test(uns);
-    //~^ ERROR `std::cell::UnsafeCell<NoSync>: std::marker::Sync` is not satisfied
+    //~^ ERROR `std::cell::UnsafeCell<NoSync>` cannot be shared between threads safely [E0277]
 
     let ms = MySync{u: uns};
     test(ms);
-    //~^ ERROR `std::cell::UnsafeCell<NoSync>: std::marker::Sync` is not satisfied
+    //~^ ERROR `std::cell::UnsafeCell<NoSync>` cannot be shared between threads safely [E0277]
 
     test(NoSync);
-    //~^ ERROR `NoSync: std::marker::Sync` is not satisfied
+    //~^ ERROR `NoSync` cannot be shared between threads safely [E0277]
 }

src/test/ui/closure-move-sync.rs

@@ -0,0 +1,32 @@
+// Copyright 2018 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.
+
+use std::thread;
+use std::sync::mpsc::channel;
+
+fn bar() {
+    let (send, recv) = channel();
+    let t = thread::spawn(|| {
+        recv.recv().unwrap();
+        //~^^ ERROR `std::sync::mpsc::Receiver<()>` cannot be shared between threads safely
+    });
+
+    send.send(());
+
+    t.join().unwrap();
+}
+
+fn foo() {
+    let (tx, _rx) = channel();
+    thread::spawn(|| tx.send(()).unwrap());
+    //~^ ERROR `std::sync::mpsc::Sender<()>` cannot be shared between threads safely
+}
+
+fn main() {}

src/test/ui/closure-move-sync.stderr

@@ -0,0 +1,25 @@
+error[E0277]: `std::sync::mpsc::Receiver<()>` cannot be shared between threads safely
+  --> $DIR/closure-move-sync.rs:16:13
+   |
+LL |     let t = thread::spawn(|| {
+   |             ^^^^^^^^^^^^^ `std::sync::mpsc::Receiver<()>` cannot be shared between threads safely
+   |
+   = help: the trait `std::marker::Sync` is not implemented for `std::sync::mpsc::Receiver<()>`
+   = note: required because of the requirements on the impl of `std::marker::Send` for `&std::sync::mpsc::Receiver<()>`
+   = note: required because it appears within the type `[closure@$DIR/closure-move-sync.rs:16:27: 19:6 recv:&std::sync::mpsc::Receiver<()>]`
+   = note: required by `std::thread::spawn`
+
+error[E0277]: `std::sync::mpsc::Sender<()>` cannot be shared between threads safely
+  --> $DIR/closure-move-sync.rs:28:5
+   |
+LL |     thread::spawn(|| tx.send(()).unwrap());
+   |     ^^^^^^^^^^^^^ `std::sync::mpsc::Sender<()>` cannot be shared between threads safely
+   |
+   = help: the trait `std::marker::Sync` is not implemented for `std::sync::mpsc::Sender<()>`
+   = note: required because of the requirements on the impl of `std::marker::Send` for `&std::sync::mpsc::Sender<()>`
+   = note: required because it appears within the type `[closure@$DIR/closure-move-sync.rs:28:19: 28:42 tx:&std::sync::mpsc::Sender<()>]`
+   = note: required by `std::thread::spawn`
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0277`.

src/test/ui/error-codes/E0044.rs

@@ -1,4 +1,4 @@
-// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
 // file at the top-level directory of this distribution and at
 // http://rust-lang.org/COPYRIGHT.
 //
@@ -8,7 +8,12 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-extern { fn some_func<T>(x: T); } //~ ERROR E0044
+extern {
+    fn sqrt<T>(f: T) -> T;
+    //~^ ERROR foreign items may not have type parameters [E0044]
+    //~| HELP use specialization instead of type parameters by replacing them with concrete types
+    //~| NOTE can't have type parameters
+}
 
 fn main() {
 }

src/test/ui/error-codes/E0044.stderr

@@ -1,14 +1,10 @@
 error[E0044]: foreign items may not have type parameters
-  --> $DIR/E0044.rs:11:10
+  --> $DIR/E0044.rs:12:5
    |
-LL | extern { fn some_func<T>(x: T); } //~ ERROR E0044
-   |          ^^^^^^^^^^^^^^^^^^^^^^
+LL |     fn sqrt<T>(f: T) -> T;
+   |     ^^^^^^^^^^^^^^^^^^^^^^ can't have type parameters
    |
-help: consider using specialization instead of type parameters
-  --> $DIR/E0044.rs:11:10
-   |
-LL | extern { fn some_func<T>(x: T); } //~ ERROR E0044
-   |          ^^^^^^^^^^^^^^^^^^^^^^
+   = help: use specialization instead of type parameters by replacing them with concrete types like `u32`
 
 error: aborting due to previous error