diff --git a/src/codegen/mod.rs b/src/codegen/mod.rs index 5ac8dc11c9..2467353eb1 100644 --- a/src/codegen/mod.rs +++ b/src/codegen/mod.rs @@ -1790,8 +1790,10 @@ impl CodeGenerator for CompInfo { if item.can_derive_default(ctx) { derives.push("Default"); } else { - needs_default_impl = - ctx.options().derive_default && !self.is_forward_declaration(); + needs_default_impl = ctx.options().derive_default && + !self.is_forward_declaration() && + ctx.lookup_can_derive_default(item.id()) == + CanDerive::Manually; } let all_template_params = item.all_template_params(ctx); diff --git a/src/ir/analysis/derive.rs b/src/ir/analysis/derive.rs index e07f6bc2c0..7d8546c1a6 100644 --- a/src/ir/analysis/derive.rs +++ b/src/ir/analysis/derive.rs @@ -140,10 +140,10 @@ impl<'ctx> CannotDerive<'ctx> { fn constrain_type(&mut self, item: &Item, ty: &Type) -> CanDerive { if !self.ctx.whitelisted_items().contains(&item.id()) { trace!( - " cannot derive {} for blacklisted type", + " cannot derive {} for blacklisted type, but it can be implemented", self.derive_trait ); - return CanDerive::No; + return CanDerive::Manually; } if self.derive_trait.not_by_name(self.ctx, &item) { @@ -156,15 +156,18 @@ impl<'ctx> CannotDerive<'ctx> { trace!("ty: {:?}", ty); if item.is_opaque(self.ctx, &()) { - if !self.derive_trait.can_derive_union() && - ty.is_union() && + if ty.is_union() && self.ctx.options().rust_features().untagged_union { - trace!( - " cannot derive {} for Rust unions", - self.derive_trait - ); - return CanDerive::No; + let can_derive = self.derive_trait.can_derive_union(); + if can_derive != CanDerive::Yes { + trace!( + " cannot derive {} for Rust unions ({:?})", + self.derive_trait, + can_derive, + ); + return can_derive; + } } let layout_can_derive = @@ -179,7 +182,13 @@ impl<'ctx> CannotDerive<'ctx> { self.derive_trait ); } - _ => { + CanDerive::Manually => { + trace!( + " we cannot derive {} for the layout, but it can be implemented", + self.derive_trait + ); + } + CanDerive::No => { trace!( " we cannot derive {} for the layout", self.derive_trait @@ -226,11 +235,12 @@ impl<'ctx> CannotDerive<'ctx> { if inner_type != CanDerive::Yes { trace!( " arrays of T for which we cannot derive {} \ - also cannot derive {}", + also cannot derive {}, ({:?})", self.derive_trait, - self.derive_trait + self.derive_trait, + inner_type ); - return CanDerive::No; + return inner_type; } if len == 0 && !self.derive_trait.can_derive_incomplete_array() @@ -308,62 +318,74 @@ impl<'ctx> CannotDerive<'ctx> { } if info.kind() == CompKind::Union { - if self.derive_trait.can_derive_union() { - if self.ctx.options().rust_features().untagged_union && - // https://github.com/rust-lang/rust/issues/36640 - (!info.self_template_params(self.ctx).is_empty() || - !item.all_template_params(self.ctx).is_empty()) - { - trace!( - " cannot derive {} for Rust union because issue 36640", self.derive_trait - ); - return CanDerive::No; - } - // fall through to be same as non-union handling - } else { - if self.ctx.options().rust_features().untagged_union { - trace!( - " cannot derive {} for Rust unions", - self.derive_trait - ); - return CanDerive::No; - } - - let layout_can_derive = - ty.layout(self.ctx).map_or(CanDerive::Yes, |l| { - l.opaque() - .array_size_within_derive_limit(self.ctx) - }); - match layout_can_derive { - CanDerive::Yes => { + match self.derive_trait.can_derive_union() { + CanDerive::Yes => { + if self.ctx.options().rust_features().untagged_union && + // https://github.com/rust-lang/rust/issues/36640 + (!info.self_template_params(self.ctx).is_empty() || + !item.all_template_params(self.ctx).is_empty()) + { trace!( - " union layout can trivially derive {}", - self.derive_trait + " cannot derive {} for Rust union because issue 36640", self.derive_trait ); + return CanDerive::No; } - _ => { + // fall through to be same as non-union handling + } + trait_can_derive => { + if self.ctx.options().rust_features().untagged_union + { trace!( - " union layout cannot derive {}", - self.derive_trait - ); + " cannot derive {} for Rust unions ({:?})", + self.derive_trait, + trait_can_derive, + ); + return trait_can_derive; } - }; - return layout_can_derive; + + let layout_can_derive = ty.layout(self.ctx).map_or( + CanDerive::Yes, + |l| { + l.opaque().array_size_within_derive_limit( + self.ctx, + ) + }, + ); + match layout_can_derive { + CanDerive::Yes => { + trace!( + " union layout can trivially derive {}", + self.derive_trait + ); + } + _ => { + trace!( + " union layout cannot derive {}, ({:?})", + self.derive_trait, + layout_can_derive, + ); + } + }; + return layout_can_derive; + } } } - if !self.derive_trait.can_derive_compound_with_vtable() && - item.has_vtable(self.ctx) - { - trace!( - " cannot derive {} for comp with vtable", - self.derive_trait - ); - return CanDerive::No; + if item.has_vtable(self.ctx) { + let can_derive = + self.derive_trait.can_derive_compound_with_vtable(); + if can_derive != CanDerive::Yes { + trace!( + " cannot derive {} for comp with vtable ({:?})", + self.derive_trait, + can_derive, + ); + return can_derive; + } } let pred = self.derive_trait.consider_edge_comp(); - return self.constrain_join(item, pred); + self.constrain_join(item, pred) } TypeKind::ResolvedTypeRef(..) | @@ -432,6 +454,7 @@ impl DeriveTrait { fn not_by_name(&self, ctx: &BindgenContext, item: &Item) -> bool { match self { DeriveTrait::Copy => ctx.no_copy_by_name(item), + DeriveTrait::Default => ctx.no_default_by_name(item), DeriveTrait::Hash => ctx.no_hash_by_name(item), DeriveTrait::PartialEqOrPartialOrd => { ctx.no_partialeq_by_name(item) @@ -476,10 +499,11 @@ impl DeriveTrait { } } - fn can_derive_union(&self) -> bool { + fn can_derive_union(&self) -> CanDerive { match self { - DeriveTrait::Copy => true, - _ => false, + DeriveTrait::Copy => CanDerive::Yes, + DeriveTrait::Default => CanDerive::Manually, + _ => CanDerive::No, } } @@ -490,10 +514,10 @@ impl DeriveTrait { } } - fn can_derive_compound_with_vtable(&self) -> bool { + fn can_derive_compound_with_vtable(&self) -> CanDerive { match self { - DeriveTrait::Default => false, - _ => true, + DeriveTrait::Default => CanDerive::Manually, + _ => CanDerive::Yes, } } @@ -549,8 +573,8 @@ impl DeriveTrait { fn can_derive_pointer(&self) -> CanDerive { match self { DeriveTrait::Default => { - trace!(" pointer cannot derive Default"); - CanDerive::No + trace!(" pointer cannot derive Default, but it may be implemented"); + CanDerive::Manually } _ => { trace!(" pointer can derive {}", self); @@ -570,8 +594,8 @@ impl DeriveTrait { (DeriveTrait::Default, TypeKind::ObjCInterface(..)) | (DeriveTrait::Default, TypeKind::ObjCId) | (DeriveTrait::Default, TypeKind::ObjCSel) => { - trace!(" types that always cannot derive Default"); - CanDerive::No + trace!(" types that always cannot derive Default, but may implement it manually"); + CanDerive::Manually } (DeriveTrait::Default, TypeKind::UnresolvedTypeRef(..)) => { unreachable!( diff --git a/src/ir/context.rs b/src/ir/context.rs index 384edb95c9..d473cd4857 100644 --- a/src/ir/context.rs +++ b/src/ir/context.rs @@ -223,7 +223,8 @@ where T: Copy + Into, { fn can_derive_default(&self, ctx: &BindgenContext) -> bool { - ctx.options().derive_default && ctx.lookup_can_derive_default(*self) + ctx.options().derive_default && + ctx.lookup_can_derive_default(*self) == CanDerive::Yes } } @@ -401,11 +402,12 @@ pub struct BindgenContext { /// and is always `None` before that and `Some` after. cannot_derive_debug: Option>, - /// The set of (`ItemId`s of) types that can't derive default. + /// Map from type `ItemId`s to why they can't derive hash. If an id is not + /// in this map, we can assume that its value would be `CanDerive::Yes`. /// /// This is populated when we enter codegen by `compute_cannot_derive_default` /// and is always `None` before that and `Some` after. - cannot_derive_default: Option>, + cannot_derive_default: Option>, /// The set of (`ItemId`s of) types that can't derive copy. /// @@ -2462,16 +2464,16 @@ If you encounter an error missing from this list, please file an issue or a PR!" assert!(self.cannot_derive_default.is_none()); if self.options.derive_default { self.cannot_derive_default = - Some(as_cannot_derive_set(analyze::(( - self, - DeriveTrait::Default, - )))); + Some(analyze::((self, DeriveTrait::Default))); } } /// Look up whether the item with `id` can /// derive default or not. - pub fn lookup_can_derive_default>(&self, id: Id) -> bool { + pub fn lookup_can_derive_default>( + &self, + id: Id, + ) -> CanDerive { let id = id.into(); assert!( self.in_codegen_phase(), @@ -2480,7 +2482,12 @@ If you encounter an error missing from this list, please file an issue or a PR!" // Look up the computed value for whether the item with `id` can // derive default or not. - !self.cannot_derive_default.as_ref().unwrap().contains(&id) + self.cannot_derive_default + .as_ref() + .unwrap() + .get(&id) + .cloned() + .unwrap_or(CanDerive::Yes) } /// Compute whether we can derive copy. @@ -2637,6 +2644,12 @@ If you encounter an error missing from this list, please file an issue or a PR!" let name = item.path_for_whitelisting(self)[1..].join("::"); self.options().no_hash_types.matches(&name) } + + /// Check if `--no-default` flag is enabled for this item. + pub fn no_default_by_name(&self, item: &Item) -> bool { + let name = item.path_for_whitelisting(self)[1..].join("::"); + self.options().no_default_types.matches(&name) + } } /// A builder struct for configuring item resolution options. diff --git a/src/lib.rs b/src/lib.rs index a818409be0..07dca92f8c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -648,6 +648,16 @@ impl Builder { }) .count(); + self.options + .no_default_types + .get_items() + .iter() + .map(|item| { + output_vector.push("--no-default".into()); + output_vector.push(item.to_owned()); + }) + .count(); + output_vector } @@ -1507,6 +1517,13 @@ impl Builder { self.options.wasm_import_module_name = Some(import_name.into()); self } + + /// Don't derive `Default` for a given type. Regular + /// expressions are supported. + pub fn no_default>(mut self, arg: T) -> Builder { + self.options.no_default_types.insert(arg.into()); + self + } } /// Configuration options for generated bindings. @@ -1769,6 +1786,9 @@ struct BindgenOptions { /// Wasm import module name. wasm_import_module_name: Option, + + /// The set of types that we should not derive `Default` for. + no_default_types: RegexSet, } /// TODO(emilio): This is sort of a lie (see the error message that results from @@ -1798,6 +1818,7 @@ impl BindgenOptions { &mut self.no_partialeq_types, &mut self.no_copy_types, &mut self.no_hash_types, + &mut self.no_default_types, ]; let record_matches = self.record_matches; for regex_set in &mut regex_sets { @@ -1895,6 +1916,7 @@ impl Default for BindgenOptions { no_hash_types: Default::default(), array_pointers_in_arguments: false, wasm_import_module_name: None, + no_default_types: Default::default(), } } } diff --git a/src/options.rs b/src/options.rs index 01982f13cf..a23dd31ff2 100644 --- a/src/options.rs +++ b/src/options.rs @@ -438,7 +438,14 @@ where .long("wasm-import-module-name") .value_name("name") .takes_value(true) - .help("The name to be used in a #[link(wasm_import_module = ...)] statement") + .help("The name to be used in a #[link(wasm_import_module = ...)] statement"), + Arg::with_name("no-default") + .long("no-default") + .help("Avoids deriving Default for types matching .") + .value_name("regex") + .takes_value(true) + .multiple(true) + .number_of_values(1), ]) // .args() .get_matches_from(args); @@ -813,6 +820,12 @@ where } } + if let Some(no_default) = matches.values_of("no-default") { + for regex in no_default { + builder = builder.no_default(regex); + } + } + let verbose = matches.is_present("verbose"); Ok((builder, output, verbose)) diff --git a/tests/expectations/tests/forward-no-default.rs b/tests/expectations/tests/forward-no-default.rs new file mode 100644 index 0000000000..34565d9b3d --- /dev/null +++ b/tests/expectations/tests/forward-no-default.rs @@ -0,0 +1,15 @@ +/* automatically generated by rust-bindgen */ + +#![allow( + dead_code, + non_snake_case, + non_camel_case_types, + non_upper_case_globals +)] + +#[repr(C)] +#[derive(Debug, Copy, Clone)] +pub struct NoDefault { + _unused: [u8; 0], +} +pub type NoDefaultT = NoDefault; diff --git a/tests/expectations/tests/layout_array.rs b/tests/expectations/tests/layout_array.rs index 3e0142d0ff..5edb82f97c 100644 --- a/tests/expectations/tests/layout_array.rs +++ b/tests/expectations/tests/layout_array.rs @@ -284,6 +284,13 @@ impl Default for rte_mempool_ops_table { unsafe { ::std::mem::zeroed() } } } +impl ::std::cmp::PartialEq for rte_mempool_ops_table { + fn eq(&self, other: &rte_mempool_ops_table) -> bool { + self.sl == other.sl && + self.num_ops == other.num_ops && + self.ops == other.ops + } +} /// Structure to hold malloc heap #[repr(C)] #[repr(align(64))] diff --git a/tests/expectations/tests/no-default-opaque.rs b/tests/expectations/tests/no-default-opaque.rs new file mode 100644 index 0000000000..7bcd88aba1 --- /dev/null +++ b/tests/expectations/tests/no-default-opaque.rs @@ -0,0 +1,28 @@ +/* automatically generated by rust-bindgen */ + +#![allow( + dead_code, + non_snake_case, + non_camel_case_types, + non_upper_case_globals +)] + +#[repr(C)] +#[repr(align(4))] +#[derive(Debug, Copy, Clone)] +pub struct NoDefault { + pub _bindgen_opaque_blob: u32, +} +#[test] +fn bindgen_test_layout_NoDefault() { + assert_eq!( + ::std::mem::size_of::(), + 4usize, + concat!("Size of: ", stringify!(NoDefault)) + ); + assert_eq!( + ::std::mem::align_of::(), + 4usize, + concat!("Alignment of ", stringify!(NoDefault)) + ); +} diff --git a/tests/expectations/tests/no-default-whitelisted.rs b/tests/expectations/tests/no-default-whitelisted.rs new file mode 100644 index 0000000000..6af8ca2b86 --- /dev/null +++ b/tests/expectations/tests/no-default-whitelisted.rs @@ -0,0 +1,37 @@ +/* automatically generated by rust-bindgen */ + +#![allow( + dead_code, + non_snake_case, + non_camel_case_types, + non_upper_case_globals +)] + +#[repr(C)] +#[derive(Debug, Copy, Clone)] +pub struct NoDefault { + pub x: ::std::os::raw::c_int, +} +#[test] +fn bindgen_test_layout_NoDefault() { + assert_eq!( + ::std::mem::size_of::(), + 4usize, + concat!("Size of: ", stringify!(NoDefault)) + ); + assert_eq!( + ::std::mem::align_of::(), + 4usize, + concat!("Alignment of ", stringify!(NoDefault)) + ); + assert_eq!( + unsafe { &(*(::std::ptr::null::())).x as *const _ as usize }, + 0usize, + concat!( + "Offset of field: ", + stringify!(NoDefault), + "::", + stringify!(x) + ) + ); +} diff --git a/tests/expectations/tests/no-default.rs b/tests/expectations/tests/no-default.rs new file mode 100644 index 0000000000..d022f21bb6 --- /dev/null +++ b/tests/expectations/tests/no-default.rs @@ -0,0 +1,39 @@ +/* automatically generated by rust-bindgen */ + +#![allow( + dead_code, + non_snake_case, + non_camel_case_types, + non_upper_case_globals +)] + +#[repr(C)] +#[derive(Debug, Copy, Clone)] +pub struct NotDefault { + pub x: ::std::os::raw::c_int, +} +#[test] +fn bindgen_test_layout_NotDefault() { + assert_eq!( + ::std::mem::size_of::(), + 4usize, + concat!("Size of: ", stringify!(NotDefault)) + ); + assert_eq!( + ::std::mem::align_of::(), + 4usize, + concat!("Alignment of ", stringify!(NotDefault)) + ); + assert_eq!( + unsafe { + &(*(::std::ptr::null::())).x as *const _ as usize + }, + 0usize, + concat!( + "Offset of field: ", + stringify!(NotDefault), + "::", + stringify!(x) + ) + ); +} diff --git a/tests/expectations/tests/whitelisted-item-references-no-default.rs b/tests/expectations/tests/whitelisted-item-references-no-default.rs new file mode 100644 index 0000000000..5e6a6c5937 --- /dev/null +++ b/tests/expectations/tests/whitelisted-item-references-no-default.rs @@ -0,0 +1,57 @@ +/* automatically generated by rust-bindgen */ + +#![allow( + dead_code, + non_snake_case, + non_camel_case_types, + non_upper_case_globals +)] + +#[repr(C)] +#[derive(Debug, Copy, Clone)] +pub struct NoDefault { + pub _address: u8, +} +#[test] +fn bindgen_test_layout_NoDefault() { + assert_eq!( + ::std::mem::size_of::(), + 1usize, + concat!("Size of: ", stringify!(NoDefault)) + ); + assert_eq!( + ::std::mem::align_of::(), + 1usize, + concat!("Alignment of ", stringify!(NoDefault)) + ); +} +#[repr(C)] +#[derive(Debug, Copy, Clone)] +pub struct Whitelisted { + pub x: NoDefault, +} +#[test] +fn bindgen_test_layout_Whitelisted() { + assert_eq!( + ::std::mem::size_of::(), + 1usize, + concat!("Size of: ", stringify!(Whitelisted)) + ); + assert_eq!( + ::std::mem::align_of::(), + 1usize, + concat!("Alignment of ", stringify!(Whitelisted)) + ); + assert_eq!( + unsafe { + &(*(::std::ptr::null::())).x as *const _ as usize + }, + 0usize, + concat!( + "Offset of field: ", + stringify!(Whitelisted), + "::", + stringify!(x) + ) + ); +} diff --git a/tests/headers/forward-no-default.h b/tests/headers/forward-no-default.h new file mode 100644 index 0000000000..0a70232be0 --- /dev/null +++ b/tests/headers/forward-no-default.h @@ -0,0 +1,3 @@ +// bindgen-flags: --no-default NoDefault + +typedef struct NoDefault NoDefaultT; diff --git a/tests/headers/no-default-opaque.hpp b/tests/headers/no-default-opaque.hpp new file mode 100644 index 0000000000..4dfd35afbd --- /dev/null +++ b/tests/headers/no-default-opaque.hpp @@ -0,0 +1,7 @@ +// bindgen-flags: --no-default "NoDefault" --opaque-type "NoDefault" + +// `--with-derive-default` is added by the test runner implicitly. + +class NoDefault { + int x; +}; diff --git a/tests/headers/no-default-whitelisted.hpp b/tests/headers/no-default-whitelisted.hpp new file mode 100644 index 0000000000..a9101dfcd7 --- /dev/null +++ b/tests/headers/no-default-whitelisted.hpp @@ -0,0 +1,7 @@ +// bindgen-flags: --no-default "NoDefault" --whitelist-type "NoDefault" + +// `--with-derive-default` is added by the test runner implicitly. + +class NoDefault { + int x; +}; diff --git a/tests/headers/no-default.hpp b/tests/headers/no-default.hpp new file mode 100644 index 0000000000..01ea7de8f6 --- /dev/null +++ b/tests/headers/no-default.hpp @@ -0,0 +1,7 @@ +// bindgen-flags: --no-default "NotDefault" + +// `--with-derive-default` is added by the test runner implicitly. + +class NotDefault { + int x; +}; diff --git a/tests/headers/whitelisted-item-references-no-default.hpp b/tests/headers/whitelisted-item-references-no-default.hpp new file mode 100644 index 0000000000..0a3a7ad745 --- /dev/null +++ b/tests/headers/whitelisted-item-references-no-default.hpp @@ -0,0 +1,9 @@ +// bindgen-flags: --no-default "NoDefault" --whitelist-type "Whitelisted" + +// `--with-derive-default` is added by the test runner implicitly. + +struct NoDefault {}; + +class Whitelisted { + NoDefault x; +};