Rust-bindgen: __IncompleteArrayField shouldn't implement `Clone` / `Copy`.

[Stargateur]

struct fam {
    size_t len;
    int data[];
};

    let bindings = bindgen::Builder::default()
        .header("fam.h")
        .generate()
        .expect("Unable to generate bindings");

#[repr(C)]
#[derive(Default)]
pub struct __IncompleteArrayField<T>(::std::marker::PhantomData<T>);
impl<T> __IncompleteArrayField<T> {
    #[inline]
    pub fn new() -> Self {
        __IncompleteArrayField(::std::marker::PhantomData)
    }
    #[inline]
    pub unsafe fn as_ptr(&self) -> *const T {
        ::std::mem::transmute(self)
    }
    #[inline]
    pub unsafe fn as_mut_ptr(&mut self) -> *mut T {
        ::std::mem::transmute(self)
    }
    #[inline]
    pub unsafe fn as_slice(&self, len: usize) -> &[T] {
        ::std::slice::from_raw_parts(self.as_ptr(), len)
    }
    #[inline]
    pub unsafe fn as_mut_slice(&mut self, len: usize) -> &mut [T] {
        ::std::slice::from_raw_parts_mut(self.as_mut_ptr(), len)
    }
}
impl<T> ::std::fmt::Debug for __IncompleteArrayField<T> {
    fn fmt(&self, fmt: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result {
        fmt.write_str("__IncompleteArrayField")
    }
}
impl<T> ::std::clone::Clone for __IncompleteArrayField<T> {
    #[inline]
    fn clone(&self) -> Self {
        Self::new()
    }
}
impl<T> ::std::marker::Copy for __IncompleteArrayField<T> {}

#[repr(C)]
#[derive(Debug)]
pub struct fam {
    pub len: usize,
    pub data: __IncompleteArrayField<::std::os::raw::c_int>,
}

I wonder if it's not a bug or something because you can't copy a FAM like that. This is not a pointer. As I expected this produce complete undefined behavior when you use Copy or Clone trait.

#[no_mangle]
pub fn print_data(fam: &fam) {
    let data = fam.data.clone();
    let data = unsafe { data.as_slice(fam.len) };
    for x in data {
        println!("{:?}", x);
    }
}

Fail completely. Or I missing something ?

[emilio]

Well, it's using the same semantics as C, bindgen doesn't know about whether it's a variable length array, or just a separator so bitfields pack differently, or something else. The following C code compiles also without problems:

struct fam {
    unsigned len;
    int data[];
};

void use(const struct fam* unused) {};
void copy(const struct fam* fam)
{
  struct fam cp = *fam;
  use(&cp);
}

In general bindgen tries not to make assumptions about your C code.

[emilio]

More to the point, there's nothing bindgen can do about this particular case if you move your array, which will effectively not copy the extra space. And there's no way to make a type immovable itself, though I believe in newer rust versions you can use Pin.

So even if we didn't derive Copy or Clone, you could always move the value out and it'd be unsound anyway.

[Stargateur]

@emilio I think you totally miss the point, struct fam cp = *fam; copy only the part of the struct that is not a FAM, that not what you introduce by implementing copy and clone on the FAM, you are doing something that don't make sense in C, you can't copy an FAM, that totally different of struct fam cp = *fam; you are doing something like that magic cp = fam.data, that simply don't make sense and will not compile on any C implementation.

[emilio]

Oh, I see what you mean. Yeah, I agree copying just the data member doesn't really make sense. And apparently we are already smart enough to prevent deriving Copy / Clone on the structs that contain it.

I agree we should probably fix that. It's trivial, assuming nothing depends on it.

[highfive]

Hi! If you have any questions regarding this issue, feel free to make a comment here, or ask it in the #servo channel in IRC.

If you intend to work on this issue, then add @highfive: assign me to your comment, and I'll assign this to you. :smile:

[emilio]

Sorry for the initial misunderstanding @Stargateur, I totally misread your example :)

[honggoff]

I'd like to take this issue to get familiar with the project.
@highfive: assign me please.

[highfive]

Hey @honggoff! Thanks for your interest in working on this issue. It's now assigned to you!

[honggoff]

It looks like the implementation for Copy has already been removed with #1477. I will remove the remaining implementation for Clone.