diff --git a/CHANGELOG.md b/CHANGELOG.md index 74ff973751..cce140e373 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -227,6 +227,7 @@ -------------------------------------------------------------------------------- # Unreleased ## Added + * Add option in CLI to use rustified repr-C enums (--rustified-repr-c-enum, #3265). ## Changed ## Removed - Removed support for generating code for rustc versions < 1.51. diff --git a/bindgen-integration/build.rs b/bindgen-integration/build.rs index 1b7c2b3b82..12867bc2de 100644 --- a/bindgen-integration/build.rs +++ b/bindgen-integration/build.rs @@ -237,6 +237,7 @@ fn setup_macro_test() { .enable_cxx_namespaces() .default_enum_style(EnumVariation::Rust { non_exhaustive: false, + repr_c: false, }) .raw_line("pub use self::root::*;") .raw_line("extern { fn my_prefixed_function_to_remove(i: i32); }") diff --git a/bindgen-tests/tests/expectations/tests/enum-doc-rusty-repr-c.rs b/bindgen-tests/tests/expectations/tests/enum-doc-rusty-repr-c.rs new file mode 100644 index 0000000000..311bbdf8e9 --- /dev/null +++ b/bindgen-tests/tests/expectations/tests/enum-doc-rusty-repr-c.rs @@ -0,0 +1,38 @@ +#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)] +#[repr(C)] +/// Document enum +#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)] +pub enum B { + /// Document field with three slashes + VAR_A = 0, + /// Document field with preceding star + VAR_B = 1, + /// Document field with preceding exclamation + VAR_C = 2, + ///< Document field with following star + VAR_D = 3, + ///< Document field with following exclamation + VAR_E = 4, + /** Document field with preceding star, with a loong long multiline + comment. + + Very interesting documentation, definitely.*/ + VAR_F = 5, +} +#[allow(clippy::unnecessary_operation, clippy::identity_op)] +const _: () = { + ["Size of B"][::std::mem::size_of::() - 4usize]; + ["Alignment of B"][::std::mem::align_of::() - 4usize]; +}; +#[repr(C)] +/// An enum with a value larger than the platform's `c_int` +#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)] +pub enum BigEnum { + /// A value that is too large to fit in a 32-bit integer + BIG_ENUM_BIG = 4294967296, +} +#[allow(clippy::unnecessary_operation, clippy::identity_op)] +const _: () = { + ["Size of BigEnum"][::std::mem::size_of::() - 8usize]; + ["Alignment of BigEnum"][::std::mem::align_of::() - 8usize]; +}; diff --git a/bindgen-tests/tests/expectations/tests/enums-repr-c.rs b/bindgen-tests/tests/expectations/tests/enums-repr-c.rs new file mode 100644 index 0000000000..d618ee3c83 --- /dev/null +++ b/bindgen-tests/tests/expectations/tests/enums-repr-c.rs @@ -0,0 +1,67 @@ +#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)] +#[repr(C)] +#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)] +pub enum small_value_t { + SMALL_VALUE = 1, +} +#[allow(clippy::unnecessary_operation, clippy::identity_op)] +const _: () = { + ["Size of small_value_t"][::std::mem::size_of::() - 4usize]; + ["Alignment of small_value_t"][::std::mem::align_of::() - 4usize]; +}; +#[repr(C)] +#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)] +pub enum medium_value_t { + MEDIUM_VALUE = 256, +} +#[allow(clippy::unnecessary_operation, clippy::identity_op)] +const _: () = { + ["Size of medium_value_t"][::std::mem::size_of::() - 4usize]; + ["Alignment of medium_value_t"][::std::mem::align_of::() - 4usize]; +}; +#[repr(C)] +#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)] +pub enum large_value_t { + LARGE_VALUE = 16777216, +} +#[allow(clippy::unnecessary_operation, clippy::identity_op)] +const _: () = { + ["Size of large_value_t"][::std::mem::size_of::() - 4usize]; + ["Alignment of large_value_t"][::std::mem::align_of::() - 4usize]; +}; +#[repr(C)] +#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)] +pub enum unsigned_value_t { + UNSIGNED_VALUE = 2147483648, +} +#[allow(clippy::unnecessary_operation, clippy::identity_op)] +const _: () = { + ["Size of unsigned_value_t"][::std::mem::size_of::() - 4usize]; + [ + "Alignment of unsigned_value_t", + ][::std::mem::align_of::() - 4usize]; +}; +#[repr(C)] +#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)] +pub enum mixed_sign_t { + NEGATIVE_VALUE = -1, + POSITIVE_VALUE = 1, +} +#[allow(clippy::unnecessary_operation, clippy::identity_op)] +const _: () = { + ["Size of mixed_sign_t"][::std::mem::size_of::() - 4usize]; + ["Alignment of mixed_sign_t"][::std::mem::align_of::() - 4usize]; +}; +#[repr(C)] +#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)] +pub enum wide_mixed_sign_t { + WIDE_NEGATIVE_VALUE = -1, + WIDE_UNSIGNED_VALUE = 2147483648, +} +#[allow(clippy::unnecessary_operation, clippy::identity_op)] +const _: () = { + ["Size of wide_mixed_sign_t"][::std::mem::size_of::() - 8usize]; + [ + "Alignment of wide_mixed_sign_t", + ][::std::mem::align_of::() - 8usize]; +}; diff --git a/bindgen-tests/tests/headers/enum-doc-rusty-repr-c.h b/bindgen-tests/tests/headers/enum-doc-rusty-repr-c.h new file mode 100644 index 0000000000..22cf4162f5 --- /dev/null +++ b/bindgen-tests/tests/headers/enum-doc-rusty-repr-c.h @@ -0,0 +1,9 @@ +// bindgen-flags: --rustified-repr-c-enum B|BigEnum + +#include "enum-doc.h" + +/** An enum with a value larger than the platform's `c_int` */ +enum BigEnum { + /** A value that is too large to fit in a 32-bit integer */ + BIG_ENUM_BIG = 4294967296, +}; diff --git a/bindgen-tests/tests/headers/enums-repr-c.hpp b/bindgen-tests/tests/headers/enums-repr-c.hpp new file mode 100644 index 0000000000..061d650ec0 --- /dev/null +++ b/bindgen-tests/tests/headers/enums-repr-c.hpp @@ -0,0 +1,39 @@ +// bindgen-flags: --rustified-repr-c-enum ".*" -- -std=c++11 + +typedef enum { + SMALL_VALUE = 0x1, +} small_value_t; + +static_assert(sizeof(small_value_t) == 4, ""); + +typedef enum { + MEDIUM_VALUE = 0x100, +} medium_value_t; + +static_assert(sizeof(medium_value_t) == 4, ""); + +typedef enum { + LARGE_VALUE = 0x1000000, +} large_value_t; + +static_assert(sizeof(large_value_t) == 4, ""); + +typedef enum { + UNSIGNED_VALUE = 0x80000000, +} unsigned_value_t; + +static_assert(sizeof(unsigned_value_t) == 4, ""); + +typedef enum { + NEGATIVE_VALUE = -1, + POSITIVE_VALUE = 1, +} mixed_sign_t; + +static_assert(sizeof(mixed_sign_t) == 4, ""); + +typedef enum { + WIDE_NEGATIVE_VALUE = -1, + WIDE_UNSIGNED_VALUE = 0x80000000, +} wide_mixed_sign_t; + +static_assert(sizeof(wide_mixed_sign_t) == 8, ""); diff --git a/bindgen/codegen/helpers.rs b/bindgen/codegen/helpers.rs index 9b86ba47b0..5d2cccfd4e 100644 --- a/bindgen/codegen/helpers.rs +++ b/bindgen/codegen/helpers.rs @@ -53,6 +53,12 @@ pub(crate) mod attributes { } } + pub(crate) fn repr_c() -> TokenStream { + quote! { + #[repr(C)] + } + } + pub(crate) fn doc(comment: &str) -> TokenStream { if comment.is_empty() { quote!() diff --git a/bindgen/codegen/mod.rs b/bindgen/codegen/mod.rs index d61f87c901..5c53500242 100644 --- a/bindgen/codegen/mod.rs +++ b/bindgen/codegen/mod.rs @@ -3304,6 +3304,8 @@ pub enum EnumVariation { Rust { /// Indicates whether the generated struct should be `#[non_exhaustive]` non_exhaustive: bool, + /// Indicates whether the generated struct should be `#[repr(C)]` + repr_c: bool, }, /// The code for this enum will use a newtype NewType { @@ -3335,11 +3337,14 @@ impl fmt::Display for EnumVariation { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let s = match self { Self::Rust { - non_exhaustive: false, - } => "rust", - Self::Rust { - non_exhaustive: true, - } => "rust_non_exhaustive", + non_exhaustive, + repr_c, + } => match (*non_exhaustive, *repr_c) { + (false, false) => "rust", + (false, true) => "rust_repr_c", + (true, false) => "rust_non_exhaustive", + (true, true) => "rust_non_exhaustive_repr_c", + }, Self::NewType { is_bitfield: true, .. } => "bitfield", @@ -3368,9 +3373,19 @@ impl FromStr for EnumVariation { match s { "rust" => Ok(EnumVariation::Rust { non_exhaustive: false, + repr_c: false, + }), + "rust_repr_c" => Ok(EnumVariation::Rust { + non_exhaustive: false, + repr_c: true, }), "rust_non_exhaustive" => Ok(EnumVariation::Rust { non_exhaustive: true, + repr_c: false, + }), + "rust_non_exhaustive_repr_c" => Ok(EnumVariation::Rust { + non_exhaustive: true, + repr_c: true, }), "bitfield" => Ok(EnumVariation::NewType { is_bitfield: true, @@ -3390,7 +3405,8 @@ impl FromStr for EnumVariation { std::io::ErrorKind::InvalidInput, concat!( "Got an invalid EnumVariation. Accepted values ", - "are 'rust', 'rust_non_exhaustive', 'bitfield', 'consts',", + "are 'rust', 'rust_repr_c', 'rust_non_exhaustive', ", + "'rust_non_exhaustive_repr_c', 'bitfield', 'consts', ", "'moduleconsts', 'newtype' and 'newtype_global'." ), )), @@ -3417,6 +3433,7 @@ struct EnumBuilder { enum EnumBuilderKind { Rust { non_exhaustive: bool, + repr_c: bool, }, NewType { is_bitfield: bool, @@ -3462,9 +3479,13 @@ impl EnumBuilder { is_anonymous: enum_is_anonymous, }, - EnumVariation::Rust { non_exhaustive } => { - EnumBuilderKind::Rust { non_exhaustive } - } + EnumVariation::Rust { + non_exhaustive, + repr_c, + } => EnumBuilderKind::Rust { + non_exhaustive, + repr_c, + }, EnumVariation::Consts => EnumBuilderKind::Consts { needs_typedef: !has_typedef, @@ -3675,14 +3696,23 @@ impl EnumBuilder { // 2. Generate the enum representation match self.kind { - EnumBuilderKind::Rust { non_exhaustive } => { + EnumBuilderKind::Rust { + non_exhaustive, + repr_c, + } => { let non_exhaustive_opt = non_exhaustive.then(attributes::non_exhaustive); + let repr = if repr_c { + attributes::repr_c() + } else { + quote! { #[repr(#enum_repr)] } + }; + quote! { // Note: repr is on top of attrs to keep the test expectations diff small. // a future commit could move it further down. - #[repr(#enum_repr)] + #repr #non_exhaustive_opt #( #attrs )* pub enum #enum_ident { @@ -4085,6 +4115,64 @@ impl CodeGenerator for Enum { let item = builder.build(ctx, &enum_rust_ty); result.push(item); + + if matches!(variation, EnumVariation::Rust { repr_c: true, .. }) { + if let Some(layout) = layout { + // rustc sizes a fieldless `#[repr(C)]` enum like the target's + // default C enum type, i.e. `c_int` on all supported targets. + if layout.size != 4 || layout.align != 4 { + warn!( + "enum `{ident}` has size {} and alignment {}, but \ + `#[repr(C)]` enums are usually 4 bytes; it and any \ + type containing it may get an incompatible layout \ + (e.g. due to -fshort-enums or an explicit underlying \ + type)", + layout.size, layout.align + ); + } + + if !ctx.options().layout_tests { + return; + } + + let compile_time = ctx.options().rust_features().offset_of; + let fn_name = if compile_time { + None + } else { + let fn_name = format!("bindgen_test_layout_{ident}"); + Some(ctx.rust_ident_raw(fn_name)) + }; + let prefix = ctx.trait_prefix(); + let size_of_expr = quote! { + ::#prefix::mem::size_of::<#ident>() + }; + let align_of_expr = quote! { + ::#prefix::mem::align_of::<#ident>() + }; + let size = layout.size; + let align = layout.align; + let size_of_err = format!("Size of {ident}"); + let align_of_err = format!("Alignment of {ident}"); + + if compile_time { + result.push(quote! { + #[allow(clippy::unnecessary_operation, clippy::identity_op)] + const _: () = { + [#size_of_err][#size_of_expr - #size]; + [#align_of_err][#align_of_expr - #align]; + }; + }); + } else { + result.push(quote! { + #[test] + fn #fn_name() { + assert_eq!(#size_of_expr, #size, #size_of_err); + assert_eq!(#align_of_expr, #align, #align_of_err); + } + }); + } + } + } } } diff --git a/bindgen/ir/enum_ty.rs b/bindgen/ir/enum_ty.rs index 8566622100..d4815d0a1a 100644 --- a/bindgen/ir/enum_ty.rs +++ b/bindgen/ir/enum_ty.rs @@ -219,6 +219,7 @@ impl Enum { ) { EnumVariation::Rust { non_exhaustive: false, + repr_c: false, } } else if self.is_matching_enum( ctx, @@ -227,6 +228,16 @@ impl Enum { ) { EnumVariation::Rust { non_exhaustive: true, + repr_c: false, + } + } else if self.is_matching_enum( + ctx, + &ctx.options().rustified_repr_c_enums, + item, + ) { + EnumVariation::Rust { + non_exhaustive: false, + repr_c: true, } } else if self.is_matching_enum( ctx, diff --git a/bindgen/lib.rs b/bindgen/lib.rs index 88e07a41ef..86d4cdd31b 100644 --- a/bindgen/lib.rs +++ b/bindgen/lib.rs @@ -471,7 +471,7 @@ impl Builder { impl BindgenOptions { fn build(&mut self) { - const REGEX_SETS_LEN: usize = 29; + const REGEX_SETS_LEN: usize = 30; let regex_sets: [_; REGEX_SETS_LEN] = [ &mut self.blocklisted_types, @@ -492,6 +492,7 @@ impl BindgenOptions { &mut self.newtype_global_enums, &mut self.rustified_enums, &mut self.rustified_non_exhaustive_enums, + &mut self.rustified_repr_c_enums, &mut self.type_alias, &mut self.new_type_alias, &mut self.new_type_alias_deref, @@ -529,6 +530,7 @@ impl BindgenOptions { "--rustified-enum-non-exhaustive", "--constified-enum-module", "--constified-enum", + "--rustified-repr-c-enum", "--type-alias", "--new-type-alias", "--new-type-alias-deref", diff --git a/bindgen/options/cli.rs b/bindgen/options/cli.rs index 18b16cfcba..fd4deae075 100644 --- a/bindgen/options/cli.rs +++ b/bindgen/options/cli.rs @@ -176,6 +176,10 @@ struct BindgenCommand { #[arg(long)] depfile: Option, /// The default STYLE of code used to generate enums. + /// + /// Warning: the repr(C) rustified styles produce ABI-incompatible bindings if Rust's + /// repr(C) does not match the C/C++ enum layout, e.g. with -fshort-enums or an explicit + /// underlying type. #[arg(long, value_name = "STYLE")] default_enum_style: Option, /// Mark any enum whose name matches REGEX as a set of bitfield flags. @@ -193,6 +197,12 @@ struct BindgenCommand { /// Mark any enum whose name matches REGEX as a non-exhaustive Rust enum. #[arg(long, value_name = "REGEX")] rustified_non_exhaustive_enum: Vec, + /// Mark any enum whose name matches REGEX as a repr(C) Rust enum. + /// + /// Warning: this produces ABI-incompatible bindings if Rust's repr(C) does not match the + /// C/C++ enum layout, e.g. with -fshort-enums or an explicit underlying type. + #[arg(long, value_name = "REGEX")] + rustified_repr_c_enum: Vec, /// Mark any enum whose name matches REGEX as a series of constants. #[arg(long, value_name = "REGEX")] constified_enum: Vec, @@ -594,6 +604,7 @@ where newtype_global_enum, rustified_enum, rustified_non_exhaustive_enum, + rustified_repr_c_enum, constified_enum, constified_enum_module, default_macro_constant_type, @@ -906,6 +917,7 @@ where newtype_global_enum, rustified_enum, rustified_non_exhaustive_enum, + rustified_repr_c_enum, constified_enum, constified_enum_module, default_macro_constant_type, diff --git a/bindgen/options/mod.rs b/bindgen/options/mod.rs index bc0cb75a33..7b901eb1b2 100644 --- a/bindgen/options/mod.rs +++ b/bindgen/options/mod.rs @@ -447,7 +447,11 @@ options! { /// To set the style for individual `enum`s, use [`Builder::bitfield_enum`], /// [`Builder::newtype_enum`], [`Builder::newtype_global_enum`], /// [`Builder::rustified_enum`], [`Builder::rustified_non_exhaustive_enum`], - /// [`Builder::constified_enum_module`] or [`Builder::constified_enum`]. + /// [`Builder::rustified_repr_c_enum`], [`Builder::constified_enum_module`], + /// or [`Builder::constified_enum`]. + /// + /// The `repr(C)` rustified styles share the layout caveats described in + /// [`Builder::rustified_repr_c_enum`]. pub fn default_enum_style( mut self, arg: EnumVariation, @@ -554,6 +558,27 @@ options! { }, as_args: "--rustified-non-exhaustive-enum", }, + /// `enum`s marked as `repr(C)` Rust `enum`s. + rustified_repr_c_enums: RegexSet { + methods: { + regex_option! { + /// Mark the given `enum` as a `repr(C)` Rust `enum`. + /// + /// This is similar to the [`Builder::rustified_enum`] style, but the `enum` is + /// tagged with the `#[repr(C)]` attribute, as required for cross-language CFI. + /// + /// **Use this with caution**, Rust's `#[repr(C)]` does not match the layout of + /// every C/C++ `enum`, e.g. ones using `-fshort-enums`, an explicit underlying + /// type, or values outside the `c_int` range. A mismatch makes the `enum` and + /// any type containing it ABI-incompatible. + pub fn rustified_repr_c_enum>(mut self, arg: T) -> Builder { + self.options.rustified_repr_c_enums.insert(arg); + self + } + } + }, + as_args: "--rustified-repr-c-enum", + }, /// `enum`s marked as modules of constants. constified_enum_modules: RegexSet { methods: {