-
Notifications
You must be signed in to change notification settings - Fork 120
ndk-sys: Switch remaining enums to newtypes with associated constants #315
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
Conversation
a7188e4 to
628bc8c
Compare
| impl HardwareBufferUsage { | ||
| pub const CPU_READ_NEVER: Self = | ||
| Self(ffi::AHardwareBuffer_UsageFlags_AHARDWAREBUFFER_USAGE_CPU_READ_NEVER); | ||
| Self(ffi::AHardwareBuffer_UsageFlags::AHARDWAREBUFFER_USAGE_CPU_READ_NEVER); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not very sure about this anymore: would have been great to use pub type HardwareBufferUsage = ffi::AHardwareBuffer_UsageFlags; (or pub use ffi::AHardwareBuffer_UsageFlags as HardwareBufferUsage; which allows HardwareBufferUsage to be used as a constructor!) but bindgen doesn't strip duplicate naming out of variants making such a reuse very unfriendly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rust-lang/rust-bindgen#777 Unfortunately this has gotten stale on the bindgen side :/
628bc8c to
697b9cb
Compare
c64b23f to
3ed2434
Compare
| }; | ||
| construct(|res| unsafe { | ||
| ffi::AHardwareBuffer_lock(self.as_ptr(), usage.0, fence, rect, res) | ||
| ffi::AHardwareBuffer_lock(self.as_ptr(), usage.0 .0, fence, rect, res) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these spaces between the .0s look a bit weird to me, was this intentional as cargo fmt doesnt seem to complain?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@msiglreith It was actually cargo fmt that did this, perhaps to prevent some possible mis-interpretation of 0.0?
We run cargo fmt in the CI and it's green, so it doesn't complain :)
Searching for `pub type` reveals a lot of typedefs for enums that could have been a newtype with associated constants. Unfortunately there are still a lot of private enums in the source, some with a `typedef` following: these cannot be converted yet, otherwise we might have gotten away with globally enabling `--default-enum-style` to apply this style to _every_ enum. At the same time certain enums could use `--bitfield-enum` yet those eligible also suffer from the same lack of a type, requiring the user to work with `_bindgen_ty_XX` which is far from ergonomic.
3ed2434 to
d5a7fe6
Compare
Searching for
pub typereveals a lot of typedefs for enums that could have been a newtype with associated constants. Unfortunately there are still a lot of private enums in the source, some with atypedeffollowing: these cannot be converted yet, otherwise we might have gotten away with globally enabling--default-enum-styleto apply this style to every enum.At the same time certain enums could use
--bitfield-enumyet those eligible also suffer from the same lack of a type, requiring the user to work with_bindgen_ty_XXwhich is far from ergonomic.