-
Notifications
You must be signed in to change notification settings - Fork 13.9k
Remove support for prefix yield #148313
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
base: master
Are you sure you want to change the base?
Remove support for prefix yield #148313
Conversation
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
I actually dislike the postfix syntax quite a lot. I think |
|
The ship has probably sailed for Note that if we ever get generators that can receive responses from the caller after being resumed, then |
| /// Parse `"yield" expr?`. | ||
| fn parse_expr_yield(&mut self) -> PResult<'a, Box<Expr>> { | ||
| let lo = self.prev_token.span; | ||
| let kind = ExprKind::Yield(YieldKind::Prefix(self.parse_expr_opt()?)); | ||
| let span = lo.to(self.prev_token.span); | ||
| self.psess.gated_spans.gate(sym::yield_expr, span); | ||
| let expr = self.mk_expr(span, kind); | ||
| self.maybe_recover_from_bad_qpath(expr) | ||
| } |
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.
Could we keep the parse, creating an error expression kind with an appropriate suggestion for the postfix?
Ideally, if maintenance cost wasn't a thing, we'd keep the yield kind too, for later more complex diagnostics, but at the very least we should be able to parse yield foo to tell people to use foo.yield instead (like we already do for await foo).
We're diverging from other language's syntax. Without an explicit check, we'd end up with something like:
error: expected one of `!`, `.`, `::`, `;`, `?`, `{`, `}`, or an operator, found `foo`
--> src/lib.rs:2:9
|
2 | yield foo;
| ^^^ expected one of 8 possible tokens
|
help: you might have meant to write a field access
|
2 | yield.foo;
| +
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.
Oh, that's a great idea! I'll add that back in before I mark this PR as ready to review.
|
Postfix Still, it seems like it'd be much better to retain bare |
Has it, though? Would it not be possible to support postfix I think that it's very fair to propose a suffix version and implement it, but I also think that it's very premature to remove the prefix version in the same change. And I think that if we're going along with suffix control flow keywords moving forward, there should probably be an RFC to propose doing so with existing keywords ( |
Part of the proposal in rust-lang/rfcs#3861 is to go forward with the postfix version of yield. This PR updates rustc to match that design.
This is currently a draft PR. Its main value at this point is to give people a feel for postfix yield by seeing how the test cases and examples change.