rust-lang / rust-bindgen

Automatically generates Rust FFI bindings to C (and some C++) libraries.
https://rust-lang.github.io/rust-bindgen/
BSD 3-Clause "New" or "Revised" License
4.38k stars 689 forks source link

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

Open CGMossa opened 10 months ago

CGMossa commented 10 months ago

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 commented 10 months ago

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

Lokathor commented 10 months ago

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.

CGMossa commented 10 months ago

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?

Lokathor commented 10 months ago

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.

HadrienG2 commented 7 months ago

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