Reword E0044 and message for `!Send` types #48138

estebank · 2018-02-11T05:07:09Z

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

rust-highfive · 2018-02-11T05:07:14Z

(rust_highfive has picked a reviewer for you, use r? to override)

estebank · 2018-02-12T05:18:38Z

CC @nikomatsakis: could you check the wording of the messages as well as the applicability across cases?

The special label for channel::{Reciever, Sender} should become a note instead once note support for rustc_on_unimplemented lands in beta.

bors · 2018-02-12T12:34:52Z

☔ The latest upstream changes (presumably #47843) made this pull request unmergeable. Please resolve the merge conflicts.

nikomatsakis · 2018-02-12T20:13:36Z

src/libcore/marker.rs

+#[rustc_on_unimplemented(
+    on(
+        _Self="std::sync::mpsc::Receiver<T>",
+        label="`{Self}` cannot be shared safely, if using a closure consider marking it `move`"


this seems fine; it'd be nice if we could be more selective about giving the hint (e.g., if we could test that the "backtrace" contains a closure). I worry that the wording is a bit vague, since what it refers to could be either the closure or the Receiver. Perhaps "consider marking the closure move".

shepmaster · 2018-02-17T16:06:19Z

Ping from triage, @bluss — will you have some time to review this PR soon?

nikomatsakis · 2018-02-19T17:13:23Z

r? @nikomatsakis

nikomatsakis · 2018-02-20T21:58:47Z

src/libcore/marker.rs

+#[rustc_on_unimplemented(
+    on(
+        _Self="std::sync::mpsc::Receiver<T>",
+        label="`{Self}` cannot be shared safely, consider marking the closure `move`"


I forget if I said this before -- but we don't know that a closure is involved, do we? It seems odd to write "the closure" here.

We don't, but it does seem to be the most common case that happens in the wild.

nikomatsakis · 2018-02-20T21:59:14Z

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]


I definitely prefer this wording =)

nikomatsakis · 2018-02-20T22:04:49Z

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

+  --> $DIR/closure-move-sync.rs:16:13
+   |
+16 |     let t = thread::spawn(|| {
+   |             ^^^^^^^^^^^^^ `std::sync::mpsc::Receiver<()>` cannot be shared safely, consider marking the closure `move`


the message happens to work here, but I imagine you could also have a scenario like this

fn gimme<T: Send>(_: T) { } fn main() { let (send, _recv) = ::std::sync::mpsc::channel(); gimme(&send); }

What gets printed here?

You'll get the label mentioning closures. That's why the original label was more ambiguous. I'll have to expand rustc_on_unimplemented to be able to tell if a closure is involved in order to a message that is always correct.

I think that I'd prefer to merge this PR with the previous ambiguous wording ("std::sync::mpsc::Receiver<()> cannot be shared safely, if using a closure consider marking it move") and work on a follow up to improve that. I'm also waiting on #47613 to land in beta so that this message can be part of a note instead of the label.

I could live with an ambiguous message for now, I suppose.

Could we file a follow-up bug though describing the ambiguous message and thoughts for improvement? (And tag the message with // FIXME and the issue number...)

Filed #48534.

bors · 2018-02-27T03:16:10Z

☔ The latest upstream changes (presumably #48449) made this pull request unmergeable. Please resolve the merge conflicts.

nikomatsakis · 2018-03-12T17:05:39Z

@Zoxc hmm -- it's not a bad idea to include Sync, I agree, but I can't think of a nice wording. Any thoughts?

(Note that the help message does mention Sync explicitly, so the info is still there, though you have to look a bit harder.)

nikomatsakis · 2018-03-13T18:47:52Z

Grumpy travis:

https://travis-ci.org/rust-lang/rust/builds/352438805#L2500

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

…ender}` Extend `rustc_on_unimplemented` to match on ADT without evaluating type arguments.

estebank · 2018-03-15T03:54:44Z

@bors r=nikomatsakis

(@nikomatsakis based on #48138 (review))

bors · 2018-03-15T03:54:45Z

📌 Commit 1bbd4fd has been approved by nikomatsakis

bors · 2018-03-15T05:17:51Z

⌛ Testing commit 1bbd4fd with merge dda6f9b8c96840cff66a23c7bc0f61626b7c33a7...

bors · 2018-03-15T08:18:54Z

💔 Test failed - status-appveyor

pietroalbini · 2018-03-15T10:10:56Z

AppVeyor timed out.

@bors retry

bors · 2018-03-15T13:16:15Z

⌛ Testing commit 1bbd4fd with merge 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.

alexcrichton · 2018-03-15T14:12:39Z

For the previous failure:

[00:01:35] Cloning into '/c/cache/rustsrc/src/src/doc/rust-by-example'...
[02:26:08] error: RPC failed; curl 18 transfer closed with outstanding read data remaining
[02:26:08] fatal: The remote end hung up unexpectedly
[02:26:08] fatal: early EOF
[02:26:08] fatal: index-pack failed
[02:26:09] fatal: clone of 'https://github.com/rust-lang/rust-by-example' into submodule path '/c/cache/rustsrc/src/src/doc/rust-by-example' failed

It took two hours to figure out cloning that repo failed 😱

bors · 2018-03-15T16:00:35Z

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing ff2d506 to master...

rust-highfive assigned bluss Feb 11, 2018

estebank force-pushed the issue-45092 branch from 08ad027 to 2fdd7a2 Compare February 11, 2018 05:48

pietroalbini added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 11, 2018

estebank force-pushed the issue-45092 branch 3 times, most recently from 555a9f6 to d6b6c3a Compare February 12, 2018 05:17

estebank force-pushed the issue-45092 branch from d6b6c3a to 611f1f7 Compare February 12, 2018 06:34

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 12, 2018

estebank force-pushed the issue-45092 branch from 611f1f7 to 207e0c4 Compare February 12, 2018 16:23

estebank added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 12, 2018

nikomatsakis reviewed Feb 12, 2018

View reviewed changes

rust-highfive assigned nikomatsakis and unassigned bluss Feb 19, 2018

nikomatsakis requested changes Feb 20, 2018

View reviewed changes

nikomatsakis added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 20, 2018

estebank added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 26, 2018

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 27, 2018

estebank force-pushed the issue-45092 branch from 730d847 to 709baac Compare March 1, 2018 20:35

pietroalbini added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 5, 2018

nikomatsakis added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 13, 2018

estebank added 4 commits March 14, 2018 18:04

Reword E0044 and message for !Send types

6d8a173

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

Suggest using move when trying to share `...::channel::{Receiver, S…

fe19754

…ender}` Extend `rustc_on_unimplemented` to match on ADT without evaluating type arguments.

Make hint clearer, with the potential of being wrong

cb5667e

Review comment: remove mention of move closure

bfc66da

estebank force-pushed the issue-45092 branch from 92eca85 to bfc66da Compare March 15, 2018 01:06

Add span label to E0044

1bbd4fd

estebank force-pushed the issue-45092 branch from 3884e55 to 1bbd4fd Compare March 15, 2018 02:14

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 15, 2018

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 15, 2018

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 15, 2018

bors merged commit 1bbd4fd into rust-lang:master Mar 15, 2018

alexcrichton mentioned this pull request Jul 4, 2018

panic code is not deterministically lto'ed #52044

Closed

iluuu1994 mentioned this pull request Jul 25, 2019

rustc recommends specialization for type parameters inside an extern block #45092

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reword E0044 and message for `!Send` types #48138

Reword E0044 and message for `!Send` types #48138

estebank commented Feb 11, 2018 •

edited

Loading

rust-highfive commented Feb 11, 2018

estebank commented Feb 12, 2018 •

edited

Loading

bors commented Feb 12, 2018

nikomatsakis Feb 12, 2018

shepmaster commented Feb 17, 2018

nikomatsakis commented Feb 19, 2018

nikomatsakis Feb 20, 2018

estebank Feb 20, 2018

nikomatsakis Feb 20, 2018

nikomatsakis Feb 20, 2018

estebank Feb 20, 2018

nikomatsakis Feb 21, 2018

nikomatsakis Feb 21, 2018

estebank Feb 25, 2018

estebank Feb 26, 2018

bors commented Feb 27, 2018

nikomatsakis commented Mar 12, 2018

nikomatsakis commented Mar 13, 2018

estebank commented Mar 15, 2018

bors commented Mar 15, 2018

bors commented Mar 15, 2018

bors commented Mar 15, 2018

pietroalbini commented Mar 15, 2018

bors commented Mar 15, 2018

alexcrichton commented Mar 15, 2018

bors commented Mar 15, 2018

Reword E0044 and message for !Send types #48138

Reword E0044 and message for !Send types #48138

Conversation

estebank commented Feb 11, 2018 • edited Loading

rust-highfive commented Feb 11, 2018

estebank commented Feb 12, 2018 • edited Loading

bors commented Feb 12, 2018

Choose a reason for hiding this comment

shepmaster commented Feb 17, 2018

nikomatsakis commented Feb 19, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bors commented Feb 27, 2018

nikomatsakis commented Mar 12, 2018

nikomatsakis commented Mar 13, 2018

estebank commented Mar 15, 2018

bors commented Mar 15, 2018

bors commented Mar 15, 2018

bors commented Mar 15, 2018

pietroalbini commented Mar 15, 2018

bors commented Mar 15, 2018

alexcrichton commented Mar 15, 2018

bors commented Mar 15, 2018

Reword E0044 and message for `!Send` types #48138

Reword E0044 and message for `!Send` types #48138

estebank commented Feb 11, 2018 •

edited

Loading

estebank commented Feb 12, 2018 •

edited

Loading