feat(bindings-cpp-ffi): add Rust FFI crate for WASM modules#4773
feat(bindings-cpp-ffi): add Rust FFI crate for WASM modules#4773euxaristia wants to merge 2 commits intoclockworklabs:masterfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb1e667316
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
bb1e667 to
22e9c5d
Compare
Rewrite the core C++ type registration and FFI dispatch layer from crates/bindings-cpp in Rust. The new crate provides: - ModuleTypeRegistration: full SATS type registration with circular reference detection, error module generation, and BSATN serialization - FFI dispatch: __describe_module__, __call_reducer__, __call_view__, __call_view_anon__, __call_procedure__ with proper Identity/ConnectionId reconstruction and BytesSource/BytesSink I/O - Preinit functions: __preinit__01_clear_global_state and __preinit__99_validate_types matching the C++ originals Replaces ~1,414 lines of C++ with ~1,139 lines of Rust (19% reduction) plus 96 unit tests (100% coverage). Clippy-clean and fmt-clean.
22e9c5d to
c07421e
Compare
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Hi @euxaristia, what's the goal of this PR? Why is this version preferable to what's there? Is it intended as a replacement? |
- Store (data, cursor) instead of raw Vec<u8> - bytes_source_read: copy from cursor position, advance cursor - bytes_source_read: return 0 while data remains, -1 when exhausted - bytes_source_read: always write *len (0 when exhausted) to avoid reading uninitialized buffer capacity as valid bytes - bytes_source_remaining_length: report remaining bytes, not total
|
@bfops rewrites the C++ binding layer in Rust. The C++ side has integration tests (type-isolation, client-comparison) but zero unit tests on the internal logic, so edge cases/errors aren't covered. This adds 96 unit tests at 100% coverage, uses shared Rust types directly, and keeps everything in one toolchain. Yes, intended as a replacement, but this PR only adds the crate — nothing removed. Follow-up would swap it in if the team's on board. |
Summary
Add
crates/bindings-cpp-ffi, a new Rust crate providing type registration and FFIdispatch for SpacetimeDB WASM modules. It re-implements the logic from
crates/bindings-cpp(specificallymodule_type_registration.cpp,module_exports.cpp, andModule.cpp) in Rust.Motivation: The existing C++ bindings have no internal unit tests — all
existing tests are integration-level (WASM compile → publish to server). This
makes it impossible to test type registration edge cases, error paths, and FFI
dispatch in isolation, and means iteration requires a full Emscripten +
SpacetimeDB server cycle. A Rust implementation gives us:
dispatch pipeline
spacetimedb-lib,spacetimedb-sats, andspacetimedb-primitives(no hand-rolled mirrors of Rust types in C++)Replacement intent: Yes. If the team is happy with this implementation, a
follow-up PR would remove
crates/bindings-cppand wire the SDK macros to thiscrate instead. This PR only adds the crate — nothing is removed.
What's included
Type Registration (
module_type_registration.rs)options → results → ScheduleAt → user-defined structs/enums
constraint errors, type registration errors) — matching the C++
__preinit__99_validate_typesbehaviorRawModuleDefV10FFI Dispatch (
ffi.rs)__describe_module__— serializes module definition to host__call_reducer__— validates ID, reads BSATN args, dispatches to registeredhandler, writes errors to sink
__call_view__/__call_view_anon__— reconstructs sender identity, readsargs, dispatches, prepends
ViewResultHeader__call_procedure__— full dispatch with sender/connection reconstruction__preinit__01_clear_global_state/__preinit__99_validate_types— WASMpreinit exports
bytes_sink_write/bytes_source_read/bytes_source_remaining_length—FFI I/O with partial read handling
Public API
register_reducer,register_view,register_view_anon,register_procedure— handler registration returning dispatch indicesregister_type— type registration with the typespacehas_registration_error/registration_error— error introspectionSize comparison
Existing C++ test infrastructure
The C++ bindings do have integration-level tests:
tests/type-isolation-test/): 60 testmodules that validate WASM compilation and publish to a running server
tests/client-comparison/): validates thatRust and C++ bindings produce equivalent generated SDK clients and schemas
crates/smoketests/):test_quickstart_cppexercises thefull quickstart flow with a C++ server module
However, there are no unit tests covering the type registration or FFI
dispatch internals. The CTest target (
test_bsatn) inCMakeLists.txtreferences source files that don't exist on disk.
Verification
Notes
memberslist; builds cleanly alongside existingC++ bindings