Suggest move closures when a Sync bound is not satisfied through a closure
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();
}
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 Timpl on the stack?
Ariel Ben-Yehuda at 2016-05-01 13:24:43
Not sure, really.
Manish Goregaokar at 2016-05-01 13:35:49
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.
Manish Goregaokar at 2016-05-01 14:25:47
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 errorAriel Ben-Yehuda at 2016-05-01 14:46:32
@Manishearth: I'd love to see it also spell out which variable is of the non-
Synctype. 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).Jon Gjengset at 2016-05-03 15:29:19
Yeah, that's a good idea. It shouldn't be too hard to do either.
Manish Goregaokar at 2016-05-03 17:18:51
@arielb1 You were assigned to this issue, but I don't believe it's been implemented. Should we unassign you?
Mark Rousskov at 2017-05-03 22:08:05
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
Sendbecause it captures some variable "by ref" - and that variable's type is not
Sync
- the closure must be
I think we have to be careful to not go advising
moveall 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)); }Niko Matsakis at 2017-05-04 22:53:19
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>]`Esteban Kuber at 2020-02-05 00:35:08