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 printing regions with -Z verbose #43458

Merged
merged 2 commits into from
Jul 26, 2017
Merged

Fix printing regions with -Z verbose #43458

merged 2 commits into from
Jul 26, 2017

Conversation

RalfJung
Copy link
Member

When dumping MIR with -Z verbose, it would print regions on types, but not in the code. It seems the Rvalue printing code tried to be smart and guessed when the Display for Region would not possibly print anything.

This PR makes it no longer be smart, and just always use the Display like all the other code (e.g. printing types) does.

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

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

@arielb1
Copy link
Contributor

arielb1 commented Jul 24, 2017

[00:50:24] failures:

[00:50:24]     [mir-opt] mir-opt/storage_live_dead_in_statics.rs

This is a real output change - now e.g. a borrow for static lifetime displays as &'static x even without -Z verbose. I think you should just use -Z verbose -Z identify-regions.

@RalfJung
Copy link
Member Author

RalfJung commented Jul 24, 2017

This is a real output change - now e.g. a borrow for static lifetime displays as &'static x even without -Z verbose.

Ah, indeed. I did not realize this. I will push a patch which just changes the conditional to be verbose || identify-regions.

I think you should just use -Z verbose -Z identify-regions.

At the very least, then the documentation of identify-regions needs changing.

I generally find it rather strange that verbose would enable regions in some places and identify-regions in others, and only the combination gives you regions everyhwere. I guess another option would be to make region printing tied to identify-regions only, and ignore verbose?

@RalfJung
Copy link
Member Author

CI failure looks like network problem.

@arielb1
Copy link
Contributor

arielb1 commented Jul 25, 2017

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jul 25, 2017

📌 Commit 4e1249d has been approved by arielb1

@RalfJung
Copy link
Member Author

Thanks :)

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jul 25, 2017

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Contributor

bors commented Jul 25, 2017

📌 Commit 4e1249d has been approved by nikomatsakis

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Jul 26, 2017
Fix printing regions with -Z verbose

When dumping MIR with `-Z verbose`, it would print regions on types, but not in the code. It seems the Rvalue printing code tried to be smart and guessed when the `Display` for `Region` would not possibly print anything.

This PR makes it no longer be smart, and just always use the `Display` like all the other code (e.g. printing types) does.
bors added a commit that referenced this pull request Jul 26, 2017
Rollup of 10 pull requests

- Successful merges: #42959, #43447, #43455, #43456, #43458, #43462, #43463, #43465, #43471, #43480
- Failed merges:
@bors bors merged commit 4e1249d into rust-lang:master Jul 26, 2017
@RalfJung RalfJung deleted the verbose branch July 27, 2017 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants