Suppress "const" prefix of FnDef constants in MIR dump #75697

tesuji · 2020-08-19T08:45:45Z

I was asked to suppress the const infront of FnDef.
I tried to suppress comments for other types, but turned out that const () and () is different: #75697 (comment)

tesuji · 2020-08-19T08:47:11Z

cc @mark-i-m Do I need to update MIR dump in rustc-dev-guide somewhere?

oli-obk · 2020-08-19T08:48:42Z

cc @rust-lang/wg-mir-opt

tesuji · 2020-08-19T09:48:32Z

src/test/ui/consts/const-extern-fn/const-extern-fn-call-extern-fn.stderr

@@ -1,4 +1,4 @@
-error[E0723]: can only call other `const fn` within a `const fn`, but `const regular_in_block` is not stable as `const fn`
+error[E0723]: can only call other `const fn` within a `const fn`, but `regular_in_block` is not stable as `const fn`


Hm, this is interesting!

bors · 2020-08-19T20:47:07Z

☔ The latest upstream changes (presumably #75715) made this pull request unmergeable. Please resolve the merge conflicts.

tesuji · 2020-08-20T11:06:56Z

cc @bjorn3 for your work with MIR in cg_cranelift .

bjorn3 · 2020-08-20T11:14:18Z

src/test/mir-opt/address_of.borrow_and_cast.SimplifyCfg-initial.after.mir

@@ -38,7 +38,7 @@ fn borrow_and_cast(_1: i32) -> () {
        _6 = &raw mut (*_7);             // scope 2 at $DIR/address-of.rs:44:13: 44:19
        FakeRead(ForLet, _6);            // scope 2 at $DIR/address-of.rs:44:9: 44:10
        StorageDead(_7);                 // scope 2 at $DIR/address-of.rs:44:31: 44:32
-        _0 = const ();                   // scope 0 at $DIR/address-of.rs:41:32: 45:2
+        _0 = ();                         // scope 0 at $DIR/address-of.rs:41:32: 45:2


There is a difference between these two. const () is a Constant, while () is an Aggregate.

bjorn3 · 2020-08-20T11:15:00Z

src/test/mir-opt/array_index_is_temporary.main.SimplifyCfg-elaborate-drops.after.mir.32bit

@@ -25,9 +25,9 @@ fn main() -> () {

    bb0: {
        StorageLive(_1);                 // scope 0 at $DIR/array-index-is-temporary.rs:13:9: 13:14
-        _1 = [const 42_u32, const 43_u32, const 44_u32]; // scope 0 at $DIR/array-index-is-temporary.rs:13:17: 13:29
+        _1 = [42_u32, 43_u32, 44_u32];   // scope 0 at $DIR/array-index-is-temporary.rs:13:17: 13:29


Similarily here, though I am not sure if the Constant could be printed as an array, or would be printed as a raw allocation.

it can be printed as an array, so yea, there would be no visible difference in the mir

I'm wondering whether the information const vs Aggregate is useful, but the original comment by me that motivated @lzutao to open this PR was solely for FnDef, where the const is actively confusing (at least in the position for function calls. Maybe we should just special case function calls to not print a const?

I don't feel super strongly but I would prefer if this was only done for FnDefs right now.

tesuji · 2020-08-20T12:29:38Z

Thanks for all comments. Now I only suppress "const" prefix for FnDef.

bors · 2020-08-20T15:17:39Z

☔ The latest upstream changes (presumably #75670) made this pull request unmergeable. Please resolve the merge conflicts.

oli-obk · 2020-08-20T15:51:21Z

@bors r+ rollup=iffy

bors · 2020-08-20T15:51:23Z

📌 Commit 4019330ee337e28ac2869ebbc47884f819c269b2 has been approved by oli-obk

tesuji · 2020-08-20T15:53:57Z

Sorry, forgot to edit the commit message. It should be more correct now.

oli-obk · 2020-08-20T16:03:22Z

@bors r+

bors · 2020-08-20T16:03:23Z

📌 Commit 3b853f396dcbcbb5ddf94373526e7598184cee1f has been approved by oli-obk

bors · 2020-08-20T18:26:37Z

☔ The latest upstream changes (presumably #75562) made this pull request unmergeable. Please resolve the merge conflicts.

mark-i-m · 2020-08-20T19:16:27Z

cc @mark-i-m Do I need to update MIR dump in rustc-dev-guide somewhere?

To be honest, I'm not sure. I don't remember how much we have about MIR dump. Perhaps someone else from @rust-lang/wg-rustc-dev-guide knows?

JohnTitor · 2020-08-20T19:29:49Z

@lzutao @mark-i-m We have https://rustc-dev-guide.rust-lang.org/mir/debugging.html but it doesn't seem to be affected.

wesleywiser · 2020-08-21T00:55:31Z

@bors r=oli-obk

bors · 2020-08-21T00:55:32Z

📌 Commit c4c017a has been approved by oli-obk

bors · 2020-08-21T02:06:47Z

⌛ Testing commit c4c017a with merge fbc600c60e44097b00ddc90cee2548c4e59d7610...

bors · 2020-08-21T02:16:30Z

💔 Test failed - checks-azure

tesuji · 2020-08-21T04:28:09Z

Spurious error,

warning: spurious network error (2 tries remaining): [28] Timeout was reached (failed to download any data for `quick-error v1.2.3` within 30s)
warning: spurious network error (1 tries remaining): [28] Timeout was reached (failed to download any data for `quick-error v1.2.3` within 30s)
error: failed to download from `https://crates.io/api/v1/crates/quick-error/1.2.3/download`

Caused by:
  [28] Timeout was reached (failed to download any data for `quick-error v1.2.3` within 30s)

oli-obk · 2020-08-21T07:27:43Z

@bors retry

bors · 2020-08-21T07:57:07Z

⌛ Testing commit c4c017a with merge b374559b5a3ce28efb78626eab441eb0c1e176d6...

bors · 2020-08-21T09:00:46Z

💔 Test failed - checks-actions

oli-obk · 2020-08-21T09:24:20Z

this appears to break rustfmt?

JohnTitor · 2020-08-21T09:53:25Z

Hm, I don't think it's caused by this PR, though I'm not sure if it's spurious or not.

2020-08-21T08:54:27.4115722Z �[0m�[1m�[38;5;10mnote�[0m�[0m�[1m�[38;5;15m: `link.exe` returned an unexpected error�[0m
2020-08-21T08:54:27.4116018Z 
2020-08-21T08:54:27.4116240Z �[0m�[1m�[38;5;10mnote�[0m�[0m�[1m�[38;5;15m: the Visual Studio build tools may need to be repaired using the Visual Studio installer�[0m
2020-08-21T08:54:27.4116388Z 
2020-08-21T08:54:27.4116584Z �[0m�[1m�[38;5;10mnote�[0m�[0m�[1m�[38;5;15m: or a necessary component may be missing from the "C++ build tools" workload�[0m
2020-08-21T08:54:27.4116722Z 
2020-08-21T08:54:27.4128039Z �[0m�[1m�[38;5;9merror�[0m�[0m�[1m�[38;5;15m: aborting due to previous error�[0m
2020-08-21T08:54:27.4128217Z 
2020-08-21T08:54:27.4220384Z [RUSTC-TIMING] build_script_build test:false 2.622
2020-08-21T08:54:27.4233457Z �[0m�[0m�[1m�[31merror�[0m�[1m:�[0m could not compile `stacker`.
2020-08-21T08:54:27.4233664Z 
2020-08-21T08:54:27.4233931Z To learn more, run the command again with --verbose.
2020-08-21T08:54:27.4234190Z �[0m�[0m�[1m�[33mwarning�[0m�[1m:�[0m build failed, waiting for other jobs to finish...
2020-08-21T08:54:28.2375366Z [RUSTC-TIMING] build_script_build test:false 3.434
2020-08-21T08:54:31.5274931Z �[0m�[0m�[1m�[31merror�[0m�[1m:�[0m build failed
2020-08-21T08:54:31.5345517Z command did not execute successfully: "D:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\stage0\\bin\\cargo.exe" "build" "--target" "x86_64-pc-windows-msvc" "-Zbinary-dep-depinfo" "-j" "8" "--release" "--locked" "--color" "always" "--manifest-path" "D:\\a\\rust\\rust\\src/tools/rustfmt\\Cargo.toml" "--features" "rustc-workspace-hack/all-static" "--message-format" "json-render-diagnostics"
2020-08-21T08:54:31.5346495Z expected success, got: exit code: 101
2020-08-21T08:54:31.5368358Z [TIMING] ToolBuild { compiler: Compiler { stage: 2, host: TargetSelection { triple: "x86_64-pc-windows-msvc", file: None } }, target: TargetSelection { triple: "x86_64-pc-windows-msvc", file: None }, tool: "rustfmt", path: "src/tools/rustfmt", mode: ToolRustc, is_optional_tool: true, source_type: Submodule, extra_features: [] } -- 15.722
2020-08-21T08:54:31.5368933Z failed to test rustfmt: could not build

mati865 · 2020-08-21T10:49:16Z

Has to be spurious: error: linking with link.exe failed: exit code: 0xc0000005
Changes in this PR are extremely unlikely to crash MSVC linker.

JohnTitor · 2020-08-21T10:51:50Z

Thanks, @mati865. And the builder on #75765 passes so let's try it again.
@bors retry

bors · 2020-08-21T11:08:36Z

⌛ Testing commit c4c017a with merge 521db88...

bors · 2020-08-21T13:32:06Z

☀️ Test successful - checks-actions, checks-azure
Approved by: oli-obk
Pushing 521db88 to master...

rust-highfive assigned oli-obk Aug 19, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 19, 2020

tesuji commented Aug 19, 2020

View reviewed changes

tesuji force-pushed the mir-dumb-const-prefix branch 3 times, most recently from 1941082 to 4089aad Compare August 20, 2020 11:04

bjorn3 reviewed Aug 20, 2020

View reviewed changes

tesuji force-pushed the mir-dumb-const-prefix branch from 4089aad to 62cff09 Compare August 20, 2020 12:21

tesuji changed the title ~~Suppress "const" prefix of constants in MIR dump~~ Suppress "const" prefix of FnDef constants in MIR dump Aug 20, 2020

tesuji marked this pull request as ready for review August 20, 2020 12:36

tesuji force-pushed the mir-dumb-const-prefix branch from 62cff09 to 4019330 Compare August 20, 2020 15:48

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 Aug 20, 2020

tesuji force-pushed the mir-dumb-const-prefix branch from 4019330 to 3b853f3 Compare August 20, 2020 15:53

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 Aug 20, 2020

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 Aug 21, 2020

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 21, 2020

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 Aug 21, 2020

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 21, 2020

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 Aug 21, 2020

bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 21, 2020

bors merged commit 521db88 into rust-lang:master Aug 21, 2020

tesuji deleted the mir-dumb-const-prefix branch August 21, 2020 13:33

cuviper added this to the 1.47.0 milestone May 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Suppress "const" prefix of FnDef constants in MIR dump #75697

Suppress "const" prefix of FnDef constants in MIR dump #75697

tesuji commented Aug 19, 2020 •

edited

Loading

tesuji commented Aug 19, 2020

oli-obk commented Aug 19, 2020

tesuji Aug 19, 2020

bors commented Aug 19, 2020

tesuji commented Aug 20, 2020

bjorn3 Aug 20, 2020

bjorn3 Aug 20, 2020

oli-obk Aug 20, 2020

oli-obk Aug 20, 2020

wesleywiser Aug 20, 2020 •

edited

Loading

tesuji commented Aug 20, 2020

bors commented Aug 20, 2020

oli-obk commented Aug 20, 2020

bors commented Aug 20, 2020

tesuji commented Aug 20, 2020

oli-obk commented Aug 20, 2020

bors commented Aug 20, 2020

bors commented Aug 20, 2020

mark-i-m commented Aug 20, 2020

JohnTitor commented Aug 20, 2020

wesleywiser commented Aug 21, 2020

bors commented Aug 21, 2020

bors commented Aug 21, 2020

bors commented Aug 21, 2020

tesuji commented Aug 21, 2020

oli-obk commented Aug 21, 2020

bors commented Aug 21, 2020

bors commented Aug 21, 2020

oli-obk commented Aug 21, 2020

JohnTitor commented Aug 21, 2020

mati865 commented Aug 21, 2020

JohnTitor commented Aug 21, 2020

bors commented Aug 21, 2020

bors commented Aug 21, 2020

		@@ -1,4 +1,4 @@
		error[E0723]: can only call other `const fn` within a `const fn`, but `const regular_in_block` is not stable as `const fn`
		error[E0723]: can only call other `const fn` within a `const fn`, but `regular_in_block` is not stable as `const fn`

Suppress "const" prefix of FnDef constants in MIR dump #75697

Suppress "const" prefix of FnDef constants in MIR dump #75697

Conversation

tesuji commented Aug 19, 2020 • edited Loading

tesuji commented Aug 19, 2020

oli-obk commented Aug 19, 2020

tesuji Aug 19, 2020

Choose a reason for hiding this comment

bors commented Aug 19, 2020

tesuji commented Aug 20, 2020

bjorn3 Aug 20, 2020

Choose a reason for hiding this comment

bjorn3 Aug 20, 2020

Choose a reason for hiding this comment

oli-obk Aug 20, 2020

Choose a reason for hiding this comment

oli-obk Aug 20, 2020

Choose a reason for hiding this comment

wesleywiser Aug 20, 2020 • edited Loading

Choose a reason for hiding this comment

tesuji commented Aug 20, 2020

bors commented Aug 20, 2020

oli-obk commented Aug 20, 2020

bors commented Aug 20, 2020

tesuji commented Aug 20, 2020

oli-obk commented Aug 20, 2020

bors commented Aug 20, 2020

bors commented Aug 20, 2020

mark-i-m commented Aug 20, 2020

JohnTitor commented Aug 20, 2020

wesleywiser commented Aug 21, 2020

bors commented Aug 21, 2020

bors commented Aug 21, 2020

bors commented Aug 21, 2020

tesuji commented Aug 21, 2020

oli-obk commented Aug 21, 2020

bors commented Aug 21, 2020

bors commented Aug 21, 2020

oli-obk commented Aug 21, 2020

JohnTitor commented Aug 21, 2020

mati865 commented Aug 21, 2020

JohnTitor commented Aug 21, 2020

bors commented Aug 21, 2020

bors commented Aug 21, 2020

tesuji commented Aug 19, 2020 •

edited

Loading

wesleywiser Aug 20, 2020 •

edited

Loading