diff --git a/CHANGELOG.md b/CHANGELOG.md index 037755ca76..357621f373 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -162,6 +162,9 @@ # Unreleased ## Added + * Added the `--non-null-fn-ptr=` flag and its equivalent builder + method to avoid wrapping function pointer arguments or fields in `Option` if + their path matches ``. ## Changed diff --git a/bindgen-cli/options.rs b/bindgen-cli/options.rs index 966e6aeced..0b4c1d6b6a 100644 --- a/bindgen-cli/options.rs +++ b/bindgen-cli/options.rs @@ -355,6 +355,10 @@ struct BindgenCommand { /// inline` functions. #[arg(long, requires = "experimental", value_name = "SUFFIX")] wrap_static_fns_suffix: Option, + /// Do not wrap function pointers in `Option` if they are used as fields/arguments whose path + /// matches . + #[arg(long, value_name = "PATH")] + non_null_fn_ptr: Vec, /// Enables experimental features. #[arg(long)] experimental: bool, @@ -478,6 +482,7 @@ where wrap_static_fns, wrap_static_fns_path, wrap_static_fns_suffix, + non_null_fn_ptr, experimental: _, version, clang_args, @@ -989,5 +994,9 @@ where builder = builder.wrap_static_fns_suffix(suffix); } + for path in non_null_fn_ptr { + builder = builder.non_null_fn_ptr(path); + } + Ok((builder, output, verbose)) } diff --git a/bindgen-tests/tests/expectations/tests/non_null_fn_ptr.rs b/bindgen-tests/tests/expectations/tests/non_null_fn_ptr.rs new file mode 100644 index 0000000000..bba10f23e2 --- /dev/null +++ b/bindgen-tests/tests/expectations/tests/non_null_fn_ptr.rs @@ -0,0 +1,177 @@ +#![allow( + dead_code, + non_snake_case, + non_camel_case_types, + non_upper_case_globals +)] + +#[repr(C)] +#[derive(Debug, Copy, Clone)] +pub struct foo { + pub bar: unsafe extern "C" fn() -> ::std::os::raw::c_int, +} +#[test] +fn bindgen_test_layout_foo() { + const UNINIT: ::std::mem::MaybeUninit = + ::std::mem::MaybeUninit::uninit(); + let ptr = UNINIT.as_ptr(); + assert_eq!( + ::std::mem::size_of::(), + 8usize, + concat!("Size of: ", stringify!(foo)) + ); + assert_eq!( + ::std::mem::align_of::(), + 8usize, + concat!("Alignment of ", stringify!(foo)) + ); + assert_eq!( + unsafe { ::std::ptr::addr_of!((*ptr).bar) as usize - ptr as usize }, + 0usize, + concat!("Offset of field: ", stringify!(foo), "::", stringify!(bar)) + ); +} +extern "C" { + pub fn new_foo(arg: unsafe extern "C" fn() -> ::std::os::raw::c_int) + -> foo; +} +#[repr(C)] +#[derive(Debug, Default, Copy, Clone)] +pub struct baz { + pub foo: + ::std::option::Option ::std::os::raw::c_int>, +} +#[test] +fn bindgen_test_layout_baz() { + const UNINIT: ::std::mem::MaybeUninit = + ::std::mem::MaybeUninit::uninit(); + let ptr = UNINIT.as_ptr(); + assert_eq!( + ::std::mem::size_of::(), + 8usize, + concat!("Size of: ", stringify!(baz)) + ); + assert_eq!( + ::std::mem::align_of::(), + 8usize, + concat!("Alignment of ", stringify!(baz)) + ); + assert_eq!( + unsafe { ::std::ptr::addr_of!((*ptr).foo) as usize - ptr as usize }, + 0usize, + concat!("Offset of field: ", stringify!(baz), "::", stringify!(foo)) + ); +} +extern "C" { + pub fn new_baz( + foo: ::std::option::Option< + unsafe extern "C" fn() -> ::std::os::raw::c_int, + >, + ) -> baz; +} +#[repr(C)] +#[derive(Copy, Clone)] +pub union union_foo { + pub fst: unsafe extern "C" fn() -> ::std::os::raw::c_int, + pub snd: unsafe extern "C" fn() -> f32, +} +#[test] +fn bindgen_test_layout_union_foo() { + const UNINIT: ::std::mem::MaybeUninit = + ::std::mem::MaybeUninit::uninit(); + let ptr = UNINIT.as_ptr(); + assert_eq!( + ::std::mem::size_of::(), + 8usize, + concat!("Size of: ", stringify!(union_foo)) + ); + assert_eq!( + ::std::mem::align_of::(), + 8usize, + concat!("Alignment of ", stringify!(union_foo)) + ); + assert_eq!( + unsafe { ::std::ptr::addr_of!((*ptr).fst) as usize - ptr as usize }, + 0usize, + concat!( + "Offset of field: ", + stringify!(union_foo), + "::", + stringify!(fst) + ) + ); + assert_eq!( + unsafe { ::std::ptr::addr_of!((*ptr).snd) as usize - ptr as usize }, + 0usize, + concat!( + "Offset of field: ", + stringify!(union_foo), + "::", + stringify!(snd) + ) + ); +} +extern "C" { + pub fn new_union_foo( + arg: unsafe extern "C" fn() -> ::std::os::raw::c_int, + ) -> union_foo; +} +#[repr(C)] +#[derive(Copy, Clone)] +pub union union_baz { + pub foo: + ::std::option::Option ::std::os::raw::c_int>, + pub bar: ::std::option::Option f32>, +} +#[test] +fn bindgen_test_layout_union_baz() { + const UNINIT: ::std::mem::MaybeUninit = + ::std::mem::MaybeUninit::uninit(); + let ptr = UNINIT.as_ptr(); + assert_eq!( + ::std::mem::size_of::(), + 8usize, + concat!("Size of: ", stringify!(union_baz)) + ); + assert_eq!( + ::std::mem::align_of::(), + 8usize, + concat!("Alignment of ", stringify!(union_baz)) + ); + assert_eq!( + unsafe { ::std::ptr::addr_of!((*ptr).foo) as usize - ptr as usize }, + 0usize, + concat!( + "Offset of field: ", + stringify!(union_baz), + "::", + stringify!(foo) + ) + ); + assert_eq!( + unsafe { ::std::ptr::addr_of!((*ptr).bar) as usize - ptr as usize }, + 0usize, + concat!( + "Offset of field: ", + stringify!(union_baz), + "::", + stringify!(bar) + ) + ); +} +impl Default for union_baz { + fn default() -> Self { + let mut s = ::std::mem::MaybeUninit::::uninit(); + unsafe { + ::std::ptr::write_bytes(s.as_mut_ptr(), 0, 1); + s.assume_init() + } + } +} +extern "C" { + pub fn new_union_baz( + foo: ::std::option::Option< + unsafe extern "C" fn() -> ::std::os::raw::c_int, + >, + ) -> union_baz; +} diff --git a/bindgen-tests/tests/headers/non_null_fn_ptr.h b/bindgen-tests/tests/headers/non_null_fn_ptr.h new file mode 100644 index 0000000000..d32a3e537a --- /dev/null +++ b/bindgen-tests/tests/headers/non_null_fn_ptr.h @@ -0,0 +1,27 @@ +// bindgen-flags: --non-null-fn-ptr=".*foo::.*" --no-default=".*foo" + +typedef struct foo { + int (*bar)(); +} foo; + +foo new_foo(int (*arg)()); + +typedef struct baz { + int (*foo)(); +} baz; + +baz new_baz(int (*foo)()); + +typedef union union_foo { + int (*fst)(); + float (*snd)(); +} union_foo; + +union_foo new_union_foo(int (*arg)()); + +typedef union union_baz { + int (*foo)(); + float (*bar)(); +} union_baz; + +union_baz new_union_baz(int (*foo)()); diff --git a/bindgen/codegen/mod.rs b/bindgen/codegen/mod.rs index b6fb70eb01..372ada9e14 100644 --- a/bindgen/codegen/mod.rs +++ b/bindgen/codegen/mod.rs @@ -3784,11 +3784,8 @@ impl TryToRustTy for Type { // they aren't NonZero), so don't *ever* use an or_opaque // variant here. let ty = fs.try_to_rust_ty(ctx, &())?; - let prefix = ctx.trait_prefix(); - Ok(quote! { - ::#prefix::option::Option<#ty> - }) + Ok(quote!(::#prefix::option::Option<#ty>)) } TypeKind::Array(item, len) | TypeKind::Vector(item, len) => { let ty = item.try_to_rust_ty(ctx, &())?; diff --git a/bindgen/codegen/postprocessing/merge_extern_blocks.rs b/bindgen/codegen/postprocessing/merge_extern_blocks.rs index 05e7e9ef39..4d2cdb23a6 100644 --- a/bindgen/codegen/postprocessing/merge_extern_blocks.rs +++ b/bindgen/codegen/postprocessing/merge_extern_blocks.rs @@ -3,7 +3,12 @@ use syn::{ Item, ItemForeignMod, ItemMod, }; -pub(super) fn merge_extern_blocks(item_mod: &mut ItemMod) { +use crate::BindgenOptions; + +pub(super) fn merge_extern_blocks( + item_mod: &mut ItemMod, + _options: &BindgenOptions, +) { Visitor.visit_item_mod_mut(item_mod) } diff --git a/bindgen/codegen/postprocessing/mod.rs b/bindgen/codegen/postprocessing/mod.rs index 1d5a4983bd..441251efcb 100644 --- a/bindgen/codegen/postprocessing/mod.rs +++ b/bindgen/codegen/postprocessing/mod.rs @@ -5,14 +5,16 @@ use syn::{parse2, ItemMod}; use crate::BindgenOptions; mod merge_extern_blocks; +mod non_null_fn_ptr; mod sort_semantically; use merge_extern_blocks::merge_extern_blocks; +use non_null_fn_ptr::non_null_fn_ptr; use sort_semantically::sort_semantically; struct PostProcessingPass { should_run: fn(&BindgenOptions) -> bool, - run: fn(&mut ItemMod), + run: fn(&mut ItemMod, &BindgenOptions), } // TODO: This can be a const fn when mutable references are allowed in const @@ -21,13 +23,19 @@ macro_rules! pass { ($pass:ident) => { PostProcessingPass { should_run: |options| options.$pass, - run: |item_mod| $pass(item_mod), + run: |item_mod, options| $pass(item_mod, options), } }; } -const PASSES: &[PostProcessingPass] = - &[pass!(merge_extern_blocks), pass!(sort_semantically)]; +const PASSES: &[PostProcessingPass] = &[ + pass!(merge_extern_blocks), + pass!(sort_semantically), + PostProcessingPass { + should_run: |options| !options.non_null_fn_ptr.is_empty(), + run: non_null_fn_ptr, + }, +]; pub(crate) fn postprocessing( items: Vec, @@ -51,7 +59,7 @@ pub(crate) fn postprocessing( for pass in PASSES { if (pass.should_run)(options) { - (pass.run)(&mut item_mod); + (pass.run)(&mut item_mod, options); } } diff --git a/bindgen/codegen/postprocessing/non_null_fn_ptr.rs b/bindgen/codegen/postprocessing/non_null_fn_ptr.rs new file mode 100644 index 0000000000..a336da95b7 --- /dev/null +++ b/bindgen/codegen/postprocessing/non_null_fn_ptr.rs @@ -0,0 +1,157 @@ +use std::fmt::Write; + +use syn::{ + visit_mut::{ + visit_field_mut, visit_fn_arg_mut, visit_item_enum_mut, + visit_item_struct_mut, visit_item_union_mut, visit_signature_mut, + visit_variant_mut, VisitMut, + }, + AngleBracketedGenericArguments, Field, FnArg, GenericArgument, Ident, + ItemMod, Pat, PatIdent, Path, PathArguments, PathSegment, Type, TypePath, +}; + +use crate::regex_set::RegexSet; + +pub(super) fn non_null_fn_ptr( + item_mod: &mut ItemMod, + options: &crate::BindgenOptions, +) { + Visitor::new(&options.non_null_fn_ptr).visit_item_mod_mut(item_mod) +} + +struct Visitor<'vis> { + path: Vec, + regex_set: &'vis RegexSet, +} + +impl<'vis> Visitor<'vis> { + fn new(regex_set: &'vis RegexSet) -> Self { + Self { + path: Default::default(), + regex_set, + } + } + + fn with_ident(&mut self, ident: Ident, f: impl FnOnce(&mut Self)) { + self.path.push(ident); + f(self); + let _ = self.path.pop(); + } + + fn path_matches(&self) -> bool { + let mut idents = self.path.iter(); + + let mut path = String::new(); + + let head = idents.next().expect("This should never be empty!"); + write!(path, "{}", head).unwrap(); + for ident in idents { + write!(path, "::{}", ident).unwrap(); + } + + self.regex_set.matches(&path) + } +} + +impl<'vis> VisitMut for Visitor<'vis> { + fn visit_item_struct_mut(&mut self, item: &mut syn::ItemStruct) { + self.with_ident(item.ident.clone(), |this| { + visit_item_struct_mut(this, item) + }); + } + + fn visit_item_union_mut(&mut self, item: &mut syn::ItemUnion) { + self.with_ident(item.ident.clone(), |this| { + visit_item_union_mut(this, item) + }); + } + + fn visit_item_enum_mut(&mut self, item: &mut syn::ItemEnum) { + self.with_ident(item.ident.clone(), |this| { + visit_item_enum_mut(this, item); + }); + } + + fn visit_variant_mut(&mut self, variant: &mut syn::Variant) { + self.with_ident(variant.ident.clone(), |this| { + visit_variant_mut(this, variant); + }); + } + + fn visit_signature_mut(&mut self, sig: &mut syn::Signature) { + self.with_ident(sig.ident.clone(), |this| { + visit_signature_mut(this, sig); + }); + } + + fn visit_field_mut(&mut self, field: &mut Field) { + if let Some(bare_fn) = extract_fn_pointer(&field.ty) { + if let Some(ident) = &field.ident { + let bare_fn = bare_fn.clone(); + self.with_ident(ident.clone(), |this| { + if this.path_matches() { + field.ty = bare_fn; + } + }); + } else if self.path_matches() { + field.ty = bare_fn.clone(); + } + } + + visit_field_mut(self, field) + } + + fn visit_fn_arg_mut(&mut self, fn_arg: &mut FnArg) { + if let FnArg::Typed(syn::PatType { pat, ty, .. }) = fn_arg { + if let Pat::Ident(PatIdent { ident, .. }) = pat.as_ref() { + if let Some(bare_fn) = extract_fn_pointer(ty).cloned() { + self.with_ident(ident.clone(), |this| { + if this.path_matches() { + **ty = bare_fn; + } + }) + } + } + } + + visit_fn_arg_mut(self, fn_arg) + } +} + +fn extract_fn_pointer(ty: &Type) -> Option<&Type> { + if let Type::Path(TypePath { + qself: None, + path: Path { segments, .. }, + }) = ty + { + let mut path = String::new(); + + for segment in segments { + write!(path, "::{}", segment.ident).unwrap(); + } + + if let "::std::option::Option" | "::core::option::Option" = + path.as_str() + { + if let Some(PathSegment { + arguments: + PathArguments::AngleBracketed(AngleBracketedGenericArguments { + args, + .. + }), + .. + }) = segments.last() + { + if args.len() == 1 { + if let GenericArgument::Type(arg_ty @ Type::BareFn(_)) = + &args[0] + { + return Some(arg_ty); + } + } + } + } + } + + None +} diff --git a/bindgen/codegen/postprocessing/sort_semantically.rs b/bindgen/codegen/postprocessing/sort_semantically.rs index 4f23ab73a3..72a03cad8e 100644 --- a/bindgen/codegen/postprocessing/sort_semantically.rs +++ b/bindgen/codegen/postprocessing/sort_semantically.rs @@ -3,7 +3,12 @@ use syn::{ Item, ItemMod, }; -pub(super) fn sort_semantically(item_mod: &mut ItemMod) { +use crate::BindgenOptions; + +pub(super) fn sort_semantically( + item_mod: &mut ItemMod, + _options: &BindgenOptions, +) { Visitor.visit_item_mod_mut(item_mod) } diff --git a/bindgen/lib.rs b/bindgen/lib.rs index 14d7a15576..298a24fb08 100644 --- a/bindgen/lib.rs +++ b/bindgen/lib.rs @@ -384,6 +384,7 @@ impl Builder { (&self.options.no_default_types, "--no-default"), (&self.options.no_hash_types, "--no-hash"), (&self.options.must_use_types, "--must-use-type"), + (&self.options.non_null_fn_ptr, "--non-null-fn-ptr"), ]; for (set, flag) in regex_sets { @@ -1834,6 +1835,15 @@ impl Builder { self.options.wrap_static_fns_suffix = Some(suffix.as_ref().to_owned()); self } + + fn_with_regex_arg! { + /// Avoid wrapping a function pointer argument/field in `Option`. Regular expressions are + /// supported. + pub fn non_null_fn_ptr>(mut self, arg: T) -> Self { + self.options.non_null_fn_ptr.insert(arg.into()); + self + } + } } /// Configuration options for generated bindings. @@ -2180,6 +2190,9 @@ struct BindgenOptions { wrap_static_fns_suffix: Option, wrap_static_fns_path: Option, + + /// The set of function pointer fields/arguments that should not be wrapped in `Option`. + non_null_fn_ptr: RegexSet, } impl BindgenOptions { @@ -2212,6 +2225,7 @@ impl BindgenOptions { &mut self.no_default_types, &mut self.no_hash_types, &mut self.must_use_types, + &mut self.non_null_fn_ptr, ]; let record_matches = self.record_matches; for regex_set in self.abi_overrides.values_mut().chain(regex_sets) { @@ -2375,6 +2389,7 @@ impl Default for BindgenOptions { wrap_static_fns, wrap_static_fns_suffix, wrap_static_fns_path, + non_null_fn_ptr, } } }