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

Shorten the MaybeUninit Debug implementation #133282

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tgross35
Copy link
Contributor

Currently the Debug implementation for MaybeUninit winds up being pretty verbose. This struct:

#[derive(Debug)]
pub struct Foo {
    pub a: u32,
    pub b: &'static str,
    pub c: MaybeUninit<u32>,
    pub d: MaybeUninit<String>,
}

Prints as:

Foo {
    a: 0,
    b: "hello",
    c: core::mem::maybe_uninit::MaybeUninit<u32>,
    d: core::mem::maybe_uninit::MaybeUninit<alloc::string::String>,
}

The goal is just to be a standin for content so the path prefix doesn't add any useful information. Change the implementation to trim MaybeUninit's leading path, meaning the new result is now:

Foo {
    a: 0,
    b: "hello",
    c: MaybeUninit<u32>,
    d: MaybeUninit<alloc::string::String>,
}

@rustbot rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Nov 21, 2024
@rustbot
Copy link
Collaborator

rustbot commented Nov 21, 2024

r? @Amanieu

rustbot has assigned @Amanieu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 21, 2024
@@ -255,7 +255,10 @@ impl<T: Copy> Clone for MaybeUninit<T> {
#[stable(feature = "maybe_uninit_debug", since = "1.41.0")]
impl<T> fmt::Debug for MaybeUninit<T> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.pad(type_name::<Self>())
let full_name = type_name::<Self>();
// NB: this needs to be adjusted if `MaybeUninit` moves modules
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a test for this? Otherwise we're likely to miss it if it does move modules.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, done with test_maybe_uninit_short

Currently the `Debug` implementation for `MaybeUninit` winds up being
pretty verbose. This struct:

    #[derive(Debug)]
    pub struct Foo {
        pub a: u32,
        pub b: &'static str,
        pub c: MaybeUninit<u32>,
        pub d: MaybeUninit<String>,
    }

Prints as:

    Foo {
        a: 0,
        b: "hello",
        c: core::mem::maybe_uninit::MaybeUninit<u32>,
        d: core::mem::maybe_uninit::MaybeUninit<alloc::string::String>,
    }

The goal is just to be a standin for content so the path prefix doesn't
add any useful information. Change the implementation to trim
`MaybeUninit`'s leading path, meaning the new result is now:

    Foo {
        a: 0,
        b: "hello",
        c: MaybeUninit<u32>,
        d: MaybeUninit<alloc::string::String>,
    }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants