Skip to content
This repository has been archived by the owner on Sep 11, 2023. It is now read-only.

Strange code generated in enum_map_derive? #23

Closed
Fuuzetsu opened this issue Jul 28, 2022 · 2 comments
Closed

Strange code generated in enum_map_derive? #23

Fuuzetsu opened this issue Jul 28, 2022 · 2 comments
Labels

Comments

@Fuuzetsu
Copy link
Contributor

If we have an enum like this:

pub enum Scens {
    Zero,
    One,
    Two,
    Three,
    Four,
    Five,
}

doing #[derive(Enum)] produces code that looks like this:

pub fn from_usize(v: usize) -> Scens {
    use Scens::*;
    if v < 1 {
        Zero
    } else if v < 2 {
        One
    } else if v < 3 {
        Two
    } else if v < 4 {
        Three
    } else if v < 5 {
        Four
    } else if v < 6 {
        Five
    } else {
        unreachable!()
    }
}

This seems strange to me and the compiler: building with optimisation results in a large chain of comparisons and conditional jumps.

If instead we would generate

pub fn from_usize(v: usize) -> Scens {
    use Scens::*;
    if v == 0 {
        Zero
    } else if v == 1 {
        One
    } else if v == 2 {
        Two
    } else if v == 3 {
        Three
    } else if v == 4 {
        Four
    } else if v == 5 {
        Five
    } else {
        unreachable!()
    }
}

or the less-verbose

pub fn from_usize(v: usize) -> Scens {
    use Scens::*;
    match v {
        0 => Zero,
        1 => One,
        2 => Two,
        3 => Three,
        4 => Four,
        5 => Five,
        _ => unreachable!()
    }
}

both of these produce simpler and faster code: check if the value is in range (< 6 in this case) and then accept an answer as-is.

However I assume the if v < x + 1 implementation wasn't pick at random: is there some information about this? Is there a downside of the proposed methods? It seems very simple to switch the code over to equality check which compiler is much happier about.

These generated methods are quite hot in our code-base which is how I stumbled upon this.

Godbolt link: https://rust.godbolt.org/z/a7E3oE81s

@Fuuzetsu
Copy link
Contributor Author

Thanks for quick reaction!

w.r.t. why it was this way: I'm looking at handle_named_variant and that looks very similar except in that case we have fields which means < comparison catches it: perhaps unit was modeled on it and never adapted to do something more efficient. Maybe a range match is better there or something but that's for another day – unit variant is what I care about in this ticket.

@KamilaBorowska
Copy link
Owner

KamilaBorowska commented Jul 28, 2022

handle_named_variant uses < because it needs to (the number of variants is variable for named variants), and I assume handle_unit_variant got < from it.

As for why if is used rather than match, this is because match arms don't accept const expressions (inline-const is currently nightly only - rust-lang/rust#76001). At some point I think I would like to implement a special case for enums that only have unit variants that uses match for those (to shorten compilation times when having a lot of enums), but I didn't do that yet

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants