Conversation
📝 WalkthroughWalkthroughThis PR extends the wasm builtin to accept WebAssembly modules in text (WAT) format alongside binary paths. The Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Evaluator as Nix Evaluator
participant PrimWasm as prim_wasm Handler
participant WAT as WAT to WASM
participant Compiler as WASM Compiler
participant Instance as NixWasmInstancePre
User->>Evaluator: builtins.wasm { wat = "...", function = "fib" }
Evaluator->>PrimWasm: Process builtin call
PrimWasm->>PrimWasm: Detect 'wat' attribute
PrimWasm->>WAT: Convert WAT text to bytes
WAT-->>PrimWasm: WASM bytecode
PrimWasm->>Compiler: compile(bytes)
Compiler->>Instance: Create NixWasmInstancePre(name)
Instance-->>Compiler: Compiled module
Compiler-->>PrimWasm: Ready instance
PrimWasm->>Instance: Instantiate & invoke function
Instance-->>PrimWasm: Function result
PrimWasm-->>User: Return value
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/libexpr/primops/wasm.cc (2)
704-710: Document the non-WASI ABI alongside the new WAT example.The example makes
watplusfunctionlook sufficient, but hand-written non-WASI modules also need the exportedmemoryandnix_wasm_init_v1hook. Calling that out here would save users from a confusing first failure.Also applies to: 723-729
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/libexpr/primops/wasm.cc` around lines 704 - 710, Update the doc text in src/libexpr/primops/wasm.cc around the WAT example to explicitly document the non-WASI ABI requirements: state that when using `wat` + `function` for non-WASI modules you must also export a linear `memory` and provide the `nix_wasm_init_v1` initialization hook (and that `function` is not allowed for WASI modules); add the same clarification to the other block around lines 723-729 so both examples mention the exported `memory` and `nix_wasm_init_v1` hook alongside `wat`/`function` and the mutual-exclusion of `path`/`wat`.
693-694: Keep the selected module source in the outer trace.Malformed
wator invalidpathfailures still get wrapped as just"while executing a Wasm module". Including the chosen source here (basename(path)or<inline wat>) would make the new compile-time failure path much easier to diagnose.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/libexpr/primops/wasm.cc` around lines 693 - 694, The trace added before rethrowing in wasm.cc currently uses e.addTrace(state.positions[pos], "while executing a Wasm module"); change that to include the actual selected module source (e.g. basename(path) when a file was used or "<inline wat>" when inline WAT was provided) so the outer trace shows which module was being compiled/executed; modify the code around e.addTrace/state.positions[pos] to build a descriptive message like "while executing Wasm module: <module-source>" using the existing path/wat variables and then call e.addTrace with that message before throw.tests/functional/wasm.sh (1)
10-17: Use a smaller input for this smoke test.
tests/functional/fib.watis the naive recursive algorithm, so40makes this check exponentially expensive and you run it twice. A much smaller value still exercises WAT parsing and Wasm/Nix interop without stretching suite time on slower builders.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/functional/wasm.sh` around lines 10 - 17, The test uses the naive recursive fib implementation with input 40 twice (once for WAT via builtins.wasm { wat = readFile ./fib.wat; function = "fib"; } and once for binary via path = ./fib.wasm), which is expensive; change the argument from 40 to a much smaller number (e.g., 10 or 15) in both occurrences so the WAT parsing and Wasm/Nix interop are exercised without long runtime while keeping the expected result consistent for function "fib".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/libexpr/primops/wasm.cc`:
- Around line 704-710: Update the doc text in src/libexpr/primops/wasm.cc around
the WAT example to explicitly document the non-WASI ABI requirements: state that
when using `wat` + `function` for non-WASI modules you must also export a linear
`memory` and provide the `nix_wasm_init_v1` initialization hook (and that
`function` is not allowed for WASI modules); add the same clarification to the
other block around lines 723-729 so both examples mention the exported `memory`
and `nix_wasm_init_v1` hook alongside `wat`/`function` and the mutual-exclusion
of `path`/`wat`.
- Around line 693-694: The trace added before rethrowing in wasm.cc currently
uses e.addTrace(state.positions[pos], "while executing a Wasm module"); change
that to include the actual selected module source (e.g. basename(path) when a
file was used or "<inline wat>" when inline WAT was provided) so the outer trace
shows which module was being compiled/executed; modify the code around
e.addTrace/state.positions[pos] to build a descriptive message like "while
executing Wasm module: <module-source>" using the existing path/wat variables
and then call e.addTrace with that message before throw.
In `@tests/functional/wasm.sh`:
- Around line 10-17: The test uses the naive recursive fib implementation with
input 40 twice (once for WAT via builtins.wasm { wat = readFile ./fib.wat;
function = "fib"; } and once for binary via path = ./fib.wasm), which is
expensive; change the argument from 40 to a much smaller number (e.g., 10 or 15)
in both occurrences so the WAT parsing and Wasm/Nix interop are exercised
without long runtime while keeping the expected result consistent for function
"fib".
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ed2126a1-78bf-48e3-9df1-b2edda35ad98
⛔ Files ignored due to path filters (1)
tests/functional/fib.wasmis excluded by!**/*.wasm
📒 Files selected for processing (4)
src/libexpr/primops/wasm.cctests/functional/fib.wattests/functional/meson.buildtests/functional/wasm.sh
Motivation
This adds support for executing Wasm modules in textual format, which is mostly useful for testing.
Context
Summary by CodeRabbit
builtins.wasm, enabling inline module definitions alongside existing binary module support.pathorwatattributes (mutually exclusive) for flexible module input methods.