fix ICE in pretty-printing `global_asm!` by folkertdev · Pull Request #138280 · rust-lang/rust

folkertdev · 2025-03-09T15:55:21Z

since #137180, global_asm! gets a fake body, that the pretty printing logic did not know what to do with.

based on #t-compiler/help > tests for MIR pretty printing I created tests/ui/unpretty/mir which seemed as good a place as any for a test. If there is a better place, let me know.

try-job: test-various
try-job: x86_64-apple-2

rustbot · 2025-03-09T15:55:26Z

r? @nnethercote

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

compiler-errors · 2025-03-09T18:28:06Z

r? compiler-errors

compiler-errors · 2025-03-09T18:30:42Z

This has nothing to do with const operands, so the reasoning is a bit off. This also ICEs on master:

(edit: fixed typo below)

//@compile-flags: -Zdump-mir=all
use std::arch::{asm, global_asm};

global_asm!("/* hi */");

Please adjust the title and description not to mention const operands. The reason this ICEs is because since #137180 we synthesize a fake MIR body for checking the types of operands in global_asm!, but this was never adjusted to handle dumping MIR for these bodies. I think this fix is right, but I'll leave an inline suggestion for a better comment.

@rustbot author

compiler/rustc_middle/src/mir/pretty.rs

tests/ui/unpretty/mir/global-asm-const.rs

folkertdev · 2025-03-09T19:13:36Z

tests/ui/unpretty/mir/global-asm.rs

@@ -0,0 +1,7 @@
+//@ compile-flags: -Zdump-mir=all --crate-type=lib


you mentioned -dump=mir in one of your comments. That doesn't exist, and --emit=mir seemed to work. So I don't know what you meant exactly, but is this the right dump command? will it not create a bunch of useless files when someone runs the tests suite?

-Zdump-mir=all. And yes, I think unfortunately your test will end up dumping a bunch of files when the test suite is executed I think 🤔 .

You should probably move this to a tests/mir-opt test, and add an annotation for // EMIT_MIR ... for just the global asm block, or however you spell that.

compiler-errors · 2025-03-09T19:15:41Z

It was a typo lol

folkertdev · 2025-03-09T19:15:45Z

@rustbot ready

folkertdev · 2025-03-09T19:42:16Z

allright, I moved the test file. The testing machinery there is much nicer.

compiler-errors · 2025-03-09T20:02:26Z

LGTM.

@bors r+ rollup

bors · 2025-03-09T20:02:29Z

📌 Commit b394c8e has been approved by compiler-errors

It is now in the queue for this repository.

…ompiler-errors fix ICE in pretty-printing `global_asm!` fixes rust-lang#138260 since rust-lang#137180, `global_asm!` gets a fake body, that the pretty printing logic did not know what to do with. based on [#t-compiler/help > tests for MIR pretty printing](https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/tests.20for.20MIR.20pretty.20printing) I created `tests/ui/unpretty/mir` which seemed as good a place as any for a test. If there is a better place, let me know.

…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#137279 (Make some invalid codegen attr errors structured/translatable) - rust-lang#137793 (Stablize anonymous pipe) - rust-lang#138238 (Fix dyn -> param suggestion in struct ICEs) - rust-lang#138270 (chore: Fix some comments) - rust-lang#138280 (fix ICE in pretty-printing `global_asm!`) - rust-lang#138286 (triagebot.toml: Don't label `test/rustdoc-json` as A-rustdoc-search (…) r? `@ghost` `@rustbot` modify labels: rollup

…ompiler-errors fix ICE in pretty-printing `global_asm!` fixes rust-lang#138260 since rust-lang#137180, `global_asm!` gets a fake body, that the pretty printing logic did not know what to do with. based on [#t-compiler/help > tests for MIR pretty printing](https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/tests.20for.20MIR.20pretty.20printing) I created `tests/ui/unpretty/mir` which seemed as good a place as any for a test. If there is a better place, let me know.

folkertdev · 2025-03-10T08:51:23Z

right, of course...

It looks like minicore is not available for the mir opt tests (yet?), so I copied from it what is needed here.

@rustbot ready

compiler-errors · 2025-03-10T13:26:08Z

tests/mir-opt/global_asm.rs

@@ -0,0 +1,18 @@
+// skip-filecheck
+#![feature(no_core, lang_items, rustc_attrs, decl_macro)]


you don't need no-core, i'm pretty sure there's a directive for limiting this test just to platforms with asm support:

//@ needs-asm-support

Yeah I guess that's fine here, because the printing logic does not change between platforms so testing only on some is probably fine. I've made the change.

compiler-errors · 2025-03-10T14:16:10Z

I'll approve when CI turns green

matthiaskrgr · 2025-03-10T16:37:56Z

@bors try

bors · 2025-03-10T16:39:08Z

⌛ Trying commit 9213cb8 with merge 9fc4032...

fix ICE in pretty-printing `global_asm!` fixes rust-lang#138260 since rust-lang#137180, `global_asm!` gets a fake body, that the pretty printing logic did not know what to do with. based on [#t-compiler/help > tests for MIR pretty printing](https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/tests.20for.20MIR.20pretty.20printing) I created `tests/ui/unpretty/mir` which seemed as good a place as any for a test. If there is a better place, let me know. try-job: test-various try-job: x86_64-apple-2

bors · 2025-03-10T18:23:54Z

☀️ Try build successful - checks-actions
Build commit: 9fc4032 (9fc4032645037b551db82b8f38855ac82a5a5fb6)

folkertdev · 2025-03-12T16:13:56Z

@rustbot ready

compiler-errors · 2025-03-12T16:48:24Z

@bors r+ rollup

bors · 2025-03-12T16:48:26Z

📌 Commit 9213cb8 has been approved by compiler-errors

It is now in the queue for this repository.

…earth Rollup of 12 pull requests Successful merges: - rust-lang#134076 (Stabilize `std::io::ErrorKind::InvalidFilename`) - rust-lang#137504 (Move methods from Map to TyCtxt, part 4.) - rust-lang#138175 (Support rmeta inputs for --crate-type=bin --emit=obj) - rust-lang#138259 (Disentangle `ForwardGenericParamBan` and `ConstParamTy` ribs) - rust-lang#138280 (fix ICE in pretty-printing `global_asm!`) - rust-lang#138318 (Rustdoc: remove a bunch of `@ts-expect-error` from main.js) - rust-lang#138331 (Use `RUSTC_LINT_FLAGS` more) - rust-lang#138357 (merge `TypeChecker` and `TypeVerifier`) - rust-lang#138394 (remove unnecessary variant) - rust-lang#138403 (Delegation: one more ICE fix for `MethodCall` generation) - rust-lang#138407 (Delegation: reject C-variadics) - rust-lang#138409 (Use sa_sigaction instead of sa_union.__su_sigaction for AIX) r? `@ghost` `@rustbot` modify labels: rollup

Manishearth · 2025-03-12T23:08:45Z

Is it possible this rollup failure is caused by this PR?

#138416 (comment)

compiler-errors · 2025-03-12T23:10:20Z

Likely not @Manishearth

jieyouxu · 2025-03-12T23:14:35Z

(Unrelated, rollup failure is spurious MSVC filesystem problems)

…earth Rollup of 12 pull requests Successful merges: - rust-lang#134076 (Stabilize `std::io::ErrorKind::InvalidFilename`) - rust-lang#137504 (Move methods from Map to TyCtxt, part 4.) - rust-lang#138175 (Support rmeta inputs for --crate-type=bin --emit=obj) - rust-lang#138259 (Disentangle `ForwardGenericParamBan` and `ConstParamTy` ribs) - rust-lang#138280 (fix ICE in pretty-printing `global_asm!`) - rust-lang#138318 (Rustdoc: remove a bunch of `@ts-expect-error` from main.js) - rust-lang#138331 (Use `RUSTC_LINT_FLAGS` more) - rust-lang#138357 (merge `TypeChecker` and `TypeVerifier`) - rust-lang#138394 (remove unnecessary variant) - rust-lang#138403 (Delegation: one more ICE fix for `MethodCall` generation) - rust-lang#138407 (Delegation: reject C-variadics) - rust-lang#138409 (Use sa_sigaction instead of sa_union.__su_sigaction for AIX) r? `@ghost` `@rustbot` modify labels: rollup

rustbot assigned nnethercote Mar 9, 2025

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 Mar 9, 2025

rustbot assigned compiler-errors and unassigned nnethercote Mar 9, 2025

rustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Mar 9, 2025

compiler-errors reviewed Mar 9, 2025

View reviewed changes

compiler/rustc_middle/src/mir/pretty.rs Outdated Show resolved Hide resolved

compiler-errors reviewed Mar 9, 2025

View reviewed changes

tests/ui/unpretty/mir/global-asm-const.rs Outdated Show resolved Hide resolved

tests/ui/unpretty/mir/global-asm-const.rs Outdated Show resolved Hide resolved

folkertdev changed the title ~~fix ICE in pretty-printing global_asm! const operands~~ fix ICE in pretty-printing global_asm! Mar 9, 2025

folkertdev force-pushed the mir-dump-asm-const branch from 65d9a66 to 4613650 Compare March 9, 2025 19:11

folkertdev commented Mar 9, 2025

View reviewed changes

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 9, 2025

folkertdev force-pushed the mir-dump-asm-const branch from 4613650 to b394c8e Compare March 9, 2025 19:40

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 Mar 9, 2025

matthiaskrgr mentioned this pull request Mar 9, 2025

Rollup of 6 pull requests #138289

Closed

matthiaskrgr mentioned this pull request Mar 10, 2025

Rollup of 9 pull requests #138298

Closed

folkertdev force-pushed the mir-dump-asm-const branch from b394c8e to 73b7a5e Compare March 10, 2025 08:49

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 10, 2025

compiler-errors suggested changes Mar 10, 2025

View reviewed changes

rustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Mar 10, 2025

fix ICE in pretty-printing global_asm!

9213cb8

folkertdev force-pushed the mir-dump-asm-const branch from 73b7a5e to 9213cb8 Compare March 10, 2025 13:46

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 12, 2025

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 Mar 12, 2025

Manishearth mentioned this pull request Mar 12, 2025

Rollup of 12 pull requests #138416

Merged

bors merged commit f5eb296 into rust-lang:master Mar 13, 2025
7 checks passed

rustbot added this to the 1.87.0 milestone Mar 13, 2025

		@@ -0,0 +1,7 @@
		//@ compile-flags: -Zdump-mir=all --crate-type=lib

		@@ -0,0 +1,18 @@
		// skip-filecheck
		#![feature(no_core, lang_items, rustc_attrs, decl_macro)]

Uh oh!

Conversation

folkertdev commented Mar 9, 2025 • edited by matthiaskrgr Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Mar 9, 2025

Uh oh!

compiler-errors commented Mar 9, 2025

Uh oh!

compiler-errors commented Mar 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

folkertdev Mar 9, 2025

Choose a reason for hiding this comment

Uh oh!

compiler-errors Mar 9, 2025

Choose a reason for hiding this comment

Uh oh!

compiler-errors commented Mar 9, 2025

Uh oh!

folkertdev commented Mar 9, 2025

Uh oh!

folkertdev commented Mar 9, 2025

Uh oh!

compiler-errors commented Mar 9, 2025

Uh oh!

bors commented Mar 9, 2025

Uh oh!

folkertdev commented Mar 10, 2025

Uh oh!

compiler-errors Mar 10, 2025

Choose a reason for hiding this comment

Uh oh!

folkertdev Mar 10, 2025

Choose a reason for hiding this comment

Uh oh!

compiler-errors commented Mar 10, 2025

Uh oh!

matthiaskrgr commented Mar 10, 2025

Uh oh!

bors commented Mar 10, 2025

Uh oh!

bors commented Mar 10, 2025

Uh oh!

folkertdev commented Mar 12, 2025

Uh oh!

compiler-errors commented Mar 12, 2025

Uh oh!

bors commented Mar 12, 2025

Uh oh!

Manishearth commented Mar 12, 2025

Uh oh!

compiler-errors commented Mar 12, 2025

Uh oh!

jieyouxu commented Mar 12, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

folkertdev commented Mar 9, 2025 •

edited by matthiaskrgr

Loading

compiler-errors commented Mar 9, 2025 •

edited

Loading