Suggest move closures when a Sync bound is not satisfied through a closure

@nikomatsakis

use std::thread;
use std::sync::mpsc;

fn main() {
    let (send, recv) = mpsc::channel();
    let t = thread::spawn(|| {
        recv.recv().unwrap();
    });

    send.send(());

    t.join().unwrap();
}

playpen

This currently gives the error

<anon>:6:13: 6:26 error: the trait `core::marker::Sync` is not implemented for the type `core::cell::UnsafeCell<std::sync::mpsc::Flavor<()>>` [E0277]
<anon>:6     let t = thread::spawn(|| {
                     ^~~~~~~~~~~~~
<anon>:6:13: 6:26 help: see the detailed explanation for E0277
<anon>:6:13: 6:26 note: `core::cell::UnsafeCell<std::sync::mpsc::Flavor<()>>` cannot be shared between threads safely
<anon>:6:13: 6:26 note: required because it appears within the type `std::sync::mpsc::Receiver<()>`
<anon>:6:13: 6:26 note: required because it appears within the type `[closure@<anon>:6:27: 8:6 recv:&std::sync::mpsc::Receiver<()>]`
<anon>:6:13: 6:26 note: required by `std::thread::spawn`
error: aborting due to previous error
playpen: application terminated with error code 101

If a closure needs to be Sync, we should perhaps suggest that it be made a move closure.

Perhaps we should verify that the closure can be made move (i.e. it is : 'static when made move), but I'm not sure how easy that is to do.

We already suggest move closures when you do the same thing with a Vec, because the error reaches borrowck. Typeck sync issues don't suggest move.

cc @nikomatsakis

Why isn't the

impl<'a, T: Sync> Send for &'a T

impl on the stack?

Not sure, really.

My patch so far

diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs
index d7ddfc9..b70a813 100644
--- a/src/librustc/traits/error_reporting.rs
+++ b/src/librustc/traits/error_reporting.rs
@@ -861,10 +861,26 @@ fn note_obligation_cause_code<'a, 'tcx, T>(infcx: &InferCtxt<'a, 'tcx>,
         }
         ObligationCauseCode::BuiltinDerivedObligation(ref data) => {
             let parent_trait_ref = infcx.resolve_type_vars_if_possible(&data.parent_trait_ref);
+            let ty = parent_trait_ref.0.self_ty();
             err.fileline_note(
                 cause_span,
                 &format!("required because it appears within the type `{}`",
-                         parent_trait_ref.0.self_ty()));
+                         ty));
+
+            // In case the requirement is that a closure be Send,
+            if Some(parent_trait_ref.0.def_id) == infcx.tcx.lang_items.send_trait() {
+                if let ty::TyClosure(did, _) = ty.sty {
+                    if let Some(span) = infcx.tcx.map.span_if_local(did) {
+                        let suggestion = match infcx.tcx.sess.codemap().span_to_snippet(span) {
+                            Ok(string) => format!("move {}", string),
+                            Err(_) => format!("move |<args>| <body>")
+                        };
+                        err.span_suggestion(span,
+                                            "Perhaps try marking this closure as a `move` closure?",
+                                            suggestion);
+                    }
+                }
+            }
             let parent_predicate = parent_trait_ref.to_predicate();
             note_obligation_cause_code(infcx,
                                        err,

Still needs to check if the closure is move already or if move-ifying it will have any effect.

After not dropping impls:

closure-example.rs:6:13: 6:26 error: the trait bound `std::sync::mpsc::Receiver<()>: std::marker::Sync` is not satisfied [E0277]
closure-example.rs:6     let t = thread::spawn(|| {
                                 ^~~~~~~~~~~~~
closure-example.rs:6:13: 6:26 help: run `rustc --explain E0277` to see a detailed explanation
closure-example.rs:6:13: 6:26 note: `std::sync::mpsc::Receiver<()>` cannot be shared between threads safely
closure-example.rs:6:13: 6:26 note: required because of the requirements on the impl of `std::marker::Send` for `&std::sync::mpsc::Receiver<()>`
closure-example.rs:6:13: 6:26 note: required because it appears within the type `[closure@closure-example.rs:6:27: 8:6 recv:&std::sync::mpsc::Receiver<()>]`
closure-example.rs:6:13: 6:26 note: required by `std::thread::spawn`
error: aborting due to previous error

@Manishearth

@Manishearth: I'd love to see it also spell out which variable is of the non-Sync type. In this example, it's obvious, but for more complex examples, it can be tricky to figure out where you accidentally borrowed something (e.g., by using the wrong variable name).

Yeah, that's a good idea. It shouldn't be too hard to do either.

@arielb1

@arielb1 You were assigned to this issue, but I don't believe it's been implemented. Should we unassign you?

So, to spell this out:

The problem is not that "closures that must be sync" should be move -- that is overly broad and might not help anything.
Rather, the problem is:
- the closure must be Send
- it is not Send because it captures some variable "by ref"
- and that variable's type is not Sync

I think we have to be careful to not go advising move all the time when it won't help. Here are some examples where it would not help:

fn want_send<T: Send>(t: T) { }
fn want_sync<T: Sync>(t: T) { }

// move will not help because `x` is itself a reference:
fn foo(x: &Cell<T>) {
    want_send(|| x.set(1));
}

// move will not help because `Cell<T>` is not sync:
fn foo(x: Cell<T>) {
    want_sync(|| x.set(1));
}

Triage: no change.

Current output:

error[E0277]: `std::sync::mpsc::Receiver<()>` cannot be shared between threads safely
   --> file4.rs:6:13
    |
6   |     let t = thread::spawn(|| {
    |             ^^^^^^^^^^^^^ `std::sync::mpsc::Receiver<()>` cannot be shared between threads safely
    |
   ::: /Users/ekuber/workspace/rust/src/libstd/thread/mod.rs:616:8
    |
616 |     F: Send + 'static,
    |        ---- required by this bound in `std::thread::spawn`
    |
    = 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@file4.rs:6:27: 8:6 recv:&std::sync::mpsc::Receiver<()>]`

error[E0277]: `std::cell::Cell<usize>` cannot be shared between threads safely
 --> file4.rs:8:5
  |
3 | fn want_send<T: Send>(t: T) { }
  |    ---------    ---- required by this bound in `want_send`
...
8 |     want_send(|| x.set(1));
  |     ^^^^^^^^^ `std::cell::Cell<usize>` cannot be shared between threads safely
  |
  = help: within `&std::cell::Cell<usize>`, the trait `std::marker::Sync` is not implemented for `std::cell::Cell<usize>`
  = note: required because it appears within the type `&std::cell::Cell<usize>`
  = note: required because of the requirements on the impl of `std::marker::Send` for `&&std::cell::Cell<usize>`
  = note: required because it appears within the type `[closure@file4.rs:8:15: 8:26 x:&&std::cell::Cell<usize>]`

error[E0277]: `std::cell::Cell<usize>` cannot be shared between threads safely
  --> file4.rs:13:5
   |
4  | fn want_sync<T: Sync>(t: T) { }
   |    ---------    ---- required by this bound in `want_sync`
...
13 |     want_sync(|| x.set(1));
   |     ^^^^^^^^^ ----------- within this `[closure@file4.rs:13:15: 13:26 x:&std::cell::Cell<usize>]`
   |     |
   |     `std::cell::Cell<usize>` cannot be shared between threads safely
   |
   = help: within `[closure@file4.rs:13:15: 13:26 x:&std::cell::Cell<usize>]`, the trait `std::marker::Sync` is not implemented for `std::cell::Cell<usize>`
   = note: required because it appears within the type `&std::cell::Cell<usize>`
   = note: required because it appears within the type `[closure@file4.rs:13:15: 13:26 x:&std::cell::Cell<usize>]`

Navigation Menu

Repository navigation

Suggest move closures when a Sync bound is not satisfied through a closure #33307

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Suggest move closures when a Sync bound is not satisfied through a closure #33307

Description

Activity

arielb1 commented on May 1, 2016

Manishearth commented on May 1, 2016

Manishearth commented on May 1, 2016

arielb1 commented on May 1, 2016

jonhoo commented on May 3, 2016

Manishearth commented on May 3, 2016

Mark-Simulacrum commented on May 3, 2017

nikomatsakis commented on May 4, 2017

estebank commented on Feb 5, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions