-
Notifications
You must be signed in to change notification settings - Fork 13.7k
Reject static async closures in AST lowering #145605
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?
Reject static async closures in AST lowering #145605
Conversation
This comment has been minimized.
This comment has been minimized.
Rejecting this pre-expansion should be covered by #145604, but I guess we don't necessarily need to parse the combination at all. |
Let's see what the fallout is tho. @bors2 try |
This comment has been minimized.
This comment has been minimized.
Reject async closures in AST lowering
TODO: add a test for |
This comment has been minimized.
This comment has been minimized.
I'll bless the spans when bors is back |
I think there is one test in rust-analyzer that this will break too |
rust-analyzer doesn't depend on rustc like that, so no, that shouldn't happen. |
@craterbot check |
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
🚧 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
72990c5
to
f999a90
Compare
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
🎉 Experiment
|
All spurious as far as I can tell. |
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.
TODO: add a test for
static gen || {}
too.
.
but I guess we don't necessarily need to parse the combination at all.
Do you mean rejecting static || {}
(and async
, gen
) in the parser unless it's #[coroutine] static || {}
? That'd be quite surprising and also unprecedented if I'm not mistaken (namely, looking through the attributes at parse time), tho I guess it's technically possible since built-in attributes can never be shadowed by user-defined attributes (we reject such cases with ambiguity).
Regarding the 'gen || {}
doesn't parse <-> static gen || {}
parses and "works"' situation, I guess we want to do that in a separate PR? Esp. if it needs further discussions with oli. E.g, we also parse async gen || {}
but bug!
during AST lowering. I don't know what the untold plans are for all of these features.
r=me with static gen closure test added unless you also want to address some of the other things I mentioned above in this PR.
No, i mean making |
Ah, sounds good! Let's do that once the results for #145604 are back! Edit: Results are back and they're clean! Feel free to proceed :) |
Bug reported in https://rust-lang.zulipchat.com/#narrow/channel/122651-general/topic/static.20async.20closures/with/535095775.
r? fmease