Enum Map derive macro

[PokeJofeJr4th]

I think it would be useful to have a map that you can index with an enum. I had the idea while working on a game inventory system where items are enums. I've started my own fork to start the macro. I'm struggling with the dependency on EnumIter and possible support for variants with attached data.

I still have a long way to go before it's PR-quality. I'm posting here to gauge whether my idea seems worthwhile and to ask for help. I'm very new to open source so I'm not sure if I'm doing all this right.

[itsjunetime]

I also think this would be really useful. One could use a hashmap but then you'd have to .unwrap() every time you .get a value even if you know that every enum variant has a valid value, and being able to make a custom struct for each enum to act as a variant map would have a lot of uses.

I created an initial implementation of a derive macro for this on my own repo here. If you have the following code:

#[derive(EnumMap)]
enum Size {
    Small,
    Medium,
    Large,
}

it expands to:

struct SizeMap<T> {
    small: T,
    medium: T,
    large: T,
}
impl<T> SizeMap<T> {
    fn new(small: T, medium: T, large: T) -> SizeMap<T> {
        SizeMap { small, medium, large }
    }
    fn get(&self, variant: Size) -> &T {
        match variant {
            Size::Small => &self.small,
            Size::Medium => &self.medium,
            Size::Large => &self.large,
        }
    }
    fn set(&mut self, variant: Size, new_value: T) {
        match variant {
            Size::Small => self.small = new_value,
            Size::Medium => self.medium = new_value,
            Size::Large => self.large = new_value,
        }
    }
}

I'd be happy to turn this into a PR if the owners of this repo would be interesting in having this macro. I also feel that there are questions to be discussed before releasing something like this, e.g.:

1. How do we specify what the visibility of the expanded struct could be? It would be useful to allow it to sometimes be pub but it can't always be pub since it may then leak an external type if the original enum is not pub
2. Should we have other initializers? Having some other options such as setting the value of all variants with a Copy value, or setting the value of all variants with a provided closure would be very useful, I think.
3. How do we deal with variants that have data (as mentioned above)? Should we even try to accommodate for that? We could perhaps look to the policy of EnumIter for this one, since it would run into a similar problem.

If any maintainers would like to help us move forward with getting this merged in by reviewing a PR or answering these questions with their thoughts, that would be really appreciated!

[PokeJofeJr4th]

That looks great! Would you like access to my version so we can start merging our implementations?

1. It should probably be the same as the base enum, as I believe EnumIter does that as well; it would be impossible to use the map without the enum.
2. I like both of those; also, a default would be nice
3. I think it makes sense to either only support unit variants or only map using the discriminator. I don't want to try to figure out how to make it recursive.

Should we do a blanket iter() that only applies when EnumIter is also derived?

fn iter(&self) -> Iterator<Item = T> {
   Enum::iter().map(|k| (k, self.get(k))
}

[itsjunetime]

Oh my god, I feel a bit foolish now - when I pulled up your version last time, github showed me no changes from its parent repo for some reason and I just assumed you hadn't committed your work yet (and thus went on to do a lot of extraneous work on my own repo). I didn't consider just implementing Index and IndexMut as well and you've already got visibility all worked out - I wholeheartedly support merging in your code, it seems to be much more complete than mine (once we've figured out the questions above, of course).

[PokeJofeJr4th]

lol I thought yours seemed more complete. I also don't really know what I'm doing with all these procedural macros. A lot of my stuff is probably cargo cult-ish, since I copied a bunch from EnumIter. I think we should also change it to the named struct fields rather than sized array

[Peternator7]

Hey folks, this sounds like a really useful macro, but I'm worried there's a number of situational concerns that will make it difficult to generalize across a range of needs. Some thoughts:

• To match the other Map apis, it might be valuable for the struct fields to internally be Option<T> so that they can be removed and whatnot.
• The memory usage of this map scales with the number of variants rather than the number of entries. For sparsely filled maps, that might be unexpected, and there's a performance trade-off there that people might not be aware of.
• Like you said, I think we need a better story about how variants with data will behave.

If we can make some of these trade-offs more configurable, I'd consider including it in strum because it is a cool idea, and it sounds like there's a need :) If not though, it might be better off as its own crate so that people have some flexibility in choosing the implementation they want for this feature.

How does that sound?

[PokeJofeJr4th]

As for your first two points, that sounds like you'd just have a wrapper around HashMap or BTreeMap. The main point for my application of this one is that there's always a value, so you can safely index into it without thinking about unwrapping.

It might make more sense to call it something other than EnumMap, since it's basically a named array and not a true map. Having said that, it is starting to sound like it doesn't quite fit into strum. On the other hand, it seems way too small to be its own crate.

[PokeJofeJr4th]

I've updated my repo with some of June's code. Here's a list of important features I have working:

• The derived MyEnumMap<T> has a field of type T for each variant of MyEnum
• Derive Clone, Default, PartialEq for MyEnumMap<T>
• fn new(#(<fields>,)*) -> Self { ... } - publically exposed constructor because we don't want to pub the actual struct fields
• Index<T> and IndexMut<T>