-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
core: Make Debug
impl of raw pointers print metadata if present
#135080
base: master
Are you sure you want to change the base?
Conversation
r? libs |
tests/ui/fmt/ptr-metadata.rs
Outdated
|
||
use std::fmt::Display; | ||
|
||
fn main() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you make this a core unit test instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure no problem, I'll fix whatever my reviewer(s) ask me to do.
Would you mind elaborating a bit on why though? Do you prefer it as a core unit test because then the test can be run with a stage0 compiler, maybe? What I like about having it as a UI test is that --bless
works when the output change.
Edit: Ok I can see we already have a bunch of related tests at library/core/tests/fmt/mod.rs, and one of them fails now. So I'll move my test there too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. I had to (with the current test code) add "regex" as an allowed library (dev-)dependency, but it is already present as an allowed rustc dependency, so it's fine I think.
This comment has been minimized.
This comment has been minimized.
b628909
to
ee8187a
Compare
This comment has been minimized.
This comment has been minimized.
Because `.as_ptr()` changes the type of the pointer (e.g. `&[u8]` becomes `*const u8` instead of `*const [u8]`), and it can't be expected that different types will be formatted the same way.
Make Rust pointers less magic by including metadata information in their `Debug` output. This does not break Rust stability guarantees because `Debug` output is explicitly exempted from stability: https://doc.rust-lang.org/std/fmt/trait.Debug.html#stability Co-authored-by: Lukas <26522220+lukas-code@users.noreply.github.com>
ee8187a
to
4c641b1
Compare
The list of allowed third-party dependencies may have been modified! You must ensure that any new dependencies have compatible licenses before merging. These commits modify the If this was unintentional then you should revert the changes before this PR is merged. |
Make Rust pointers appear less magic by including metadata information in their
Debug
output.This does not break Rust stability guarantees because
Debug
impl are explicitly exempted from stability:https://doc.rust-lang.org/std/fmt/trait.Debug.html#stability
Note that a regression test is added as a separate commit to make it clear what impact the last commit has on the output.
Closes #128684 because the output of that code now becomes: