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

MIR dump prints the location of each statement as a comment #70400

Closed
oli-obk opened this issue Mar 25, 2020 · 7 comments
Closed

MIR dump prints the location of each statement as a comment #70400

oli-obk opened this issue Mar 25, 2020 · 7 comments
Labels
A-mir Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html A-mir-opt Area: MIR optimizations A-testsuite Area: The testsuite used to check the correctness of rustc C-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Mar 25, 2020

originally mentioned in #69916 (comment)

Basically all the lines look like

return;                          // bb2[0]: scope 0 at $DIR/const-promotion-extern-static.rs:13:1: 13:56

which is a lot of noise if used with mir-opt tests and diff mode, because mir optimizations that don't even touch the current statement may still cause diff fallout because they touch the comments.

Should we just stop emitting the location? Or maybe just specifically for mir-opt tests by creating a new -Z flag?

cc @nikomatsakis @pnkfelix @matthewjasper @eddyb

@oli-obk oli-obk added A-mir Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html A-mir-opt Area: MIR optimizations A-testsuite Area: The testsuite used to check the correctness of rustc labels Mar 25, 2020
@jonas-schievink jonas-schievink added C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 25, 2020
@Centril
Copy link
Contributor

Centril commented Mar 25, 2020

That info seems useful to me when e.g., dumping the MIR in the playground -- I think if we're going to make tweaks, let's do it only for mir-opt tests (but please find a way to thread this info without hacks in Debug impls).

@eddyb
Copy link
Member

eddyb commented Mar 25, 2020

The comment is printed outside of any formatting impls so we should be fine.

@Centril do you mean the bb2[0], or the scope/span?

I specifically meant the mir::Location i.e. the bbX[Y], maybe this issue is unclear.

@Centril
Copy link
Contributor

Centril commented Mar 25, 2020

@eddyb both seem useful :)

@eddyb
Copy link
Member

eddyb commented Mar 25, 2020

But have you actually used it? I can't remember having used it or imagine how, other than maybe correlating it with some debug! output?
If it was missing, worst case you have to count statements or something.

We could maybe normalize // bb\d+\[\d+\]: to just // by default for mir-opt tests in compiletest, that might be enough.

@eddyb
Copy link
Member

eddyb commented Mar 25, 2020

Looks like printing mir::Locations started with #46319, so they're likely used for debugging MIR/NLL borrowck, for which we've already pinged @nikomatsakis and @matthewjasper, I guess?

@Centril
Copy link
Contributor

Centril commented Mar 25, 2020

I don't remember it vividly, but I recall that the info was useful when doing my initial investigation in #67565. Could be wrong though.

If it was missing, worst case you have to count statements or something.

Yeah I could do that, but it was a bit of a time saver not to have to do that work in my head. :)

@oli-obk oli-obk added the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Jul 19, 2020
@oli-obk
Copy link
Contributor Author

oli-obk commented Jul 19, 2020

We're not printing the basic blocks without -Zverbose anymore, so I think there's nothing actionable here

@oli-obk oli-obk closed this as completed Jul 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-mir Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html A-mir-opt Area: MIR optimizations A-testsuite Area: The testsuite used to check the correctness of rustc C-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants