Skip to content

Fix coroutine MIR saved local remapping#158655

Open
dingxiangfei2009 wants to merge 2 commits into
rust-lang:mainfrom
dingxiangfei2009:fix-remapping
Open

Fix coroutine MIR saved local remapping#158655
dingxiangfei2009 wants to merge 2 commits into
rust-lang:mainfrom
dingxiangfei2009:fix-remapping

Conversation

@dingxiangfei2009

@dingxiangfei2009 dingxiangfei2009 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

View all comments

The unsound analysis was found while I was working on the async-drop code. Apparently the counter variable in the array drop glue is not correctly identified as a saved local.

Also we did not patch up indices in ProjectionElem::Index when they are saved locals.

The reproduction is in the older commit with the expected output.

@rustbot

rustbot commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@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 Jul 1, 2026
@rustbot

rustbot commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

r? @TaKO8Ki

rustbot has assigned @TaKO8Ki.
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 15 candidates

@dingxiangfei2009

Copy link
Copy Markdown
Contributor Author

r? cjgillot

who probably has more context

@rustbot rustbot assigned cjgillot and unassigned TaKO8Ki Jul 1, 2026
@dingxiangfei2009 dingxiangfei2009 requested a review from cjgillot July 1, 2026 12:33
Comment thread compiler/rustc_mir_transform/src/coroutine/mod.rs Outdated
Comment thread compiler/rustc_mir_transform/src/coroutine/mod.rs Outdated
Comment thread tests/ui/async-await/async-drop/async-drop-initial.rs
@cjgillot cjgillot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 2, 2026
@dingxiangfei2009

Copy link
Copy Markdown
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 3, 2026
Comment thread compiler/rustc_mir_transform/src/coroutine/mod.rs Outdated
Comment thread compiler/rustc_mir_transform/src/coroutine/mod.rs Outdated
Comment thread compiler/rustc_mir_transform/src/coroutine/mod.rs Outdated
Comment thread compiler/rustc_mir_transform/src/coroutine/mod.rs
Comment thread compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs
Signed-off-by: Xiangfei Ding <dingxiangfei2009@protonmail.ch>
@rustbot

rustbot commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator

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.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

Storage is still required even if the local is moved out and
immediately moved in again.

Signed-off-by: Xiangfei Ding <dingxiangfei2009@protonmail.ch>
@dingxiangfei2009

Copy link
Copy Markdown
Contributor Author

@rustbot ready

@dingxiangfei2009 dingxiangfei2009 requested a review from cjgillot July 6, 2026 10:54
@dingxiangfei2009

Copy link
Copy Markdown
Contributor Author

@cjgillot Thank you for your review!

I have applied the suggestions. Let me know any other outstanding concerns. Cheers!

@cjgillot

cjgillot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Great !
@bors r+

rust-bors Bot pushed a commit that referenced this pull request Jul 8, 2026
…uwer

Rollup of 18 pull requests

Successful merges:

 - #158871 (add relnotes for 1.97.0)
 - #150946 (intrinsics: Add a fallback for non-const libm float functions)
 - #158617 (allow mGCA const arguments to fall back to anon consts)
 - #158645 (Fix splat ICEs and ban it in closures)
 - #158655 (Fix coroutine MIR saved local remapping)
 - #158666 (Carry the `b_offset` inside `BackendRepr::ScalarPair`)
 - #158920 (Update wasm-component-ld to 0.5.26)
 - #158926 (wrapping_sh* methods: clarify underspecified reference)
 - #158927 (add core test run with `-Zforce-intrinsic-fallback`)
 - #151379 (Stabilize `VecDeque::retain_back` from `truncate_front`)
 - #158807 (Add regression test for CString::clone_into unwind safety)
 - #158862 (Fix the span for parameter suggestion )
 - #158883 (tests: fix enum-match.rs to handle LLVM 23)
 - #158894 (Make the ordering of non-terminal binds in ambiguity error messages deterministic)
 - #158902 (add codegen test for range length bound propagation)
 - #158913 (Update `browser-ui-test` version to `0.24.1`)
 - #158935 (std: support real fd methods on Emscripten)
 - #158951 (Merge three `MaxUniverse`s into one)
rust-bors Bot pushed a commit that referenced this pull request Jul 8, 2026
…uwer

Rollup of 18 pull requests

Successful merges:

 - #158871 (add relnotes for 1.97.0)
 - #150946 (intrinsics: Add a fallback for non-const libm float functions)
 - #158617 (allow mGCA const arguments to fall back to anon consts)
 - #158645 (Fix splat ICEs and ban it in closures)
 - #158655 (Fix coroutine MIR saved local remapping)
 - #158666 (Carry the `b_offset` inside `BackendRepr::ScalarPair`)
 - #158920 (Update wasm-component-ld to 0.5.26)
 - #158926 (wrapping_sh* methods: clarify underspecified reference)
 - #158927 (add core test run with `-Zforce-intrinsic-fallback`)
 - #151379 (Stabilize `VecDeque::retain_back` from `truncate_front`)
 - #158807 (Add regression test for CString::clone_into unwind safety)
 - #158862 (Fix the span for parameter suggestion )
 - #158883 (tests: fix enum-match.rs to handle LLVM 23)
 - #158894 (Make the ordering of non-terminal binds in ambiguity error messages deterministic)
 - #158902 (add codegen test for range length bound propagation)
 - #158913 (Update `browser-ui-test` version to `0.24.1`)
 - #158935 (std: support real fd methods on Emscripten)
 - #158951 (Merge three `MaxUniverse`s into one)
rust-bors Bot pushed a commit that referenced this pull request Jul 8, 2026
…uwer

Rollup of 18 pull requests

Successful merges:

 - #158871 (add relnotes for 1.97.0)
 - #150946 (intrinsics: Add a fallback for non-const libm float functions)
 - #158617 (allow mGCA const arguments to fall back to anon consts)
 - #158645 (Fix splat ICEs and ban it in closures)
 - #158655 (Fix coroutine MIR saved local remapping)
 - #158666 (Carry the `b_offset` inside `BackendRepr::ScalarPair`)
 - #158920 (Update wasm-component-ld to 0.5.26)
 - #158926 (wrapping_sh* methods: clarify underspecified reference)
 - #158927 (add core test run with `-Zforce-intrinsic-fallback`)
 - #151379 (Stabilize `VecDeque::retain_back` from `truncate_front`)
 - #158807 (Add regression test for CString::clone_into unwind safety)
 - #158862 (Fix the span for parameter suggestion )
 - #158883 (tests: fix enum-match.rs to handle LLVM 23)
 - #158894 (Make the ordering of non-terminal binds in ambiguity error messages deterministic)
 - #158902 (add codegen test for range length bound propagation)
 - #158913 (Update `browser-ui-test` version to `0.24.1`)
 - #158935 (std: support real fd methods on Emscripten)
 - #158951 (Merge three `MaxUniverse`s into one)
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Jul 8, 2026
…cjgillot

Fix coroutine MIR saved local remapping

The unsound analysis was found while I was working on the `async-drop` code. Apparently the counter variable in the array drop glue is not correctly identified as a saved local.

Also we did not patch up indices in `ProjectionElem::Index` when they are saved locals.

The reproduction is in the older commit with the expected output.
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Jul 8, 2026
…cjgillot

Fix coroutine MIR saved local remapping

The unsound analysis was found while I was working on the `async-drop` code. Apparently the counter variable in the array drop glue is not correctly identified as a saved local.

Also we did not patch up indices in `ProjectionElem::Index` when they are saved locals.

The reproduction is in the older commit with the expected output.
rust-bors Bot pushed a commit that referenced this pull request Jul 8, 2026
…uwer

Rollup of 25 pull requests

Successful merges:

 - #158871 (add relnotes for 1.97.0)
 - #158968 (stdarch subtree update)
 - #154445 (rustdoc: Represent `--output-format=json` coverage and ir differently)
 - #156370 (Reject linked dylib EII default overrides)
 - #157153 (allow `Allocator`s to be used as `#[global_allocator]`s)
 - #158495 (Rename HAS_CT_PROJECTION to HAS_CONST_ALIAS)
 - #158617 (allow mGCA const arguments to fall back to anon consts)
 - #158645 (Fix splat ICEs and ban it in closures)
 - #158655 (Fix coroutine MIR saved local remapping)
 - #158666 (Carry the `b_offset` inside `BackendRepr::ScalarPair`)
 - #158912 (Introduce new bootstrap config section for PGO configuration)
 - #158920 (Update wasm-component-ld to 0.5.26)
 - #158926 (wrapping_sh* methods: clarify underspecified reference)
 - #158927 (add core test run with `-Zforce-intrinsic-fallback`)
 - #158932 (Do not build the compiler when invoking `x perf compare`)
 - #158937 (Emit the emscripten entry point as `__main_argc_argv`)
 - #151379 (Stabilize `VecDeque::retain_back` from `truncate_front`)
 - #156548 ( Library support for aarch64-unknown-linux-pauthtest target)
 - #158307 (CI job for parallel frontend ui tests)
 - #158347 (Improve generic parameters handling for #[diagnostic::on_const])
 - #158722 (delegation: do not always inherit `ConstArgHasType` predicates)
 - #158741 (Simplify `Option::into_flat_iter` signature)
 - #158807 (Add regression test for CString::clone_into unwind safety)
 - #158862 (Fix the span for parameter suggestion )
 - #158883 (tests: fix enum-match.rs to handle LLVM 23)
jhpratt added a commit to jhpratt/rust that referenced this pull request Jul 8, 2026
…cjgillot

Fix coroutine MIR saved local remapping

The unsound analysis was found while I was working on the `async-drop` code. Apparently the counter variable in the array drop glue is not correctly identified as a saved local.

Also we did not patch up indices in `ProjectionElem::Index` when they are saved locals.

The reproduction is in the older commit with the expected output.
rust-bors Bot pushed a commit that referenced this pull request Jul 8, 2026
Rollup of 25 pull requests

Successful merges:

 - #158871 (add relnotes for 1.97.0)
 - #158968 (stdarch subtree update)
 - #157690 (codegen_ssa: pack small const aggregates into immediate stores)
 - #158541 (Move `std::io::Write` to `core::io`)
 - #154445 (rustdoc: Represent `--output-format=json` coverage and ir differently)
 - #156370 (Reject linked dylib EII default overrides)
 - #157153 (allow `Allocator`s to be used as `#[global_allocator]`s)
 - #158495 (Rename HAS_CT_PROJECTION to HAS_CONST_ALIAS)
 - #158617 (allow mGCA const arguments to fall back to anon consts)
 - #158645 (Fix splat ICEs and ban it in closures)
 - #158655 (Fix coroutine MIR saved local remapping)
 - #158666 (Carry the `b_offset` inside `BackendRepr::ScalarPair`)
 - #158870 (std: merge the unix-like io::error modules into one file)
 - #158920 (Update wasm-component-ld to 0.5.26)
 - #158926 (wrapping_sh* methods: clarify underspecified reference)
 - #158927 (add core test run with `-Zforce-intrinsic-fallback`)
 - #158932 (Do not build the compiler when invoking `x perf compare`)
 - #158937 (Emit the emscripten entry point as `__main_argc_argv`)
 - #151379 (Stabilize `VecDeque::retain_back` from `truncate_front`)
 - #156144 (Better docs for PartialEq (includes macro rename))
 - #156548 ( Library support for aarch64-unknown-linux-pauthtest target)
 - #158307 (CI job for parallel frontend ui tests)
 - #158347 (Improve generic parameters handling for #[diagnostic::on_const])
 - #158722 (delegation: do not always inherit `ConstArgHasType` predicates)
 - #158741 (Simplify `Option::into_flat_iter` signature)
@jhpratt

jhpratt commented Jul 9, 2026

Copy link
Copy Markdown
Member

I think this may be the cause of #158986 (comment), but I'm not certain.

@bors try jobs=aarch64-gnu-debug

@bors r-

@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Jul 9, 2026
Fix coroutine MIR saved local remapping


try-job: aarch64-gnu-debug
@rust-bors rust-bors Bot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 9, 2026
@rust-bors

rust-bors Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

This pull request was unapproved.

This PR was contained in a rollup (#158986), which was unapproved.

View changes since this unapproval

@rust-bors

rust-bors Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: fea2ff1 (fea2ff15d501ff090e07e8b8e0e3923a45526ac9)
Base parent: 14cae68 (14cae681329a63c622a6e1fbe1d30f9374bc51d8)

@dingxiangfei2009

Copy link
Copy Markdown
Contributor Author

@bors try

@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Jul 11, 2026
Fix coroutine MIR saved local remapping
@rust-bors

rust-bors Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: 47885b7 (47885b7d3a2f833e136633bcfcf7622f33f1004b)
Base parent: ad49852 (ad49852b6f3dd9c1c73fe909d8843bb7e464b387)

@dingxiangfei2009

dingxiangfei2009 commented Jul 11, 2026

Copy link
Copy Markdown
Contributor Author

@bors try jobs=aarch64-gnu-debug

@rust-bors

rust-bors Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Unknown command "jobs". Run @bors help or go to https://bors.rust-lang.org/help to see available commands.

@dingxiangfei2009

Copy link
Copy Markdown
Contributor Author

@bors try jobs=aarch64-gnu-debug

@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Jul 11, 2026
Fix coroutine MIR saved local remapping


try-job: aarch64-gnu-debug
@rust-bors

rust-bors Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: 5f08f47 (5f08f47e109fef96c446a1db4e145e1e70bc4d71)
Base parent: acf6a00 (acf6a00aff5d8aa8892aadff0b48364b50f97cf4)

@dingxiangfei2009

Copy link
Copy Markdown
Contributor Author

@jhpratt Okay it looks like the previous failure was due to another PR in the rollup.

Shall we proceed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants