Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
martin-t opened this issue Jul 9, 2023 · 2 comments
Open

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

martin-t opened this issue Jul 9, 2023 · 2 comments

Comments

@martin-t
Copy link

martin-t commented Jul 9, 2023

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.

@Peternator7
Copy link
Owner

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

@Bubbler-4
Copy link

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.

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

No branches or pull requests

3 participants