Skip to content

Conversation

itsjunetime
Copy link
Collaborator

The from_enum macro, before this PR, defined each macro to have variants of isize type. While this probably didn't really cause any issues in practice, this always made me a bit nervous. As far as I can tell, rust doesn't guarantee that enums with repr(rust) are automatically sized down to the smallest representation they could use (see the reference chapters on type layout and enums, maybe I missed something), so we might be passing all enums around as isizes right now (as opposed to their most optimal representation, which is a u8 in most cases).

So this PR, to fix that, allows specifying a repr for all enums created with from_enum and ensures that every constant passed in to be the value of a variant in that enum will actually fit within the given repr.

This pr also cleans up a few other small things, like using a fully-qualified path for $crate::error::Error and providing a way to convert from an enum into its c representation (e.g. c_int) losslessly (to ensure that we don't accidentally lose information when we add a new enum without checking what its largest value actually is).

@ginnyTheCat
Copy link
Collaborator

According to https://doc.rust-lang.org/reference/items/enumerations.html#r-items.enum.discriminant.repr-rust the compiler is allowed to choose a smaller repr and does this in practice as well. Using a enum U2 { Zero, One, Two, Three } is quite common for example and the compiler not doing this optimization would "break" crates like smol_str by increasing their size and therefore removing the performance benefit they give.

Not sure about the repr part of this PR therefore. The rest looks good.

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

Successfully merging this pull request may close these issues.

2 participants