Skip to content

Expand async drops during drop elaboration#156649

Merged
rust-bors[bot] merged 12 commits into
rust-lang:mainfrom
cjgillot:move-async-expand
Jun 1, 2026
Merged

Expand async drops during drop elaboration#156649
rust-bors[bot] merged 12 commits into
rust-lang:mainfrom
cjgillot:move-async-expand

Conversation

@cjgillot
Copy link
Copy Markdown
Contributor

@cjgillot cjgillot commented May 16, 2026 •
edited by rustbot
Loading

View all comments

Async drops implemented by #123948 are expanded in await loops inside StateTransform. I find this odd, as StateTransform is tasked with removing yields from MIR.

This PR moves this async drop -> loop transformation to drop elaboration.

The first few commits create many mir-opt tests to gauge the effect of following commits. I can easily trim those.
The next few commits are from #156422.
Then come a few commits performing elementary simplifications to drop elaboration code.

The most important commit is Expand async drops during drop elaboration. It performs the main change and attempts to document what I had to understand doing that.

The last commit is a cleanup.

This is a large PR, but most of the diff is in generated files. I can easily split them, each commit compiles and passes tests on its own.

cc @zetanumbers

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 16, 2026

This PR changes MIR

cc @oli-obk, @RalfJung, @JakobDegen, @vakaras

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

This PR changes rustc_public

cc @oli-obk, @celinval, @ouz-a, @makai410

The Cranelift subtree was changed

cc @bjorn3

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

@rustbot 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 16, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 16, 2026

r? @JohnTitor

rustbot has assigned @JohnTitor.
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 16 candidates

@cjgillot cjgillot force-pushed the move-async-expand branch from 1ed6528 to 20114b0 Compare May 16, 2026 18:51
@bjorn3
Copy link
Copy Markdown
Member

bjorn3 commented May 16, 2026

Is there some relation to #156495 or would that PR be orthogonal to this one?

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@cjgillot
Copy link
Copy Markdown
Contributor Author

cjgillot commented May 16, 2026

Is there some relation to #156495 or would that PR be orthogonal to this one?

It should be orthogonal.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@zetanumbers
Copy link
Copy Markdown
Contributor

zetanumbers commented May 18, 2026

uuh
cc @azhogin

@cjgillot cjgillot force-pushed the move-async-expand branch from 1a5cdbb to a4336bc Compare May 22, 2026 16:20
@rustbot

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@cjgillot cjgillot force-pushed the move-async-expand branch from a4336bc to 7fe62e4 Compare May 22, 2026 19:46
@JohnTitor
Copy link
Copy Markdown
Member

JohnTitor commented May 27, 2026

@rustbot reroll
I'm not familiar with this 🙏

@rustbot rustbot assigned nnethercote and unassigned JohnTitor May 27, 2026
@cjgillot cjgillot force-pushed the move-async-expand branch from 7fe62e4 to a3420ce Compare May 29, 2026 20:24
@rustbot

This comment has been minimized.

Sign in to view
@cjgillot cjgillot force-pushed the move-async-expand branch from a3420ce to 49f7a10 Compare May 30, 2026 15:07
@rustbot

This comment has been minimized.

Sign in to view
@cjgillot cjgillot mentioned this pull request May 30, 2026
Merged
@nnethercote
Copy link
Copy Markdown
Contributor

nnethercote commented May 30, 2026

@cjgillot: looks like this is still evolving. Can you mark it as a draft? Or otherwise make it clear when it is ready for review? Thanks.

@cjgillot
Copy link
Copy Markdown
Contributor Author

cjgillot commented May 30, 2026

This is ready for review. Just rebasing regularly.

@nnethercote
Copy link
Copy Markdown
Contributor

nnethercote commented May 30, 2026

Ok! Thanks.

@rust-bors

This comment has been minimized.

Sign in to view
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request May 30, 2026
@JonathanBrouwer
Rollup merge of rust-lang#157162 - cjgillot:as_mut_unwrap, r=mejrs
d662f5a 
elaborate_drop: Avoid as_mut.unwrap dance, empty vectors are cheap.

Nit I found while working on rust-lang#156649
nnethercote
nnethercote reviewed May 31, 2026
View reviewed changes
Comment thread compiler/rustc_mir_transform/src/shim/async_destructor_ctor.rs Outdated
patch.apply(&mut body);
}

// We did not bother respectig deref separation, do it here.
Copy link
Copy Markdown
Contributor

@nnethercote nnethercote May 31, 2026 •
edited by rustbot
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*respecting

View changes since the review

nnethercote
nnethercote reviewed May 31, 2026
View reviewed changes
Comment thread compiler/rustc_mir_transform/src/shim/async_destructor_ctor.rs Outdated
),
);

// We did not bother respectig deref separation, do it here.
Copy link
Copy Markdown
Contributor

@nnethercote nnethercote May 31, 2026 •
edited by rustbot
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*respecting

View changes since the review

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 31, 2026
@cjgillot
Copy link
Copy Markdown
Contributor Author

cjgillot commented May 31, 2026

@nnethercote I added a few lines in each commit message. I'd like your review on those. And I added an extra commit with comment changes.

@nnethercote
Copy link
Copy Markdown
Contributor

nnethercote commented May 31, 2026

The expanded commit messages look great! Thanks.

@bors r+

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented May 31, 2026

📌 Commit d0e0cea has been approved by nnethercote

It is now in the queue for this repository.

@rust-bors 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 31, 2026
@rust-bors

This comment has been minimized.

Sign in to view
rust-bors Bot pushed a commit that referenced this pull request May 31, 2026
@bors
Auto merge of #156649 - cjgillot:move-async-expand, r=nnethercote
6387066 
Expand async drops during drop elaboration



Async drops implemented by #123948 are expanded in await loops inside `StateTransform`. I find this odd, as `StateTransform` is tasked with removing yields from MIR.

This PR moves this `async drop` -> loop transformation to drop elaboration.

The first few commits create many mir-opt tests to gauge the effect of following commits. I can easily trim those.
The next few commits are from #156422.
Then come a few commits performing elementary simplifications to drop elaboration code.

The most important commit is `Expand async drops during drop elaboration.` It performs the main change and attempts to document what I had to understand doing that.

The last commit is a cleanup.

This is a large PR, but most of the diff is in generated files. I can easily split them, each commit compiles and passes tests on its own.

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

rust-bors Bot commented May 31, 2026

💔 Test for 6387066 failed: CI. Failed job:

@rust-log-analyzer
Copy link
Copy Markdown
Collaborator

rust-log-analyzer commented May 31, 2026

A job failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@cjgillot
Copy link
Copy Markdown
Contributor Author

cjgillot commented Jun 1, 2026

Docker error
@bors retry

@rust-bors 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 Jun 1, 2026
@JonathanBrouwer
Copy link
Copy Markdown
Contributor

JonathanBrouwer commented Jun 1, 2026

@bors p=6
Scheduling this before the next rollup

@rust-bors

This comment has been minimized.

Sign in to view
@rust-bors rust-bors Bot added merged-by-bors This PR was explicitly merged by bors. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 1, 2026
@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented Jun 1, 2026

☀️ Test successful - CI
Approved by: nnethercote
Duration: 3h 11m 22s
Pushing c0bb140 to main...

@rust-bors rust-bors Bot merged commit c0bb140 into rust-lang:main Jun 1, 2026
14 checks passed
@rustbot rustbot added this to the 1.98.0 milestone Jun 1, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 1, 2026

What is this? This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 968d50a (parent) -> c0bb140 (this PR)

Test differences

Show 54 test diffs

Stage 1

  • [mir-opt] tests/mir-opt/coroutine/async_drop.rs: [missing] -> pass (J0)
  • [mir-opt] tests/mir-opt/coroutine/async_fn.rs: [missing] -> pass (J0)

Stage 2

  • [mir-opt] tests/mir-opt/coroutine/async_drop.rs: [missing] -> pass (J1)
  • [mir-opt] tests/mir-opt/coroutine/async_fn.rs: [missing] -> pass (J1)

Additionally, 50 doctest diffs were found. These are ignored, as they are noisy.

Job group index

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard c0bb140a37c81cf59a0b40c21c9413059644e294 --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

  1. x86_64-gnu-gcc-core-tests: 7m 29s -> 14m 28s (+93.1%)
  2. x86_64-gnu-llvm-21: 45m 46s -> 1h 20m (+75.3%)
  3. x86_64-msvc-1: 1h 42m -> 2h 40m (+56.7%)
  4. i686-gnu-nopt-1: 1h 37m -> 2h 21m (+44.8%)
  5. pr-check-2: 30m 46s -> 44m 30s (+44.6%)
  6. x86_64-gnu-debug: 1h 34m -> 2h 11m (+39.1%)
  7. aarch64-apple: 3h 26m -> 2h 22m (-30.9%)
  8. dist-aarch64-linux: 2h 31m -> 1h 52m (-26.1%)
  9. dist-x86_64-illumos: 1h 27m -> 1h 7m (-23.0%)
  10. i686-msvc-1: 3h 5m -> 2h 22m (-23.0%)
How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

This was referenced Jun 1, 2026
Draft
Closed
Open
Open
@rust-timer
Copy link
Copy Markdown
Collaborator

rust-timer commented Jun 1, 2026

Finished benchmarking commit (c0bb140): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -0.1% [-0.1%, -0.1%] 1
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (secondary -5.4%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -5.4% [-5.6%, -5.2%] 2
All ❌✅ (primary) - - 0

Cycles

This perf run didn't have relevant results for this metric.

Binary size

Results (primary -0.2%, secondary -0.3%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 -0.2% [-0.4%, -0.1%] 28
Improvements ✅
(secondary)		 -0.3% [-0.4%, -0.1%] 25
All ❌✅ (primary) -0.2% [-0.4%, -0.1%] 28

Bootstrap: 511.125s -> 511.375s (0.05%)
Artifact size: 400.79 MiB -> 400.63 MiB (-0.04%)

@cjgillot cjgillot deleted the move-async-expand branch June 1, 2026 21:10
@cjgillot cjgillot mentioned this pull request Jun 1, 2026
Open
20 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nnethercote nnethercote nnethercote left review comments

Assignees

@nnethercote nnethercote

Labels

merged-by-bors This PR was explicitly merged by bors. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.98.0

Development

Successfully merging this pull request may close these issues.

9 participants

@cjgillot @rustbot @bjorn3 @rust-log-analyzer @zetanumbers @JohnTitor @nnethercote @rust-timer @JonathanBrouwer