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

Cannot define async fn with no_std #56974

Closed
Nemo157 opened this issue Dec 19, 2018 · 23 comments · Fixed by #69033
Closed

Cannot define async fn with no_std #56974

Nemo157 opened this issue Dec 19, 2018 · 23 comments · Fixed by #69033
Assignees
Labels
A-async-await Area: Async & Await AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-embedded Working group: Embedded systems

Comments

@Nemo157
Copy link
Member

Nemo157 commented Dec 19, 2018

#![feature(async_await, futures_api)]

#![no_std]

pub async fn foo() {
}

(playground) currently fails with

error[E0433]: failed to resolve: could not find `from_generator` in `future`

This should also work with whatever the syntax for await is, currently it fails earlier than this error because of await! only being defined in std, see #56767.

@oli-obk

This comment has been minimized.

@Centril Centril added C-bug Category: This is a bug. A-async-await Area: Async & Await labels Dec 20, 2018
@nikomatsakis

This comment has been minimized.

@nikomatsakis

This comment has been minimized.

@nikomatsakis nikomatsakis added the AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. label Mar 5, 2019
@nikomatsakis
Copy link
Contributor

Marking as not blocking stabilization -- obviously we want things to work from core eventually, but it's ok if that doesn't work to start, and I don't believe there are any major design questions at play here (right @cramertj?)

@cramertj
Copy link
Member

cramertj commented Mar 5, 2019

Yup-- this is an unfortunate restriction, but I think it's not a blocker for an MVP and resolving it requires some more complex investigation into generator arguments + some special-sauce :)

@rubdos
Copy link

rubdos commented Jul 20, 2019

Could someone indicate how difficult this would be to tackle as a new Rust contributor? I fear like this might be a bit over my head, but I'd love to know.

@cramertj
Copy link
Member

It's not easy. There needs to be a way to pass an argument into the generator resume function which can be accessed by any .await call. Neither generator arguments nor any mechanism for accessing such a bizarrely scoped local variable exist today.

@Nemo157
Copy link
Member Author

Nemo157 commented Jul 22, 2019

There is a way to implement it today using unsafe, as far as I know this proc-macro based implementation is sound (other than one non-hygienic local variable that could be an unnameable gensym in a compiler implementation). Although, I have seen one benchmark comparing this to the builtin transform and it seems to have much worse performance for some reason (but I don't know how that overhead compares to the actual operations that are going to be occurring inside the async fn).

@leo60228
Copy link
Contributor

For platforms with ELF TLS support, https://github.com/sunriseos/core-futures-tls solves this issue. It could be trivially modified for single-core platforms without ELF TLS, but multi-core ones would be much more difficult.

@phil-opp
Copy link
Contributor

See also https://internals.rust-lang.org/t/pre-rfc-generator-resume-args/10011 for a Pre-RFC for generator resume arguments.

@cramertj
Copy link
Member

cc @cavedweller

@birktj
Copy link

birktj commented Nov 10, 2019

Now that async fn is stabilized, is there any news on this?

@leo60228
Copy link
Contributor

@birktj This is fully supported in the compiler, however functions used internally by async/await are not implemented in libcore due to requiring TLS. core-futures-tls uses the unstable #[thread_local] attribute as an alternative.

@birktj
Copy link

birktj commented Nov 10, 2019

@leo60228 so if I understand correctly, all that is required to use async fn on a single-core mcu with stable rust is to copy core-futures-tls and removing the #[thread_local] attribute? (Making the context global)

@benbrittain
Copy link

The underlying implementation is depending on it being thread local, it just happens to be sufficient on a MCU that you can define a how thread_local works as a Cell. (which is what core-futures-tls does) This is currently at least a little off on being able to run on stable, the TLS dependency needs to be removed from the underlying implementation.

This won't change the API hence why async/await was stabilizable, it just happens that no_std will get a strict upgrade when this is done. (I keep planning on getting around to it, but it's non-trivial)

@Nemo157
Copy link
Member Author

Nemo157 commented Nov 11, 2019

@birktj that will not be interrupt-safe, polling a Future inside an interrupt would be a very weird thing to do, but without the context variable being #[thread_local] doing so with an async created Future would allow UB from safe code.

@jonas-schievink
Copy link
Contributor

Triage: Highly relevant for embedded folks, tagging WG-embedded.

rust-lang/rfcs#2781 proposes generator resume arguments, which would solve this issue once implemented (stabilization is not necessary).

@jonas-schievink
Copy link
Contributor

Actually we shouldn't need an RFC at all for this, generators are experimental anyways

@jonas-schievink
Copy link
Contributor

#68524 removes one of the roadblocks for this by implementing generator resume arguments

bors added a commit that referenced this issue Jan 26, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Feb 6, 2020
…guments, r=Zoxc

Generator Resume Arguments

cc rust-lang#43122 and rust-lang#56974

Blockers:
* [x] Fix miscompilation when resume argument is live across a yield point (rust-lang#68524 (comment))
* [x] Fix 10% compile time regression in `await-call-tree` benchmarks (rust-lang#68524 (comment))
  * [x] Fix remaining 1-3% regression (rust-lang#68524 (comment)) - resolved (rust-lang#68524 (comment))
* [x] Make dropck rules account for resume arguments (rust-lang#68524 (comment))

Follow-up work:
* Change async/await desugaring to make use of this feature
* Rewrite [`box_region.rs`](https://github.com/rust-lang/rust/blob/3d8778d767f0dde6fe2bc9459f21ead8e124d8cb/src/librustc_data_structures/box_region.rs) to use resume arguments (this shows up in profiles too)
@Arnavion
Copy link

Arnavion commented Feb 8, 2020

I guess what's left is to update the Future impl of GenFuture to pass the Context instead of (), update the lowering of async fn to get new cx from yield instead of TLS, and move the whole thing from libstd/... to libcore/... ? Ala https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=eea6daa5f5a3b81fd65a3786f760c084

@jonas-schievink
Copy link
Contributor

@Arnavion yep, I've started to work on that already. Expect a PR next week.

@jonas-schievink
Copy link
Contributor

Opened #69033 with the final fix for this

killercup added a commit to technocreatives/capnproto-rust that referenced this issue Feb 11, 2020
So, [currently][1], `async` fns/closures prevent crates from building in
a no_std environment. Since the only occurrence is this single `send`
method, I've moved it to the rpc crate.

[1]: rust-lang/rust#56974
killercup added a commit to technocreatives/capnproto-rust that referenced this issue Feb 11, 2020
So, [currently][1], `async` fns/closures prevent crates from building in
a no_std environment. Since the only occurrence is this single `send`
method, I've moved it to the rpc crate.

[1]: rust-lang/rust#56974
killercup added a commit to technocreatives/capnproto-rust that referenced this issue Feb 11, 2020
So, [currently][1], `async` fns/closures prevent crates from building in
a no_std environment. Since the only occurrence is this single `send`
method, I've moved it to the rpc crate.

[1]: rust-lang/rust#56974
Centril added a commit to Centril/rust that referenced this issue Mar 21, 2020
…, r=tmandry

Use generator resume arguments in the async/await lowering

This removes the TLS requirement from async/await and enables it in `#![no_std]` crates.

Closes rust-lang#56974

I'm not confident the HIR lowering is completely correct, there seem to be quite a few undocumented invariants in there. The `async-std` and tokio test suites are passing with these changes though.
@bors bors closed this as completed in ef7c8a1 Mar 21, 2020
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-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-embedded Working group: Embedded systems
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.