-
Notifications
You must be signed in to change notification settings - Fork 42
Extend from_enum macro to keep all enums at smallest repr #172
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
Extend from_enum macro to keep all enums at smallest repr #172
Conversation
|
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 Not sure about the repr part of this PR therefore. The rest looks good. |
|
Huh, yeah you're right. I kinda assumed that explicitly I'll remove the from_enum changes. |
…omfortably in its c_type
|
Ok, I've pushed some changes so that we don't have the unnecessary |
|
Ok @ginnyTheCat sorry to keep this PR open so long as i fiddle with types here and there I've now made it so that the If you'd be willing to just give it a once-over to make sure I'm not crazy about anything else, that would be so appreciated :) |
|
Looks good. Thanks for this work.
Yea, I think C leaves it open to the implementation whether enums are |
The
from_enummacro, before this PR, defined each macro to have variants ofisizetype. 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 withrepr(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 asisizes 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
reprfor all enums created withfrom_enumand 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::Errorand 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).