Misc improvements to coroutine transform code by cjgillot · Pull Request #156672 · rust-lang/rust

cjgillot · 2026-05-17T15:56:23Z

Several quality-of-life improvements:

A dedicated mir-opt directory for coroutines, there are very few tests right now, but more should come.

A dedicated pretty-printer for coroutine layout. In particular, it does not rely on Ty as Debug which has unstable output. This is important for async fns which capture opaque types, in particular other async fns.

A drive-by simplification.

Last, I change how the coroutine entry block is inserted. The current implementation shifts everything by 1. I prefer swapping with the current entry, which makes debugging and MIR diffing much easier.

rustbot · 2026-05-17T15:56:25Z

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

rustbot · 2026-05-17T15:56:28Z

r? @tiif

rustbot has assigned @tiif.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: compiler
compiler expanded to 73 candidates
Random selection from 18 candidates

tiif

Sorry for the delay :)

I don't really understand the final commit, so I am going to roll the right person to review it.

View changes since this review

tiif · 2026-05-28T08:29:36Z

+    writeln!(w, "{INDENT}{INDENT}variant_fields = {{")?;
+    for (variant, fields) in variant_fields.iter_enumerated() {
+        let variant_name = ty::CoroutineArgs::variant_name(variant);
+        let header = format!("{INDENT}{INDENT}{INDENT}{variant_name:9}({variant:?}): {fields:?},");


I am trying to figure out what is the reason of having {variant_name:9}, does the variant name have at most 9 characters?

Mainly: because impl Debug for CoroutineLayout uses that.
Variant names are Unresumed (9), Returned (8), Poisoned (8) and Suspend followed by a number.

tiif · 2026-05-28T08:45:36Z

+            if let Some(name) = name
+                && let source_info = layout.field_tys[field].source_info
+                && source_info.scope == parent
+            {


Suggested change

if let Some(name) = name

&& let source_info = layout.field_tys[field].source_info

&& source_info.scope == parent

{

let source_info = layout.field_tys[field].source_info;

if let Some(name) = name

&& source_info.scope == parent

{

It might be clearer to separate source_info from the condition statement?

tiif · 2026-05-28T09:20:26Z

-            | TerminatorKind::Unreachable
-            | TerminatorKind::CoroutineDrop
-            | TerminatorKind::FalseEdge { .. }
-            | TerminatorKind::FalseUnwind { .. } => {}


In this PR, can_unwind will return true if there is a TerminatorKind::FalseUnwind, but previously it will not. Will this introduce any kind of behavioural change?

FalseUnwind cannot happen at this stage of compilation.

tiif · 2026-05-28T09:21:47Z

@@ -1173,40 +1173,8 @@ fn can_unwind<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>) -> bool {
    }

    // Unwinds can only start at certain terminators.


Suggested change

// Unwinds can only start at certain terminators.

tiif · 2026-05-28T09:23:28Z

+    body.basic_blocks.iter().any(|block| block.terminator().unwind().is_some())
    // If we didn't find an unwinding terminator, the function cannot unwind.


Suggested change

body.basic_blocks.iter().any(|block| block.terminator().unwind().is_some())

// If we didn't find an unwinding terminator, the function cannot unwind.

// If we didn't find an unwinding terminator, the function cannot unwind.

body.basic_blocks.iter().any(|block| block.terminator().unwind().is_some())

tiif · 2026-05-28T09:59:25Z

r? @oli-obk

rustbot · 2026-05-29T21:03:29Z

This PR was rebased onto a different main 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.

oli-obk

@bors r+

View changes since this review

rust-bors · 2026-05-30T08:34:14Z

📌 Commit f6a5b0b has been approved by oli-obk

It is now in the queue for this repository.

🌲 The tree is currently closed for pull requests below priority 1. This pull request will be tested once the tree is reopened.

…uwer Rollup of 12 pull requests Successful merges: - #154591 (Remove `will_cache_on_disk_for_key_fn`) - #156672 (Misc improvements to coroutine transform code) - #157027 (HIR ty lowering: Move some things into submodules) - #157051 (Allow two object files for a single CGU in CompiledModule) - #157100 (Some more per owner things) - #153497 (Use `trait_object_dummy_self` more & heavily fix+update related docs) - #155638 (Fix tupled closure signature in `AsyncFn` arg mismatch diagnostic) - #156826 (style: Clarify nullary call and `()` no-break rule applies past max width) - #157004 (Remove unused functions in `value_analysis.rs`) - #157032 (Fixed more &x ->&mut x suggestions) - #157033 (Note irrefutable while let in loop type errors) - #157139 (compiler: `ops::RangeInclusive` → `range::RangeInclusive`) Failed merges: - #156875 (Correct and document semantics of `yield` terminator)

Rollup merge of #156672 - cjgillot:coroutine-qol, r=oli-obk Misc improvements to coroutine transform code Several quality-of-life improvements: A dedicated mir-opt directory for coroutines, there are very few tests right now, but more should come. A dedicated pretty-printer for coroutine layout. In particular, it does not rely on `Ty as Debug` which has unstable output. This is important for async fns which capture opaque types, in particular other async fns. A drive-by simplification. Last, I change how the coroutine entry block is inserted. The current implementation shifts everything by 1. I prefer swapping with the current entry, which makes debugging and MIR diffing much easier.

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 17, 2026

rustbot assigned tiif May 17, 2026

This comment has been minimized.

Sign in to view

cjgillot force-pushed the coroutine-qol branch 2 times, most recently from 0c96073 to 757cfe5 Compare May 22, 2026 16:19

This comment has been minimized.

Sign in to view

tiif reviewed May 28, 2026

View reviewed changes

rustbot assigned oli-obk and unassigned tiif May 28, 2026

cjgillot force-pushed the coroutine-qol branch from 757cfe5 to 8544e80 Compare May 29, 2026 20:00

This comment has been minimized.

Sign in to view

cjgillot added 5 commits May 29, 2026 20:11

Make a dedicated coroutine mir-opt test directory.

5878c57

Change how coroutine layout is dumped in MIR.

d332a12

Simplify can_unwind.

3b77583

Put entry block at the end instead of shifting everything.

9e5c064

Review nits.

f6a5b0b

cjgillot force-pushed the coroutine-qol branch from 8544e80 to f6a5b0b Compare May 29, 2026 21:03

oli-obk approved these changes May 30, 2026

View reviewed changes

rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 30, 2026

JonathanBrouwer mentioned this pull request May 30, 2026

Rollup of 12 pull requests #157149

Merged

rust-bors Bot merged commit 3a53860 into rust-lang:main May 30, 2026
12 checks passed

rustbot added this to the 1.98.0 milestone May 30, 2026

cjgillot deleted the coroutine-qol branch May 30, 2026 14:08

		@@ -1173,40 +1173,8 @@ fn can_unwind<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>) -> bool {
		}

		// Unwinds can only start at certain terminators.

		body.basic_blocks.iter().any(\|block\| block.terminator().unwind().is_some())
		// If we didn't find an unwinding terminator, the function cannot unwind.

Uh oh!

Conversation

cjgillot commented May 17, 2026

Uh oh!

rustbot commented May 17, 2026

Uh oh!

rustbot commented May 17, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

tiif left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tiif May 28, 2026

Choose a reason for hiding this comment

Uh oh!

cjgillot May 29, 2026

Choose a reason for hiding this comment

Uh oh!

tiif May 28, 2026

Choose a reason for hiding this comment

Uh oh!

tiif May 28, 2026

Choose a reason for hiding this comment

Uh oh!

cjgillot May 29, 2026

Choose a reason for hiding this comment

Uh oh!

tiif May 28, 2026

Choose a reason for hiding this comment

Uh oh!

tiif May 28, 2026

Choose a reason for hiding this comment

Uh oh!

tiif commented May 28, 2026

Uh oh!

This comment has been minimized.

rustbot commented May 29, 2026

Uh oh!

oli-obk left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rust-bors Bot commented May 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tiif left a comment •

edited by rustbot

Loading

oli-obk left a comment •

edited by rustbot

Loading