Clarify MIR dialects and phases by nnethercote · Pull Request #137204 · rust-lang/rust (original) (raw)

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation23 Commits3 Checks6 Files changed

Conversation

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

[ Show hidden characters]({{ revealButtonHref }})

I found the existing code and docs hard to understand.

r? @Zalathar

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

Feb 18, 2025

Best reviewed one commit at a time.

Looks good to me, but I'm happy to wait for a second opinion from someone with more experience in this area.

Could not assign reviewer from: oli-obk.
User(s) oli-obk are either the PR author, already assigned, or on vacation. Please use r? to specify someone else to assign.

Could not assign reviewer from: oli-obk. User(s) oli-obk are either the PR author, already assigned, or on vacation. Please use r? to specify someone else to assign.

Ok, then, r? @davidtwco for a second opinion.

Choose a reason for hiding this comment

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

I added another commit tweaking the dialect/phase description.

/// differences come in two forms: Dialects and phases.
/// The MIR pipeline is structured into a few major dialects, with one or more phases within each
/// dialect. A MIR flavor is identified by a dialect-phase pair. A single `MirPhase` value
/// specifies such a pair. All flavors of MIR use the same data structure.

Choose a reason for hiding this comment

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

/// specifies such a pair. All flavors of MIR use the same data structure.
/// specifies such a pair. All flavors of MIR use the same data structure to represent the program.

r=me with a final nit while we are at it.

Currently many of them exceed 100 chars, which makes them painful to read on a terminal that is 100 chars wide.

I found the dialect/phase distinction quite confusing when I first read these comments. This commit clarifies things a bit.

The only visible change is to the filenames produce by -Zdump-mir. E.g. before and after:

h.main.003-000.analysis-post-cleanup.after.mir
h.main.2-2-000.analysis-post-cleanup.after.mir

It also fixes a FIXME comment.

I fixed the nit.

@bors r=RalfJung rollup

📌 Commit 83a7fb6 has been approved by RalfJung

It is now in the queue for this repository.

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

Feb 20, 2025

workingjubilee added a commit to workingjubilee/rustc that referenced this pull request

Feb 20, 2025

…nd-phases, r=RalfJung

Clarify MIR dialects and phases

I found the existing code and docs hard to understand.

r? @Zalathar

bors added a commit to rust-lang-ci/rust that referenced this pull request

Feb 20, 2025

workingjubilee added a commit to workingjubilee/rustc that referenced this pull request

Feb 20, 2025

…nd-phases, r=RalfJung

Clarify MIR dialects and phases

I found the existing code and docs hard to understand.

r? @Zalathar

bors added a commit to rust-lang-ci/rust that referenced this pull request

Feb 20, 2025

Rollup of 12 pull requests

Successful merges:

rust-lang#128080 (Specify scope in out_of_scope_macro_calls lint)
rust-lang#135354 ([Debuginfo] Add MSVC Synthetic and Summary providers to LLDB)
rust-lang#135630 (add more s390x target features)
rust-lang#136089 (Reduce Box::default stack copies in debug mode)
rust-lang#136148 (Optionally add type names to TypeIds.)
rust-lang#137192 (Remove obsolete Windows ThinLTO+TLS workaround)
rust-lang#137204 (Clarify MIR dialects and phases)
rust-lang#137299 (Simplify Postorder customization.)
rust-lang#137302 (Use a probe to avoid registering stray region obligations when re-checking drops in MIR typeck)
rust-lang#137305 (Tweaks in and around rustc_middle)
rust-lang#137313 (Some codegen_llvm cleanups)
rust-lang#137333 (Use edition = "2024" in the compiler (redux))

r? @ghost @rustbot modify labels: rollup

try-job: test-various try-job: x86_64-msvc-1 try-job: x86_64-msvc-2 try-job: i686-msvc-1 try-job: i686-msvc-2 try-job: i686-mingw-1 try-job: i686-mingw-2 try-job: i686-mingw-3 try-job: x86_64-gnu-nopt

workingjubilee added a commit to workingjubilee/rustc that referenced this pull request

Feb 20, 2025

…nd-phases, r=RalfJung

Clarify MIR dialects and phases

I found the existing code and docs hard to understand.

r? @Zalathar

bors added a commit to rust-lang-ci/rust that referenced this pull request

Feb 21, 2025

Rollup of 10 pull requests

Successful merges:

rust-lang#128080 (Specify scope in out_of_scope_macro_calls lint)
rust-lang#135630 (add more s390x target features)
rust-lang#136089 (Reduce Box::default stack copies in debug mode)
rust-lang#137192 (Remove obsolete Windows ThinLTO+TLS workaround)
rust-lang#137204 (Clarify MIR dialects and phases)
rust-lang#137299 (Simplify Postorder customization.)
rust-lang#137302 (Use a probe to avoid registering stray region obligations when re-checking drops in MIR typeck)
rust-lang#137305 (Tweaks in and around rustc_middle)
rust-lang#137313 (Some codegen_llvm cleanups)
rust-lang#137333 (Use edition = "2024" in the compiler (redux))

r? @ghost @rustbot modify labels: rollup

try-job: aarch64-gnu try-job: armhf-gnu try-job: i686-mingw-1 try-job: i686-mingw-2 try-job: i686-mingw-3 try-job: test-various try-job: x86_64-gnu-nopt try-job: x86_64-msvc-1 try-job: x86_64-msvc-2

bors added a commit to rust-lang-ci/rust that referenced this pull request

Feb 21, 2025

User(s) oli-obk are either the PR author, already assigned, or on vacation. Please use r? to specify someone else to assign.

huh?

bors added a commit to rust-lang-ci/rust that referenced this pull request

Feb 21, 2025

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request

Feb 21, 2025

Rollup merge of rust-lang#137204 - nnethercote:clarify-MIR-dialects-and-phases, r=RalfJung

Clarify MIR dialects and phases

I found the existing code and docs hard to understand.

r? @Zalathar

huh?

So I think you were indeed already assigned.

github-actions bot pushed a commit to tautschnig/verify-rust-std that referenced this pull request

Mar 11, 2025

github-actions bot pushed a commit to tautschnig/verify-rust-std that referenced this pull request

Mar 11, 2025

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.