Skip to content

Conversation

cjgillot
Copy link
Contributor

MIR parameters are not explicitly assigned-to when entering the MIR body. If we want to save their values inside the coroutine state, we need to do so explicitly.

This was done by renaming the _2 local, and introducing an explicit assignment pre-transform. This particular trick confuses me.

This version makes explicit that we are assigning parameters to saved locals.

r? @dingxiangfei2009

@rustbot
Copy link
Collaborator

rustbot commented Sep 14, 2025

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 Sep 14, 2025
@rustbot
Copy link
Collaborator

rustbot commented Sep 14, 2025

Failed to set assignee to dingxiangfei2009: invalid assignee

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@cjgillot
Copy link
Contributor Author

r? compiler

@jackh726
Copy link
Member

Seems reasonable to me. Curious if there was some particular reason this was written this way.

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 16, 2025

📌 Commit c1bda59 has been approved by jackh726

It is now in the queue for this repository.

@bors bors 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 Sep 16, 2025
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 16, 2025
StateTransform: Do not renumber resume local. MIR parameters are not explicitly assigned-to when entering the MIR body. If we want to save their values inside the coroutine state, we need to do so explicitly. This was done by renaming the `_2` local, and introducing an explicit assignment pre-transform. This particular trick confuses me. This version makes explicit that we are assigning parameters to saved locals. r? `@dingxiangfei2009`
bors added a commit that referenced this pull request Sep 16, 2025
Rollup of 4 pull requests Successful merges: - #143613 (Fix backtraces with `-C panic=abort` on linux; emit unwind tables by default) - #145181 (remove FIXME block from `has_significant_drop`, it never encounters inference variables) - #146552 (StateTransform: Do not renumber resume local.) - #146588 (tests/run-make: Update list of statically linked musl targets) r? `@ghost` `@rustbot` modify labels: rollup
@Zalathar
Copy link
Contributor

Probably failed in rollup: #146625 (comment)

@bors r-

@bors bors 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 Sep 16, 2025
@rustbot
Copy link
Collaborator

rustbot commented Sep 16, 2025

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.

@cjgillot
Copy link
Contributor Author

@bors r=jackh726

@bors
Copy link
Collaborator

bors commented Sep 16, 2025

📌 Commit eddd755 has been approved by jackh726

It is now in the queue for this repository.

@bors bors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 16, 2025
Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 17, 2025
StateTransform: Do not renumber resume local. MIR parameters are not explicitly assigned-to when entering the MIR body. If we want to save their values inside the coroutine state, we need to do so explicitly. This was done by renaming the `_2` local, and introducing an explicit assignment pre-transform. This particular trick confuses me. This version makes explicit that we are assigning parameters to saved locals. r? `@dingxiangfei2009`
bors added a commit that referenced this pull request Sep 17, 2025
Rollup of 14 pull requests Successful merges: - #142807 (libtest: expose --fail-fast as an unstable command-line option) - #144871 (Stabilize `btree_entry_insert` feature) - #145071 (Update the minimum external LLVM to 20) - #145181 (remove FIXME block from `has_significant_drop`, it never encounters inference variables) - #145660 (initial implementation of the darwin_objc unstable feature) - #145838 (don't apply temporary lifetime extension rules to non-extended `super let`) - #146259 (Suggest removing Box::new instead of unboxing it) - #146410 (Iterator repeat: no infinite loop for `last` and `count`) - #146460 (Add tidy readme) - #146552 (StateTransform: Do not renumber resume local.) - #146564 (Remove Rvalue::Len again.) - #146581 (Detect attempt to use var-args in closure) - #146588 (tests/run-make: Update list of statically linked musl targets) - #146631 (cg_llvm: Replace some DIBuilder wrappers with LLVM-C API bindings (part 3)) r? `@ghost` `@rustbot` modify labels: rollup
@bors bors merged commit 577f18f into rust-lang:master Sep 17, 2025
10 checks passed
@rustbot rustbot added this to the 1.92.0 milestone Sep 17, 2025
rust-timer added a commit that referenced this pull request Sep 17, 2025
Rollup merge of #146552 - cjgillot:resume-noremap, r=jackh726 StateTransform: Do not renumber resume local. MIR parameters are not explicitly assigned-to when entering the MIR body. If we want to save their values inside the coroutine state, we need to do so explicitly. This was done by renaming the `_2` local, and introducing an explicit assignment pre-transform. This particular trick confuses me. This version makes explicit that we are assigning parameters to saved locals. r? ``@dingxiangfei2009``
@cjgillot cjgillot deleted the resume-noremap branch September 17, 2025 10:51
github-actions bot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request Sep 22, 2025
Rollup of 14 pull requests Successful merges: - rust-lang/rust#142807 (libtest: expose --fail-fast as an unstable command-line option) - rust-lang/rust#144871 (Stabilize `btree_entry_insert` feature) - rust-lang/rust#145071 (Update the minimum external LLVM to 20) - rust-lang/rust#145181 (remove FIXME block from `has_significant_drop`, it never encounters inference variables) - rust-lang/rust#145660 (initial implementation of the darwin_objc unstable feature) - rust-lang/rust#145838 (don't apply temporary lifetime extension rules to non-extended `super let`) - rust-lang/rust#146259 (Suggest removing Box::new instead of unboxing it) - rust-lang/rust#146410 (Iterator repeat: no infinite loop for `last` and `count`) - rust-lang/rust#146460 (Add tidy readme) - rust-lang/rust#146552 (StateTransform: Do not renumber resume local.) - rust-lang/rust#146564 (Remove Rvalue::Len again.) - rust-lang/rust#146581 (Detect attempt to use var-args in closure) - rust-lang/rust#146588 (tests/run-make: Update list of statically linked musl targets) - rust-lang/rust#146631 (cg_llvm: Replace some DIBuilder wrappers with LLVM-C API bindings (part 3)) r? `@ghost` `@rustbot` modify labels: rollup
rust-cloud-vms bot pushed a commit to makai410/rustc_public that referenced this pull request Oct 12, 2025
Rollup of 14 pull requests Successful merges: - rust-lang/rust#142807 (libtest: expose --fail-fast as an unstable command-line option) - rust-lang/rust#144871 (Stabilize `btree_entry_insert` feature) - rust-lang/rust#145071 (Update the minimum external LLVM to 20) - rust-lang/rust#145181 (remove FIXME block from `has_significant_drop`, it never encounters inference variables) - rust-lang/rust#145660 (initial implementation of the darwin_objc unstable feature) - rust-lang/rust#145838 (don't apply temporary lifetime extension rules to non-extended `super let`) - rust-lang/rust#146259 (Suggest removing Box::new instead of unboxing it) - rust-lang/rust#146410 (Iterator repeat: no infinite loop for `last` and `count`) - rust-lang/rust#146460 (Add tidy readme) - rust-lang/rust#146552 (StateTransform: Do not renumber resume local.) - rust-lang/rust#146564 (Remove Rvalue::Len again.) - rust-lang/rust#146581 (Detect attempt to use var-args in closure) - rust-lang/rust#146588 (tests/run-make: Update list of statically linked musl targets) - rust-lang/rust#146631 (cg_llvm: Replace some DIBuilder wrappers with LLVM-C API bindings (part 3)) r? `@ghost` `@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

6 participants