xfix / enum-map Goto Github PK

It's technically not needed, and it's probably simpler to just have a single module.

Hello. Is there a sensible way to have EnumMap use default values for each key if their key is not present when deserializing from a human-readable format?

Ord and PartialOrd impls are missing.

It seems to be missing, but I can't think of a reason why it wouldn't be possible to implement offhand without having tried it yet.

We have from_array, but it would be nice if we could get to_array() if Rust's const implementation allows it (it might not yet, but presumably should at some point).

enum_map! is probably not fixable in stable Rust without the unstable const fn support, unless there's some clever approach I'm not seeing.

However, making the field public would allow to manually do it.

There's also the possibility of writing an enum_with_map! macro that would create both an enum and an enum map const (by just writing both the variants and the array items in the same order as provided by the user), which is easy and might be good enough in practice.

Possible implementation of such a macro (after changing EnumMap to be a tuple struct with a public array field):

#[macro_export]
macro_rules! enum_with_map {
	{$(#[$m:meta])* enum $e:ident, const $n:ident : EnumMap<_, $d:ty> = {$($k:ident => $v:expr),*};} => {
        $(#[$m])* #[derive(EnumMap)] enum $e {
            $($k),*
        }

        const $n: EnumMap<$e, $d> = EnumMap([
            $($v),*
        ]);
    };
}

For 1.0.0, an Internal interface which will likely be renamed to Enum will need to be stabilized. The plan is once const generics become stable (see rust-lang/rust#44580) - although worth noting that this doesn't need full const generics, just being able to use associated consts within a struct, an Internal trait would be rewritten as such.

trait Enum {
    const DISCRIMINANTS: usize;
    fn discriminant(self) -> usize;
    fn from_discriminant(discriminant: usize) -> Self;
}

Since it is really an array, which itself probably implements Copy.

It's currently possible to:

• Create EnumMap from K::Array with from_array
• Destructure EnumMap to K::Array with into_array
• Get a slice reference with as_slice and as_mut_slice

But it's currently not possible to get a reference to K::Array directly, mutable or not. This is useful if the contents aren't Clone-able but we still want to iterate over the values directly (without concerning ourselves with the keys).

At this point, it seems that just exposing the array field of EnumMap as public should be fair game: we already have setter/getter and hiding it just makes it inconvenient.

This would be broadly useful when using EnumMap<K,V> inside other structs where it's nice to be able to compare by value. If you think this would be good to add, I'm happy to take a shot, especially if you have some thoughts/opinions on how you think it should be done!

Thank you!

error: any number modulo 1 will be 0
  --> some_filename.rs:13:10
   |
13 | #[derive(enum_map::Enum, Debug, Copy, Clone)]

#[derive(enum_map::Enum)]
pub enum Side {
    Bottom,
}

I'm not entirely sure what to do about it, just putting it here anyway.

Currently, the implementation of from_usize for derive(Enum) will panic if given an out-of-bounds integer. That seems fine.

However, the panic message is currently:

panicked at 'internal error: entered unreachable code'

To me, that suggests an internal logic error in the derived impl, which doesn't appear to be the case. This case is trivially reachable if the caller provides a bogus integer. unreachable is an unusual way of handling this; for instance, slice index out of bounds does not print unreachable but rather that a slice index was out of bounds.

As a user I think it'd be a better experience if the message were something like index out of range for Enum::from_usize.

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

It seems logical that (A, B) where A: Enum, B: Enum would be basically the same as a struct's derivation, which is a cartesian product. I don't think we're allowed to implement this ourselves as tuples are always a foreign type, but it could be implemented for up to some length of tuples in the crate.

I personally want this so that I can produce enums via this utility function I've made:

#[inline(always)]
pub fn enum_iter<E: enum_map::Enum>() -> impl DoubleEndedIterator<Item = E> + Clone {
    (0..E::LENGTH).map(|i| E::from_usize(i))
}

without having to nest the enum_iter calls if there's multiple which I want to produce from.

(Side note: the enum_iter function might also be useful to have put in.)

I could make an MR if it's an amenable feature to include.