[WIP] async functions #50850
[WIP] async functions #50850
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
|
|
rust-highfive
added
the
S-waiting-on-review
withoutboats
added
S-waiting-on-author
|
So is the plan to have |
|
@eddyb I figured we'd desugar to generators as early as possible at least for the MVP, since that will likely get us results the fastest. @alexcrichton and @aturon even suggested adding a temporary lang item to do TLS so we don't have to worry about threading the context arg through. |
|
r? @cramertj who knows more about this than me. |
|
@eddyb I'd like to desugar them to after lifetime elisions, because the return type is |
|
You can use a custom visitor like the one we use to collect lifetimes for |
|
Talked with @eddyb and @cramertj and my current understanding of the plan for handling the return type is to transform it into Then there's the matter of the body. Talking to @eddyb, it seems like we will probably translate everything to
Then there's the question of how to handle async blocks and await, which are essentially sugar for generators yielding |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Hmm, I thought (I haven't thought about the WF bit until now and I think this is one of the best reasons to not try and guess the set of lifetimes at any point before typeck). |
This didn't work in the macro based implementation, but I believe, looking at the code, that the limitation is entirely in parsing, so yea we don't need to manufacture a lifetime, just collect all of the resolved input lifetimes. |
|
Talked with eddyb more and came up with this (hacky) solution that works, pretty mcuh the elaboration of cramertj's & eddyb's original vision:
Then we just piggy back the generator code the rest of the way down. We'll need a pass to make sure that |
|
HIR lowering of the return types is now implemented, as well as this "future" API (pending further movement on the lib RFC): trait Future { }
struct GenFuture<G, R> {
gen: G,
marker: PhantomData<R>,
}
impl<G: Generator<Yield = ()>> Future for GenFuture<G, G::Return> { }
fn gen_future<G: Generator>(gen: G) -> GenFuture<G, G::Return> { /* ... */ } |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
src/libcore/raw.rs
Outdated
| /// depend on it. | ||
| #[allow(missing_debug_implementations, dead_code)] | ||
| #[unstable(feature = "gen_future", issue = "50547")] | ||
| pub struct GenFuture<G, R> { |
There was a problem hiding this comment.
If you add a bound on G that mentions R, I think you can both get rid of the PhantomData, and force the type-checker to infer the return type from the type you put in R, from GenFuture alone, although gen_future below is probably enough either way.
There was a problem hiding this comment.
I didn't know you could get rid of PhantomData this way. If that works I'll just put the bound here, make the field public, and get rid of the constructor.
|
|
||
| struct NonAsyncRet; | ||
| struct AsyncClosureRet; | ||
| struct AsyncFunctionRet; |
There was a problem hiding this comment.
Is there any particular reason for not using an enum here?
There was a problem hiding this comment.
I watched a talk at a conference about how great dependency injection is right before I wrot this code :) (the talk was by Sandi Metz)
I don't mind switching to a method on an enum which then delegates to three other methods, but it introduces the opportunity for irregularity (adding an argument that only applies to 1 case, for example, which I think is usually suspicious). I'd love if Rust made it more natural to write this pattern.
There was a problem hiding this comment.
the main thing I don't want to do is pass an enum down and then write the match inside of this function. I think the proliferation of that pattern in lowering has made the code more difficult to understand (but I like OO, so YMMV).
There was a problem hiding this comment.
Our mileage does vary - enum represent finite state, whereas a trait introduces an open-world source of ambiguity - every single time there's a trait object involved, all the impls must be found to reason about anything at all.
I can's speak for all of @rust-lang/compiler, but I personally wouldn't use a trait unless the implementers are used in unrelated situations, but they can reuse some common abstraction through the trait (e.g. visitor/folder pattern) - in other words, bridging gaps in an open-world.
And even then I'd almost never employ a trait object (or fn pointers for that matter) except for bypassing polymorphic recursion (typically needed when a recursive function takes a closure) or when polymorphism is somehow prohibited (e.g. the query engine, which does dependency injection with fn pointers, or the codegen backend, which is dynamically loaded nowadays for LLVM reasons). There's also the oddball case of "half of this crate would be monomorphized if you used generics" but AFAIK that's rare.
In this case, all the implementors are in the same location, they're not recursive data types (or any data at all, but most data wouldn't be relevant), they're created from similar match-es on asyncness, and the sole method of the trait has exactly one caller.
I don't mind switching to a method on an enum which then delegates to three other methods, but it introduces the opportunity for irregularity (adding an argument that only applies to 1 case, for example, which I think is usually suspicious).
I don't understand this - are you referring to an argument that would come from a field of the structs (which are currently all units)? Or from the caller of lower? Because in the latter case I don't know how that'd change anything, and in the former you just move all the struct fields to the respective enum variant.
| let LoweredNodeId { node_id, hir_id } = ctx.next_id(); | ||
| let gen_future_path = hir::Path { | ||
| segments: segments.map_slice(|mut v| { | ||
| v.last_mut().unwrap().parameters = Some(P(hir::PathParameters { |
There was a problem hiding this comment.
Maybe add an Option<P<hir::PathParameters>> argument to std_path instead?
There was a problem hiding this comment.
And then perform this operation there, or is there way to do this eventually without the map_sice if I keep passing the argument down? std_path delegates to something else and I stopped investigating how it worked at that point.
There was a problem hiding this comment.
It constructs a hir::Path at some point and map_slice should be avoidable, yes.
There was a problem hiding this comment.
It's in the implementation of resolve_str_path, turns out:
rust/src/librustc_resolve/lib.rs
Line 1507 in d0456c6
I don't know why the path is constructed there instead of its sole caller in hir::lowering, but @petrochenkov might.
|
☔ The latest upstream changes (presumably #50930) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Now also contains code to lower the bodies of async items to generators |
This is gated on edition 2018 & the `async_await` feature gate. The parser will accept `async fn` and `async unsafe fn` as fn items. Along the same lines as `const fn`, only `async unsafe fn` is permitted, not `unsafe async fn`.The parser will not accept `async` functions as trait methods. To do a little code clean up, four fields of the function type struct have been merged into the new `FnHeader` struct: constness, asyncness, unsafety, and ABI. Also, a small bug in HIR printing is fixed: it previously printed `const unsafe fn` as `unsafe const fn`, which is grammatically incorrect.
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
☔ The latest upstream changes (presumably #51082) made this pull request unmergeable. Please resolve the merge conflicts. |
| /// depend on it. | ||
| #[allow(missing_debug_implementations, dead_code)] | ||
| #[unstable(feature = "gen_future", issue = "50547")] | ||
| pub struct GenFuture<G: ::ops::Generator<Yield = (), Return = R>, R>(pub G); |
|
@withoutboats Your first commit (accidentally?) modifies submodules. You need to rebase those changes out (I'd use |
|
@cramertj is going to take over this work building off this branch |
|
@withoutboats should we close this in the meantime? |
This PR implements async functions under the
async_awaitfeature flag.Checklist:
async fnasync struct Foo;.async fnand async closures to returning their outer return type.async fnreturn type after lifetime elisionCurrently built off of #50307, will be rebased once that has been merged.