MIR dump: print pointers consistently with Miri output #71590
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
r? @eddyb |
|
Cc @oli-obk |
src/test/ui/consts/const-eval/const-pointer-values-in-various-types.stderr
Outdated
Show resolved
Hide resolved
RalfJung
force-pushed
the
mir-dump-pointers
branch
from
7966807
to
b12faeb
Compare
|
📌 Commit b12faeb has been approved by |
bors
added
S-waiting-on-bors
Rollup of 8 pull requests Successful merges: - rust-lang#71148 (Vec drop and truncate: drop using raw slice *mut [T]) - rust-lang#71465 (Add a convenience method on `TyCtxt` for checking for thread locals) - rust-lang#71567 (Handle build completion message from Cargo) - rust-lang#71590 (MIR dump: print pointers consistently with Miri output) - rust-lang#71682 (Bump pulldown-cmark) - rust-lang#71688 (Allow `Downcast` projections unconditionally in const-checking) - rust-lang#71691 (Allow `Unreachable` terminators unconditionally in const-checking) - rust-lang#71719 (Update backtrace-sys) Failed merges: r? @ghost
| 0x00 │ 00 00 00 00 __ __ __ __ ╾─a4+0x0──╼ 00 00 00 00 │ ....░░░░╾──╼.... | ||
| 0x10 │ 00 00 00 00 __ __ __ __ ╾─a8+0x0──╼ 02 00 00 00 │ ....░░░░╾──╼.... | ||
| 0x20 │ 01 00 00 00 2a 00 00 00 ╾─a13+0x0─╼ 03 00 00 00 │ ....*...╾──╼.... |
There was a problem hiding this comment.
Hmpf, for the common case (offset is 0, or otherwise small), this seems unnecessarily noisy compared to decimal (due to the prefix).
I know @RalfJung doesn't like it, but my preferred approach of printing arbitrary integers involves using a decimal digit when unambiguous (i.e. <= 9), and +0 feels unnecessary so I would skip it too.
But maybe this won't matter long-term, if we improve our ty::Const printing to move beyond the need for these (otherwise really cool!) allocation dumps. (We would need to detect if the allocations are entirely reproducible from the pretty value, and don't have e.g. initialized padding, to avoid losing information)
There was a problem hiding this comment.
I'm okay introducing a special case for 0, to not print the offset at all.
But alloc13+4 and elsewhere alloc13+0x10 seems oddly inconsistent to me.
Miri: tweak error print I started by adjusting the "invalid use of int as pointer" message (it wasn't really clear what is invalid about the use). But then I realized that these are all `Debug` impls we use for these errors, for some reason, so I fixed that to use `Display` instead. ~~This includes rust-lang#71590 (to get the `Display` impl for `Pointer`), so the diff will look better once that finally lands. Here's the [relative diff](RalfJung/rust@e72ebf5...RalfJung:miri-error-print).~~ r? @oli-obk
This makes MIR allocation dump pointer printing consistent with Miri output: both use hexadecimal offsets with a
0xprefix. To save some space, MIR dump replaces theallocprefix byawhen necessary.I also made AllocId/Pointer printing more consistent in their Debug/Display handling, and adjusted Display printing for Scalar a bit to avoid using decimal printing when we do not know the sign with which to interpret the value (IMO using decimal then is misleading).