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 fn-ptr pretty-printing #82176

Merged
merged 2 commits into from
Feb 21, 2021
Merged

Conversation

RalfJung
Copy link
Member

An uninitialized function pointer would get printed as {{uninit fn()} (notice the unbalanced parentheses), and a dangling fn ptr would ICE. This fixes both of that.

However, I have no idea how to add tests for this.

Also, I don't understand this MIR pretty-printing code. Somehow the print function pretty_print_const_scalar actually returns a transformed form of the const (but there is no doc comment explaining what is being returned); some match arms do p! while others do self =, and there's a wild mixture of p! and write!... all very mysterious and confusing.^^

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 16, 2021
" as ",
)?;
}
Some(_) => p!("<non-executable memory>"),
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should just print the pointer as if it were a raw pointer here, otherwise we lose information that may be helpful in debugging the situation.

Copy link
Member Author

Choose a reason for hiding this comment

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

You mean like this? (I don't actually know what I am doing here, as said before, this code is rather confusing.^^)

@oli-obk
Copy link
Contributor

oli-obk commented Feb 20, 2021

@bors r+ rollup

yea this looks correct. I have no idea how to test this either, as we'd need to disable validation in order to ever show this in a regular MIR dump

@bors
Copy link
Contributor

bors commented Feb 20, 2021

📌 Commit e906745 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 20, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 20, 2021
fix MIR fn-ptr pretty-printing

An uninitialized function pointer would get printed as `{{uninit  fn()}` (notice the unbalanced parentheses), and a dangling fn ptr would ICE. This fixes both of that.

However, I have no idea how to add tests for this.

Also, I don't understand this MIR pretty-printing code. Somehow the print function `pretty_print_const_scalar` actually *returns* a transformed form of the const (but there is no doc comment explaining what is being returned); some match arms do `p!` while others do `self =`, and there's a wild mixture of `p!` and `write!`... all very mysterious and confusing.^^

r? `@oli-obk`
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 20, 2021
…laumeGomez

Rollup of 7 pull requests

Successful merges:

 - rust-lang#80595 (`impl PartialEq<Punct> for char`; symmetry for rust-lang#78636)
 - rust-lang#81991 (Fix panic in 'remove semicolon' when types are not local)
 - rust-lang#82176 (fix MIR fn-ptr pretty-printing)
 - rust-lang#82244 (Keep consistency in example for Stdin StdinLock)
 - rust-lang#82260 (rustc: Show ``@path`` usage in stable)
 - rust-lang#82316 (Fix minor mistake in LTO docs.)
 - rust-lang#82332 (Don't generate src link on dummy spans)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 2d39300 into rust-lang:master Feb 21, 2021
@rustbot rustbot added this to the 1.52.0 milestone Feb 21, 2021
@RalfJung RalfJung deleted the mir-fn-ptr-pretty branch February 21, 2021 16:09
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