rustc_mir_build: Clarify that 'mirrored' does not mean 'flipped' or 'reversed' #134197

Enselic · 2024-12-12T05:36:29Z

My intuition for 'mirrored' is that it means 'flipped' or 'reversed'. Clarify that that is not what is meant to 'mirror' the THIR from the HIR.

rustbot · 2024-12-12T05:36:37Z

r? @lcnr

rustbot has assigned @lcnr.
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

jieyouxu · 2024-12-12T05:47:45Z

compiler/rustc_mir_build/src/thir/cx/expr.rs

+    // Create a THIR expression that is "the same" as the HIR expression. The
+    // term "mirror" in this case does not refer to "flipped" or "reversed".


Nit: maybe "of the same shape"?

hmm not sure about "of the same shape". Maybe

Suggested change

// Create a THIR expression that is "the same" as the HIR expression. The

// term "mirror" in this case does not refer to "flipped" or "reversed".

// Create a THIR expression with the same structure as a HIR expression. (The

// term "mirror" in this case does not refer to "flipped" or "reversed".)

?

SGTM, I don't know this part too well so I'll let other reviewers double-check the wording.

lcnr · 2024-12-13T12:28:19Z

@bors r+ rollup

bors · 2024-12-13T12:28:23Z

📌 Commit c25f51a has been approved by lcnr

It is now in the queue for this repository.

lcnr · 2024-12-13T12:28:47Z

@bors r-

lcnr · 2024-12-13T12:32:41Z

compiler/rustc_mir_build/src/thir/cx/expr.rs

+    // Create a THIR expression with the same structure as a HIR expression.
+    // (The term "mirror" in this case does not refer to "flipped" or "reversed".)


Suggested change

// Create a THIR expression with the same structure as a HIR expression.

// (The term "mirror" in this case does not refer to "flipped" or "reversed".)

/// Create a THIR expression for the given HIR expression. This expands

/// all adjustments and directly adds the type information from the

/// `typeck_results`. See the [dev-guide] for more details.

///

/// [dev-guide]: https://rustc-dev-guide.rust-lang.org/thir.html

Feel free to keep the explanation for "mirror" around if you think it's useful

Thanks for the improved text. I added it now. Personally I think the "mirror" clarification is helpful so I kept it around.

Maybe you expected me to click "Commit suggestion" so you would get attribution in the form of your own commit? I apologize if so. I can retroactively fix that if you prefer?

👍 would have been nice but also not something I necessarily mind 😁 so no worries

…reversed' My intuition for 'mirrored' is that it means 'flipped' or 'reversed'. Clarify that that is not what is meant to 'mirror' the THIR from the HIR.

lcnr · 2024-12-16T15:27:54Z

@bors r+ rollup

bors · 2024-12-16T15:27:56Z

📌 Commit 165f37e has been approved by lcnr

It is now in the queue for this repository.

rustc_mir_build: Clarify that 'mirrored' does not mean 'flipped' or 'reversed' My intuition for 'mirrored' is that it means 'flipped' or 'reversed'. Clarify that that is not what is meant to 'mirror' the THIR from the HIR.

…llaumeGomez Rollup of 9 pull requests Successful merges: - rust-lang#132056 (Stabilize `#[diagnostic::do_not_recommend]`) - rust-lang#134124 (CI: use free runners for x86_64-gnu-llvm jobs) - rust-lang#134197 (rustc_mir_build: Clarify that 'mirrored' does not mean 'flipped' or 'reversed') - rust-lang#134260 (Correctly handle comments in attributes in doctests source code) - rust-lang#134277 (rustdoc-search: handle `impl Into<X>` better) - rust-lang#134284 (Keep track of patterns that could have introduced a binding, but didn't) - rust-lang#134337 (reject unsound toggling of RISCV target features) - rust-lang#134385 (tests/ui/asm: Remove uses of rustc_attrs, lang_items, and decl_macro features by using minicore) - rust-lang#134386 (Some trait method vs impl method signature difference diagnostic cleanups) r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#134124 (CI: use free runners for x86_64-gnu-llvm jobs) - rust-lang#134197 (rustc_mir_build: Clarify that 'mirrored' does not mean 'flipped' or 'reversed') - rust-lang#134260 (Correctly handle comments in attributes in doctests source code) - rust-lang#134277 (rustdoc-search: handle `impl Into<X>` better) - rust-lang#134284 (Keep track of patterns that could have introduced a binding, but didn't) - rust-lang#134337 (reject unsound toggling of RISCV target features) - rust-lang#134371 (Check for array lengths that aren't actually `usize`) - rust-lang#134385 (tests/ui/asm: Remove uses of rustc_attrs, lang_items, and decl_macro features by using minicore) - rust-lang#134386 (Some trait method vs impl method signature difference diagnostic cleanups) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#134197 - Enselic:mirror, r=lcnr rustc_mir_build: Clarify that 'mirrored' does not mean 'flipped' or 'reversed' My intuition for 'mirrored' is that it means 'flipped' or 'reversed'. Clarify that that is not what is meant to 'mirror' the THIR from the HIR.

rustbot assigned lcnr Dec 12, 2024

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 Dec 12, 2024

jieyouxu reviewed Dec 12, 2024

View reviewed changes

Enselic force-pushed the mirror branch from aaff8b1 to c25f51a Compare December 12, 2024 19:15

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 Dec 13, 2024

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 Dec 13, 2024

lcnr reviewed Dec 13, 2024

View reviewed changes

rustc_mir_build: Clarify that 'mirrored' does not mean 'flipped' or '…

165f37e

…reversed' My intuition for 'mirrored' is that it means 'flipped' or 'reversed'. Clarify that that is not what is meant to 'mirror' the THIR from the HIR.

Enselic force-pushed the mirror branch from c25f51a to 165f37e Compare December 13, 2024 15:45

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 Dec 16, 2024

GuillaumeGomez mentioned this pull request Dec 16, 2024

Rollup of 10 pull requests #134391

Closed

GuillaumeGomez mentioned this pull request Dec 16, 2024

Rollup of 10 pull requests #134392

Closed

GuillaumeGomez mentioned this pull request Dec 16, 2024

Rollup of 9 pull requests #134393

Closed

matthiaskrgr mentioned this pull request Dec 16, 2024

Rollup of 9 pull requests #134395

Merged

bors merged commit d03e3c8 into rust-lang:master Dec 16, 2024
6 checks passed

rustbot added this to the 1.85.0 milestone Dec 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rustc_mir_build: Clarify that 'mirrored' does not mean 'flipped' or 'reversed' #134197

rustc_mir_build: Clarify that 'mirrored' does not mean 'flipped' or 'reversed' #134197

Enselic commented Dec 12, 2024

rustbot commented Dec 12, 2024

jieyouxu Dec 12, 2024

Enselic Dec 12, 2024

jieyouxu Dec 12, 2024 •

edited

Loading

lcnr commented Dec 13, 2024

bors commented Dec 13, 2024

lcnr commented Dec 13, 2024

lcnr Dec 13, 2024

Enselic Dec 13, 2024

Enselic Dec 15, 2024 •

edited

Loading

lcnr Dec 16, 2024

lcnr commented Dec 16, 2024

bors commented Dec 16, 2024

		// Create a THIR expression that is "the same" as the HIR expression. The
		// term "mirror" in this case does not refer to "flipped" or "reversed".

		// Create a THIR expression with the same structure as a HIR expression.
		// (The term "mirror" in this case does not refer to "flipped" or "reversed".)

-    // Create a THIR expression with the same structure as a HIR expression.
-    // (The term "mirror" in this case does not refer to "flipped" or "reversed".)
+    /// Create a THIR expression for the given HIR expression. This expands
+    /// all adjustments and directly adds the type information from the
+    /// `typeck_results`. See the [dev-guide] for more details.
+    ///
+    /// [dev-guide]: https://rustc-dev-guide.rust-lang.org/thir.html

rustc_mir_build: Clarify that 'mirrored' does not mean 'flipped' or 'reversed' #134197

rustc_mir_build: Clarify that 'mirrored' does not mean 'flipped' or 'reversed' #134197

Conversation

Enselic commented Dec 12, 2024

rustbot commented Dec 12, 2024

jieyouxu Dec 12, 2024

Choose a reason for hiding this comment

Enselic Dec 12, 2024

Choose a reason for hiding this comment

jieyouxu Dec 12, 2024 • edited Loading

Choose a reason for hiding this comment

lcnr commented Dec 13, 2024

bors commented Dec 13, 2024

lcnr commented Dec 13, 2024

lcnr Dec 13, 2024

Choose a reason for hiding this comment

Enselic Dec 13, 2024

Choose a reason for hiding this comment

Enselic Dec 15, 2024 • edited Loading

Choose a reason for hiding this comment

lcnr Dec 16, 2024

Choose a reason for hiding this comment

lcnr commented Dec 16, 2024

bors commented Dec 16, 2024

jieyouxu Dec 12, 2024 •

edited

Loading

Enselic Dec 15, 2024 •

edited

Loading