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.

Sign up for GitHub

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

Jump to bottom

Generic async function not_send #6947

Open
GilRtr opened this issue Mar 21, 2021 · 3 comments · May be fixed by #13590
Open

Generic async function not_send #6947

GilRtr opened this issue Mar 21, 2021 · 3 comments · May be fixed by #13590
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have L-nursery Lint: Currently in the nursery group

Comments

@GilRtr
Copy link

GilRtr commented Mar 21, 2021

Lint name: future_not_send

I tried this code:

async fn recv_msg<'b, R: AsyncRead + Unpin>(
    reader: &'_ mut R,
    buf: &'b mut Vec<u8>,
) -> io::Result<&'b mut Vec<u8>> {
    let len = reader.read_u16().await? as usize;
    buf.resize_with(len, u8::default);
    let bytes_read = reader.read_exact(buf).await?;
    debug_assert_eq!(bytes_read, len);
    Ok(buf)
}

I expected to see this happen: nothing - I think this code is correct, the returned future is not Send only if R isn't.

Instead, this happened: warning - Clippy complains that the future isn't Send because reader is used across await (by drop) and reader does not implement Send

Meta

  • cargo clippy -V: 0.0.212 (cb75ad5d 2021-02-10)
  • rustc -Vv: 
    rustc 1.50.0 (cb75ad5db 2021-02-10)
binary: rustc
commit-hash: cb75ad5db02783e8b0222fee363c5f63f7e2cf5b
commit-date: 2021-02-10
host: x86_64-pc-windows-msvc
release: 1.50.0
@GilRtr GilRtr added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Mar 21, 2021
@camsteffen
Copy link
Contributor

the returned future is not Send only if R isn't

So then shouldn't you add the constraint R: Send to ensure that the future is Send?

@GnomedDev
Copy link
Contributor

This is stopping this lint being used in https://github.com/serenity-rs/serenity because a lot of methods take impl Into<T>, and it would take a lot of unnecessary change to add + Send to all of these. There should at least be a setting to allow this lint not to trigger for generics.

@kangalio
Copy link

kangalio commented Jul 25, 2022 •
edited
Loading

the returned future is not Send only if R isn't

So then shouldn't you add the constraint R: Send to ensure that the future is Send?

That means users with a runtime that doesn't require Send (i.e. single threaded runtime) can no longer use the function, for no good reason.

I believe this lint should only lint async functions whose future is unconditionally !Send. Because library authors want to verify "does my function support Send environment", not "does my function force Send environment"

I'd also be fine with a new lint future_not_send_unconditionally. As long as a lint in that form exists at all, because it would be very useful 😇

@mkrasnitski mkrasnitski mentioned this issue Jul 25, 2022
Merged
@jplatte jplatte mentioned this issue Aug 25, 2022
Open
@oddgrd oddgrd mentioned this issue Jul 31, 2023
Merged
@jplatte jplatte mentioned this issue Feb 2, 2024
Closed
@J-ZhengLi J-ZhengLi added the L-nursery Lint: Currently in the nursery group label Apr 25, 2024
@github-actions github-actions bot mentioned this issue Sep 14, 2024
Merged
@y21 y21 linked a pull request Oct 23, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have L-nursery Lint: Currently in the nursery group
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Allow conditional Send futures in future_not_send y21/rust-clippy
5 participants
@camsteffen @kangalio @J-ZhengLi @GnomedDev @GilRtr