fix ICE in pretty-printing `global_asm!` #138280

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

bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Mar 10, 2025

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 requested 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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix ICE in pretty-printing `global_asm!` #138280

fix ICE in pretty-printing `global_asm!` #138280

folkertdev commented Mar 9, 2025 •

edited by matthiaskrgr

Loading

rustbot commented Mar 9, 2025

compiler-errors commented Mar 9, 2025

compiler-errors commented Mar 9, 2025 •

edited

Loading

folkertdev Mar 9, 2025

compiler-errors Mar 9, 2025

compiler-errors commented Mar 9, 2025

folkertdev commented Mar 9, 2025

folkertdev commented Mar 9, 2025

compiler-errors commented Mar 9, 2025

bors commented Mar 9, 2025

folkertdev commented Mar 10, 2025

compiler-errors Mar 10, 2025

folkertdev Mar 10, 2025

compiler-errors commented Mar 10, 2025

matthiaskrgr commented Mar 10, 2025

bors commented Mar 10, 2025

bors commented Mar 10, 2025

folkertdev commented Mar 12, 2025

compiler-errors commented Mar 12, 2025

bors commented Mar 12, 2025

Manishearth commented Mar 12, 2025

compiler-errors commented Mar 12, 2025

jieyouxu commented Mar 12, 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)]

fix ICE in pretty-printing global_asm! #138280

fix ICE in pretty-printing global_asm! #138280

Conversation

folkertdev commented Mar 9, 2025 • edited by matthiaskrgr Loading

rustbot commented Mar 9, 2025

compiler-errors commented Mar 9, 2025

compiler-errors commented Mar 9, 2025 • edited Loading

folkertdev Mar 9, 2025

Choose a reason for hiding this comment

compiler-errors Mar 9, 2025

Choose a reason for hiding this comment

compiler-errors commented Mar 9, 2025

folkertdev commented Mar 9, 2025

folkertdev commented Mar 9, 2025

compiler-errors commented Mar 9, 2025

bors commented Mar 9, 2025

folkertdev commented Mar 10, 2025

compiler-errors Mar 10, 2025

Choose a reason for hiding this comment

folkertdev Mar 10, 2025

Choose a reason for hiding this comment

compiler-errors commented Mar 10, 2025

matthiaskrgr commented Mar 10, 2025

bors commented Mar 10, 2025

bors commented Mar 10, 2025

folkertdev commented Mar 12, 2025

compiler-errors commented Mar 12, 2025

bors commented Mar 12, 2025

Manishearth commented Mar 12, 2025

compiler-errors commented Mar 12, 2025

jieyouxu commented Mar 12, 2025

fix ICE in pretty-printing `global_asm!` #138280

fix ICE in pretty-printing `global_asm!` #138280

folkertdev commented Mar 9, 2025 •

edited by matthiaskrgr

Loading

compiler-errors commented Mar 9, 2025 •

edited

Loading