Skip to content

feat: add generate-binding command#9

Merged
twoeths merged 4 commits intomainfrom
cayman/generate-binding
Jun 16, 2025
Merged

feat: add generate-binding command#9
twoeths merged 4 commits intomainfrom
cayman/generate-binding

Conversation

@wemeetagain
Copy link
Copy Markdown
Member

  • Add command to generate ts binding code
    • It will scan zig source code for exported functions, parsing function signatures into ffi function definitions
    • ffi function definitions can be overridden with a comment before the exported function like so:
      // bun-ffi-z: functionName (arg1, arg2) return
    • the generated binding is written to ${bunCwd}/src/binding.ts

@twoeths
Copy link
Copy Markdown
Contributor

twoeths commented May 30, 2025

this is amazing @wemeetagain , I wanna test this on state-transition-z and blst-z before we merge this

@twoeths
Copy link
Copy Markdown
Contributor

twoeths commented Jun 4, 2025

got this error error: Unsupported Zig type: committee_indices.ByteCount

it comes from this function declaration

const committee_indices = @import("committee_indices.zig");
export fn computeProposerIndex(seed: [*c]u8, seed_len: usize, active_indices: [*c]u32, active_indices_len: usize, effective_balance_increments: [*c]u16, effective_balance_increments_len: usize, rand_byte_count: committee_indices.ByteCount, max_effective_balance: u64, effective_balance_increment: u32, rounds: u32) u32 {

inside committee_indices.zig

pub const ByteCount = enum(u8) {
    One = 1,
    Two = 2,
};

I guess it's too much to try to support this example. Instead may be just warn, and generate "unknown" instead, and notice users, wdyt @wemeetagain ?

@wemeetagain
Copy link
Copy Markdown
Member Author

The bun ffi library definition doesn't understand "unknown", and so the output of generate-binding would require additional user input after-the-fact.

Because this isn't something that can be enforced directly at the ffi boundary, even if we were able to support generating the ffi (because enum types aren't directly supported by ffi), I don't think we should support this, and we should require all exported functions to use simple types and not use type aliases.

One alternative would be to have zig generate a header file (.h) which includes all exported symbols. Unfortunately, this is currently broken on the zig side.

Another alternative would be to do the binding generation on the zig side. Unfortunately, this complicates consumption of this library, (imo a lot) as now consumers must install an additional zig dependency, add a generate-binding.zig file, and also add this as an executable to their build.zig. And unless everything is named to some specific standard, this can't be controlled easily by the bun side, eg: for building things in CI. Something like:

zig fetch https://github.com/chainsafe/bun-ffi-z
// build.zig

// add an executable and link everything
```zig
// build.zig

...
const options_module = b.addOptionsModule(... // set the bun directory here
const bun_ffi_z = b.dependency(...
const generate_binding = b.addExecutable(..., // link bun-ffi-z and the bun directory configuration
...
// User has defined the library in this file
const lib = @import("./c_abi_lib.zig");
// User has defined an options module to help configure the "bun directory"
const bun_dir = @import("build_options").bun_dir;
// User has defined and linked the bun-ffi-z zig dependency
const bun_ffi_z = @import("bun_ffi_z");

pub fn main() !void {
    bun_ffi_z.generateBinding(lib, bun_dir);
}

@twoeths
Copy link
Copy Markdown
Contributor

twoeths commented Jun 5, 2025

Because this isn't something that can be enforced directly at the ffi boundary, even if we were able to support generating the ffi (because enum types aren't directly supported by ffi), I don't think we should support this, and we should require all exported functions to use simple types and not use type aliases.

my first through was zig C-ABI functions can use whatever types they want
but given this whole C-ABI functions are in the end for Bun which resolve to basic types only, I agree to go with simple types there

@twoeths
Copy link
Copy Markdown
Contributor

twoeths commented Jun 5, 2025

it works fine with state-transition-z however it's more complex in blst-z, still working on that

@twoeths
Copy link
Copy Markdown
Contributor

twoeths commented Jun 16, 2025

works fine with blst-z too

@twoeths twoeths merged commit f210c56 into main Jun 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants