From 102152cbd6c48aabcf0e8d839c81a6b90ca0c023 Mon Sep 17 00:00:00 2001 From: Jonathan Schwender Date: Sun, 2 Feb 2025 09:53:11 +0100 Subject: [PATCH] Prevent default derives for Forward declared types. We do not know the layout for forward declared types. Deriving Copy, Clone, Hash, PartialEq etc. is simply wrong. Deriving Debug is debatable, since it can't print anything useful. Since we previously would derive Debug, lets just keep it, to avoid needless breaking changes. Changed tests: - func-return-must-use: Don't use forward declared structs in the test, since that would result in wrong bindings! - issue-1238-fwd-no-copy test no longer requires an annotation, since Forward declared types should always be no copy - forward-declaration-autoptr.rs: Manually implement clone. The inner type is unknown, so requiring clone/copy is not correct. --- .../tests/expectations/tests/anon_union.rs | 4 +-- .../tests/forward-declaration-autoptr.rs | 4 +-- .../tests/forward_declared_complex_types.rs | 6 ++-- .../tests/forward_declared_opaque.rs | 4 +-- .../tests/func_return_must_use.rs | 32 ++++++++++++++----- .../tests/expectations/tests/issue-1443.rs | 2 +- .../tests/issue-654-struct-fn-collision.rs | 2 +- .../tests/issue-710-must-use-type.rs | 6 ++-- .../tests/issue-801-opaque-sloppiness.rs | 2 +- .../tests/expectations/tests/layout_array.rs | 2 +- .../tests/expectations/tests/operator.rs | 2 +- .../tests/expectations/tests/parm-union.rs | 2 +- ...ame_struct_name_in_different_namespaces.rs | 2 +- .../tests/expectations/tests/template.rs | 2 +- .../tests/typedef-pointer-overlap.rs | 2 +- .../expectations/tests/wrap-static-fns.rs | 2 +- .../tests/wrap_unsafe_ops_anon_union.rs | 4 +-- .../tests/headers/func_return_must_use.h | 12 +++++-- .../tests/headers/issue-1238-fwd-no-copy.h | 1 - bindgen/codegen/mod.rs | 12 ++++++- bindgen/ir/analysis/derive.rs | 2 +- 21 files changed, 69 insertions(+), 38 deletions(-) diff --git a/bindgen-tests/tests/expectations/tests/anon_union.rs b/bindgen-tests/tests/expectations/tests/anon_union.rs index d9bf3cf183..8a5891bed0 100644 --- a/bindgen-tests/tests/expectations/tests/anon_union.rs +++ b/bindgen-tests/tests/expectations/tests/anon_union.rs @@ -15,12 +15,12 @@ pub enum TErrorResult_UnionState { HasMessage = 0, } #[repr(C)] -#[derive(Debug, Copy, Clone)] +#[derive(Debug)] pub struct TErrorResult_Message { _unused: [u8; 0], } #[repr(C)] -#[derive(Debug, Copy, Clone)] +#[derive(Debug)] pub struct TErrorResult_DOMExceptionInfo { _unused: [u8; 0], } diff --git a/bindgen-tests/tests/expectations/tests/forward-declaration-autoptr.rs b/bindgen-tests/tests/expectations/tests/forward-declaration-autoptr.rs index b74b408841..aeb2b7b4e5 100644 --- a/bindgen-tests/tests/expectations/tests/forward-declaration-autoptr.rs +++ b/bindgen-tests/tests/expectations/tests/forward-declaration-autoptr.rs @@ -1,6 +1,6 @@ #![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)] #[repr(C)] -#[derive(Debug, Copy, Clone)] +#[derive(Debug)] pub struct Foo { _unused: [u8; 0], } @@ -20,7 +20,7 @@ impl Default for RefPtr { } } #[repr(C)] -#[derive(Debug, Copy, Clone)] +#[derive(Debug)] pub struct Bar { pub m_member: RefPtr, } diff --git a/bindgen-tests/tests/expectations/tests/forward_declared_complex_types.rs b/bindgen-tests/tests/expectations/tests/forward_declared_complex_types.rs index 79554d58e4..89cf957633 100644 --- a/bindgen-tests/tests/expectations/tests/forward_declared_complex_types.rs +++ b/bindgen-tests/tests/expectations/tests/forward_declared_complex_types.rs @@ -10,7 +10,7 @@ const _: () = { ["Alignment of Foo_empty"][::std::mem::align_of::() - 1usize]; }; #[repr(C)] -#[derive(Debug, Copy, Clone)] +#[derive(Debug)] pub struct Foo { _unused: [u8; 0], } @@ -39,7 +39,7 @@ unsafe extern "C" { pub fn baz_struct(f: *mut Foo); } #[repr(C)] -#[derive(Copy, Clone)] +#[derive(Debug)] pub struct Union { _unused: [u8; 0], } @@ -48,7 +48,7 @@ unsafe extern "C" { pub fn baz_union(u: *mut Union); } #[repr(C)] -#[derive(Debug, Copy, Clone)] +#[derive(Debug)] pub struct Quux { _unused: [u8; 0], } diff --git a/bindgen-tests/tests/expectations/tests/forward_declared_opaque.rs b/bindgen-tests/tests/expectations/tests/forward_declared_opaque.rs index 1f92194045..5c3a1f5412 100644 --- a/bindgen-tests/tests/expectations/tests/forward_declared_opaque.rs +++ b/bindgen-tests/tests/expectations/tests/forward_declared_opaque.rs @@ -1,11 +1,11 @@ #![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)] #[repr(C)] -#[derive(Copy, Clone)] +#[derive(Debug)] pub struct a { _unused: [u8; 0], } #[repr(C)] -#[derive(Debug, Default, Copy, Clone)] +#[derive(Debug)] pub struct b { _unused: [u8; 0], } diff --git a/bindgen-tests/tests/expectations/tests/func_return_must_use.rs b/bindgen-tests/tests/expectations/tests/func_return_must_use.rs index 904c71cbe3..9a4293c458 100644 --- a/bindgen-tests/tests/expectations/tests/func_return_must_use.rs +++ b/bindgen-tests/tests/expectations/tests/func_return_must_use.rs @@ -5,11 +5,19 @@ unsafe extern "C" { pub fn return_int() -> MustUseInt; } #[repr(C)] -#[derive(Debug, Copy, Clone)] +#[derive(Debug, Default, Copy, Clone)] #[must_use] pub struct MustUseStruct { - _unused: [u8; 0], + pub a: ::std::os::raw::c_int, } +#[allow(clippy::unnecessary_operation, clippy::identity_op)] +const _: () = { + ["Size of MustUseStruct"][::std::mem::size_of::() - 4usize]; + ["Alignment of MustUseStruct"][::std::mem::align_of::() - 4usize]; + [ + "Offset of field: MustUseStruct::a", + ][::std::mem::offset_of!(MustUseStruct, a) - 0usize]; +}; unsafe extern "C" { #[must_use] pub fn return_struct() -> MustUseStruct; @@ -27,11 +35,16 @@ unsafe extern "C" { #[repr(C)] #[derive(Debug, Default, Copy, Clone)] #[must_use] -pub struct AnnotatedStruct {} +pub struct AnnotatedStruct { + pub a: ::std::os::raw::c_int, +} #[allow(clippy::unnecessary_operation, clippy::identity_op)] const _: () = { - ["Size of AnnotatedStruct"][::std::mem::size_of::() - 0usize]; - ["Alignment of AnnotatedStruct"][::std::mem::align_of::() - 1usize]; + ["Size of AnnotatedStruct"][::std::mem::size_of::() - 4usize]; + ["Alignment of AnnotatedStruct"][::std::mem::align_of::() - 4usize]; + [ + "Offset of field: AnnotatedStruct::a", + ][::std::mem::offset_of!(AnnotatedStruct, a) - 0usize]; }; unsafe extern "C" { #[must_use] @@ -39,11 +52,14 @@ unsafe extern "C" { } #[repr(C)] #[derive(Debug, Default, Copy, Clone)] -pub struct PlainStruct {} +pub struct PlainStruct { + pub a: ::std::os::raw::c_int, +} #[allow(clippy::unnecessary_operation, clippy::identity_op)] const _: () = { - ["Size of PlainStruct"][::std::mem::size_of::() - 0usize]; - ["Alignment of PlainStruct"][::std::mem::align_of::() - 1usize]; + ["Size of PlainStruct"][::std::mem::size_of::() - 4usize]; + ["Alignment of PlainStruct"][::std::mem::align_of::() - 4usize]; + ["Offset of field: PlainStruct::a"][::std::mem::offset_of!(PlainStruct, a) - 0usize]; }; ///
pub type TypedefPlainStruct = PlainStruct; diff --git a/bindgen-tests/tests/expectations/tests/issue-1443.rs b/bindgen-tests/tests/expectations/tests/issue-1443.rs index ee1ffca8e5..e1617582d3 100644 --- a/bindgen-tests/tests/expectations/tests/issue-1443.rs +++ b/bindgen-tests/tests/expectations/tests/issue-1443.rs @@ -1,6 +1,6 @@ #![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)] #[repr(C)] -#[derive(Debug, Copy, Clone)] +#[derive(Debug)] pub struct Foo { _unused: [u8; 0], } diff --git a/bindgen-tests/tests/expectations/tests/issue-654-struct-fn-collision.rs b/bindgen-tests/tests/expectations/tests/issue-654-struct-fn-collision.rs index 6bf02be74e..bd670a00f2 100644 --- a/bindgen-tests/tests/expectations/tests/issue-654-struct-fn-collision.rs +++ b/bindgen-tests/tests/expectations/tests/issue-654-struct-fn-collision.rs @@ -1,6 +1,6 @@ #![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)] #[repr(C)] -#[derive(Debug, Copy, Clone)] +#[derive(Debug)] pub struct foo { _unused: [u8; 0], } diff --git a/bindgen-tests/tests/expectations/tests/issue-710-must-use-type.rs b/bindgen-tests/tests/expectations/tests/issue-710-must-use-type.rs index d7006dd011..25da6eb498 100644 --- a/bindgen-tests/tests/expectations/tests/issue-710-must-use-type.rs +++ b/bindgen-tests/tests/expectations/tests/issue-710-must-use-type.rs @@ -1,19 +1,19 @@ #![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)] #[repr(C)] -#[derive(Debug, Copy, Clone)] +#[derive(Debug)] #[must_use] pub struct A { _unused: [u8; 0], } ///
#[repr(C)] -#[derive(Debug, Copy, Clone)] +#[derive(Debug)] #[must_use] pub struct B { _unused: [u8; 0], } #[repr(C)] -#[derive(Debug, Copy, Clone)] +#[derive(Debug)] pub struct C { _unused: [u8; 0], } diff --git a/bindgen-tests/tests/expectations/tests/issue-801-opaque-sloppiness.rs b/bindgen-tests/tests/expectations/tests/issue-801-opaque-sloppiness.rs index 90eb048640..50bc573fc4 100644 --- a/bindgen-tests/tests/expectations/tests/issue-801-opaque-sloppiness.rs +++ b/bindgen-tests/tests/expectations/tests/issue-801-opaque-sloppiness.rs @@ -1,6 +1,6 @@ #![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)] #[repr(C)] -#[derive(Debug, Copy, Clone)] +#[derive(Debug)] pub struct A { _unused: [u8; 0], } diff --git a/bindgen-tests/tests/expectations/tests/layout_array.rs b/bindgen-tests/tests/expectations/tests/layout_array.rs index b910159beb..b0654815d6 100644 --- a/bindgen-tests/tests/expectations/tests/layout_array.rs +++ b/bindgen-tests/tests/expectations/tests/layout_array.rs @@ -4,7 +4,7 @@ pub const RTE_MEMPOOL_OPS_NAMESIZE: u32 = 32; pub const RTE_MEMPOOL_MAX_OPS_IDX: u32 = 16; pub const RTE_HEAP_NUM_FREELISTS: u32 = 13; #[repr(C)] -#[derive(Debug, Copy, Clone)] +#[derive(Debug)] pub struct rte_mempool { _unused: [u8; 0], } diff --git a/bindgen-tests/tests/expectations/tests/operator.rs b/bindgen-tests/tests/expectations/tests/operator.rs index ce5a652d8d..1a5a6859a2 100644 --- a/bindgen-tests/tests/expectations/tests/operator.rs +++ b/bindgen-tests/tests/expectations/tests/operator.rs @@ -4,7 +4,7 @@ unsafe extern "C" { pub fn operator_information() -> ::std::os::raw::c_int; } #[repr(C)] -#[derive(Debug, Copy, Clone)] +#[derive(Debug)] pub struct Foo { _unused: [u8; 0], } diff --git a/bindgen-tests/tests/expectations/tests/parm-union.rs b/bindgen-tests/tests/expectations/tests/parm-union.rs index 6dee4ec7d5..1a4f654038 100644 --- a/bindgen-tests/tests/expectations/tests/parm-union.rs +++ b/bindgen-tests/tests/expectations/tests/parm-union.rs @@ -20,7 +20,7 @@ impl Struct { } } #[repr(C)] -#[derive(Copy, Clone)] +#[derive(Debug)] pub struct Union { _unused: [u8; 0], } diff --git a/bindgen-tests/tests/expectations/tests/same_struct_name_in_different_namespaces.rs b/bindgen-tests/tests/expectations/tests/same_struct_name_in_different_namespaces.rs index e6e4088abf..b7dd3a579f 100644 --- a/bindgen-tests/tests/expectations/tests/same_struct_name_in_different_namespaces.rs +++ b/bindgen-tests/tests/expectations/tests/same_struct_name_in_different_namespaces.rs @@ -1,6 +1,6 @@ #![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)] #[repr(C)] -#[derive(Debug, Copy, Clone)] +#[derive(Debug)] pub struct JS_Zone { _unused: [u8; 0], } diff --git a/bindgen-tests/tests/expectations/tests/template.rs b/bindgen-tests/tests/expectations/tests/template.rs index 94678cb49e..b1beae9930 100644 --- a/bindgen-tests/tests/expectations/tests/template.rs +++ b/bindgen-tests/tests/expectations/tests/template.rs @@ -36,7 +36,7 @@ unsafe extern "C" { pub fn bar(foo: Foo<::std::os::raw::c_int>); } #[repr(C)] -#[derive(Debug, Copy, Clone)] +#[derive(Debug)] pub struct mozilla_Foo { _unused: [u8; 0], } diff --git a/bindgen-tests/tests/expectations/tests/typedef-pointer-overlap.rs b/bindgen-tests/tests/expectations/tests/typedef-pointer-overlap.rs index 122059b4e1..1579b6ddf7 100644 --- a/bindgen-tests/tests/expectations/tests/typedef-pointer-overlap.rs +++ b/bindgen-tests/tests/expectations/tests/typedef-pointer-overlap.rs @@ -24,7 +24,7 @@ const _: () = { }; pub type bar_ptr = *mut bar; #[repr(C)] -#[derive(Debug, Copy, Clone)] +#[derive(Debug)] pub struct baz { _unused: [u8; 0], } diff --git a/bindgen-tests/tests/expectations/tests/wrap-static-fns.rs b/bindgen-tests/tests/expectations/tests/wrap-static-fns.rs index bafcad8a7e..4c13c365ea 100644 --- a/bindgen-tests/tests/expectations/tests/wrap-static-fns.rs +++ b/bindgen-tests/tests/expectations/tests/wrap-static-fns.rs @@ -81,7 +81,7 @@ unsafe extern "C" { } pub type __builtin_va_list = [__va_list_tag; 1usize]; #[repr(C)] -#[derive(Debug, Copy, Clone)] +#[derive(Debug)] pub struct __va_list_tag { pub gp_offset: ::std::os::raw::c_uint, pub fp_offset: ::std::os::raw::c_uint, diff --git a/bindgen-tests/tests/expectations/tests/wrap_unsafe_ops_anon_union.rs b/bindgen-tests/tests/expectations/tests/wrap_unsafe_ops_anon_union.rs index 50cefed043..20bbf9037c 100644 --- a/bindgen-tests/tests/expectations/tests/wrap_unsafe_ops_anon_union.rs +++ b/bindgen-tests/tests/expectations/tests/wrap_unsafe_ops_anon_union.rs @@ -10,12 +10,12 @@ pub const TErrorResult_UnionState_HasMessage: TErrorResult_UnionState = 0; pub const TErrorResult_UnionState_HasException: TErrorResult_UnionState = 0; pub type TErrorResult_UnionState = i32; #[repr(C)] -#[derive(Debug, Copy, Clone)] +#[derive(Debug)] pub struct TErrorResult_Message { _unused: [u8; 0], } #[repr(C)] -#[derive(Debug, Copy, Clone)] +#[derive(Debug)] pub struct TErrorResult_DOMExceptionInfo { _unused: [u8; 0], } diff --git a/bindgen-tests/tests/headers/func_return_must_use.h b/bindgen-tests/tests/headers/func_return_must_use.h index f05bd2de40..da727fe559 100644 --- a/bindgen-tests/tests/headers/func_return_must_use.h +++ b/bindgen-tests/tests/headers/func_return_must_use.h @@ -4,7 +4,9 @@ typedef int MustUseInt; MustUseInt return_int(); -struct MustUseStruct; +struct MustUseStruct { + int a; +}; struct MustUseStruct return_struct(); @@ -20,11 +22,15 @@ int return_plain_int(); /** *
*/ -struct AnnotatedStruct {}; +struct AnnotatedStruct { + int a; +}; struct AnnotatedStruct return_annotated_struct(); -struct PlainStruct {}; +struct PlainStruct { + int a; +}; /** *
diff --git a/bindgen-tests/tests/headers/issue-1238-fwd-no-copy.h b/bindgen-tests/tests/headers/issue-1238-fwd-no-copy.h index 150bbbeb77..dba7ab1462 100644 --- a/bindgen-tests/tests/headers/issue-1238-fwd-no-copy.h +++ b/bindgen-tests/tests/headers/issue-1238-fwd-no-copy.h @@ -1,3 +1,2 @@ -// bindgen-flags: --no-copy MyType typedef struct MyType MyTypeT; diff --git a/bindgen/codegen/mod.rs b/bindgen/codegen/mod.rs index f58a234117..84b7aa46d3 100644 --- a/bindgen/codegen/mod.rs +++ b/bindgen/codegen/mod.rs @@ -2444,7 +2444,17 @@ impl CodeGenerator for CompInfo { } } - let derivable_traits = derives_of_item(item, ctx, packed); + let derivable_traits = if self.is_forward_declaration() { + // The only trait we can derive for forward declared types is `Debug`, + // since we don't know anything about the layout or type. + let mut derivable_traits = DerivableTraits::empty(); + if !item.annotations().disallow_debug() { + derivable_traits |= DerivableTraits::DEBUG; + }; + derivable_traits + } else { + derives_of_item(item, ctx, packed) + }; if !derivable_traits.contains(DerivableTraits::DEBUG) { needs_debug_impl = ctx.options().derive_debug && ctx.options().impl_debug && diff --git a/bindgen/ir/analysis/derive.rs b/bindgen/ir/analysis/derive.rs index 6c66998bee..3e216b1f0b 100644 --- a/bindgen/ir/analysis/derive.rs +++ b/bindgen/ir/analysis/derive.rs @@ -510,7 +510,7 @@ impl DeriveTrait { } fn can_derive_compound_forward_decl(self) -> bool { - matches!(self, DeriveTrait::Copy | DeriveTrait::Debug) + matches!(self, DeriveTrait::Debug) } fn can_derive_incomplete_array(self) -> bool {