-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Document the correct FFI equivalent to a C opaque struct #27303
Comments
It looks like the canonical answer here is a But @crumblingstatue wants to newtype this for type safety, and it isn't clear whether newtyping this structure ( A few places in the rust repo, some external docs, and some projects (including Servo) use I can see few options here:
Whatever the answer, it should be documented in the book's FFI section as well as in the doc comment on |
Note that the only reason I personally would not recommend |
The reason we don't accept |
This is exactly what I've done in my projects, although I haven't tried with the newly rewritten improper-ctypes lint. |
Servo uses |
@Ms2ger Doesn't the ffi-safe lint trigger on that too? |
@bluss raw pointers to Sized types are always allowed in FFI signatures, AFAIK. |
A consequence of using enums for opaque structs is that the generated documentation will move from the Structs section to the Enums section. This makes the documentation confusing and misleading especially when there are also "normal" structs and enums. |
Wouldn't |
Oh yeah, I guess given the advice above that It might be nice to have |
Maybe not Void, but an empty enum named Opaque or something like that. There is a void crate with a Void type that implements various traits already. We don't want such a thing. |
What about this to leave it named a "struct" in rustdoc:
or perhaps
Note that I'm avoiding |
Nah, this isn't a better idea, as this type is actually constructible (although it takes a smidge more work). |
See rust-lang/rust#27303 and rust-lang/libc#48 for discussion of the optimization, or just try compiling `unsafe {free(malloc(16))}` with various Rust definitions of `void *` in the FFI binding.
See rust-lang/rust#27303 and rust-lang/libc#48 for discussion of the optimization, or just try compiling `unsafe {free(malloc(16))}` with various Rust definitions of `void *` in the FFI binding.
Fix core-text related warnings (#62) ### Fix private-in-public warnings (error E0446) Fixed by prepending `pub` to the offending types. Not sure if its the best way but it was the only thing that worked. Other methods I've tried to maintain privateness: * Replace with an empty enum. That threw an error. * Make it public and wrap within a private module. That threw an error too as `#[repr(C)]` does not accept modules ### Fix use of extern static warnings by wrapping in unsafe block (error E0133) I think this is self-explanatory. ### Fix zero-size struct warnings on certain CT* types Wrap around c_void as I believe the types are only used as pointers. Again, let me know if there is a better way to do this. I've tried following [this discussion](rust-lang/rust#27303) on opaque C types but I don't think there is a consensus. I came across this [discussion](Rust-SDL2/rust-sdl2#442) from the Rust SDL2 bindings and they eventually [opted](Rust-SDL2/rust-sdl2#445) for the solution that I implemented. This is my first pull request (hence, first open source contribution) and I've been playing with Rust for the past few weeks. So please, if anything isnt up to standards or correctness, let me know! Also, to squash the other warnings, those changes would have to made in the modules where those types reside (core-graphics and core-foundation). I would gladly open a PR to fix those too if all looks good. Thanks! <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/core-text-rs/65) <!-- Reviewable:end -->
This follows the servo pattern mentioned in rust-lang/rust#27303 to replace the zero-sized structs with an `enum`.
This follows the servo pattern mentioned in rust-lang/rust#27303 to replace the zero-sized structs with an `enum`.
With recent changes to the
improper_ctypes
lint, it now rejects#[repr(C)] struct Foo;
for representing a C opaque struct.On the #rust IRC channel, I have asked what is the proper way to represent a C opaque struct, but no definitive answer was reached.
Some of the answers were:
struct Foo;
is incorrect.struct Foo;
is fine, but it can break certain LLVM optimizations.c_void
instead. This isn't satisfactory when it is desirable to represent a C opaque struct as a distinct type in your API.c_void
. Someone said this might not be correct either.#repr(C) enum Foo {}
. It doesn't currently allow this.This should be properly researched and then documented in the official documentation, so users know what to use for representing C opaque struct types in Rust when interfacing with C code.
Some relevant issues:
The text was updated successfully, but these errors were encountered: