rust-bindgen icon indicating copy to clipboard operation
rust-bindgen copied to clipboard

Opaque types don't include `!Send`, `!Sync`, `!Pin`

Open CGMossa opened this issue 1 year ago • 5 comments

We have a pointer type to an opaque type in our FFI project.

Bindgen generates this definition for the opaque type:

#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct SEXPREC {
    _unused: [u8; 0],
}

But then I was reading https://stackoverflow.com/questions/38315383/whats-the-rust-idiom-to-define-a-field-pointing-to-a-c-opaque-pointer, and https://doc.rust-lang.org/nomicon/ffi.html#representing-opaque-structs, which says that the definition of the above should really be:

#[repr(C)]
pub struct SEXPREC {
    _data: [u8; 0],
    _marker:
        core::marker::PhantomData<(*mut u8, core::marker::PhantomPinned)>,
}

Is this something I should replace the bindgen-version with this?

Lastly, I merely have these in the C-headers:

typedef struct SEXPREC s_object;

with no definition of s_object, and I'm quite certain this is intended behavior.

CGMossa avatar Nov 16 '23 13:11 CGMossa

See https://github.com/rust-lang/nomicon/issues/250

CGMossa avatar Nov 18 '23 12:11 CGMossa

Hello. I was asked to look at this issue but I don't have any wider context of your specific project.

That said, I'd advise the following:

/// S-expression data that is opaque to Rust
#[repr(transparent)]
pub struct SEXPREC(c_void);

// if you want this alias too
pub type s_object = SEXPREC;

I'm genuinely not sure why the Nomicon is concerned with the value itself being Send/Sync or not because pointers to it will naturally be !Send and !Sync. The c_void field will also correctly make the type Unpin.

Lokathor avatar Nov 18 '23 15:11 Lokathor

Thanks! My use case requires pub type SEXP = *mut SEXPREC;.

So

/// S-expression data that is opaque to Rust
#[repr(transparent)]
pub struct SEXPREC(c_void);

// if you want this alias too
pub type SEXP = *mut SEXPREC;

Is it possible to make bindgen produce this pattern?

CGMossa avatar Nov 18 '23 15:11 CGMossa

I haven't used bindgen in years, I just make bindings by hand generally. You would probably have to file an issue to make them support this.

Lokathor avatar Nov 18 '23 15:11 Lokathor

I agree that the current codegen for opaque types is unnecessarily dangerous and should be changed to follow the current nomicon guidelines.

HadrienG2 avatar Feb 12 '24 08:02 HadrienG2