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

Do not require Sync on unused shared reference arguments #232

Closed
dtolnay opened this issue Jan 22, 2023 · 0 comments · Fixed by #233
Closed

Do not require Sync on unused shared reference arguments #232

dtolnay opened this issue Jan 22, 2023 · 0 comments · Fixed by #233

Comments

@dtolnay
Copy link
Owner

dtolnay commented Jan 22, 2023

use async_trait::async_trait;

#[async_trait]
trait Generic<T> {
    async fn takes_ref(&self, thing: &T);
}

#[async_trait]
impl<T> Generic<T> for () {
    async fn takes_ref(&self, _: &T) {}
}

This currently fails because it expands to something like:

impl<T> Generic<T> for () {
    fn takes_ref<'life0, 'life1, 'async_trait>(
        &'life0 self,
        __arg1: &'life1 T,
    ) -> Pin<Box<dyn Future<Output = ()> + Send + 'async_trait>>
    where
        'life0: 'async_trait,
        'life1: 'async_trait,
        Self: 'async_trait,
    {
        Box::pin(async move {
            let __self = self;
            let __arg1 = __arg1;
            let _: () = {};
        })
    }
}

in which the anonymous future type captures a local variable of type &T, implying a need for T: Sync in order to get a Send future.

error: future cannot be sent between threads safely
  --> src/main.rs:10:38
   |
10 |     async fn takes_ref(&self, _: &T) {}
   |                                      ^^ future created by async block is not `Send`
   |
note: captured value is not `Send` because `&` references cannot be sent unless their referent is `Sync`
  --> src/main.rs:10:31
   |
8  | #[async_trait]
   | -------------- in this procedural macro expansion
9  | impl<T> Generic<T> for () {
10 |     async fn takes_ref(&self, _: &T) {}
   |                               ^ has type `&T` which is not `Send`, because `T` is not `Sync`
   = note: required for the cast from `[async block@src/main.rs:10:38: 10:40]` to the object type `dyn Future<Output = ()> + Send`
   = note: this error originates in the attribute macro `async_trait` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider restricting type parameter `T`
   |
9  | impl<T: std::marker::Sync> Generic<T> for () {
   |       +++++++++++++++++++

The following compiles:

#[async_trait]
impl<T: Sync> Generic<T> for () {
    async fn takes_ref(&self, _: &T) {}
}

However that T: Sync bound should be unnecessary as long as the &T argument is unused within the future.

crapStone added a commit to Calciumdibromid/CaBr2 that referenced this issue Feb 1, 2023
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [async-trait](https://github.com/dtolnay/async-trait) | dependencies | patch | `0.1.62` -> `0.1.64` |

---

### Release Notes

<details>
<summary>dtolnay/async-trait</summary>

### [`v0.1.64`](https://github.com/dtolnay/async-trait/releases/tag/0.1.64)

[Compare Source](dtolnay/async-trait@0.1.63...0.1.64)

-   Suppress async_yields_async clippy correctness lint in generated code ([#&#8203;236](dtolnay/async-trait#236), [#&#8203;237](dtolnay/async-trait#237))

### [`v0.1.63`](https://github.com/dtolnay/async-trait/releases/tag/0.1.63)

[Compare Source](dtolnay/async-trait@0.1.62...0.1.63)

-   Do not require Sync on unused shared reference arguments ([#&#8203;232](dtolnay/async-trait#232), [#&#8203;233](dtolnay/async-trait#233))
-   Make expansion of nested `_` and `..` patterns edition independent ([#&#8203;234](dtolnay/async-trait#234), [#&#8203;235](dtolnay/async-trait#235))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4xMTQuMSIsInVwZGF0ZWRJblZlciI6IjM0LjExNi4xIn0=-->

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Co-authored-by: crapStone <crapstone01@gmail.com>
Co-authored-by: crapStone <crapstone@noreply.codeberg.org>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1750
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant