Clarify which kinds of MIR are allowed during which phases. #94655

JakobDegen · 2022-03-06T01:45:24Z

This enhances documentation with these details and extends the validator to check these requirements more thoroughly. Most of these conditions were already being checked.

There was also some disagreement between the MirPhase docs and validator as to what it meant for the body.phase field to have a certain value. This PR resolves those disagreements in favor of the MirPhase docs (which is what the pass manager implemented), adjusting the validator accordingly. The result is now that the DropLowering phase begins with the end of the elaborate drops pass, and lasts until the beginning of the generator lowring pass. This doesn't feel entirely natural to me, but as long as it's documented accurately it should be ok.

r? rust-lang/mir-opt

compiler/rustc_const_eval/src/transform/validate.rs

JakobDegen · 2022-03-06T20:54:33Z

The new commit adds an two unrelated documentation improvements that I wanted to tack onto some PR

compiler/rustc_middle/src/mir/mod.rs

JakobDegen · 2022-03-17T22:46:09Z

Added the new phase and renamed the existing phases.

@rustbot ready

oli-obk · 2022-03-23T16:31:22Z

@bors r+

Thanks! this is awesome

bors · 2022-03-23T16:51:40Z

📌 Commit 90810c0eabd718ec573878c9bf6d2ffc11896f38 has been approved by oli-obk

bors · 2022-03-23T19:44:52Z

⌛ Testing commit 90810c0eabd718ec573878c9bf6d2ffc11896f38 with merge 70e77598bd1415c77053c17bb47f6371b0683c66...

bors · 2022-03-23T20:18:48Z

💔 Test failed - checks-actions

JakobDegen · 2022-03-23T22:32:45Z

Yeah, documentation is not one of the tests I usually run locally. New commits switch Terminator out for TerminatorKind

This enhances documentation with these details and extends the validator to check these requirements more thoroughly. As a part of this, we add a new `Deaggregated` phase, and rename other phases so that their names more naturally correspond to what they represent.

oli-obk · 2022-03-24T06:56:29Z

@bors r+

bors · 2022-03-24T06:56:31Z

📌 Commit 26d7b8d has been approved by oli-obk

bors · 2022-03-24T08:35:05Z

⌛ Testing commit 26d7b8d with merge a566bb2d22e9a17a6090811d09c5d96db02d7bc0...

bors · 2022-03-24T09:11:57Z

💔 Test failed - checks-actions

rust-log-analyzer · 2022-03-24T09:17:03Z

The job x86_64-gnu-tools failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

error: 1 unexpected errors found, 1 expected errors not found
status: exit status: 1
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools-bin/miri" "tests/compile-fail/null_pointer_deref.rs" "-L" "/tmp/compiletestSKklR3" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-C" "prefer-dynamic" "-o" "/tmp/compiletestSKklR3/null_pointer_deref.stage-id" "-A" "unused" "--edition" "2018" "-Astable-features" "--sysroot" "/home/user/.cache/miri/HOST" "-L" "/tmp/compiletestSKklR3/null_pointer_deref.stage-id.aux"
    Error {
        line_num: 3,
        kind: Some(
            Error,
---
error: tests/compile-fail/null_pointer_write.rs:3: expected error not found: null pointer is not a valid pointer for this operation

error: 1 unexpected errors found, 1 expected errors not found
status: exit status: 1
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools-bin/miri" "tests/compile-fail/null_pointer_write.rs" "-L" "/tmp/compiletestSKklR3" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-C" "prefer-dynamic" "-o" "/tmp/compiletestSKklR3/null_pointer_write.stage-id" "-A" "unused" "--edition" "2018" "-Astable-features" "--sysroot" "/home/user/.cache/miri/HOST" "-L" "/tmp/compiletestSKklR3/null_pointer_write.stage-id.aux"
    Error {
        line_num: 3,
        kind: Some(
            Error,
---
.......... (60/61)
          (61/61)


/checkout/src/test/rustdoc-gui/search-tab-selection-if-current-is-empty.goml search-tab-selection-if-current-is-empty... FAILED
[ERROR] (line 6) TimeoutError: waiting for selector "#titles" failed: timeout 30000ms exceeded: for command `wait-for: "#titles"`
[ERROR] (line 7) "#titles > button:nth-of-type(1)" not found: for command `assert-attribute: ("#titles > button:nth-of-type(1)", {"class": "selected"})`
Build completed unsuccessfully in 0:00:48

oli-obk · 2022-03-24T10:24:22Z

oh... we are in beta breakage prevention week already. I guess the best thing would be to wait for beta to branch and then merge this PR. miri can be patched up easily afterwards. Doing it during beta week is hard

JakobDegen · 2022-03-24T10:58:58Z

Was that two different unrelated failures? The miri failure and the rustdoc timeout? (how did this even cause the miri failure?)

oli-obk · 2022-03-24T16:58:54Z

hmm.. the miri failure looks like the one in a recent miri update PR. so possibly not related to your PR... maybe we broke miri before beta breakage week and now every PR errors due to that? looking into it, this is indeed not related to this PR

Clarify which kinds of MIR are allowed during which phases. This enhances documentation with these details and extends the validator to check these requirements more thoroughly. Most of these conditions were already being checked. There was also some disagreement between the `MirPhase` docs and validator as to what it meant for the `body.phase` field to have a certain value. This PR resolves those disagreements in favor of the `MirPhase` docs (which is what the pass manager implemented), adjusting the validator accordingly. The result is now that the `DropLowering` phase begins with the end of the elaborate drops pass, and lasts until the beginning of the generator lowring pass. This doesn't feel entirely natural to me, but as long as it's documented accurately it should be ok. r? rust-lang/mir-opt

Dylan-DPC · 2022-03-24T20:47:34Z

@bors r-

(till we solve the miri issue )

Clarify which kinds of MIR are allowed during which phases. This enhances documentation with these details and extends the validator to check these requirements more thoroughly. Most of these conditions were already being checked. There was also some disagreement between the `MirPhase` docs and validator as to what it meant for the `body.phase` field to have a certain value. This PR resolves those disagreements in favor of the `MirPhase` docs (which is what the pass manager implemented), adjusting the validator accordingly. The result is now that the `DropLowering` phase begins with the end of the elaborate drops pass, and lasts until the beginning of the generator lowring pass. This doesn't feel entirely natural to me, but as long as it's documented accurately it should be ok. r? rust-lang/mir-opt

JakobDegen · 2022-03-24T22:13:47Z

Yeah, and the rustdoc timeout seems possibly spurious? Also unsure how I would have caused that

Rollup of 5 pull requests Successful merges: - rust-lang#94391 (Fix ice when error reporting recursion errors) - rust-lang#94655 (Clarify which kinds of MIR are allowed during which phases.) - rust-lang#95179 (Try to evaluate in try unify and postpone resolution of constants that contain inference variables) - rust-lang#95270 (debuginfo: Fix debuginfo for Box<T> where T is unsized.) - rust-lang#95276 (add diagnostic items for clippy's `trim_split_whitespace`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

rust-highfive assigned oli-obk Mar 6, 2022

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 6, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 6, 2022

JakobDegen commented Mar 6, 2022

View reviewed changes

compiler/rustc_const_eval/src/transform/validate.rs Outdated Show resolved Hide resolved

JakobDegen force-pushed the mir-phase-docs branch from 534331f to 9cdcbbf Compare March 7, 2022 00:20

oli-obk reviewed Mar 7, 2022

View reviewed changes

compiler/rustc_middle/src/mir/mod.rs Show resolved Hide resolved

oli-obk 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, 2022

JakobDegen force-pushed the mir-phase-docs branch from 9cdcbbf to 90810c0 Compare March 17, 2022 22:45

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 17, 2022

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 23, 2022

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 Mar 23, 2022

This comment has been minimized.

Sign in to view

JakobDegen force-pushed the mir-phase-docs branch from 90810c0 to 00938bc Compare March 23, 2022 22:31

JakobDegen added 2 commits March 23, 2022 18:34

Clarify more MIR docs

26d7b8d

JakobDegen force-pushed the mir-phase-docs branch from 00938bc to 26d7b8d Compare March 23, 2022 22:34

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 24, 2022

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 Mar 24, 2022

Dylan-DPC mentioned this pull request Mar 24, 2022

Rollup of 5 pull requests #95279

Closed

Dylan-DPC mentioned this pull request Mar 24, 2022

Rollup of 6 pull requests #95287

Closed

Dylan-DPC mentioned this pull request Mar 24, 2022

Rollup of 5 pull requests #95288

Closed

Dylan-DPC mentioned this pull request Mar 25, 2022

Rollup of 5 pull requests #95291

Merged

bors merged commit c66e0c8 into rust-lang:master Mar 25, 2022

rustbot added this to the 1.61.0 milestone Mar 25, 2022

JakobDegen deleted the mir-phase-docs branch August 10, 2022 00:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clarify which kinds of MIR are allowed during which phases. #94655

Clarify which kinds of MIR are allowed during which phases. #94655

JakobDegen commented Mar 6, 2022

JakobDegen commented Mar 6, 2022 •

edited

Loading

JakobDegen commented Mar 17, 2022

oli-obk commented Mar 23, 2022

bors commented Mar 23, 2022

bors commented Mar 23, 2022

bors commented Mar 23, 2022

This comment has been minimized.

JakobDegen commented Mar 23, 2022

oli-obk commented Mar 24, 2022

bors commented Mar 24, 2022

bors commented Mar 24, 2022

bors commented Mar 24, 2022

rust-log-analyzer commented Mar 24, 2022

oli-obk commented Mar 24, 2022

JakobDegen commented Mar 24, 2022

oli-obk commented Mar 24, 2022

Dylan-DPC commented Mar 24, 2022

JakobDegen commented Mar 24, 2022

Clarify which kinds of MIR are allowed during which phases. #94655

Clarify which kinds of MIR are allowed during which phases. #94655

Conversation

JakobDegen commented Mar 6, 2022

JakobDegen commented Mar 6, 2022 • edited Loading

JakobDegen commented Mar 17, 2022

oli-obk commented Mar 23, 2022

bors commented Mar 23, 2022

bors commented Mar 23, 2022

bors commented Mar 23, 2022

This comment has been minimized.

JakobDegen commented Mar 23, 2022

oli-obk commented Mar 24, 2022

bors commented Mar 24, 2022

bors commented Mar 24, 2022

bors commented Mar 24, 2022

rust-log-analyzer commented Mar 24, 2022

oli-obk commented Mar 24, 2022

JakobDegen commented Mar 24, 2022

oli-obk commented Mar 24, 2022

Dylan-DPC commented Mar 24, 2022

JakobDegen commented Mar 24, 2022

JakobDegen commented Mar 6, 2022 •

edited

Loading