Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

suggestion that &T requires Sync to be Send could be clearer #86507

Closed
nikomatsakis opened this issue Jun 21, 2021 · 2 comments · Fixed by #87322
Closed

suggestion that &T requires Sync to be Send could be clearer #86507

nikomatsakis opened this issue Jun 21, 2021 · 2 comments · Fixed by #87322
Labels
A-async-await Area: Async & Await A-diagnostics Area: Messages for errors, warnings, and lints AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-enhancement Category: An issue proposing an enhancement or a PR with one. D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

Given the following code: playground

use async_trait::async_trait; // 0.1.50
use serde::Serialize;

#[async_trait]
trait Foo {
    async fn bar(x: &(impl Serialize + Send));
}

#[async_trait]
impl Foo for () {
    async fn bar(x: &(impl Serialize + Send)) {
    }
}

fn main() { }

The current output is:

error: future cannot be sent between threads safely
  --> src/main.rs:11:47
   |
11 |       async fn bar(x: &(impl Serialize + Send)) {
   |  _______________________________________________^
12 | |     }
   | |_____^ future created by async block is not `Send`
   |
note: captured value is not `Send`
  --> src/main.rs:11:18
   |
11 |     async fn bar(x: &(impl Serialize + Send)) {
   |                  ^ has type `&impl Serialize + Send` which is not `Send`
   = note: required for the cast to the object type `dyn Future<Output = ()> + Send`
help: consider further restricting this bound
   |
11 |     async fn bar(x: &(impl Serialize + Send + std::marker::Sync)) {
   |                                             ^^^^^^^^^^^^^^^^^^^

The compiler helpfully points out that Sync is required, but nonetheless @chazkiker2 and I both got confused by this error for some time before we noticed it. The suggestion was kind of buried by all the other details.

@nikomatsakis nikomatsakis added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 21, 2021
@nikomatsakis
Copy link
Contributor Author

I'm not sure the ideal output, but it might be something like this:

error: future cannot be sent between threads safely
  --> src/main.rs:11:47
   |
11 |       async fn bar(x: &(impl Serialize + Send)) {
   |  _______________________________________________^
12 | |     }
   | |_____^ future created by async block is not `Send`
   |
note: `&` references cannot be sent unless their referent is `Sync`
  --> src/main.rs:11:18
   |
11 |     async fn bar(x: &(impl Serialize + Send)) {
   |                  ^ refers to `impl Serialize + Send` which is not `Sync`
   = note: required for the cast to the object type `dyn Future<Output = ()> + Send`
help: consider further restricting this bound
   |
11 |     async fn bar(x: &(impl Serialize + Send + std::marker::Sync)) {
   |                                             ^^^^^^^^^^^^^^^^^^^

@nikomatsakis nikomatsakis added the A-async-await Area: Async & Await label Jun 21, 2021
@JohnTitor JohnTitor added C-enhancement Category: An issue proposing an enhancement or a PR with one. D-confusing Diagnostics: Confusing error or lint that should be reworked. labels Jun 21, 2021
@tmandry tmandry added the AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. label Jun 25, 2021
@chazkiker2
Copy link
Contributor

This error can be replicated without any external crates (i.e., without #[async_trait]) like so:

Link to Playground

use ::core::pin::Pin;
use ::core::future::Future;
use ::core::marker::Send;

trait Foo {
    fn bar<'me, 'async_trait, T: Send>(x: &'me T)
        -> Pin<Box<dyn Future<Output = ()> + Send + 'async_trait>>
        where 'me: 'async_trait;
}

impl Foo for () {
    fn bar<'me, 'async_trait, T: Send>(x: &'me T)
        -> Pin<Box<dyn Future<Output = ()> + Send + 'async_trait>>
        where 'me: 'async_trait {
            Box::pin(
                async move {
                    let x = x;
                }
            )
         }
}

fn main() { }

JohnTitor added a commit to JohnTitor/rust that referenced this issue Jul 23, 2021
…send, r=estebank

fix: clarify suggestion that `&T` must refer to `T: Sync` for `&T: Send`

### Description

- [x] fix rust-lang#86507
- [x] add UI test for relevant code from issue
- [x] change `rustc_trait_selection/src/traits/error_reporting/suggestions.rs` to include a more clear suggestion when `&T` fails to satisfy `Send` bounds due to the fact that `T` fails to implement `Sync`
@bors bors closed this as completed in 3fc79fd Jul 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-async-await Area: Async & Await A-diagnostics Area: Messages for errors, warnings, and lints AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-enhancement Category: An issue proposing an enhancement or a PR with one. D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants