Desugar async blocks in HIR instead of MIR#157309
Draft
cjgillot wants to merge 14 commits into
Draft
Conversation
Debug-printing MIR is not readable. We have dedicated infra to dump MIR, use it. Added benefit, this avoid hardcoding that `AddMovesForPackedDrops` is the first pass in make_shim (it is not).
Closures, coroutines, coroutine witnesses and coroutine closures use the same syntax. This used to be copy-paste but gradually departed.
rustbot
added
PG-exploit-mitigations
This comment has been minimized.
This comment has been minimized.
Member
|
cc @Swatinem as you have done work on this in the past |
cjgillot
force-pushed
the
coroutine-hir-desugar
branch
from
3aee865 to
d17eeb1
Compare
Contributor
Author
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
rustbot
added
the
S-waiting-on-perf
This comment has been minimized.
This comment has been minimized.
rust-bors Bot
pushed a commit
that referenced
this pull request
Jun 2, 2026
Desugar async blocks in HIR instead of MIR
Collaborator
|
The job Click to see the possible cause of the failure (guessed by this bot) |
Contributor
This comment has been minimized.
This comment has been minimized.
Collaborator
|
Finished benchmarking commit (ec00386): comparison URL. Overall result: ❌✅ regressions and improvements - please read:Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf. Next, please: If you can, justify the regressions found in this try perf run in writing along with @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary -0.2%, secondary 0.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary 6.3%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary 0.1%, secondary 6.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 510.698s -> 509.663s (-0.20%) |
rustbot
added
perf-regression
estebank
reviewed
Jun 2, 2026
Comment on lines
+21
to
+31
| --> $DIR/issue-76168-hr-outlives-3.rs:13:1 | ||
| | | ||
| LL | / { | ||
| LL | | | ||
| LL | | | ||
| LL | | let mut i = 41; | ||
| LL | | &mut i; | ||
| LL | | } | ||
| | |_^ expected an `FnOnce(&'a mut i32)` closure, found `i32` | ||
| | | ||
| = help: the trait `for<'a> FnOnce(&'a mut i32)` is not implemented for `i32` |
Contributor
There was a problem hiding this comment.
Not sure why the later error isn't marked as duplicate automatically. I don't see any visual difference between the two. It'd be a shame to start producing redundant errors here. (but so far things look reasonable in general.)
Comment on lines
10
to
+19
| error[E0038]: the trait `Foo` is not dyn compatible | ||
| --> $DIR/inference_var_self_argument.rs:5:33 | ||
| --> $DIR/inference_var_self_argument.rs:5:34 | ||
| | | ||
| LL | async fn foo(self: &dyn Foo) { | ||
| | ^ `Foo` is not dyn compatible | ||
| LL | async fn foo(self: &dyn Foo) { | ||
| | __________________________________^ | ||
| LL | | | ||
| LL | | | ||
| LL | | todo!() | ||
| LL | | } | ||
| | |_____^ `Foo` is not dyn compatible |
Contributor
There was a problem hiding this comment.
We should have a better span (ideally pointing at dyn Foo). Errors that point at the whole body are bad when presenting information in IDEs, as then you end up with a whole lot of text with a red underline, generally masking other errors.
Comment on lines
+51
to
+52
| = note: no two async blocks, even if identical, have the same type | ||
| = help: consider pinning your async block and casting it to a trait object |
Contributor
There was a problem hiding this comment.
These two are redundant with the ones immediately above.
| | ----- ^^^^^^^^ | ||
| | | | | ||
| | | expected `async` block, found a different `async` block | ||
| | | arguments to this function are incorrect |
Contributor
There was a problem hiding this comment.
We need to customize the E0308 logic for function calls to account for this new desugaring, as talking about "arguments to this function" when pointing at an async block are less than ideal. Just not including the label would be enough., but ideally we'd make the previous output whihch points at fun still work even with the additional level of indirection.
Contributor
There was a problem hiding this comment.
Ideally we'd point at the tail expression at least, instead of the whole block.
Contributor
There was a problem hiding this comment.
This is a problem: we are no longer pointing at user code in the recursion error. There's no way a normal user can figure out what caused there to be a problem with the new output.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements MCP rust-lang/compiler-team#997
Based on #157166
In the current implementation,
gen/async/async genblocks and closures have typeCoroutine(..)andCoroutineClosure(..). Those types implementIterator,FutureorAsyncIteratordepending on the initial desugaring.This creates a lot of complexity:
I propose to change the desugaring for coroutines to:
gen { .. }becomesCoroutineIterator::from_coroutine(#[coroutine] { .. });async { .. }becomesCoroutineFuture::from_coroutine(#[coroutine] { .. });async gen { .. }becomesCoroutineAsyncIterator::from_coroutine(#[coroutine] { .. }).This way, all coroutines implement
std::ops::Coroutineandcoreis responsible for translating this to user-friendly traits. All the complexity is pushed to error-reporting code, which is not soundness-critical.Coroutine closures are a little more complex, as we need to keep the
CoroutineClosuretype for borrow-checking.Main design point: I create two methods on
TyCtxtthat are meant to do the back-and-forth between wrapped and unwrapped coroutines.coroutine_desugared_typewraps a coroutine inside the adapter struct.try_unwrap_desugared_coroutineunwraps it.r? @oli-obk
cc @lcnr @RalfJung
cc @estebank as I modify quite a lot of diagnostic code