-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Add expectation for {
when parsing lone coroutine qualifiers
#142362
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
// `Span::at_least_rust_2018()` is somewhat expensive; don't get it repeatedly. | ||
let at_async = this.check_keyword(exp!(Async)); | ||
// check for `gen {}` and `gen move {}` | ||
// or `async gen {}` and `async gen move {}` | ||
// FIXME: (async) gen closures aren't yet parsed. | ||
// FIXME(gen_blocks): Parse `gen async` and suggest swap | ||
if this.token_uninterpolated_span().at_least_rust_2024() | ||
// check for `gen {}` and `gen move {}` | ||
// or `async gen {}` and `async gen move {}` | ||
&& (this.is_gen_block(kw::Gen, 0) | ||
|| (this.check_keyword(exp!(Async)) && this.is_gen_block(kw::Gen, 1))) | ||
&& this.is_gen_block(kw::Gen, at_async as usize) | ||
{ | ||
// FIXME: (async) gen closures aren't yet parsed. | ||
this.parse_gen_block() | ||
} else if this.check_keyword(exp!(Async)) { | ||
// FIXME(gen_blocks): Parse `gen async` and suggest swap | ||
if this.is_gen_block(kw::Async, 0) { | ||
// Check for `async {` and `async move {`, | ||
this.parse_gen_block() | ||
} else { | ||
this.parse_expr_closure() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shuffled this around while looking through the call path as I found the {async} gen
condition a bit heavy on the eyes. Happy to undo this
This comment has been minimized.
This comment has been minimized.
36b11cf
to
ef92bb9
Compare
&& coroutine_kind.is_some() | ||
{ | ||
// couroutine closures and generators can have the same qualifiers, so we might end up | ||
// in here if there is a missing `|` but also no `{`. Adjust the expectations in that case. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... this feels like sth that we'd regularly hit elsewhere, too, because is_gen_block
and similar functions won't put their things in the expectation list. Unsure how to resolve that, so what you wrote is ok, but it does feel a bit like papering over a more common issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was feeling the same about this actually, but I couldn't figure out a solution for this either for the time being. We would need to start consuming the qualifier list before dispatching into the actual coroutine / generator parsing routines I think given we can't build expectations for lookaheads.
ef92bb9
to
edc405d
Compare
@bors r=oli-obk |
Rollup of 9 pull requests Successful merges: - #142305 (Remove unneeded `check_id` calls as they are already called in `visit_id` in `EarlyContextAndPass` type) - #142314 (remove ice group pings from `triagebot.toml`) - #142343 (remove myself from the project) - #142346 (Add tracing import to execution context) - #142356 (Fix enter_trace_span!() using wrong $crate paths) - #142362 (Add expectation for `{` when parsing lone coroutine qualifiers) - #142364 (Do not warn on `rust.incremental` when using download CI rustc) - #142369 (Improve some attribute docs and rename groups) - #142374 (Fix missing newline trim in bootstrap) r? `@ghost` `@rustbot` modify labels: rollup
Fixes #80931