Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support deriving copy for large array #874

Merged
merged 2 commits into from
Aug 1, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/ir/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -531,8 +531,7 @@ impl<'a> CanDeriveCopy<'a> for Type {

fn can_derive_copy(&self, ctx: &BindgenContext, item: &Item) -> bool {
match self.kind {
TypeKind::Array(t, len) => {
len <= RUST_DERIVE_IN_ARRAY_LIMIT &&
TypeKind::Array(t, _) => {
t.can_derive_copy_in_array(ctx, ())
}
TypeKind::ResolvedTypeRef(t) |
Expand Down
20 changes: 20 additions & 0 deletions tests/expectations/tests/class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ impl <T> ::std::fmt::Debug for __BindgenUnionField<T> {
}
}
#[repr(C)]
#[derive(Copy)]
pub struct C {
pub a: ::std::os::raw::c_int,
pub big_array: [::std::os::raw::c_char; 33usize],
Expand All @@ -82,10 +83,14 @@ fn bindgen_test_layout_C() {
"Alignment of field: " , stringify ! ( C ) , "::" , stringify
! ( big_array ) ));
}
impl Clone for C {
fn clone(&self) -> Self { *self }
}
impl Default for C {
fn default() -> Self { unsafe { ::std::mem::zeroed() } }
}
#[repr(C)]
#[derive(Copy)]
pub struct C_with_zero_length_array {
pub a: ::std::os::raw::c_int,
pub big_array: [::std::os::raw::c_char; 33usize],
Expand Down Expand Up @@ -118,10 +123,14 @@ fn bindgen_test_layout_C_with_zero_length_array() {
C_with_zero_length_array ) , "::" , stringify ! (
zero_length_array ) ));
}
impl Clone for C_with_zero_length_array {
fn clone(&self) -> Self { *self }
}
impl Default for C_with_zero_length_array {
fn default() -> Self { unsafe { ::std::mem::zeroed() } }
}
#[repr(C)]
#[derive(Copy)]
pub struct C_with_incomplete_array {
pub a: ::std::os::raw::c_int,
pub big_array: [::std::os::raw::c_char; 33usize],
Expand All @@ -136,10 +145,14 @@ fn bindgen_test_layout_C_with_incomplete_array() {
concat ! (
"Alignment of " , stringify ! ( C_with_incomplete_array ) ));
}
impl Clone for C_with_incomplete_array {
fn clone(&self) -> Self { *self }
}
impl Default for C_with_incomplete_array {
fn default() -> Self { unsafe { ::std::mem::zeroed() } }
}
#[repr(C)]
#[derive(Copy)]
pub struct C_with_zero_length_array_and_incomplete_array {
pub a: ::std::os::raw::c_int,
pub big_array: [::std::os::raw::c_char; 33usize],
Expand All @@ -157,6 +170,9 @@ fn bindgen_test_layout_C_with_zero_length_array_and_incomplete_array() {
"Alignment of " , stringify ! (
C_with_zero_length_array_and_incomplete_array ) ));
}
impl Clone for C_with_zero_length_array_and_incomplete_array {
fn clone(&self) -> Self { *self }
}
impl Default for C_with_zero_length_array_and_incomplete_array {
fn default() -> Self { unsafe { ::std::mem::zeroed() } }
}
Expand All @@ -178,6 +194,7 @@ fn bindgen_test_layout_WithDtor() {
stringify ! ( b ) ));
}
#[repr(C)]
#[derive(Copy)]
pub struct IncompleteArrayNonCopiable {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what to do with this. Change the name?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should really be kept non-copiable though... That's a huge footgun, because incomplete arrays tend to be used to allocate to the end of the object, and copying it would lose the trailing information.

pub whatever: *mut ::std::os::raw::c_void,
pub incomplete_array: __IncompleteArrayField<C>,
Expand All @@ -192,6 +209,9 @@ fn bindgen_test_layout_IncompleteArrayNonCopiable() {
"Alignment of " , stringify ! ( IncompleteArrayNonCopiable )
));
}
impl Clone for IncompleteArrayNonCopiable {
fn clone(&self) -> Self { *self }
}
impl Default for IncompleteArrayNonCopiable {
fn default() -> Self { unsafe { ::std::mem::zeroed() } }
}
Expand Down
16 changes: 16 additions & 0 deletions tests/expectations/tests/layout_eth_conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -727,6 +727,7 @@ pub enum rte_eth_nb_pools {
/// A default pool may be used, if desired, to route all traffic which
/// does not match the vlan filter rules.
#[repr(C)]
#[derive(Copy)]
pub struct rte_eth_vmdq_dcb_conf {
/// < With DCB, 16 or 32 pools
pub nb_queue_pools: rte_eth_nb_pools,
Expand Down Expand Up @@ -814,6 +815,9 @@ fn bindgen_test_layout_rte_eth_vmdq_dcb_conf() {
"Alignment of field: " , stringify ! ( rte_eth_vmdq_dcb_conf )
, "::" , stringify ! ( dcb_tc ) ));
}
impl Clone for rte_eth_vmdq_dcb_conf {
fn clone(&self) -> Self { *self }
}
impl Default for rte_eth_vmdq_dcb_conf {
fn default() -> Self { unsafe { ::std::mem::zeroed() } }
}
Expand Down Expand Up @@ -941,6 +945,7 @@ impl Default for rte_eth_vmdq_tx_conf {
fn default() -> Self { unsafe { ::std::mem::zeroed() } }
}
#[repr(C)]
#[derive(Copy)]
pub struct rte_eth_vmdq_rx_conf {
/// < VMDq only mode, 8 or 64 pools
pub nb_queue_pools: rte_eth_nb_pools,
Expand Down Expand Up @@ -1036,6 +1041,9 @@ fn bindgen_test_layout_rte_eth_vmdq_rx_conf() {
"Alignment of field: " , stringify ! ( rte_eth_vmdq_rx_conf )
, "::" , stringify ! ( pool_map ) ));
}
impl Clone for rte_eth_vmdq_rx_conf {
fn clone(&self) -> Self { *self }
}
impl Default for rte_eth_vmdq_rx_conf {
fn default() -> Self { unsafe { ::std::mem::zeroed() } }
}
Expand Down Expand Up @@ -1458,6 +1466,7 @@ impl Clone for rte_intr_conf {
/// Depending upon the RX multi-queue mode, extra advanced
/// configuration settings may be needed.
#[repr(C)]
#[derive(Copy)]
pub struct rte_eth_conf {
/// < bitmap of ETH_LINK_SPEED_XXX of speeds to be
/// used. ETH_LINK_SPEED_FIXED disables link
Expand Down Expand Up @@ -1490,6 +1499,7 @@ pub struct rte_eth_conf {
pub intr_conf: rte_intr_conf,
}
#[repr(C)]
#[derive(Copy)]
pub struct rte_eth_conf__bindgen_ty_1 {
/// < Port RSS configuration
pub rss_conf: rte_eth_rss_conf,
Expand Down Expand Up @@ -1531,6 +1541,9 @@ fn bindgen_test_layout_rte_eth_conf__bindgen_ty_1() {
rte_eth_conf__bindgen_ty_1 ) , "::" , stringify ! (
vmdq_rx_conf ) ));
}
impl Clone for rte_eth_conf__bindgen_ty_1 {
fn clone(&self) -> Self { *self }
}
impl Default for rte_eth_conf__bindgen_ty_1 {
fn default() -> Self { unsafe { ::std::mem::zeroed() } }
}
Expand Down Expand Up @@ -1625,6 +1638,9 @@ fn bindgen_test_layout_rte_eth_conf() {
"Alignment of field: " , stringify ! ( rte_eth_conf ) , "::" ,
stringify ! ( intr_conf ) ));
}
impl Clone for rte_eth_conf {
fn clone(&self) -> Self { *self }
}
impl Default for rte_eth_conf {
fn default() -> Self { unsafe { ::std::mem::zeroed() } }
}
8 changes: 8 additions & 0 deletions tests/expectations/tests/struct_with_derive_debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ impl Clone for LittleArray {
fn clone(&self) -> Self { *self }
}
#[repr(C)]
#[derive(Copy)]
pub struct BigArray {
pub a: [::std::os::raw::c_int; 33usize],
}
Expand All @@ -40,6 +41,9 @@ fn bindgen_test_layout_BigArray() {
"Alignment of field: " , stringify ! ( BigArray ) , "::" ,
stringify ! ( a ) ));
}
impl Clone for BigArray {
fn clone(&self) -> Self { *self }
}
impl Default for BigArray {
fn default() -> Self { unsafe { ::std::mem::zeroed() } }
}
Expand All @@ -64,6 +68,7 @@ impl Clone for WithLittleArray {
fn clone(&self) -> Self { *self }
}
#[repr(C)]
#[derive(Copy)]
pub struct WithBigArray {
pub a: BigArray,
}
Expand All @@ -79,6 +84,9 @@ fn bindgen_test_layout_WithBigArray() {
"Alignment of field: " , stringify ! ( WithBigArray ) , "::" ,
stringify ! ( a ) ));
}
impl Clone for WithBigArray {
fn clone(&self) -> Self { *self }
}
impl Default for WithBigArray {
fn default() -> Self { unsafe { ::std::mem::zeroed() } }
}
37 changes: 37 additions & 0 deletions tests/expectations/tests/struct_with_large_array.rs
Original file line number Diff line number Diff line change
@@ -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(Copy)]
pub struct S {
pub large_array: [::std::os::raw::c_char; 33usize],
}
#[test]
fn bindgen_test_layout_S() {
assert_eq!(::std::mem::size_of::<S>() , 33usize , concat ! (
"Size of: " , stringify ! ( S ) ));
assert_eq! (::std::mem::align_of::<S>() , 1usize , concat ! (
"Alignment of " , stringify ! ( S ) ));
assert_eq! (unsafe {
& ( * ( 0 as * const S ) ) . large_array as * const _ as usize
} , 0usize , concat ! (
"Alignment of field: " , stringify ! ( S ) , "::" , stringify
! ( large_array ) ));
}
impl Clone for S {
fn clone(&self) -> Self { *self }
}
impl Default for S {
fn default() -> Self { unsafe { ::std::mem::zeroed() } }
}
#[repr(C)]
pub struct ST<T> {
pub large_array: [T; 33usize],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, so why did this suddenly start working? There have been multiple refactors of the derive debug code, @fitzgen, do you know which one could cause this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@emilio I'm unsure what "this" that suddenly started working is? We aren't deriving Debug here, which it seems like you're implying, so I am confused.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant that the length check was effectively fixing that, so it eventually became unneeded, and I wondered if you knew off-hand why :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my memory, deriving Copy was working before bindgen gained the ability to derive Debug. It may as well just be that the implementor treat all derive the same, while not knowing Copy is a special trait to the compiler in regard to large array derive. Should be able to verify this aginst the commit history, but a bit tedious...

pub _phantom_0: ::std::marker::PhantomData<::std::cell::UnsafeCell<T>>,
}
impl <T> Default for ST<T> {
fn default() -> Self { unsafe { ::std::mem::zeroed() } }
}
7 changes: 7 additions & 0 deletions tests/headers/struct_with_large_array.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
struct S {
char large_array[33];
};

template<typename T> struct ST {
T large_array[33];
};