From 99309215afd29ed6cb7683a708370fa2aed4f060 Mon Sep 17 00:00:00 2001 From: Jonathan Schwender Date: Wed, 29 Jan 2025 18:49:02 +0100 Subject: [PATCH 1/3] Support mapping Enums to Result type Some enums, where the Ok variant is 0, can be mapped to a Rust Result, with a NonZero enum representation. Future improvements could add a callback which allows customizing the name error enum. --- .../expectations/tests/result-error-enum.rs | 22 ++++++ .../tests/headers/result-error-enum.h | 13 ++++ bindgen/codegen/mod.rs | 67 +++++++++++++++++++ bindgen/ir/enum_ty.rs | 29 +++++++- bindgen/lib.rs | 4 +- bindgen/options/cli.rs | 5 ++ bindgen/options/mod.rs | 21 ++++++ 7 files changed, 159 insertions(+), 2 deletions(-) create mode 100644 bindgen-tests/tests/expectations/tests/result-error-enum.rs create mode 100644 bindgen-tests/tests/headers/result-error-enum.h diff --git a/bindgen-tests/tests/expectations/tests/result-error-enum.rs b/bindgen-tests/tests/expectations/tests/result-error-enum.rs new file mode 100644 index 0000000000..3f0ffcbcdd --- /dev/null +++ b/bindgen-tests/tests/expectations/tests/result-error-enum.rs @@ -0,0 +1,22 @@ +#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)] +impl MyResultError { + pub const MyResultErr1: MyResultError = MyResultError(const { + core::num::NonZero::new(1).unwrap() + }); +} +impl MyResultError { + pub const MyResultErr2: MyResultError = MyResultError(const { + core::num::NonZero::new(2).unwrap() + }); +} +pub type MyResult = Result<(), MyResultError>; +#[repr(transparent)] +#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)] +pub struct MyResultError(pub core::num::NonZero<::std::os::raw::c_uint>); +pub use self::MyResult as ResultTypedef; +extern "C" { + pub fn do_something() -> MyResult; +} +extern "C" { + pub fn do_something2() -> ResultTypedef; +} diff --git a/bindgen-tests/tests/headers/result-error-enum.h b/bindgen-tests/tests/headers/result-error-enum.h new file mode 100644 index 0000000000..ba9f3f3003 --- /dev/null +++ b/bindgen-tests/tests/headers/result-error-enum.h @@ -0,0 +1,13 @@ +// bindgen-flags: --result-error-enum "MyResult" --rust-target 1.79 + +enum MyResult { + MyResultOk = 0, + MyResultErr1, + MyResultErr2, +}; + +typedef enum MyResult ResultTypedef; + +enum MyResult do_something(void); + +ResultTypedef do_something2(void); diff --git a/bindgen/codegen/mod.rs b/bindgen/codegen/mod.rs index cf819950db..597b950c70 100644 --- a/bindgen/codegen/mod.rs +++ b/bindgen/codegen/mod.rs @@ -3178,6 +3178,9 @@ pub enum EnumVariation { is_bitfield: bool, /// Indicates whether the variants will be represented as global constants is_global: bool, + /// Indicates whether this enum is a result type, where 0 indicates success. + /// The enum will then be a NonZero type, and usages wrapped in Result. + is_result_type: bool, }, /// The code for this enum will use consts #[default] @@ -3210,9 +3213,15 @@ impl fmt::Display for EnumVariation { Self::NewType { is_bitfield: true, .. } => "bitfield", + Self::NewType { + is_bitfield: false, + is_global: false, + is_result_type: true, + } => "result_error_enum", Self::NewType { is_bitfield: false, is_global, + .. } => { if *is_global { "newtype_global" @@ -3242,16 +3251,24 @@ impl FromStr for EnumVariation { "bitfield" => Ok(EnumVariation::NewType { is_bitfield: true, is_global: false, + is_result_type: false, }), "consts" => Ok(EnumVariation::Consts), "moduleconsts" => Ok(EnumVariation::ModuleConsts), "newtype" => Ok(EnumVariation::NewType { is_bitfield: false, is_global: false, + is_result_type: false, }), "newtype_global" => Ok(EnumVariation::NewType { is_bitfield: false, is_global: true, + is_result_type: false, + }), + "result_error_enum" => Ok(EnumVariation::NewType { + is_bitfield: false, + is_global: false, + is_result_type: true, }), _ => Err(std::io::Error::new( std::io::ErrorKind::InvalidInput, @@ -3278,6 +3295,7 @@ enum EnumBuilder<'a> { tokens: proc_macro2::TokenStream, is_bitfield: bool, is_global: bool, + result_error_enum_ident: Option, }, Consts { variants: Vec, @@ -3309,6 +3327,27 @@ impl<'a> EnumBuilder<'a> { EnumVariation::NewType { is_bitfield, is_global, + is_result_type: true, + } => { + // Todo: This identifier perhaps could be determined by a ParseCallback. + let error_ident = format_ident!("{}Error", ident); + EnumBuilder::NewType { + canonical_name: name, + tokens: quote! { + pub type #ident = Result<(), #error_ident>; + + #( #attrs )* + pub struct #error_ident (pub core::num::NonZero<#repr>); + }, + is_bitfield, + is_global, + result_error_enum_ident: Some(error_ident), + } + } + EnumVariation::NewType { + is_bitfield, + is_global, + .. } => EnumBuilder::NewType { canonical_name: name, tokens: quote! { @@ -3317,6 +3356,7 @@ impl<'a> EnumBuilder<'a> { }, is_bitfield, is_global, + result_error_enum_ident: None, }, EnumVariation::Rust { .. } => { @@ -3411,6 +3451,33 @@ impl<'a> EnumBuilder<'a> { } } + EnumBuilder::NewType { + result_error_enum_ident: Some(ref enum_ident), + .. + } => { + assert!(is_ty_named); + if matches!( + variant.val(), + EnumVariantValue::Signed(0) | EnumVariantValue::Unsigned(0) + ) { + return self; + } + + let variant_ident = ctx.rust_ident(variant_name); + // Wrapping the unwrap in the const block ensures we get + // a compile-time panic. + let expr = quote! { const { core::num::NonZero::new(#expr).unwrap() } }; + + result.push(quote! { + impl #enum_ident { + #doc + pub const #variant_ident : #enum_ident = #enum_ident ( #expr ); + } + }); + + self + } + EnumBuilder::NewType { canonical_name, is_global, diff --git a/bindgen/ir/enum_ty.rs b/bindgen/ir/enum_ty.rs index 9b08da3bce..d1ffcbb881 100644 --- a/bindgen/ir/enum_ty.rs +++ b/bindgen/ir/enum_ty.rs @@ -4,10 +4,11 @@ use super::super::codegen::EnumVariation; use super::context::{BindgenContext, TypeId}; use super::item::Item; use super::ty::{Type, TypeKind}; -use crate::clang; use crate::ir::annotations::Annotations; use crate::parse::ParseError; use crate::regex_set::RegexSet; +use crate::{clang, RustTarget}; +use std::str::FromStr; /// An enum representing custom handling that can be given to a variant. #[derive(Copy, Clone, Debug, PartialEq, Eq)] @@ -193,12 +194,37 @@ impl Enum { EnumVariation::NewType { is_bitfield: true, is_global: false, + is_result_type: false, + } + } else if self.is_matching_enum( + ctx, + &ctx.options().result_error_enums, + item, + ) && ctx + .options() + .rust_target + .ge(&RustTarget::from_str("1.79").unwrap()) + { + let zero_variant = self.variants.iter().find(|x| { + matches!( + x.val, + EnumVariantValue::Signed(0) | EnumVariantValue::Unsigned(0) + ) + }); + if zero_variant.is_none() { + panic!("Result-error-enum must have a zero variant. (Item: {item:?})"); + } + EnumVariation::NewType { + is_bitfield: false, + is_global: false, + is_result_type: true, } } else if self.is_matching_enum(ctx, &ctx.options().newtype_enums, item) { EnumVariation::NewType { is_bitfield: false, is_global: false, + is_result_type: false, } } else if self.is_matching_enum( ctx, @@ -208,6 +234,7 @@ impl Enum { EnumVariation::NewType { is_bitfield: false, is_global: true, + is_result_type: false, } } else if self.is_matching_enum( ctx, diff --git a/bindgen/lib.rs b/bindgen/lib.rs index 9e22e37ce6..b13af83994 100644 --- a/bindgen/lib.rs +++ b/bindgen/lib.rs @@ -457,7 +457,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, @@ -476,6 +476,7 @@ impl BindgenOptions { &mut self.constified_enum_modules, &mut self.newtype_enums, &mut self.newtype_global_enums, + &mut self.result_error_enums, &mut self.rustified_enums, &mut self.rustified_non_exhaustive_enums, &mut self.type_alias, @@ -511,6 +512,7 @@ impl BindgenOptions { "--bitfield-enum", "--newtype-enum", "--newtype-global-enum", + "--result-error-enum", "--rustified-enum", "--rustified-enum-non-exhaustive", "--constified-enum-module", diff --git a/bindgen/options/cli.rs b/bindgen/options/cli.rs index 8c4c05bc84..41ab3a18cb 100644 --- a/bindgen/options/cli.rs +++ b/bindgen/options/cli.rs @@ -162,6 +162,9 @@ struct BindgenCommand { /// Mark any enum whose name matches REGEX as a global newtype. #[arg(long, value_name = "REGEX")] newtype_global_enum: Vec, + /// Mark any enum whose name matches REGEX as a `Result<(), NonZero>`. + #[arg(long, value_name = "REGEX")] + result_error_enum: Vec, /// Mark any enum whose name matches REGEX as a Rust enum. #[arg(long, value_name = "REGEX")] rustified_enum: Vec, @@ -537,6 +540,7 @@ where bitfield_enum, newtype_enum, newtype_global_enum, + result_error_enum, rustified_enum, rustified_non_exhaustive_enum, constified_enum, @@ -839,6 +843,7 @@ where default_enum_style, bitfield_enum, newtype_enum, + result_error_enum, newtype_global_enum, rustified_enum, rustified_non_exhaustive_enum, diff --git a/bindgen/options/mod.rs b/bindgen/options/mod.rs index 6bf652d4e1..374433d3fc 100644 --- a/bindgen/options/mod.rs +++ b/bindgen/options/mod.rs @@ -438,6 +438,27 @@ options! { }, as_args: "--newtype-enum", }, + /// `enum`s marked as `Result<(), ErrorEnum>`. + result_error_enums: RegexSet { + methods: { + regex_option! { + /// Mark the given `enum` as a `Result<(), ErrorEnum>`, where ErrorEnum is a newtupe. + /// + /// This means that a NonZero integer newtype will be declared to represent the `enum` + /// type, without the zero variant, and the error variants will be represented as + /// constants inside of this type's `impl` block. The 0 variant of the enum will + /// be mapped to the `Ok(())` variant of the Result. + /// + /// Note: Using This API requires --rust-target 1.79 or newer due to the usage + /// of `NonZero`! + pub fn result_error_enum>(mut self, arg: T) -> Builder { + self.options.result_error_enums.insert(arg); + self + } + } + }, + as_args: "--result-error-enum", + }, /// `enum`s marked as global newtypes . newtype_global_enums: RegexSet { methods: { From 1464db3e0254018605edd61a8aa633b9539d5a1e Mon Sep 17 00:00:00 2001 From: Jonathan Schwender Date: Thu, 30 Jan 2025 09:46:20 +0100 Subject: [PATCH 2/3] Add callback to allow renaming result error enum --- .../tests/parsecb-result-error-enum-name.rs | 22 ++++++++ .../headers/parsecb-result-error-enum-name.h | 14 +++++ bindgen-tests/tests/parse_callbacks/mod.rs | 15 ++++++ bindgen/callbacks.rs | 54 +++++++++++++++++++ bindgen/codegen/mod.rs | 11 ++-- 5 files changed, 113 insertions(+), 3 deletions(-) create mode 100644 bindgen-tests/tests/expectations/tests/parsecb-result-error-enum-name.rs create mode 100644 bindgen-tests/tests/headers/parsecb-result-error-enum-name.h diff --git a/bindgen-tests/tests/expectations/tests/parsecb-result-error-enum-name.rs b/bindgen-tests/tests/expectations/tests/parsecb-result-error-enum-name.rs new file mode 100644 index 0000000000..6b11d34baf --- /dev/null +++ b/bindgen-tests/tests/expectations/tests/parsecb-result-error-enum-name.rs @@ -0,0 +1,22 @@ +#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)] +impl MyError { + pub const MyResultInvalid: MyError = MyError(const { + core::num::NonZero::new(1).unwrap() + }); +} +impl MyError { + pub const MyResultAnotherError: MyError = MyError(const { + core::num::NonZero::new(2).unwrap() + }); +} +pub type MyResult = Result<(), MyError>; +#[repr(transparent)] +#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)] +pub struct MyError(pub core::num::NonZero<::std::os::raw::c_uint>); +pub use self::MyResult as AnotherResult; +extern "C" { + pub fn some_function() -> MyResult; +} +extern "C" { + pub fn another_function() -> AnotherResult; +} diff --git a/bindgen-tests/tests/headers/parsecb-result-error-enum-name.h b/bindgen-tests/tests/headers/parsecb-result-error-enum-name.h new file mode 100644 index 0000000000..8bf714e956 --- /dev/null +++ b/bindgen-tests/tests/headers/parsecb-result-error-enum-name.h @@ -0,0 +1,14 @@ +// bindgen-flags: --result-error-enum MyResult --rust-target 1.79 +// bindgen-parse-callbacks: result-error-enum-rename + +enum MyResult { + MyResultOk = 0, + MyResultInvalid, + MyResultAnotherError, +}; + +typedef enum MyResult AnotherResult; + +enum MyResult some_function(void); + +AnotherResult another_function(void); diff --git a/bindgen-tests/tests/parse_callbacks/mod.rs b/bindgen-tests/tests/parse_callbacks/mod.rs index 7aca0fd1a1..284a3d51d0 100644 --- a/bindgen-tests/tests/parse_callbacks/mod.rs +++ b/bindgen-tests/tests/parse_callbacks/mod.rs @@ -72,6 +72,20 @@ impl ParseCallbacks for EnumVariantRename { } } +#[derive(Debug)] +struct ResultErrorEnumRename; + +impl ParseCallbacks for ResultErrorEnumRename { + fn result_error_enum_name( + &self, + original_enum_name: &str, + ) -> Option { + original_enum_name + .strip_suffix("Result") + .map(|base| format!("{base}Error")) + } +} + #[derive(Debug)] struct BlocklistedTypeImplementsTrait; @@ -149,6 +163,7 @@ impl ParseCallbacks for WrapAsVariadicFn { pub fn lookup(cb: &str) -> Box { match cb { "enum-variant-rename" => Box::new(EnumVariantRename), + "result-error-enum-rename" => Box::new(ResultErrorEnumRename), "blocklisted-type-implements-trait" => { Box::new(BlocklistedTypeImplementsTrait) } diff --git a/bindgen/callbacks.rs b/bindgen/callbacks.rs index 8a21e98dea..14ea097024 100644 --- a/bindgen/callbacks.rs +++ b/bindgen/callbacks.rs @@ -79,6 +79,60 @@ pub trait ParseCallbacks: fmt::Debug { None } + /// Allows to rename the error enum name for a result-error-enum + /// + /// When the original C-enum is translated to a `Result<(), NonZero>` representation, + /// the original enum name is used as a type alias for the result. + /// The error enum hence must have a different name. By default, we simply append an `Error` to + /// the typename, but this callback allows overriding the name for the error enum. + /// Please note that the new enum name returned by this callback must be distinct from the + /// original enum name. + /// + /// ### Example + /// + /// Given a header file like this: + /// ```c + /// enum MyResult { + /// MyResultOk = 0, + /// MyResultInvalid, + /// MyResultAnotherError, + /// }; + /// ``` + /// A user could the implement the following callback: + /// ```rust + /// # use bindgen::callbacks::ParseCallbacks; + /// #[derive(Debug)] + /// struct ResultErrorEnumRename; + /// + /// impl ParseCallbacks for ResultErrorEnumRename { + /// fn result_error_enum_name( + /// &self, + /// original_enum_name: &str, + /// ) -> Option { + /// original_enum_name + /// .strip_suffix("Result") + /// .map(|base| format!("{base}Error")) + /// } + /// } + /// ``` + /// + /// The above callback would result in the following rust binding: + /// + /// ```no_run + /// # // Note: doctests break when running with MSRV. + /// pub type MyResult = Result<(), MyError>; + /// #[repr(transparent)] + /// #[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)] + /// pub struct MyError(pub core::num::NonZero<::std::os::raw::c_uint>); + /// // + /// ``` + fn result_error_enum_name( + &self, + _original_enum_name: &str, + ) -> Option { + None + } + /// Allows to rename an enum variant, replacing `_original_variant_name`. fn enum_variant_name( &self, diff --git a/bindgen/codegen/mod.rs b/bindgen/codegen/mod.rs index 597b950c70..bbd5b59c42 100644 --- a/bindgen/codegen/mod.rs +++ b/bindgen/codegen/mod.rs @@ -3316,6 +3316,7 @@ impl<'a> EnumBuilder<'a> { /// the representation, and which variation it should be generated as. fn new( name: &'a str, + ctx: &BindgenContext, mut attrs: Vec, repr: syn::Type, enum_variation: EnumVariation, @@ -3329,8 +3330,12 @@ impl<'a> EnumBuilder<'a> { is_global, is_result_type: true, } => { - // Todo: This identifier perhaps could be determined by a ParseCallback. - let error_ident = format_ident!("{}Error", ident); + let error_enum_name = ctx + .options() + .last_callback(|c| c.result_error_enum_name(&name)) + .unwrap_or(format!("{name}Error")); + let error_ident = + Ident::new(&error_enum_name, Span::call_site()); EnumBuilder::NewType { canonical_name: name, tokens: quote! { @@ -3837,7 +3842,7 @@ impl CodeGenerator for Enum { let has_typedef = ctx.is_enum_typedef_combo(item.id()); let mut builder = - EnumBuilder::new(&name, attrs, repr, variation, has_typedef); + EnumBuilder::new(&name, ctx, attrs, repr, variation, has_typedef); // A map where we keep a value -> variant relation. let mut seen_values = HashMap::<_, Ident>::default(); From ce5478ab550a1d4b8426aa1c3b9caaa2773888f0 Mon Sep 17 00:00:00 2001 From: Jonathan Schwender Date: Thu, 30 Jan 2025 10:53:54 +0100 Subject: [PATCH 3/3] Ignore example to avoid msrv doctest failures There is a `no_run`, but not `no_build` doctest attribute, so we need to ignore it to prevent CI from failing in the msrv build. --- bindgen/callbacks.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/bindgen/callbacks.rs b/bindgen/callbacks.rs index 14ea097024..1f63b879a2 100644 --- a/bindgen/callbacks.rs +++ b/bindgen/callbacks.rs @@ -118,8 +118,7 @@ pub trait ParseCallbacks: fmt::Debug { /// /// The above callback would result in the following rust binding: /// - /// ```no_run - /// # // Note: doctests break when running with MSRV. + /// ```ignore /// pub type MyResult = Result<(), MyError>; /// #[repr(transparent)] /// #[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)]