optionally use associated constants in bitfields #1293
Conversation
|
Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @emilio (or someone else) soon. |
Which tests do fail? And which OS are you on? |
src/lib.rs
Outdated
| @@ -1235,6 +1235,12 @@ impl Builder { | |||
| self.options.no_hash_types.insert(arg.into()); | |||
| self | |||
| } | |||
|
|
|||
| /// Use associated constants for bitfields. | |||
| pub fn use_associated_consts(mut self, doIt: bool) -> Builder { | |||
There was a problem hiding this comment.
This probably deserves a bit more specific name. And please use snake_case for the argument name.
| result.push(quote! { | ||
| impl #enum_ident { | ||
| #doc | ||
| pub const #variant_ident : #rust_ty = #rust_ty ( #expr ); |
There was a problem hiding this comment.
This is probably even not worth the option, and just worth guarding it after a feature guard, wdyt? see src/features.rs.
|
Also, thanks a lot for the patch! :) |
|
How do they fail? Can you post theerror? Do you happen to be in a linux32 OS or something of the sort? |
|
All pass now, PTAL |
| @@ -68,8 +84,12 @@ impl ::std::ops::BitAndAssign for Buz { | |||
| #[repr(C)] | |||
| #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] | |||
| pub struct Buz(pub i8); | |||
| pub const NS_FOO: _bindgen_ty_1 = _bindgen_ty_1(1); | |||
| pub const NS_BAR: _bindgen_ty_1 = _bindgen_ty_1(2); | |||
| impl _bindgen_ty_1 { | |||
There was a problem hiding this comment.
Hmm, I didn't think about unnamed enums, but this is a problem... Because the constants are no longer in the top-level namespace, and their name is not deterministic.
src/codegen/mod.rs
Outdated
| pub const #ident : #rust_ty = #rust_ty ( #expr ); | ||
| }); | ||
| EnumBuilder::Bitfield { canonical_name, .. } => { | ||
| if ctx.options().rust_features().associated_const { |
There was a problem hiding this comment.
So we need to guard this condition to prevent entering here if the enum is unnamed.
There was a problem hiding this comment.
How can we check? I see "_bindgen_ty_{}" in Item::base_name but am not sure how this information is propagated further. (I'm not sure whether we care about cases where the user actually defines a type called _bindgen_ty_1 or such.)
There was a problem hiding this comment.
Unnamed enums can be detected using ir::Type::name, in which case the name would be None`.
|
PTAL |
|
Looks great, thank you! @bors-servo r+ |
|
📌 Commit 7024cf6 has been approved by |
|
☀️ Test successful - status-travis |
|
Thanks! I think we can close #1166 now. |
See #1166
r? @emilio