async/await #51580
async/await #51580
Conversation
rust-highfive
added
the
S-waiting-on-review
|
Why are My inclination would be to try to make the feature gating as pleasant as possible for users; I'd prefer merging both compiler features into #. |
| | | | ||
| | first lifetime here | ||
| | | ||
| = help: `async fn` can only accept borrowed values identical lifetimes |
There was a problem hiding this comment.
async fn can only accept borrowed values with identical lifetimes (<-- added "with")
src/libsyntax/parse/parser.rs
Outdated
| @@ -6755,6 +6807,28 @@ impl<'a> Parser<'a> { | |||
| maybe_append(attrs, extra_attrs)); | |||
| return Ok(Some(item)); | |||
| } | |||
| if self.check_keyword(keywords::Async) && | |||
| (self.look_ahead(1, |t| t.is_keyword(keywords::Fn)) || | |||
| self.look_ahead(1, |t| t.is_keyword(keywords::Unsafe))) | |||
There was a problem hiding this comment.
The order a lot of people seemed to agree on in the RFC discussion was different. rust-lang/rfcs#2394 (comment)
"const unsafe async move": unsafe was in front of async back then (of course move doesn't apply here) To me unsafe in front of async seems more "natural" because unsafety should be featured more prominently.
|
I notice this is still using TLS for passing the context in to the generated futures. Has resurrecting generator arguments for this been considered? (big reason I want this is to easily support |
|
@Nemo157 as far as I know, we definitely want to stop using TLS in the long run (before we stabilize anything), and the current use in this PR is a hack to get the next iteration of async/await up and running. |
|
@Nemo157 @withoutboats is absolutely correct-- we definitely want to support no-TLS and no-std |
|
One thing that occurred to me: |
|
@cramertj why would we want to experiment on the 2015 edition over the 2018 edition (since both are available on nightly)? In the final stable version I think I'd like to make things consistent by not support any async and await features on 2015 (so that the story is very simple, instead of having to remember which half of the features are available on 2015). But if its expedient to support some things on 2015 in this iteration, that seems fine. |
Because it would allow for specifically testing out async/await without having to change the rest of the project over to the 2018 edition. |
Currently standard library features must be gated separately from language features. This is also why |
|
@cramertj I didn't see it was literally implemented as a macro. We don't intend to stabilize it with the current syntax anyway, do you object to using the |
Hm... it seems sort of surprising to me that |
This comment has been minimized.
This comment has been minimized.
| @@ -213,6 +213,26 @@ macro_rules! eprintln { | |||
| ($fmt:expr, $($arg:tt)*) => (eprint!(concat!($fmt, "\n"), $($arg)*)); | |||
| } | |||
|
|
|||
| #[macro_export] | |||
| #[unstable(feature = "await_macro", issue = "50547")] | |||
| #[allow_internal_unstable] | |||
There was a problem hiding this comment.
This should probably use #[allow_internal_unsafe] as well.
src/libsyntax/ptr.rs
Outdated
| @@ -95,6 +95,16 @@ impl<T: 'static> P<T> { | |||
| } | |||
| } | |||
|
|
|||
| impl<T: 'static> P<[T]> { | |||
| pub fn map_slice<F>(self, f: F) -> P<[T]> where | |||
| F: FnOnce(Vec<T>) -> Vec<T> | |||
There was a problem hiding this comment.
This looks similar to existing move_map/move_flat_map.
There was a problem hiding this comment.
It is similar, but move_map and move_flat_map expect to receive one element at a time, whereas this takes the whole list at once, allowing it to do things like modifying the last element.
There was a problem hiding this comment.
Should be able to remove this now.
src/libsyntax/parse/parser.rs
Outdated
| @@ -2246,6 +2258,15 @@ impl<'a> Parser<'a> { | |||
| hi = path.span; | |||
| return Ok(self.mk_expr(lo.to(hi), ExprKind::Path(Some(qself), path), attrs)); | |||
| } | |||
| if syntax_pos::hygiene::default_edition() >= Edition::Edition2018 && | |||
There was a problem hiding this comment.
You probably want self.token.span.edition() >= Edition::Edition2018 here instead of default edition in case async comes from a macro defined with other edition.
There was a problem hiding this comment.
Do you mean self.span.edition()? Token doesn't have a span field.
There was a problem hiding this comment.
Yes, sorry (self.span is the span of self.token).
src/libsyntax/parse/parser.rs
Outdated
| @@ -3240,6 +3261,13 @@ impl<'a> Parser<'a> { | |||
| } else { | |||
| Movability::Movable | |||
| }; | |||
| let asyncness = if syntax_pos::hygiene::default_edition() >= Edition::Edition2018 | |||
There was a problem hiding this comment.
| IsAsync::Async(ast::DUMMY_NODE_ID) | ||
| } else { | ||
| IsAsync::NotAsync | ||
| }; |
There was a problem hiding this comment.
So it's fixed-order static async move |args...| body now.
The number of modifiers becomes closer to the point where nobody can remember the correct order :)
| @@ -4274,6 +4320,18 @@ impl<'a> Parser<'a> { | |||
| }) | |||
| } | |||
|
|
|||
| fn is_async_block(&mut self) -> bool { | |||
| self.token.is_keyword(keywords::Async) && | |||
There was a problem hiding this comment.
Nit: This is kind of fragile due to assumption that edition check was already performed and the function is used only once, so it may be better to just inline it.
| // `unsafe async fn` or `async fn` | ||
| if ( | ||
| self.check_keyword(keywords::Unsafe) && | ||
| self.look_ahead(1, |t| t.is_keyword(keywords::Async)) |
There was a problem hiding this comment.
Looks like edition checks are missing here and below.
There was a problem hiding this comment.
That's intentional-- async fn is supposed to work on edition 2015. It's just closures and blocks that don't.
| let is_const_fn = self.eat_keyword(keywords::Const); | ||
| let const_span = self.prev_span; | ||
| let unsafety = self.parse_unsafety(); | ||
| let asyncness = self.parse_asyncness(); |
There was a problem hiding this comment.
So the syntax is fixed-order default const unsafe async extern "C" fn foo() {} now.
Fascinating.
There are two groups of things here - stuff inherent to the fn type (unsafe, extern "ABI" and fn itself) and possibly context-dependent "modifiers" (default, const, async).
So far the logic was that modifiers were grouped together on the left, and type-things were grouped together with fn on the right - default const | unsafe extern "C" fn so the modifiers can't get "inside the fn type" type F = unsafe extern "C" fn();.
I think we should turn the syntax into (default const async in arbitrary order) | (unsafe extern "C" in arbitrary order) fn eventually, but for now let's keep it fixed-order while keeping the grouping rule - default const async | unsafe extern "C" fn.
|
Reviewed. |
|
Thanks @petrochenkov! |
|
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 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 |
|
Is there a reason that the error codes number order got broken once again? |
|
@petrochenkov sorry, I misunderstood what you were requesting when I bought read that comment the first time. When I opened the PR it was "async unsafe" but then @MajorBreakfast pointed out that the opposite decision was made on the rfc: #51580 (comment) I don't have a strong preference here-- I'm happy to change it around again if you think it makes more sense. |
|
@GuillaumeGomez I'm not sure what you're asking-- is there a specific policy over the order in which error codes are added now? |
|
There always has been one: you take the current biggest and you add one. Otherwise it's nearly impossible to keep track of what's going on. Anyway, I'll open a PR to fix it... |
|
@guillaumegomes I've never done that because it means you have to constantly edit your PR when someone else merges something using the same error codes. |
|
It doesn't happen very often, and in addition to that, it's a very small change. Going from 700 to 900+ is kind of a big step. |
+1, that's the reason I don't add errors with codes at all and leave that for separate error-code-assigning PRs |
|
@petrochenkov: That's pretty bad but whatever. If no one cares I'll just stop doing it at some point... |
|
@GuillaumeGomez Error codes are a complete disaster IMO and I'd argue we should just remove them and figure out another indexing mechanism. Every single time someone mentions an error by code, I have to ask them to replace the code with something readable. |
|
Sorry for being off-topic (on the digression into error codes), and I admit that I have no idea about any of this, but has anyone suggested or brought up using some kind of short, descriptive error identifiers (I'll call them "shortcodes") instead of opaque numeric error codes? Something like eslint rules. Yes, an error code isn't meant to be a self-contained exhaustive explanation of an error, but I find "shortcodes" to be qualitatively better than basic numeric codes. Whenever I see them emitted by eslint, they're much more helpful to me than numeric error codes (emitted by TypeScript for example), they're usually enough for me to remember what the error/mistake pertains to so that I can quickly fix it without having to spend time (even if mere seconds) deciphering the compiler/linter output, but in the unlikely event that that's not the case, then of course I could still look it up since the shortcode is uniquely identifying (for example no-dupe-args). For example, the error codes from this PR: Might become (first draft, see below): Some others might be straightforward: - E0300, // unexpanded macro
+ unexpanded-macro, // unexpanded macroOthers maybe trickier: - E0315, // cannot invoke closure outside of its lifetime
+ outlived-closure-invocation, // cannot invoke closure outside of its lifetimeObviously as a first draft that may not look great. I admit that I don't have a deep enough understanding of the errors in order to name them both precisely and concisely, and I'm sure people can pile on with bikeshedding those particular codes, but the idea is that some convention/standard would evolve around how to abbreviate/shorten certain words, what prefixes/suffixes to use, what words to use to connote/convey different types of errors/mistakes, and so on, such that each code would be somewhat uniform/consistent There would preferably be no hard limit on the length as that may eventually lead to overly terse error codes due to unforeseen future Rust terminology or overly niche/specific error situations, defeating the point of them in the first place, but obviously there would be a very strong preference toward short and concise while balancing for descriptive, and definitely unique. Yes, it's unlikely that any one scheme would be perfect, but "shortcodes" may serve more useful and readable than clearly opaque (to end users) numeric error codes, and would at least as one consequence remedy this PR contribution/conflict situation. I'm not sure how error codes relate to backwards compatibility, or whether this can ever be done going forward, even at the edition level. Maybe the ship has long since sailed on this and it's not feasible to introduce something like this at this point. |
|
@eddyb: I agree on your points but as long as a new (better?) system is put in place, let's just keep to stop destroying the current one, as bad as it might seems. It'll make the migration simpler. |
|
This appears to have uncovered a miscompilation bug on old macOS: servo/servo#21089 (comment). I haven’t managed to run lldb so far, but both affected build scripts use |
|
I suspect this PR broke |
FWIW, the direct equivalent is our lint names. I'm not sure we need identifiers at all, although ones that could be name types would be better, since we could use them for structural errors. |
Addresses the errors produced by (re)moving, merging or renaming structs, fields and methods by rust-lang/rust#48149 and rust-lang/rust#51580
|
@blaenk That is indeed very off-topic for this. You should create a new issue if you want further discussion of it. One remark I will make: some tooling is designed around using numbers. Vim’s 'errorformat' looks for error numbers and treats them specially. Very mild damage would be done there if that were changed. |
This PR implements
async/awaitsyntax forasync fnin Rust 2015 andasyncclosures andasyncblocks in Rust 2018 (tracking issue: #50547). Limitations: non-moveasync closures with arguments are currently not supported, nor areasync fnwith multiple different input lifetimes. These limitations are not fundamental and will be removed in the future, however I'd like to go ahead and get this PR merged so we can start experimenting with this in combination with futures 0.3.Based on #51414.
cc @petrochenkov for parsing changes.
r? @eddyb