Conversation
There was a problem hiding this comment.
Pull request overview
Adds initial ReScript bindings for Bun’s bun:ffi module (FFI^2) and introduces accompanying FFI-focused tests plus small C fixtures to validate dlopen, typed pointers, and cc compilation.
Changes:
- Add
src/FFI.resbindings forbun:ffi(dlopen/linkSymbols, pointers, CString/JSCallback/CFunction,cc, viewSource, read helpers). - Add a new FFI test suite and small C sources used by the tests.
- Add a precompiled
test/ffi_testlib.soartifact fordlopen-based tests (plusbun.lock).
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/ffi_testlib.so | Adds a precompiled shared library used for dlopen tests. |
| test/ffi_testlib.c | C fixture implementing simple functions (add/multiply/etc.) for FFI tests. |
| test/ffi_square.c | C fixture for validating CC.compile from a file. |
| test/FFITest.res | ReScript FFI test suite exercising suffix/dlopen/cstring/pointers/read/cc. |
| test/FFITest.js | Generated JS output for the ReScript FFI test suite. |
| src/FFI.res | New bun:ffi bindings for ReScript, including Read, CString, JSCallback, and CC. |
| src/FFI.js | Generated JS output for the new FFI bindings module. |
| bun.lock | Adds a Bun lockfile snapshot. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| external i64: (pointer, ~byteOffset: int=?) => bigint = "i64" | ||
|
|
||
| @module("bun:ffi") @scope("read") | ||
| external ptr: (pointer, ~byteOffset: int=?) => int = "ptr" |
There was a problem hiding this comment.
Read.ptr currently returns int, which makes it impossible (at the type level) to pass the result into other APIs that expect pointer (e.g. toBuffer, CString.make, etc.) without unsafe casting. Since this accessor is for reading a pointer value from memory, it should likely return pointer to keep the API composable.
| external ptr: (pointer, ~byteOffset: int=?) => int = "ptr" | |
| external ptr: (pointer, ~byteOffset: int=?) => pointer = "ptr" |
| open Test | ||
| open FFI | ||
|
|
||
| let libPath = Globals.import.meta.dir ++ "/ffi_testlib.so" |
There was a problem hiding this comment.
libPath is hard-coded to a .so file, which will fail on macOS/Windows (and is inconsistent with the suffix API being tested below). Consider building the filename using suffix (e.g. ffi_testlib.${suffix}) or switching this test to compile the C source via CC.compile instead of checking in/loading a platform-specific binary.
| let libPath = Globals.import.meta.dir ++ "/ffi_testlib.so" | |
| let libPath = Globals.import.meta.dir ++ "/ffi_testlib." ++ suffix |
|
@fabrv I think the remarks are on point, can you take a look. |
Bun's C ABI FFI bindings (FFI^2)