Generate and add options for bitfield-like enums. #226
Conversation
|
|
|
Is it possible to generate a |
|
Ok, if it's not urgent, I can hold this off, otherwise I'd rather merge it and fix #227 separately. |
|
I'm fine with this solution. Although bitflag is probably better, we can do that later if we want. I'm not a fan of adding dependency as well. |
| @@ -91,6 +92,10 @@ Options: | |||
| matching <regex>. Same behavior on emptyness | |||
| than the type whitelisting. | |||
|
|
|||
| --bitfield-enum=<regex> Mark a bitfield as being used as an enum. | |||
| This makes bindgen don't generate a rust enum | |||
| @@ -91,6 +92,10 @@ Options: | |||
| matching <regex>. Same behavior on emptyness | |||
| than the type whitelisting. | |||
|
|
|||
| --bitfield-enum=<regex> Mark a bitfield as being used as an enum. | |||
There was a problem hiding this comment.
Nitpick: since this is a regex that presumably makes anything that matches it a bitfield enum (I haven't read the rest of the patch yet), we should rephrase this a little bit:
Mark any union whose name matches
<regex>as a set of bitfield flags instead of an enumeration. Bindgen will generate a type with bitwise arithmetic operators, instead of a Rustenum. If the union is anonymous, match<regex>against its members.
(Does this patch handle anonymous unions as bitfields?)
I also think it might make sense to rename this option to --bitfield-union or simply --bitfield since enum is purely on the Rust side of things.
| } else { | ||
| None | ||
| } | ||
| if rt.is_valid() { Some(rt) } else { None } |
There was a problem hiding this comment.
All these changes are simply rustfmt right?
There was a problem hiding this comment.
Yup, just for the record, you can review individual commits in the github UI, if that's easier :)
| @@ -1224,6 +1224,79 @@ impl MethodCodegen for Method { | |||
| } | |||
| } | |||
|
|
|||
| enum EnumBuilder<'a> { | |||
There was a problem hiding this comment.
Nitpick: it would be nice to have brief documentation comments here and throughout the new code (I know I never got around to documenting the codegen module, but we should aim to hold ourselves to documenting new code)
There was a problem hiding this comment.
Fair enough, thanks for pointing it out :)
| } | ||
| } | ||
|
|
||
| fn with_variant_(self, |
There was a problem hiding this comment.
It was for being consistent with the aster builder method, but it doesn't make any sense now.
| self.variants() | ||
| .iter() | ||
| .any(|v| ctx.options().bitfield_enums.matches(&v.name()))) | ||
| }; |
There was a problem hiding this comment.
We should factor this out to a method on the context or on the Enum which takes a context as a parameter. No need for the logic of determining whether a type should be a bitfield to live inside code generation.
| /// Mark the given enum (or set of enums, if using a pattern) as being | ||
| /// bitfield-like. | ||
| /// | ||
| /// This makes bindgen to generate a type for it that isn't a rust `enum`. |
There was a problem hiding this comment.
Nitpick:
This makes bindgen
togenerate a typefor itthat isn't a rustenum.
| Baz = 1 << 2, | ||
| Duplicated = 1 << 2, | ||
| Negative = -3, | ||
| }; |
There was a problem hiding this comment.
Also add an anonymous union whose variants match the --bitfield-enum regex.
…s, generate default BitOr implementation.
|
@bors-servo r=fitzgen I ended up adding the BitOr impl, but it's straight-forward. I couldn't move |
|
📌 Commit 4062713 has been approved by |
Generate and add options for bitfield-like enums. r? @fitzgen or @Manishearth cc @bholley @upsuper @heycam Followups: * Reduce duplication a bit in that code (probably E-easy) * Generate an impl of `BitOr` for bitfield-like enums (E-easy/E-less-easy).
|
☀️ Test successful - status-travis |
r? @fitzgen or @Manishearth
cc @bholley @upsuper @heycam
Followups:
BitOrfor bitfield-like enums (E-easy/E-less-easy).