Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rustc_mir: remove extra space when pretty-printing MIR. #70080

Merged
merged 1 commit into from
Mar 23, 2020

Conversation

anyska
Copy link
Contributor

@anyska anyska commented Mar 17, 2020

No description provided.

@rust-highfive
Copy link
Collaborator

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 17, 2020
@eddyb
Copy link
Member

eddyb commented Mar 17, 2020

r? @oli-obk

@rust-highfive rust-highfive assigned oli-obk and unassigned eddyb Mar 17, 2020
@@ -114,7 +114,7 @@ fn main() {
// }
// END rustc.main-{{closure}}.EraseRegions.after.mir
// START rustc.ptr-drop_in_place.Test.SimplifyCfg-make_shim.after.mir
// fn std::intrinsics::drop_in_place(_1: *mut Test) -> () {
Copy link
Contributor

@oli-obk oli-obk Mar 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get how this is the only test affected. Shouldn't

// fn foo(_1: T, _2: &i32) -> i32{
have been printing the space before this PR, too?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also I just saw that my linked thing is missing a space in the end... Maybe MIR opt tests ignore whitespaces even in the middle of things?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe MIR opt tests ignore whitespaces even in the middle of things?

We were confused by this too, I think that's the case, and I'd rather it not.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we move loads of mir-opt tests to the blessable system the leftover cases that still want to use the manual system should be few enough for that to be doable in a painless way.

@oli-obk
Copy link
Contributor

oli-obk commented Mar 23, 2020

@bors r+ rollup

Not testable right now, but improves mir printing output

@bors
Copy link
Contributor

bors commented Mar 23, 2020

📌 Commit 7d40019 has been approved by oli-obk

@bors 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, 2020
Centril added a commit to Centril/rust that referenced this pull request Mar 23, 2020
rustc_mir: remove extra space when pretty-printing MIR.
This was referenced Mar 23, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 23, 2020
Rollup of 9 pull requests

Successful merges:

 - rust-lang#68700 (Add Wake trait for safe construction of Wakers.)
 - rust-lang#69494 (Stabilize --crate-version option in rustdoc)
 - rust-lang#70080 (rustc_mir: remove extra space when pretty-printing MIR.)
 - rust-lang#70195 (Add test for issue rust-lang#53275)
 - rust-lang#70199 (Revised span-to-lines conversion to produce an empty vec on DUMMY_SP.)
 - rust-lang#70299 (add err_machine_stop macro)
 - rust-lang#70300 (Reword unused variable warning)
 - rust-lang#70315 (Rename remaining occurences of Void to Opaque.)
 - rust-lang#70318 (Split long derive lists into two derive attributes.)

Failed merges:

r? @ghost
@bors bors merged commit 4d5ecca into rust-lang:master Mar 23, 2020
@anyska anyska deleted the mir-double-space branch March 27, 2020 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants