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

Make async futures Unpin if they don't borrow anything across .awaits #82187

Open
jplatte opened this issue Feb 16, 2021 · 14 comments
Open

Make async futures Unpin if they don't borrow anything across .awaits #82187

jplatte opened this issue Feb 16, 2021 · 14 comments
Labels
A-async-await Area: Async & Await AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-feature-request Category: A feature request, i.e: not implemented / a PR.

Comments

@jplatte
Copy link
Contributor

jplatte commented Feb 16, 2021

Am I missing something or could rustc in principle make the anonymous (return) type of async fn and async {} blocks implement Unpin (or maybe it should be "not implement !Unpin") if the body does not contain any borrows across .await points? I couldn't find an existing issue about this.

@JakobDegen
Copy link
Contributor

This seems like it would cause problems regarding accidental breaking changes. Changing a function to now borrow something across an await point is an incredibly easy change to accidentally make and would result in the Unpin impl being removed from the return type.

An alternative would be to make it possible to explicitly mark that an async function/block should be Unpin (removing the possibility of an accidental breaking change). I don't see how to do this in function signatures without introducing an additional keyword though (pub unpin async fn or such) and that's probably not worth the effort.

@jplatte
Copy link
Contributor Author

jplatte commented Feb 16, 2021

@JakobDegen I don't see how Unpin would be different to Send and Sync in this regard.

@ghost
Copy link

ghost commented Feb 16, 2021

How about doing this only for async blocks? async fns will remain returning impl Future + !Unpin, and function authors that want to make it Unpin can specify the return type as impl Future + Unpin manually. I think this would not easily cause accidental breaking changes?

@jplatte
Copy link
Contributor Author

jplatte commented Feb 16, 2021

function authors that want to make it Unpin can specify the return type as impl Future + Unpin manually

You mean by making it a regular fn and wrapping the entire body in async move {}? I think a similar strategy could be used to create an attribute macro that asserts Send / Sync / Unpin for async fn (not sure about async blocks, can proc-macro attrs be used on expressions?), but doing it manually would be super annoying.

@ghost
Copy link

ghost commented Feb 16, 2021

You mean by making it a regular fn and wrapping the entire body in async move {}? That would be pretty annoying. I think a similar strategy could be used to create an attribute macro that asserts Send / Sync / Unpin for async fn (not sure about async blocks, can proc-macro attrs be used on expressions?), but doing it manually would be super annoying.

Yes, and I agree that could be annoying (sorry for not thinking of this point before commenting, stability itself is annoying)...
An attribute macro is also a good (maybe even better) idea (FYI attribute macros on expressions are currently nightly-only, tracking at #15701).

@jplatte
Copy link
Contributor Author

jplatte commented Feb 16, 2021

I guess I'll create that then 😄

@jplatte
Copy link
Contributor Author

jplatte commented Feb 16, 2021

The amount of code required is smaller than I expected. I've hardly tested it, but I think it should work: https://docs.rs/async_auto_traits/0.1

@ghost
Copy link

ghost commented Feb 16, 2021

(Off-topic content removed)

It turns out that my suggestion has a breaking change hole because impl Future does not hide auto traits: if the function author forgot to + Unpin, it's still easy to cause accidental breaking changes. I didn't consider that point carefully before commenting. Sorry about that!

@jplatte

This comment has been minimized.

@ghost

This comment has been minimized.

@jplatte
Copy link
Contributor Author

jplatte commented Feb 17, 2021

Okay, let's move this discussion to an issue on https://github.com/jplatte/async_auto_traits.

@hyd-dev I suggest hiding your previous two comments here.

@csmoe csmoe added the A-async-await Area: Async & Await label Feb 18, 2021
@tmandry tmandry added the AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. label Feb 19, 2021
@Dylan-DPC
Copy link
Member

Closing this as it's better tracked in the jplatte/async_auto_traits#1 issue, and it is better to have it contained in one issue instead of having 2 issues tracking it in different places

@Dylan-DPC Dylan-DPC closed this as not planned Won't fix, can't repro, duplicate, stale Jan 15, 2024
@jplatte
Copy link
Contributor Author

jplatte commented Jan 15, 2024

@Dylan-DPC I think you misunderstood something, that is about preventing the leak of auto-traits of async fn future types. It only relates to Unpin in that it would sometime be useful to be able to stop leaking an Unpin implementation in a public interface if the compiler generated those impls at all (which this issue is about).

@djc
Copy link
Contributor

djc commented Sep 17, 2024

I was experimenting today with replacing the likes of futures_util::future::ok() (which is IMO a little ugly -- it reeks of the futures 0.1 times) with async move {} blocks and was surprised that I needed an explicit pin!() call wrapped around the async move {} block even with trivial Copy types (like usize).

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 AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-feature-request Category: A feature request, i.e: not implemented / a PR.
Projects
None yet
Development

No branches or pull requests

7 participants