Skip to content

optionally use associated constants in bitfields #1293

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Apr 7, 2018
Merged

optionally use associated constants in bitfields #1293

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8021a42
optionally use associated constants in bitfields
strake Apr 2, 2018
0ada942
feature guard rather than option
strake Apr 2, 2018
7c5b2e6
test associated consts
strake Apr 4, 2018
7024cf6
not generate associated constants to unnamed types
strake Apr 7, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 23 additions & 14 deletions src/codegen/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2126,7 +2126,7 @@ impl EnumVariation {

fn is_bitfield(&self) -> bool {
match *self {
EnumVariation::Bitfield => true,
EnumVariation::Bitfield {..} => true,
_ => false
}
}
Expand Down Expand Up @@ -2278,19 +2278,28 @@ impl<'a> EnumBuilder<'a> {
}
}

EnumBuilder::Bitfield { .. } => {
let constant_name = match mangling_prefix {
Some(prefix) => {
Cow::Owned(format!("{}_{}", prefix, variant_name))
}
None => variant_name,
};

let ident = ctx.rust_ident(constant_name);
result.push(quote! {
#doc
pub const #ident : #rust_ty = #rust_ty ( #expr );
});
EnumBuilder::Bitfield { canonical_name, .. } => {
if ctx.options().rust_features().associated_const {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we need to guard this condition to prevent entering here if the enum is unnamed.

Copy link
Contributor Author

@strake strake Apr 6, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnamed enums can be detected using ir::Type::name, in which case the name would be None`.

let enum_ident = ctx.rust_ident(canonical_name);
let variant_ident = ctx.rust_ident(variant_name);
result.push(quote! {
impl #enum_ident {
#doc
pub const #variant_ident : #rust_ty = #rust_ty ( #expr );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably even not worth the option, and just worth guarding it after a feature guard, wdyt? see src/features.rs.

}
});
} else {
let ident = ctx.rust_ident(match mangling_prefix {
Some(prefix) => {
Cow::Owned(format!("{}_{}", prefix, variant_name))
}
None => variant_name,
});
result.push(quote! {
#doc
pub const #ident : #rust_ty = #rust_ty ( #expr );
});
}

self
}
Expand Down
8 changes: 8 additions & 0 deletions src/features.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ macro_rules! rust_target_base {
=> Stable_1_0 => 1.0;
/// Rust stable 1.19
=> Stable_1_19 => 1.19;
/// Rust stable 1.20
=> Stable_1_20 => 1.20;
/// Rust stable 1.21
=> Stable_1_21 => 1.21;
/// Rust stable 1.25
Expand Down Expand Up @@ -142,6 +144,8 @@ rust_feature_def!(
=> builtin_clone_impls;
/// repr(align) https://github.com/rust-lang/rust/pull/47006
=> repr_align;
/// associated constants https://github.com/rust-lang/rust/issues/29646
=> associated_const;
);

impl From<RustTarget> for RustFeatures {
Expand All @@ -152,6 +156,10 @@ impl From<RustTarget> for RustFeatures {
features.untagged_union = true;
}

if rust_target >= RustTarget::Stable_1_20 {
features.associated_const = true;
}

if rust_target >= RustTarget::Stable_1_21 {
features.builtin_clone_impls = true;
}
Expand Down
48 changes: 36 additions & 12 deletions tests/expectations/tests/bitfield-enum-basic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,18 @@

#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]

pub const Foo_Bar: Foo = Foo(2);
pub const Foo_Baz: Foo = Foo(4);
pub const Foo_Duplicated: Foo = Foo(4);
pub const Foo_Negative: Foo = Foo(-3);
impl Foo {
pub const Bar: Foo = Foo(2);
}
impl Foo {
pub const Baz: Foo = Foo(4);
}
impl Foo {
pub const Duplicated: Foo = Foo(4);
}
impl Foo {
pub const Negative: Foo = Foo(-3);
}
impl ::std::ops::BitOr<Foo> for Foo {
type Output = Self;
#[inline]
Expand Down Expand Up @@ -35,10 +43,18 @@ impl ::std::ops::BitAndAssign for Foo {
#[repr(C)]
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
pub struct Foo(pub i32);
pub const Buz_Bar: Buz = Buz(2);
pub const Buz_Baz: Buz = Buz(4);
pub const Buz_Duplicated: Buz = Buz(4);
pub const Buz_Negative: Buz = Buz(-3);
impl Buz {
pub const Bar: Buz = Buz(2);
}
impl Buz {
pub const Baz: Buz = Buz(4);
}
impl Buz {
pub const Duplicated: Buz = Buz(4);
}
impl Buz {
pub const Negative: Buz = Buz(-3);
}
impl ::std::ops::BitOr<Buz> for Buz {
type Output = Self;
#[inline]
Expand Down Expand Up @@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

pub const NS_FOO: _bindgen_ty_1 = _bindgen_ty_1(1);
}
impl _bindgen_ty_1 {
pub const NS_BAR: _bindgen_ty_1 = _bindgen_ty_1(2);
}
impl ::std::ops::BitOr<_bindgen_ty_1> for _bindgen_ty_1 {
type Output = Self;
#[inline]
Expand Down Expand Up @@ -104,8 +124,12 @@ pub struct _bindgen_ty_1(pub u32);
pub struct Dummy {
pub _address: u8,
}
pub const Dummy_DUMMY_FOO: Dummy__bindgen_ty_1 = Dummy__bindgen_ty_1(1);
pub const Dummy_DUMMY_BAR: Dummy__bindgen_ty_1 = Dummy__bindgen_ty_1(2);
impl Dummy__bindgen_ty_1 {
pub const DUMMY_FOO: Dummy__bindgen_ty_1 = Dummy__bindgen_ty_1(1);
}
impl Dummy__bindgen_ty_1 {
pub const DUMMY_BAR: Dummy__bindgen_ty_1 = Dummy__bindgen_ty_1(2);
}
impl ::std::ops::BitOr<Dummy__bindgen_ty_1> for Dummy__bindgen_ty_1 {
type Output = Self;
#[inline]
Expand Down
42 changes: 27 additions & 15 deletions tests/expectations/tests/enum-doc-bitfield.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,33 @@

#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]

/// Document field with three slashes
pub const B_VAR_A: B = B(0);
/// Document field with preceeding star
pub const B_VAR_B: B = B(1);
/// Document field with preceeding exclamation
pub const B_VAR_C: B = B(2);
/// < Document field with following star
pub const B_VAR_D: B = B(3);
/// < Document field with following exclamation
pub const B_VAR_E: B = B(4);
/// Document field with preceeding star, with a loong long multiline
/// comment.
///
/// Very interesting documentation, definitely.
pub const B_VAR_F: B = B(5);
impl B {
/// Document field with three slashes
pub const VAR_A: B = B(0);
}
impl B {
/// Document field with preceeding star
pub const VAR_B: B = B(1);
}
impl B {
/// Document field with preceeding exclamation
pub const VAR_C: B = B(2);
}
impl B {
/// < Document field with following star
pub const VAR_D: B = B(3);
}
impl B {
/// < Document field with following exclamation
pub const VAR_E: B = B(4);
}
impl B {
/// Document field with preceeding star, with a loong long multiline
/// comment.
///
/// Very interesting documentation, definitely.
pub const VAR_F: B = B(5);
}
impl ::std::ops::BitOr<B> for B {
type Output = Self;
#[inline]
Expand Down
24 changes: 18 additions & 6 deletions tests/expectations/tests/issue-1198-alias-rust-bitfield-enum.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,15 @@

#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]

pub const MyDupeEnum_A: MyDupeEnum = MyDupeEnum(0);
pub const MyDupeEnum_A_alias: MyDupeEnum = MyDupeEnum(0);
pub const MyDupeEnum_B: MyDupeEnum = MyDupeEnum(1);
impl MyDupeEnum {
pub const A: MyDupeEnum = MyDupeEnum(0);
}
impl MyDupeEnum {
pub const A_alias: MyDupeEnum = MyDupeEnum(0);
}
impl MyDupeEnum {
pub const B: MyDupeEnum = MyDupeEnum(1);
}
impl ::std::ops::BitOr<MyDupeEnum> for MyDupeEnum {
type Output = Self;
#[inline]
Expand Down Expand Up @@ -34,9 +40,15 @@ impl ::std::ops::BitAndAssign for MyDupeEnum {
#[repr(C)]
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
pub struct MyDupeEnum(pub u32);
pub const MyOtherDupeEnum_C: MyOtherDupeEnum = MyOtherDupeEnum(0);
pub const MyOtherDupeEnum_C_alias: MyOtherDupeEnum = MyOtherDupeEnum(0);
pub const MyOtherDupeEnum_D: MyOtherDupeEnum = MyOtherDupeEnum(1);
impl MyOtherDupeEnum {
pub const C: MyOtherDupeEnum = MyOtherDupeEnum(0);
}
impl MyOtherDupeEnum {
pub const C_alias: MyOtherDupeEnum = MyOtherDupeEnum(0);
}
impl MyOtherDupeEnum {
pub const D: MyOtherDupeEnum = MyOtherDupeEnum(1);
}
impl ::std::ops::BitOr<MyOtherDupeEnum> for MyOtherDupeEnum {
type Output = Self;
#[inline]
Expand Down