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

Fix mir-cfg dumps #82362

Merged
merged 1 commit into from
Feb 23, 2021
Merged

Fix mir-cfg dumps #82362

merged 1 commit into from
Feb 23, 2021

Conversation

osa1
Copy link
Contributor

@osa1 osa1 commented Feb 21, 2021

Fixes #81918
Fixes #82326 (duplicate)
Fixes #82325


r? @oli-obk

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 21, 2021
@osa1
Copy link
Contributor Author

osa1 commented Feb 21, 2021

Should I add a test to this? I have a minimal repro:

const TAG: &'static str = "ABCD";

fn main() {
    if TAG == "" {}
}

but I couldn't find a way to accept a test based on rustc exit code (without comparing stdout contents), and I'm not sure if we want to check dot outputs.

@oli-obk
Copy link
Contributor

oli-obk commented Feb 22, 2021

but I couldn't find a way to accept a test based on rustc exit code (without comparing stdout contents), and I'm not sure if we want to check dot outputs.

we already have pretty printing tests in https://github.com/rust-lang/rust/tree/master/src/test/pretty I think you should be able to just add it there

@osa1
Copy link
Contributor Author

osa1 commented Feb 22, 2021

@oli-obk I couldn't put this in test/pretty because outputs of tests in that directory is passed to the compiler again, to check the the printer prints valid Rust. I realized that stdouts of tests in test/mir-opt are not checked so I put the test there. Let me know if you want me to move it elsewhere.

@oli-obk
Copy link
Contributor

oli-obk commented Feb 22, 2021

Heh that's a neat trick. Please leave the explanation on why it is here and not in the ui test suite in a comment in that file, otherwise someone will be confused by that test in the future

@osa1
Copy link
Contributor Author

osa1 commented Feb 22, 2021

Sigh.. it seems like you can't easily ignore stdout contents of a test.. Putting the test in mir-opt just to ignore stdout seemed hacky so I moved the test once again, and committed stdout contents. Ping @oli-obk

@osa1
Copy link
Contributor Author

osa1 commented Feb 22, 2021

@oli-obk nvm, figured it out. Could you review?

@oli-obk
Copy link
Contributor

oli-obk commented Feb 22, 2021

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Feb 22, 2021

📌 Commit 2145a87 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 Feb 22, 2021
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Feb 22, 2021
Fix mir-cfg dumps

Fixes rust-lang#81918
Fixes rust-lang#82326 (duplicate)
Fixes rust-lang#82325

---

r? `@oli-obk`
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 23, 2021
Rollup of 10 pull requests

Successful merges:

 - rust-lang#81629 (Point out implicit deref coercions in borrow)
 - rust-lang#82113 (Improve non_fmt_panic lint.)
 - rust-lang#82258 (Implement -Z hir-stats for nested foreign items)
 - rust-lang#82296 (Support `pub` on `macro_rules`)
 - rust-lang#82297 (Consider auto derefs before warning about write only fields)
 - rust-lang#82305 (Remove many RefCells from DocContext)
 - rust-lang#82308 (Lower condition of `if` expression before it's "then" block)
 - rust-lang#82311 (Jsondocck improvements)
 - rust-lang#82362 (Fix mir-cfg dumps)
 - rust-lang#82391 (disable atomic_max/min tests in Miri)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit aa1405f into rust-lang:master Feb 23, 2021
@rustbot rustbot added this to the 1.52.0 milestone Feb 23, 2021
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
5 participants