Fix misc printing issues in emit=stable_mir by ouz-a · Pull Request #118364 · rust-lang/rust (original) (raw)
Previously our } closed early and looked bit weird, now it closes when function call ends, and previously Adt printing was too verbose now it's similar to mir output
r? @celinval
Status: Awaiting review from the assignee but also interested parties.
Relevant to the compiler team, which will review and decide on the PR/issue.
labels
I've asked before but I will ask again -- is it possible for you to prioritize working on some sort of test?
It's kinda sketchy to be putting up PRs like this with absolutely no evidence that it's doing anything.
I've asked before but I will ask again -- is it possible for you to prioritize working on some sort of test?
It's kinda sketchy to be putting up PRs like this with absolutely no evidence that it's doing anything.
Alright will start working on a test immediately
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
…=compiler-errors
Add -Zunpretty=stable-mir output test
As strongly suggested here rust-lang#118364 (comment) this adds output test for -Zunpretty=stable-mir, added test shows almost all the functionality of the current printer.
r? @compiler-errors
Do you mind rebasing this on top of #118375? Then you can bless the tests in your commit and see the output changes due to these tweaks.
Area: The testsuite used to check the correctness of rustc
Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
labels
Do you mind rebasing this on top of #118375? Then you can bless the tests in your commit and see the output changes due to these tweaks.
Just rebased and run bless
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a good opportunity to make more tweaks to the output since I can now see what's broken 😃 thanks for that
Also, plz squash the "rebase" commit into the "fix" commit.
I think this is a good opportunity to make more tweaks to the output since I can now see what's broken 😃 thanks for that
Also, plz squash the "rebase" commit into the "fix" commit.
No problem, it's better for someone other than me to also look at this and have some opinion about it 😄
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request
…=compiler-errors
Add -Zunpretty=stable-mir output test
As strongly suggested here rust-lang#118364 (comment) this adds output test for -Zunpretty=stable-mir, added test shows almost all the functionality of the current printer.
r? @compiler-errors
This comment has been minimized.
This comment has been minimized.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed that the successor for Goto terminators is missing the bb in the label. For example, we're printing something like:
when I expected:
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
…=celinval
Add -Zunpretty=stable-mir output test
As strongly suggested here rust-lang#118364 (comment) this adds output test for -Zunpretty=stable-mir, added test shows almost all the functionality of the current printer.
r? @compiler-errors
bors added a commit to rust-lang-ci/rust that referenced this pull request
…elinval
Add -Zunpretty=stable-mir output test
As strongly suggested here rust-lang#118364 (comment) this adds output test for -Zunpretty=stable-mir, added test shows almost all the functionality of the current printer.
r? @compiler-errors
bors added a commit to rust-lang-ci/rust that referenced this pull request
…elinval
Add -Zunpretty=stable-mir output test
As strongly suggested here rust-lang#118364 (comment) this adds output test for -Zunpretty=stable-mir, added test shows almost all the functionality of the current printer.
r? @compiler-errors
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request
Add -Zunpretty=stable-mir output test
As strongly suggested here rust-lang/rust#118364 (comment) this adds output test for -Zunpretty=stable-mir, added test shows almost all the functionality of the current printer.
r? @compiler-errors
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking so much better. I'm hoping we can merge this after the next revision. Btw, have you compared the expected file with the internal MIR dump? It might be helpful to spot what's missing or what's wrong.
| _2 = 1 Add const 1_i32 |
|---|
| assert(!move _2 bool),"attempt to compute `{} + {}`, which would overflow", 1, const 1_i32) -> [success: bb1, unwind continue] |
| _2 = CheckedAdd(_1, const 1_i32) |
| assert(!move _2 bool),"attempt to compute `{} + {}`, which would overflow", _1, const 1_i32) -> [success: bb1, unwind continue] |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where's this bool coming from?
| bb0: { |
|---|
| _3 = refShared1 |
| _3 = &1 |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be _1?
| _5 = refMut { |
|---|
| kind: TwoPhaseBorrow, |
| }2 |
| _5 = &mut 2 |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where's _5 declaration?
| } |
|---|
| fn bar(_0: &mut std::vec::Vec) -> std::vec::Vec { |
| let mut _0: std::vec::Vec; |
| let mut _1: &std::vec::Vec; |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we missing _1 initialization?
This is looking so much better. I'm hoping we can merge this after the next revision. Btw, have you compared the expected file with the internal MIR dump? It might be helpful to spot what's missing or what's wrong.
Will go trough another pass and then compare with mir output
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
@ouz-a anything interesting to report w.r.t. to your prev. comment? No hurry, I'll just flip the review switch
@rustbot author
I been meaning to look at this for some time but I been extremely busy with life/job in past couple months 😓
Hey @ouz-a do you mind if I take a stab at this?
Hey @ouz-a do you mind if I take a stab at this?
Yeah, go ahead.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
…rrors
Fix misc printing issues in emit=stable_mir
Trying to continue the work that @ouz-a started here: rust-lang#118364
Few modifications beyond fixes:
- I made the
pretty_*functions private. - I added a function to print the instance body
- Changed a bunch of signatures to write to the writer directly.
- Added a function to translate the place to its internal representation, so we could use the internal debug implementation.
- Also removed
pretty_ty, replaced by Display implementation of Ty which uses the internal display.
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request
…rrors
Fix misc printing issues in emit=stable_mir
Trying to continue the work that @ouz-a started here: rust-lang#118364
Few modifications beyond fixes:
- I made the
pretty_*functions private. - I added a function to print the instance body
- Changed a bunch of signatures to write to the writer directly.
- Added a function to translate the place to its internal representation, so we could use the internal debug implementation.
- Also removed
pretty_ty, replaced by Display implementation of Ty which uses the internal display.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
…rrors
Fix misc printing issues in emit=stable_mir
Trying to continue the work that @ouz-a started here: rust-lang#118364
Few modifications beyond fixes:
- I made the
pretty_*functions private. - I added a function to print the instance body
- Changed a bunch of signatures to write to the writer directly.
- Added a function to translate the place to its internal representation, so we could use the internal debug implementation.
- Also removed
pretty_ty, replaced by Display implementation of Ty which uses the internal display.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
…rrors
Fix misc printing issues in emit=stable_mir
Trying to continue the work that @ouz-a started here: rust-lang#118364
Few modifications beyond fixes:
- I made the
pretty_*functions private. - I added a function to print the instance body
- Changed a bunch of signatures to write to the writer directly.
- Added a function to translate the place to its internal representation, so we could use the internal debug implementation.
- Also removed
pretty_ty, replaced by Display implementation of Ty which uses the internal display.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request
Rollup merge of rust-lang#122801 - celinval:smir-pretty, r=compiler-errors
Fix misc printing issues in emit=stable_mir
Trying to continue the work that @ouz-a started here: rust-lang#118364
Few modifications beyond fixes:
- I made the
pretty_*functions private. - I added a function to print the instance body
- Changed a bunch of signatures to write to the writer directly.
- Added a function to translate the place to its internal representation, so we could use the internal debug implementation.
- Also removed
pretty_ty, replaced by Display implementation of Ty which uses the internal display.