strum icon indicating copy to clipboard operation
strum copied to clipboard

Maybe EnumCount should respect `#[repr(...)]` like FromRepr

Open martin-t opened this issue 1 year ago • 2 comments

The parameter of the generated from_repr has the type specified by the repr on the enum. However the COUNT associated const always has the type usize. This requires casts when composing the two (for example when doing arithmetic to get the prev/next variant).

I think it makes sense to also give COUNT the type from the repr (and usize of not specified). This is obviously a breaking change but it'll only affect people who specify a repr.

The most common repr type will probably be u8 which can convert to usize infallibly so if people specify #[repr(u8)] but want count as usize, they can convert using .into() without any unwraps. This is not the case currently - converting COUNT to the type required by from_repr is a fallible conversion and therefore needs unwrapping.

martin-t avatar Jul 09 '23 18:07 martin-t

Agree this is probably more consistent with what people expect. Happy to take a PR if you're interested

Peternator7 avatar Jul 29 '23 22:07 Peternator7

A potential problem with this is that, if a #[repr(u8)] enum happens to have exactly 256 variants, the value of COUNT does not fit in u8.

Bubbler-4 avatar Sep 26 '23 04:09 Bubbler-4