Skip to content

derive(Debug) on simple enums should emit a single call to Formatter::write_str #106875

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

Closed
saethlin opened this issue Jan 14, 2023 · 0 comments · Fixed by #106884
Closed

derive(Debug) on simple enums should emit a single call to Formatter::write_str #106875

saethlin opened this issue Jan 14, 2023 · 0 comments · Fixed by #106884
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-proc-macros Area: Procedural macros C-enhancement Category: An issue proposing an enhancement or a PR with one.

Comments

@saethlin
Copy link
Member

saethlin commented Jan 14, 2023

Currently, this code:

#[derive(Debug)]
pub enum ErrorKind {
    A,
    B,
    C,
}

Expands to this:

#[automatically_derived]
impl ::core::fmt::Debug for ErrorKind {
    fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result {
        match self {
            ErrorKind::A => ::core::fmt::Formatter::write_str(f, "A"),
            ErrorKind::B => ::core::fmt::Formatter::write_str(f, "B"),
            ErrorKind::C => ::core::fmt::Formatter::write_str(f, "C"),
        }
    }
}

It should expand to this instead:

#[automatically_derived]
impl ::core::fmt::Debug for ErrorKind {
    fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result {
        ::core::fmt::Formatter::write_str(f, match self {
            ErrorKind::A => "A",
            ErrorKind::B => "B",
            ErrorKind::C => "C",
        })
    }
}

The current version compiles to MIR that looks like this:

    bb3: {
        _4 = &mut (*_2);
        _6 = const "A";
        _5 = _6;
        _0 = Formatter::<'_>::write_str(move _4, move _5) -> bb5; // bb5 is the return block
    }   

Hoisting the write_str out of the match should dramatically reduce the amount of MIR we generate.

@Kixiron suggested that storing all the string literals in an array and indexing into it with the discriminant might be even better, with the extra requirement that the discriminants are consecutive. That sounds more complicated to me in implementation, though it would have the nice extra benefit of generating a function body that is O(1) with respect to the number of variants in the enum.

This is similar to #88793, the change I'm looking for here would let such an optimization apply.

@rustbot label +C-enhancement +A-macros +A-proc-macros

@rustbot rustbot added A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-proc-macros Area: Procedural macros C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Jan 14, 2023
@bors bors closed this as completed in b22aa57 Jan 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-proc-macros Area: Procedural macros C-enhancement Category: An issue proposing an enhancement or a PR with one.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants