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

Manishearth · 2016-05-01T11:30:32Z

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

The text was updated successfully, but these errors were encountered:

arielb1 · 2016-05-01T13:24:43Z

Why isn't the

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

impl on the stack?

Manishearth · 2016-05-01T13:35:49Z

Not sure, really.

Manishearth · 2016-05-01T14:25:47Z

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.

arielb1 · 2016-05-01T14:46:32Z

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

jonhoo · 2016-05-03T15:29:19Z

@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).

Manishearth · 2016-05-03T17:18:51Z

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

Mark-Simulacrum · 2017-05-03T22:08:05Z

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

nikomatsakis · 2017-05-04T22:53:19Z

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));
}

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.

estebank · 2020-02-05T00:35:08Z

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>]`

Manishearth added the A-diagnostics Area: Messages for errors, warnings, and lints label May 1, 2016

Manishearth assigned arielb1 May 1, 2016

Mark-Simulacrum added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jul 25, 2017

estebank mentioned this issue Feb 11, 2018

Reword E0044 and message for !Send types #48138

Merged

estebank mentioned this issue Feb 25, 2018

Extend rustc_on_unimplemented to query for closures #48534

Open

bors added a commit that referenced this issue Mar 15, 2018

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

ff2d506

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.

estebank added A-closures Area: closures (`|args| { .. }`) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 5, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

Manishearth commented May 1, 2016

arielb1 commented May 1, 2016 •

edited

Loading

Manishearth commented May 1, 2016

Manishearth commented May 1, 2016

arielb1 commented May 1, 2016

jonhoo commented May 3, 2016

Manishearth commented May 3, 2016

Mark-Simulacrum commented May 3, 2017

nikomatsakis commented May 4, 2017

estebank commented Feb 5, 2020

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

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

Comments

Manishearth commented May 1, 2016

arielb1 commented May 1, 2016 • edited Loading

Manishearth commented May 1, 2016

Manishearth commented May 1, 2016

arielb1 commented May 1, 2016

jonhoo commented May 3, 2016

Manishearth commented May 3, 2016

Mark-Simulacrum commented May 3, 2017

nikomatsakis commented May 4, 2017

estebank commented Feb 5, 2020

arielb1 commented May 1, 2016 •

edited

Loading