strum
strum copied to clipboard
Add Enum Map Derive Macro
This started as Issue #272
#[derive(EnumMap)]
enum MyEnum {
VariantName,*
}
This macro creates a new type MyEnumMap<T>
with a field of type T
for each variant of MyEnum
. It also adds the following:
- Automatic derive for Debug, Clone, Default, PartialEq, Eq, Hash
- Implement
Index<T>
andIndexMut<T>
-
fn new(variant_name: T,*) -> MyEnumMap<T>
constructor to specify each struct field -
fn from_closure(func: Fn(MyEnum)->T) -> MyEnumMap<T>
constructor to initialize each struct field using a function -
fn transform(&self: MyEnumMap<T>, func: Fn(MyEnum,&T) -> U) -> MyEnumMap<U>
method to create a new map by applying a function to each field - if
T
implementsClone
, generatefn filled(value: T) -> MyEnumMap<T>
constructor to fill each struct field with the given vaalue -
fn all(self: MyEnumMap<Option<T>>) -> Option<MyEnumMap<T>>
checks if all fields areSome
-
fn all_ok(self: MyEnumMap<Result<T, E>>) -> Option<MyEnumMap<T>>
checks if all fields areOk
- variants marked with
#[strum(disabled)]
are not included as struct fields and panic when passed as an index.
Questions:
- Should
EnumMap
be renamed to clarify that it doesn't have many features of conventional maps? - Are the
all
andall_ok
functions helpful? Are they within the scope of Strum? - I'm currently disallowing extra data on variants. Should I just ignore it and use the discriminant? Should I treat the underlying data as a key to a more conventional map?
- Are there any other traits/functions I'm forgetting about that should be included?
the build is only failing because the version of rust they're using is out of date
In response to some of your other questions which I've been thinking about:
-
I'm not quite certain what a better name would be. I guess if there was some way to specify that it's a map that has
Some
value for every possible key, that could be more clear, but I don't know what that would be. -
I feel like the
all_ok
function is definitely useful, but would be better if it returned aResult<T, E>
where it returnsErr
with the first error encountered while looking through the variants (plus, this would also make it really simple to implement - justEnumMap { #(variant: variant?,)* }
in the body). I was previously going to say thatall
wouldn't be super useful, but I was thinking about my use cases and realized it could actually be really useful. E.g. I would like to store data in anEnumMap
to disk, and the library I'm using for that can store aHashMap
right out of the box. So I plan to convert theEnumMap
to aHashMap
when storing like so:let mut map = HashMap::new(); for var in Enum::iter() { map[var] = enum_map[var] }
But to convert back after reading the
HashMap
from the disk, The only safe and clean way I can think of (besides manually getting the value for each variant) would be:let Some(enum_map) = EnumMap::from_closure(|v| hash_map.get(v)).all() else { println!("oops!"); };
So actually, I feel like
all
could definitely be useful if you need to convert from other similar storage mediums. -
I put my thoughts about non-unit variants in another comment
-
I think
impl
ingClone
whereT: Clone
could be nice. I guess if you wanted to work around it, you could just doEnumMap::from_closure(|v| other_map[v].clone())
, but just doingother_map.clone()
would be cleaner and clearer a bit.
- ChatGPT's best suggestion was
MyEnumTable
, which I kinda like - I did change the
all_ok
to what you suggested. However, for your exact case, would it be better to implement a way to directly store the EnumMap? - yeah that's still a hard one
- That's already a thing with the
#[derive(...)]
thingy; I've added a test to clarify that
- Yeah, I think calling it a table could be good; if people would prefer that, then I have no objections
- It would definitely be better for me to directly store the
EnumMap
, but with the library that I'm using, that requiresderive
ing a bespoke macro, which I don't think I can do right now for the generatedEnumMap
. However, with that in mind, perhaps we could add an attribute like#[enum_map(derive = "MyTrait, MyOtherTrait")]
so that you can add other traits to the generated struct'sderive
attribute? I'm not attached to that syntax, just the general idea of adding extra traits to derive.
And thanks for pointing out that it already derives Clone
; I must've missed that.
found a significant issue https://github.com/PokeJofeJr4th/strum/issues/8
Could you clarify what the significant issue was? It's good to consider that someone could use a reserved word; can we fix that by prepending an underscore to their identifier?
Sorry, I see now that you have a perfectly fine fix (prepending _
). I previously thought we'd have to figure out if it was a reserved keyword and use r#
to escape it; but since it's not publicly facing the underscore solution is perfectly valid
I feel like it would be really useful to add custom derive commands for each struct this is used on, e.g.
#[derive(EnumTable)]
#[strum(table_derives = "Serialize, Deserialize")]
enum Color {
...
}
this way, ColorTable
would have #[derive(Serialize, Deserialize)]
added on its declaration since you can't do that otherwise (at least, as far as I know). Would anyone be opposed to this?
I like that but will the namespaces work on it? Would the user having macros like Serialize and Deserialize in the same scope as #[derive(EnumTable)]
be enough?
Published strum version 0.26.0
that includes this feature. I've labeled it experimental because I'm not totally sure the api surface won't change, but think it's an interesting start. Thanks for the work on this and sorry for the delay merging it.