Skip to content

Codegen ctors in Runtime mir phase#158040

Merged
rust-bors[bot] merged 2 commits into
rust-lang:mainfrom
jdonszelmann:fix-crash
Jun 19, 2026
Merged

Codegen ctors in Runtime mir phase#158040
rust-bors[bot] merged 2 commits into
rust-lang:mainfrom
jdonszelmann:fix-crash

Conversation

@jdonszelmann

@jdonszelmann jdonszelmann commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

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::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 for work towards making bootstraps with the next solver a CI job.

r? @oli-obk

@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 Jun 17, 2026
Comment thread stats_waffle_lcnr Outdated
@jdonszelmann

Copy link
Copy Markdown
Contributor Author

I'm not confident that the same ICE cannot happen with other variants though. We just have no examples that trigger it.

@jdonszelmann

Copy link
Copy Markdown
Contributor Author

cc @lcnr could you also take a look at this?

@lcnr

lcnr commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

alright, there is a dubious bug here. codegen should never use any tpyingmode but PostAnalysis.

The fact that we have a body whose mode is still borrowck during codegen is very questionable :<

@rustbot

rustbot commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@jdonszelmann jdonszelmann changed the title Allow getting opaque types for ctors and variants Codegen ctors in Runtime mir phase Jun 18, 2026
@jdonszelmann

Copy link
Copy Markdown
Contributor Author

r? @lcnr

@lcnr lcnr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

View changes since this review

@oli-obk

oli-obk commented Jun 18, 2026

Copy link
Copy Markdown
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...

@rust-log-analyzer

This comment has been minimized.

@rustbot

rustbot commented Jun 18, 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.

@oli-obk oli-obk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@rust-bors

rust-bors Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 119335c 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 5. This pull request will be tested once the tree is reopened.

@rustbot rustbot assigned oli-obk and unassigned lcnr Jun 19, 2026
@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 19, 2026
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-bors rust-bors Bot merged commit e37d7b5 into rust-lang:main Jun 19, 2026
13 checks passed
@rustbot rustbot added this to the 1.98.0 milestone Jun 19, 2026
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
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rustc doesn't compile with the new solver

6 participants