From ccc43cd24e66263587fbcc0ad6161c7efa18a413 Mon Sep 17 00:00:00 2001 From: uk <50893351+unterkontrolle@users.noreply.github.com> Date: Mon, 10 Jun 2019 15:16:19 -0300 Subject: [PATCH 1/6] Add support for non_exhaustive rustified enums. Implements the feature discussed in https://github.com/rust-lang/rust-bindgen/issues/1554. --- src/codegen/helpers.rs | 6 ++++ src/codegen/mod.rs | 46 ++++++++++++++------------ src/ir/enum_ty.rs | 4 ++- src/lib.rs | 33 +++++++++++++++--- src/options.rs | 2 +- tests/expectations/tests/issue-1554.rs | 16 +++++++++ tests/headers/issue-1554.h | 6 ++++ 7 files changed, 85 insertions(+), 28 deletions(-) create mode 100644 tests/expectations/tests/issue-1554.rs create mode 100644 tests/headers/issue-1554.h diff --git a/src/codegen/helpers.rs b/src/codegen/helpers.rs index b630a70bd..1e8534ee0 100644 --- a/src/codegen/helpers.rs +++ b/src/codegen/helpers.rs @@ -42,6 +42,12 @@ pub mod attributes { } } + pub fn non_exhaustive() -> TokenStream { + quote! { + #[non_exhaustive] + } + } + pub fn doc(comment: String) -> TokenStream { // NOTE(emilio): By this point comments are already preprocessed and in // `///` form. Quote turns them into `#[doc]` comments, but oh well. diff --git a/src/codegen/mod.rs b/src/codegen/mod.rs index ceb2f5b86..362068929 100644 --- a/src/codegen/mod.rs +++ b/src/codegen/mod.rs @@ -2170,7 +2170,10 @@ impl MethodCodegen for Method { #[derive(Copy, Clone, PartialEq, Debug)] pub enum EnumVariation { /// The code for this enum will use a Rust enum - Rust, + /// + /// When the boolean parameter on this variant is set to true, + /// the generated enum should be non_exhaustive. + Rust(bool), /// The code for this enum will use a bitfield Bitfield, /// The code for this enum will use consts @@ -2182,14 +2185,7 @@ pub enum EnumVariation { impl EnumVariation { fn is_rust(&self) -> bool { match *self { - EnumVariation::Rust => true, - _ => false - } - } - - fn is_bitfield(&self) -> bool { - match *self { - EnumVariation::Bitfield {..} => true, + EnumVariation::Rust(_) => true, _ => false } } @@ -2216,13 +2212,14 @@ impl std::str::FromStr for EnumVariation { /// Create a `EnumVariation` from a string. fn from_str(s: &str) -> Result { match s { - "rust" => Ok(EnumVariation::Rust), + "rust" => Ok(EnumVariation::Rust(false)), + "rust_non_exhaustive" => Ok(EnumVariation::Rust(true)), "bitfield" => Ok(EnumVariation::Bitfield), "consts" => Ok(EnumVariation::Consts), "moduleconsts" => Ok(EnumVariation::ModuleConsts), _ => Err(std::io::Error::new(std::io::ErrorKind::InvalidInput, concat!("Got an invalid EnumVariation. Accepted values ", - "are 'rust', 'bitfield', 'consts', and ", + "are 'rust', 'rust_non_exhaustive', 'bitfield', 'consts', and ", "'moduleconsts'."))), } } @@ -2288,7 +2285,7 @@ impl<'a> EnumBuilder<'a> { } } - EnumVariation::Rust => { + EnumVariation::Rust(_) => { let tokens = quote!(); EnumBuilder::Rust { codegen_depth: enum_codegen_depth + 1, @@ -2580,15 +2577,22 @@ impl CodeGenerator for Enum { let variation = self.computed_enum_variation(ctx, item); // TODO(emilio): Delegate this to the builders? - if variation.is_rust() { - attrs.push(attributes::repr(repr_name)); - } else if variation.is_bitfield() { - if ctx.options().rust_features.repr_transparent { - attrs.push(attributes::repr("transparent")); - } else { - attrs.push(attributes::repr("C")); - } - } + match variation { + EnumVariation::Rust(non_exhaustive) => { + attrs.push(attributes::repr(repr_name)); + if non_exhaustive { + attrs.push(attributes::non_exhaustive()); + } + }, + EnumVariation::Bitfield => { + if ctx.options().rust_features.repr_transparent { + attrs.push(attributes::repr("transparent")); + } else { + attrs.push(attributes::repr("C")); + } + }, + _ => {}, + }; if let Some(comment) = item.comment(ctx) { attrs.push(attributes::doc(comment)); diff --git a/src/ir/enum_ty.rs b/src/ir/enum_ty.rs index f3da21997..d4fbdf3ab 100644 --- a/src/ir/enum_ty.rs +++ b/src/ir/enum_ty.rs @@ -164,7 +164,9 @@ impl Enum { } else if self.is_matching_enum(ctx, &ctx.options().bitfield_enums, item) { EnumVariation::Bitfield } else if self.is_matching_enum(ctx, &ctx.options().rustified_enums, item) { - EnumVariation::Rust + EnumVariation::Rust(false) + } else if self.is_matching_enum(ctx, &ctx.options().rustified_enums_non_exhaustive, item) { + EnumVariation::Rust(true) } else if self.is_matching_enum(ctx, &ctx.options().constified_enums, item) { EnumVariation::Consts } else { diff --git a/src/lib.rs b/src/lib.rs index 4a695f603..26d15f1d4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -226,7 +226,8 @@ impl Builder { if self.options.default_enum_style != Default::default() { output_vector.push("--default-enum-style=".into()); output_vector.push(match self.options.default_enum_style { - codegen::EnumVariation::Rust => "rust", + codegen::EnumVariation::Rust(false) => "rust", + codegen::EnumVariation::Rust(true) => "rust_non_exhaustive", codegen::EnumVariation::Bitfield => "bitfield", codegen::EnumVariation::Consts => "consts", codegen::EnumVariation::ModuleConsts => "moduleconsts", @@ -253,6 +254,16 @@ impl Builder { }) .count(); + self.options + .rustified_enums_non_exhaustive + .get_items() + .iter() + .map(|item| { + output_vector.push("--rustified-enum-non-exhaustive".into()); + output_vector.push(item.to_owned()); + }) + .count(); + self.options .constified_enum_modules .get_items() @@ -810,15 +821,24 @@ impl Builder { /// This makes bindgen generate enums instead of constants. Regular /// expressions are supported. /// - /// **Use this with caution.** You should not be using Rust enums unless - /// you have complete control of the C/C++ code that you're binding to. - /// Take a look at https://github.com/rust-lang/rust/issues/36927 for - /// more information. + /// **Use this with caution,** you probably want to use the non_exhaustive + /// flavor of rust enums instead of this one. Take a look at + /// https://github.com/rust-lang/rust/issues/36927 for more information. pub fn rustified_enum>(mut self, arg: T) -> Builder { self.options.rustified_enums.insert(arg); self } + /// Mark the given enum (or set of enums, if using a pattern) as a Rust + /// enum with the #[non_exhaustive] attribute. + /// + /// This makes bindgen generate enums instead of constants. Regular + /// expressions are supported. + pub fn rustified_enum_non_exhaustive>(mut self, arg: T) -> Builder { + self.options.rustified_enums_non_exhaustive.insert(arg); + self + } + /// Mark the given enum (or set of enums, if using a pattern) as a set of /// constants that are not to be put into a module. pub fn constified_enum>(mut self, arg: T) -> Builder { @@ -1367,6 +1387,8 @@ struct BindgenOptions { /// The enum patterns to mark an enum as a Rust enum. rustified_enums: RegexSet, + rustified_enums_non_exhaustive: RegexSet, + /// The enum patterns to mark an enum as a module of constants. constified_enum_modules: RegexSet, @@ -1620,6 +1642,7 @@ impl Default for BindgenOptions { default_enum_style: Default::default(), bitfield_enums: Default::default(), rustified_enums: Default::default(), + rustified_enums_non_exhaustive: Default::default(), constified_enums: Default::default(), constified_enum_modules: Default::default(), builtins: false, diff --git a/src/options.rs b/src/options.rs index 300036e0a..43ad2edc4 100644 --- a/src/options.rs +++ b/src/options.rs @@ -31,7 +31,7 @@ where .help("The default style of code used to generate enums.") .value_name("variant") .default_value("consts") - .possible_values(&["consts", "moduleconsts", "bitfield", "rust"]) + .possible_values(&["consts", "moduleconsts", "bitfield", "rust", "rust_non_exhaustive"]) .multiple(false), Arg::with_name("bitfield-enum") .long("bitfield-enum") diff --git a/tests/expectations/tests/issue-1554.rs b/tests/expectations/tests/issue-1554.rs new file mode 100644 index 000000000..49da62bbf --- /dev/null +++ b/tests/expectations/tests/issue-1554.rs @@ -0,0 +1,16 @@ +/* automatically generated by rust-bindgen */ + +#![allow( + dead_code, + non_snake_case, + non_camel_case_types, + non_upper_case_globals +)] + +#[repr(u32)] +#[non_exhaustive] +#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] +pub enum Planet { + earth = 0, + mars = 1, +} diff --git a/tests/headers/issue-1554.h b/tests/headers/issue-1554.h new file mode 100644 index 000000000..3f840fddf --- /dev/null +++ b/tests/headers/issue-1554.h @@ -0,0 +1,6 @@ +// bindgen-flags: --default-enum-style rust_non_exhaustive + +enum Planet { + earth, + mars +}; From ac78e077c70586d66d19aee21ecececc16cb90f8 Mon Sep 17 00:00:00 2001 From: uk <50893351+unterkontrolle@users.noreply.github.com> Date: Tue, 11 Jun 2019 11:19:59 -0300 Subject: [PATCH 2/6] Rename a couple miscalled identifiers. See https://github.com/rust-lang/rust-bindgen/pull/1575 --- src/codegen/mod.rs | 20 ++++++++++---------- src/ir/enum_ty.rs | 6 +++--- src/lib.rs | 14 +++++++------- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/codegen/mod.rs b/src/codegen/mod.rs index 362068929..aa920c9b7 100644 --- a/src/codegen/mod.rs +++ b/src/codegen/mod.rs @@ -2170,10 +2170,10 @@ impl MethodCodegen for Method { #[derive(Copy, Clone, PartialEq, Debug)] pub enum EnumVariation { /// The code for this enum will use a Rust enum - /// - /// When the boolean parameter on this variant is set to true, - /// the generated enum should be non_exhaustive. - Rust(bool), + Rust { + /// Indicates whether the generated struct should be #[non_exhaustive] + non_exhaustive: bool + }, /// The code for this enum will use a bitfield Bitfield, /// The code for this enum will use consts @@ -2185,7 +2185,7 @@ pub enum EnumVariation { impl EnumVariation { fn is_rust(&self) -> bool { match *self { - EnumVariation::Rust(_) => true, + EnumVariation::Rust{ non_exhaustive: _ } => true, _ => false } } @@ -2212,8 +2212,8 @@ impl std::str::FromStr for EnumVariation { /// Create a `EnumVariation` from a string. fn from_str(s: &str) -> Result { match s { - "rust" => Ok(EnumVariation::Rust(false)), - "rust_non_exhaustive" => Ok(EnumVariation::Rust(true)), + "rust" => Ok(EnumVariation::Rust{ non_exhaustive: false }), + "rust_non_exhaustive" => Ok(EnumVariation::Rust{ non_exhaustive: true }), "bitfield" => Ok(EnumVariation::Bitfield), "consts" => Ok(EnumVariation::Consts), "moduleconsts" => Ok(EnumVariation::ModuleConsts), @@ -2285,7 +2285,7 @@ impl<'a> EnumBuilder<'a> { } } - EnumVariation::Rust(_) => { + EnumVariation::Rust { non_exhaustive: _ } => { let tokens = quote!(); EnumBuilder::Rust { codegen_depth: enum_codegen_depth + 1, @@ -2578,9 +2578,9 @@ impl CodeGenerator for Enum { // TODO(emilio): Delegate this to the builders? match variation { - EnumVariation::Rust(non_exhaustive) => { + EnumVariation::Rust { non_exhaustive: nh } => { attrs.push(attributes::repr(repr_name)); - if non_exhaustive { + if nh { attrs.push(attributes::non_exhaustive()); } }, diff --git a/src/ir/enum_ty.rs b/src/ir/enum_ty.rs index d4fbdf3ab..be33ed4b7 100644 --- a/src/ir/enum_ty.rs +++ b/src/ir/enum_ty.rs @@ -164,9 +164,9 @@ impl Enum { } else if self.is_matching_enum(ctx, &ctx.options().bitfield_enums, item) { EnumVariation::Bitfield } else if self.is_matching_enum(ctx, &ctx.options().rustified_enums, item) { - EnumVariation::Rust(false) - } else if self.is_matching_enum(ctx, &ctx.options().rustified_enums_non_exhaustive, item) { - EnumVariation::Rust(true) + EnumVariation::Rust { non_exhaustive: false } + } else if self.is_matching_enum(ctx, &ctx.options().rustified_non_exhaustive_enums, item) { + EnumVariation::Rust { non_exhaustive: true } } else if self.is_matching_enum(ctx, &ctx.options().constified_enums, item) { EnumVariation::Consts } else { diff --git a/src/lib.rs b/src/lib.rs index 26d15f1d4..b669bcb26 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -226,8 +226,8 @@ impl Builder { if self.options.default_enum_style != Default::default() { output_vector.push("--default-enum-style=".into()); output_vector.push(match self.options.default_enum_style { - codegen::EnumVariation::Rust(false) => "rust", - codegen::EnumVariation::Rust(true) => "rust_non_exhaustive", + codegen::EnumVariation::Rust { non_exhaustive: false } => "rust", + codegen::EnumVariation::Rust { non_exhaustive: true } => "rust_non_exhaustive", codegen::EnumVariation::Bitfield => "bitfield", codegen::EnumVariation::Consts => "consts", codegen::EnumVariation::ModuleConsts => "moduleconsts", @@ -255,7 +255,7 @@ impl Builder { .count(); self.options - .rustified_enums_non_exhaustive + .rustified_non_exhaustive_enums .get_items() .iter() .map(|item| { @@ -834,8 +834,8 @@ impl Builder { /// /// This makes bindgen generate enums instead of constants. Regular /// expressions are supported. - pub fn rustified_enum_non_exhaustive>(mut self, arg: T) -> Builder { - self.options.rustified_enums_non_exhaustive.insert(arg); + pub fn rustified_non_exhaustive_enum>(mut self, arg: T) -> Builder { + self.options.rustified_non_exhaustive_enums.insert(arg); self } @@ -1387,7 +1387,7 @@ struct BindgenOptions { /// The enum patterns to mark an enum as a Rust enum. rustified_enums: RegexSet, - rustified_enums_non_exhaustive: RegexSet, + rustified_non_exhaustive_enums: RegexSet, /// The enum patterns to mark an enum as a module of constants. constified_enum_modules: RegexSet, @@ -1642,7 +1642,7 @@ impl Default for BindgenOptions { default_enum_style: Default::default(), bitfield_enums: Default::default(), rustified_enums: Default::default(), - rustified_enums_non_exhaustive: Default::default(), + rustified_non_exhaustive_enums: Default::default(), constified_enums: Default::default(), constified_enum_modules: Default::default(), builtins: false, From b2c478f9ccb971f350e364d16a45cdd128dca3a8 Mon Sep 17 00:00:00 2001 From: uk <50893351+unterkontrolle@users.noreply.github.com> Date: Tue, 11 Jun 2019 11:49:48 -0300 Subject: [PATCH 3/6] Add rust target configuration for feature #1554 Will panic if the feature is being used outside nightly. See https://github.com/rust-lang/rust-bindgen/pull/1575#discussion_r292231027 --- src/codegen/mod.rs | 4 +++- src/features.rs | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/codegen/mod.rs b/src/codegen/mod.rs index aa920c9b7..2863387f5 100644 --- a/src/codegen/mod.rs +++ b/src/codegen/mod.rs @@ -2580,8 +2580,10 @@ impl CodeGenerator for Enum { match variation { EnumVariation::Rust { non_exhaustive: nh } => { attrs.push(attributes::repr(repr_name)); - if nh { + if nh && ctx.options().rust_features().non_exhaustive { attrs.push(attributes::non_exhaustive()); + } else if nh && !ctx.options().rust_features().non_exhaustive { + panic!("The rust target you're using doesn't seem to support non_exhaustive enums"); } }, EnumVariation::Bitfield => { diff --git a/src/features.rs b/src/features.rs index 05f5dc422..4dc526ec6 100644 --- a/src/features.rs +++ b/src/features.rs @@ -206,6 +206,8 @@ rust_feature_def!( Nightly { /// `thiscall` calling convention ([Tracking issue](https://github.com/rust-lang/rust/issues/42202)) => thiscall_abi; + /// `non_exhaustive` enums/structs ([Tracking issue](https://github.com/rust-lang/rust/issues/44109)) + => non_exhaustive; } ); From 26d7f0aa88f78f568931528f8a528124af6ff6be Mon Sep 17 00:00:00 2001 From: uk <50893351+unterkontrolle@users.noreply.github.com> Date: Tue, 11 Jun 2019 11:57:17 -0300 Subject: [PATCH 4/6] Hopefully fixes automated tests for #1554. See https://github.com/rust-lang/rust-bindgen/pull/1575#discussion_r292230729 --- tests/expectations/tests/issue-1554.rs | 3 +++ tests/headers/issue-1554.h | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/expectations/tests/issue-1554.rs b/tests/expectations/tests/issue-1554.rs index 49da62bbf..c3fb0d486 100644 --- a/tests/expectations/tests/issue-1554.rs +++ b/tests/expectations/tests/issue-1554.rs @@ -6,6 +6,9 @@ non_camel_case_types, non_upper_case_globals )] +#![cfg(feature = "nightly")] +#![feature(non_exhaustive)] + #[repr(u32)] #[non_exhaustive] diff --git a/tests/headers/issue-1554.h b/tests/headers/issue-1554.h index 3f840fddf..edf3541c0 100644 --- a/tests/headers/issue-1554.h +++ b/tests/headers/issue-1554.h @@ -1,4 +1,4 @@ -// bindgen-flags: --default-enum-style rust_non_exhaustive +// bindgen-flags: --default-enum-style rust_non_exhaustive --raw-line '#![cfg(feature = "nightly")]' --raw-line '#![feature(non_exhaustive)]' enum Planet { earth, From 7ba2cae8c825baf411488ded7d0eb1e6cc17452d Mon Sep 17 00:00:00 2001 From: uk <50893351+unterkontrolle@users.noreply.github.com> Date: Wed, 12 Jun 2019 17:23:19 -0300 Subject: [PATCH 5/6] Add missing --rust-target flags on test case. See https://github.com/rust-lang/rust-bindgen/pull/1575#discussion_r293079226 --- tests/headers/issue-1554.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/headers/issue-1554.h b/tests/headers/issue-1554.h index edf3541c0..13452923f 100644 --- a/tests/headers/issue-1554.h +++ b/tests/headers/issue-1554.h @@ -1,4 +1,4 @@ -// bindgen-flags: --default-enum-style rust_non_exhaustive --raw-line '#![cfg(feature = "nightly")]' --raw-line '#![feature(non_exhaustive)]' +// bindgen-flags: --default-enum-style rust_non_exhaustive --rust-target nightly --raw-line '#![cfg(feature = "nightly")]' --raw-line '#![feature(non_exhaustive)]' enum Planet { earth, From 2a522304b7302191bff8fe0cb540f8bba0da395c Mon Sep 17 00:00:00 2001 From: uk <50893351+unterkontrolle@users.noreply.github.com> Date: Wed, 12 Jun 2019 17:30:33 -0300 Subject: [PATCH 6/6] Style fixes. See https://github.com/rust-lang/rust-bindgen/pull/1575 --- src/codegen/mod.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/codegen/mod.rs b/src/codegen/mod.rs index 2863387f5..15ace9e91 100644 --- a/src/codegen/mod.rs +++ b/src/codegen/mod.rs @@ -2185,7 +2185,7 @@ pub enum EnumVariation { impl EnumVariation { fn is_rust(&self) -> bool { match *self { - EnumVariation::Rust{ non_exhaustive: _ } => true, + EnumVariation::Rust{ .. } => true, _ => false } } @@ -2285,7 +2285,7 @@ impl<'a> EnumBuilder<'a> { } } - EnumVariation::Rust { non_exhaustive: _ } => { + EnumVariation::Rust { .. } => { let tokens = quote!(); EnumBuilder::Rust { codegen_depth: enum_codegen_depth + 1, @@ -2578,11 +2578,11 @@ impl CodeGenerator for Enum { // TODO(emilio): Delegate this to the builders? match variation { - EnumVariation::Rust { non_exhaustive: nh } => { + EnumVariation::Rust { non_exhaustive } => { attrs.push(attributes::repr(repr_name)); - if nh && ctx.options().rust_features().non_exhaustive { + if non_exhaustive && ctx.options().rust_features().non_exhaustive { attrs.push(attributes::non_exhaustive()); - } else if nh && !ctx.options().rust_features().non_exhaustive { + } else if non_exhaustive && !ctx.options().rust_features().non_exhaustive { panic!("The rust target you're using doesn't seem to support non_exhaustive enums"); } },