Codegen ctors in Runtime mir phase#158040
Merged
Merged
Conversation
rustbot
added
S-waiting-on-review
lqd
reviewed
Jun 17, 2026
Contributor
Author
|
I'm not confident that the same ICE cannot happen with other variants though. We just have no examples that trigger it. |
Contributor
Author
|
cc @lcnr could you also take a look at this? |
Contributor
|
alright, there is a dubious bug here. codegen should never use any tpyingmode but The fact that we have a body whose mode is still borrowck during codegen is very questionable :< |
Collaborator
|
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
jdonszelmann
changed the title
Contributor
Author
|
r? @lcnr |
lcnr
reviewed
Jun 18, 2026
Contributor
There was a problem hiding this comment.
cc @rust-lang/opsem @oli-obk we previously built the constructor shims in MirPhase::Built and kept them there. Given that MIR phase transitions change what some of the MIR stmts mean, just changing the phase to Runtime without doing anything could be wrong.
Given that we already used this MIR for CTFE and codegen, that's probably not actually an issue 😀 still want to ask whether there's something we should do here/your thoughts on how to handle shims and MIR phases
Contributor
|
seems fine to build shims directly in the latter phases (unsure if we need a different one for ctfe than for codegen). Especially if we're not running the passes that change the phases anyway... |
This comment has been minimized.
This comment has been minimized.
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. |
Contributor
rust-bors
Bot
added
S-waiting-on-bors
rust-bors Bot
pushed a commit
that referenced
this pull request
Jun 19, 2026
…uwer Rollup of 5 pull requests Successful merges: - #157878 (`impl [const] Default for BTreeMap`) - #158040 (Codegen ctors in Runtime mir phase) - #141266 (Stabilize `substr_range` and `subslice_range`) - #158109 (Change `EarlyCheckNode` from a trait to an enum.) - #158118 (Convert '.' to '_' in bootstrap envify)
rust-bors Bot
pushed a commit
that referenced
this pull request
Jun 19, 2026
…uwer Rollup of 5 pull requests Successful merges: - #157878 (`impl [const] Default for BTreeMap`) - #158040 (Codegen ctors in Runtime mir phase) - #141266 (Stabilize `substr_range` and `subslice_range`) - #158109 (Change `EarlyCheckNode` from a trait to an enum.) - #158118 (Convert '.' to '_' in bootstrap envify)
rust-bors Bot
pushed a commit
that referenced
this pull request
Jun 19, 2026
…uwer Rollup of 5 pull requests Successful merges: - #157878 (`impl [const] Default for BTreeMap`) - #158040 (Codegen ctors in Runtime mir phase) - #141266 (Stabilize `substr_range` and `subslice_range`) - #158109 (Change `EarlyCheckNode` from a trait to an enum.) - #158118 (Convert '.' to '_' in bootstrap envify)
rust-timer
added a commit
that referenced
this pull request
Jun 19, 2026
Rollup merge of #158040 - jdonszelmann:fix-crash, r=oli-obk Codegen ctors in Runtime mir phase https://github.com/rust-lang/rust/pull/156141/changes#diff-f18405dedc545b19aa3ee04cd08b17e1e0fa1b5876e46c6b445eaaa7e54618eaR421 (part of #156141) The mir of constructors used to be generated in MirPhase::Built, but are only used during codegen. That means they'd have a weird MirPhase, since all other items have MirPhase::Runtime during codegen. This PR still generates ctor mir in MirPhase::Built, but then immediately does a phase change to Runtime after running no passes. Fixes #158037 <summary> Old description of another, worse way to solve this <details> starts using `TypingMode::PostTypeckUntilBorrowck` during mir building with the next solver. This is the right typing mode, and the typing mode contains a list of opaque types. However, previously, we instead just used `TypingMode::Typeck` with an empty list of opaques. That is the wrong typing mode, but it also meant we collected opaque types of slightly fewer things. Now that we do collect their opaque types, we're doing so for items that we've never gathered opaque types for before and get an ICE. There are no tests associated with this change. @lqd tried to reproduce it, but it seems surprisingly hard except by bootstrapping rustc itself. This change makes `RUSTFLAGS_NOT_BOOTSTRAP=-Znext-solver x build compiler --stage 2` work, which I verified locally. See [#t-infra/announcements > Next solver / polonius alpha pre-stabilization CI job](https://rust-lang.zulipchat.com/#narrow/channel/533458-t-infra.2Fannouncements/topic/Next.20solver.20.2F.20polonius.20alpha.20pre-stabilization.20CI.20job/with/602503884) for work towards making bootstraps with the next solver a CI job. </details> </summary> r? @oli-obk
jhpratt
added a commit
to jhpratt/rust
that referenced
this pull request
Jun 19, 2026
ensure the new solver bootstraps on CI With rust-lang#158040, the new solver should now be able to bootstrap, and this PR ensures on CI it doesn't regress. This is part of the plan in rust-lang#157780 until stabilization. This renames the job as well to something hopefully clearer, from `x86_64-gnu-pre-stabilization` to `x86_64-gnu-next-trait-solver-polonius`. And thus, also needs rust-lang/rustc-dev-guide#2904 to have the correct `doc_url`. r? kobzol
JonathanBrouwer
added a commit
to JonathanBrouwer/rust
that referenced
this pull request
Jun 19, 2026
ensure the new solver bootstraps on CI With rust-lang#158040, the new solver should now be able to bootstrap, and this PR ensures on CI it doesn't regress. This is part of the plan in rust-lang#157780 until stabilization. This renames the job as well to something hopefully clearer, from `x86_64-gnu-pre-stabilization` to `x86_64-gnu-next-trait-solver-polonius`. And thus, also needs rust-lang/rustc-dev-guide#2904 to have the correct `doc_url`. r? kobzol
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
6 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.
https://github.com/rust-lang/rust/pull/156141/changes#diff-f18405dedc545b19aa3ee04cd08b17e1e0fa1b5876e46c6b445eaaa7e54618eaR421
(part of #156141)
The mir of constructors used to be generated in MirPhase::Built, but are only used during codegen. That means they'd have a weird MirPhase, since all other items have MirPhase::Runtime during codegen. This PR still generates ctor mir in MirPhase::Built, but then immediately does a phase change to Runtime after running no passes.
Fixes #158037
Old description of another, worse way to solve this
Details
starts using
TypingMode::PostTypeckUntilBorrowckduring mir building with the next solver. This is the right typing mode, and the typing mode contains a list of opaque types. However, previously, we instead just usedTypingMode::Typeckwith an empty list of opaques. That is the wrong typing mode, but it also meant we collected opaque types of slightly fewer things. Now that we do collect their opaque types, we're doing so for items that we've never gathered opaque types for before and get an ICE.There are no tests associated with this change. @lqd tried to reproduce it, but it seems surprisingly hard except by bootstrapping rustc itself. This change makes
RUSTFLAGS_NOT_BOOTSTRAP=-Znext-solver x build compiler --stage 2work, which I verified locally. See #t-infra/announcements > Next solver / polonius alpha pre-stabilization CI job for work towards making bootstraps with the next solver a CI job.r? @oli-obk