fix(cli): warn when meld fuse uses the multi-memory default (#172)#177
Merged
Conversation
`meld fuse` defaults to `--memory multi`, which produces a core module with one linear memory per input component. The canonical optimise → AOT chain then rejects it: `wasm-opt` needs `--enable-multimemory`, and `synth --cortex-m` has no single flat address space for multiple memories. A user on the happy path (`meld fuse a b -o fused.wasm`) hit that wall silently at the very next tool. This is #172 options 2 + 3 (warn + document): - `meld fuse` now prints a warning whenever `--memory multi` is in effect — including the default — naming the downstream implication and pointing at `--memory shared --address-rebase`, the combination that flows through `wasm-opt` → `synth` cleanly. - The `--memory` `--help` text documents the same. #172 option 1 (flip the default to `shared`) is deliberately NOT done here. Flipping the core memory default is high-blast-radius, and `shared` is not an unambiguously safe default: it is currently labelled "legacy mode" and documented as broken when any component uses `memory.grow`. The default flip is a separate decision and is left to the issue owner; this PR makes the current default non-surprising in the meantime. Tests: `test_cli_memory_default_is_multi` pins the default so a future flip is a deliberate edit; `test_cli_memory_shared_parses`. 7/7 meld-cli tests pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
LS-N verification gate✅ 19/19 approved LS entries verified
Approved Failed LS entries(none) Missing regression tests(none) Updated automatically by |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Addresses #172 —
meld fusedefaults to--memory multi, producing a fused module the canonical optimise → AOT chain can't consume.A user running the happy path (
meld fuse a b -o fused.wasm) gets a multi-memory core module.wasm-optthen rejects it without--enable-multimemory, andsynth --cortex-mhas no single flat address space for multiple memories. The wall is hit silently at the next tool.What this PR does (#172 options 2 + 3)
meld fuseprints a warning whenever--memory multiis in effect (including the default), naming the downstream implication and pointing at--memory shared --address-rebase.--memory--helptext documents the same.Why not option 1 (flip the default)
#172's recommended option 1 is to default to
--memory shared. That's deliberately not done here:meld fuseinvocation.sharedis not an unambiguously safe default:meld-coredocuments it as broken when any component usesmemory.grow("one component's growth corrupts every other component's address space"), and the CLI currently labels it "legacy mode".shared --address-rebase— two flags, not just a default flip.So the default flip is a separate decision for the issue owner. This PR makes the current default non-surprising in the meantime — the lowest-risk of #172's three suggested fixes, shippable now.
Tests
test_cli_memory_default_is_multi— pins the default so a future flip is a deliberate edittest_cli_memory_shared_parses🤖 Generated with Claude Code