-
-
Notifications
You must be signed in to change notification settings - Fork 14
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
Feat/stone ffi #364
base: main
Are you sure you want to change the base?
Feat/stone ffi #364
Conversation
8fa216d
to
f20be1a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mildly terrifying =) Was it not possible to autogenerate the header from the extern "C" functions..? My only nitpicks atm are missing docs/SPDX headers. I assume the renames are required for FFI exports, and unavoidably introduce the verbosity / far reach of changes?
The header is being auto generated by cbindgen, checkout build.rs!
Lol yeah cbindgen operates on type names only, not fully qualified paths, so any "duplicate" name, even from different modules, collide and it'll only emit one of them. So I had to switch the entire crate to fully qualified type syntax :/
👍 Yup we need 100% documentation and outlining safety & lifetimes will be important. Any necessary allocations happen on the rust side. The consumer needs to copy out any StoneStrings we allocate in Rust to C before destroying the reader and Rust drops it all. Since C is devoid of lifetimes, le documentation must be solid. |
This is required to properly use cbindgen for the future C FFI bindings. cbindgen completely breaks when struct names collide since it doesn't consider fully qualified paths and there's no apparent workaround.
No description provided.