compiler: string-wall slice 8a — guard string ++ against silent miscompile (#575)

## Phase F slice 8a — guard string `++` against silent miscompile

The "guard first" half of the owner-approved string-concat plan
(`proposals/DESIGN-string-concat.adoc`, merged #574). Depends on: #574
(design, merged).

### The bug this guards

String `++` **silently miscompiles** to wasm: it typechecks (`++` is
type-dispatched in `typecheck.ml`), the interpreter is correct, but
codegen lowers `++` only as *list* concat (4-byte element stride).
Applied to a string's `[len][utf8]` layout it copies the length word +
bytes as i32 elements — `"ab" ++ "cd"` yields byte 2 = `2` (the length
word of `"cd"`) instead of `'c'`. A #555-class silent mis-lowering.

The "add a `string_concat` builtin / ban `++`" route is **ruled out**
(`++` is the canonical, stdlib-pervasive surface; `string_concat` was
deliberately removed in #330), so `++` must be made to lower correctly —
that's **slice 8b** (type-directed lowering). This PR is the interim
**safety guard**.

### This PR (8a)

The wasm `OpConcat` handler now rejects the syntactically-obvious string
`++` with a loud `UnsupportedFeature` error instead of emitting garbage.
`is_string_concat_operand` is a **sound-rejection** heuristic:

- **Fires** only on operands that are unambiguously strings — a string
literal, a String-returning builtin call (`int_to_string`, `trim`,
`to_lowercase`, …), or a nested string `++`.
- **Never** fires on list `++` (arrays / array literals / bare
variables), so genuine list concatenation is unaffected.
- Deliberately **incomplete**: a pure variable-to-variable string `++`
still slips through — closing that gap needs the type channel and is
**slice 8b**. The footgun is currently unreachable in practice (no
migrated kernel compiles string `++` to wasm), so the partial guard is a
safe interim.

### Tests / verification

- `test/test_e2e.ml` group **"E2E String-wall slice 8 guard"** — string
`++` is rejected at wasm codegen (error names "concatenation"); list
`++` still compiles (no false positive).
- Full `run_codegen_wasm_tests.sh` green, **including `list_concat`**
and all 7 string fixtures (slices 1–7 still compile). The list-concat
lowering is untouched.

`dune runtest` couldn't run in-sandbox (no `alcotest`), but the guard
logic + the `wasm_codegen`/`Parse_driver` test pattern were validated
against the real `affinescript` library (string `++` → rejected with
"concatenation"; list `++` → compiles).

### Next

**Slice 8b** — the type-directed string-`++` lowering
(`ExprStringConcat` elaboration + byte two-copy-loop) makes `++`
*correct and complete* (incl. var-var), replacing this guard. Plan in
`proposals/DESIGN-string-concat.adoc` §"Implementation plan".

https://claude.ai/code/session_01WoKhFQePiRsAj7aqnxbG8s

---
_Generated by [Claude
Code](https://claude.ai/code/session_01WoKhFQePiRsAj7aqnxbG8s)_

Co-authored-by: Claude <noreply@anthropic.com>

Author: hyperpolymath

lib/codegen.ml

@@ -354,6 +354,35 @@ let dedup (lst : string list) : string list =
     if List.mem x acc then acc else x :: acc
   ) [] lst |> List.rev
 
+(** PHASE-F string-wall slice 8 (guard half). Syntactic recogniser for a
+    string-typed operand of `++`. The wasm `OpConcat` handler lowers `++` as
+    *list* concatenation (4-byte element stride); applied to a string's
+    `[len][utf8]` layout it silently miscompiles (a source string's length
+    word leaks in as a payload byte — see proposals/DESIGN-string-concat.adoc).
+    Codegen is type-blind, so until the type-directed lowering lands (slice 8
+    full fix) this recogniser lets the handler reject the *obvious* string `++`
+    cases with a loud error instead of emitting garbage.
+
+    It is a *sound-rejection* heuristic: it only returns true for operands that
+    are unambiguously strings — a string literal, a call to a String-returning
+    builtin, or a nested string `++`. List `++` operands (arrays / array
+    literals / bare variables) are never matched, so genuine list
+    concatenation is unaffected. It is deliberately *incomplete*: a pure
+    variable-to-variable string `++` (e.g. `a ++ b` with both string-typed
+    vars) is not recognised syntactically and still slips through; closing that
+    gap needs the type channel and is the slice-8 full fix. *)
+let rec is_string_concat_operand (e : expr) : bool =
+  match e with
+  | ExprLit (LitString _) -> true
+  | ExprApp (ExprVar id, _) ->
+    (* String-returning builtins (Int/Char-returning ones are excluded). *)
+    List.mem id.name
+      [ "int_to_string"; "float_to_string"; "to_lowercase"; "to_uppercase";
+        "trim"; "string_sub"; "string_from_char_code"; "show" ]
+  | ExprBinary (a, OpConcat, b) ->
+    is_string_concat_operand a || is_string_concat_operand b
+  | _ -> false
+
 (** Generate code for a literal *)
 let gen_literal (ctx : context) (lit : literal) : (context * instr) result =
   match lit with
@@ -463,6 +492,22 @@ let rec gen_expr (ctx : context) (expr : expr) : (context * instr list) result =
         end
     end
 
+  | ExprBinary (left, OpConcat, right)
+    when is_string_concat_operand left || is_string_concat_operand right ->
+    (* PHASE-F string-wall slice 8 (guard half). String `++` reaching the
+       *list*-concat lowering below would silently miscompile (the source
+       string's `[len][utf8]` is copied as 4-byte elements — see
+       proposals/DESIGN-string-concat.adoc, where `"ab" ++ "cd"` yields byte
+       2 = 2 instead of 'c'). Codegen is type-blind, so we reject the
+       syntactically-obvious string `++` cases with a loud error rather than
+       emit garbage; the type-directed lowering is the slice-8 full fix. *)
+    let _ = (left, right) in
+    Error (UnsupportedFeature
+      "string `++` (concatenation) is not yet lowered to wasm. The list-concat \
+       lowering would silently miscompile a string's [len][utf8] bytes as i32 \
+       elements. See proposals/DESIGN-string-concat.adoc (string-wall slice 8); \
+       the type-directed string-concat lowering is the follow-up fix.")
+
   | ExprBinary (left, OpConcat, right) ->
     (* List concatenation `a ++ b`. `OpConcat` was a placeholder `I32Add`
        (it just summed the two list pointers), so every `++` produced a

proposals/DESIGN-string-concat.adoc

@@ -3,11 +3,32 @@
 = Design: lowering string `++` (and `slice`) to wasm
 :toc: macro
 
-*Status:* PROPOSAL — awaiting owner sign-off before implementation.
+*Status:* ACCEPTED (owner sign-off 2026-06-13, "guard first, then full fix").
+*Slice 8a (guard): LANDED.* Slice 8b (type-directed lowering): next.
 *Author:* string-wall slice-8 design (Phase F, `proposals/MIGRATION-PLAN.adoc`).
 *Scope:* the last remaining string-wall items — string concatenation (`++`)
 and polymorphic `slice` — which slices 1-7 deferred as "type-directed".
 
+[NOTE]
+====
+*Slice 8a — guard (LANDED).* Per the owner's "guard first" decision, the wasm
+`OpConcat` handler now *rejects the syntactically-obvious string `++`* with a
+loud `UnsupportedFeature` error (via `is_string_concat_operand` in
+`lib/codegen.ml`) instead of silently miscompiling it. This is a
+*sound-rejection* heuristic: it fires only on operands that are unambiguously
+strings (a string literal, a String-returning builtin call, or a nested string
+`++`), so genuine list `++` is never affected; it is deliberately *incomplete*
+— a pure variable-to-variable string `++` still slips through, and closing that
+gap is exactly what slice 8b (the type-directed lowering) does. The latent
+footgun is currently unreachable in practice (no migrated kernel compiles
+string `++` to wasm yet), so the partial guard is a safe interim that stops the
+common cases loudly while 8b is built.
+
+*Slice 8b — full fix (next).* The type-directed lowering of §"Implementation
+plan" makes string `++` *correct* (and complete, incl. variable-to-variable),
+at which point the guard is replaced by the real byte-concat lowering.
+====
+
 toc::[]
 
 == TL;DR

proposals/MIGRATION-PLAN.adoc

@@ -216,7 +216,8 @@ Heuristic:
 | F (string slice 5) | DONE | String-wall slice 5 — *whitespace trimming landed in the wasm backend* (2026-06-13, Opus). `trim` (already wired in resolve/typecheck/interp) gained a wasm lowering: two flat scans bracket the non-whitespace core (front for `lo`, back for `hi` exclusive; whitespace set `{32,9,10,12,13}` via an `is_ws` `I32Or` chain), then a slice-3-style byte copy of `[lo, hi)`. Matches `String.trim`: internal whitespace preserved, all-whitespace -> empty. *Gates:* G1 builds; G2 parity 7/7 (interp vs wasm: both-end/leading/trailing, all-ws, empty, internal-space preservation, tab+newline). Tests: `test/test_e2e.ml` group "E2E String-wall slice 5 (trim)" (interp, `dune runtest`) + `tests/codegen/string_trim.{affine,…mjs}` (executable wasm parity; full `run_codegen_wasm_tests.sh` green, no regressions). Evidence: `proposals/EVIDENCE-stringwall-slice5.adoc`. *The clean name-dispatched builtins (read/write/copy/transform/scan) are now done.* NEXT decision point — see F+ row.
 | F (string slice 6) | DONE | String-wall slice 6 — *substring search landed in the wasm backend + an interp bugfix* (2026-06-13, Opus). `string_find(haystack, needle)` (already wired in resolve/typecheck/interp) gained a wasm lowering: a *flat-safe* nested scan (inner AND-accumulates a match over `nlen` bytes with no early break; outer exits at top on `res != -1` or `i > hlen - nlen`; result update is a branchless `Select`) returning the first-occurrence index or -1, no allocation. *Also fixed a latent interp bug:* `string_find` called `String.get needle 0` before its `nlen = 0` guard, so an empty needle crashed the interpreter (uncaught `Invalid_argument`); the guard is reordered so an empty needle returns `0`, making both backends agree. *Gates:* G1 builds; G2 parity 8/8 (found/not-found/first-occurrence/needle-longer/empty-needle/empty-haystack/both-empty). Tests: `test/test_e2e.ml` group "E2E String-wall slice 6 (string_find)" (interp, incl. the formerly-crashing empty-needle case, `dune runtest`) + `tests/codegen/string_find.{affine,…mjs}` (executable wasm parity; full `run_codegen_wasm_tests.sh` green, no regressions). Evidence: `proposals/EVIDENCE-stringwall-slice6.adoc`. *All name-dispatched string builtins with a trap-free wasm analogue are now lowered.* NEXT: `int_to_string` (last clean op), then the gated concat/`slice` decision.
 | F (string slice 7) | DONE | String-wall slice 7 — *decimal rendering landed in the wasm backend* (2026-06-13, Opus). `int_to_string(n)` (already wired in resolve/typecheck/interp) gained a wasm lowering over the slice-3 allocation idiom: `neg = n<0`; `m = neg ? n : -n` (non-positive magnitude via `Select`); a do-while digit-count loop (so `n=0` yields one '0' with no special case); allocate `4 + dc + neg`; a do-while backward-write loop emitting `48 - (m%10)` per digit; leading `'-'` when negative. *INT_MIN-safe* — the un-representable positive magnitude is never formed; `div_s`/`rem_s` by 10 never trap. *Gates:* G1 builds; G2 parity over 0/single/multi-digit/negative/powers-of-ten/INT_MAX/INT_MIN (wasm string read back from memory vs `string_of_int`). Tests: `test/test_e2e.ml` new `eval_string_fn` + group "E2E String-wall slice 7 (int_to_string)" (5 full-string cases incl. INT_MIN, `dune runtest`) + `tests/codegen/string_int_to_string.{affine,…mjs}` (executable wasm parity; full `run_codegen_wasm_tests.sh` green, no regressions). Evidence: `proposals/EVIDENCE-stringwall-slice7.adoc`. *Clean lane CLOSED — every name-dispatched string builtin with a trap-free wasm analogue is lowered.* NEXT: the gated concat/`slice` decision, or the effect wall.
-| F+ | TODO (design decision) | The *clean* string slices are done (1-7). What remains: (a) *concat `++` / polymorphic `slice`* — **blocked on an architectural decision**: codegen has *no per-expression type access* (`ExprBinary` is untyped, no type env threaded in), so string `++` (vs the existing list `OpConcat`) and `slice` (array|string) need either typed-AST threading or a dedicated `string_concat` builtin (language-surface call). *This is the highest-value remaining string work and needs owner direction.* (b) *`float_to_string`* — stays host-side under the float-wall. (c) *`string_get`/`int_to_char`* — deferred (raise on out-of-range; no trap-free wasm analogue). Then the *effect-codegen wall* (module-state / `Date.now` / `Console.log`) — the migration's other compiler half. See `proposals/EVIDENCE-stringwall-slice7.adoc` §"what remains". *Cluster migration complete; string slices 1-7 landed (read + single-byte write + runtime-length copy + per-byte transform + scan-and-copy + substring search + decimal rendering).*
+| F (string slice 8a) | DONE | String-wall slice 8a — *string-`++` footgun GUARD landed in the wasm backend* (2026-06-13, Opus). *Finding:* string `++` silently miscompiles to wasm — it typechecks (`++` is type-dispatched at `typecheck.ml:1062`), the interp is correct, but codegen lowers `++` only as *list*-concat (4-byte element stride), so a string's `[len][utf8]` is copied as i32 elements (`"ab" ++ "cd"` -> byte 2 = 2, the length word of "cd", instead of 'c'). A #555-class silent mis-lowering. *Decision (owner, "guard first, then full fix"):* the "add a `string_concat` builtin / ban `++`" route is ruled out — `++` is the canonical, stdlib-pervasive surface and `string_concat` was deliberately removed (#330). *This slice:* the wasm `OpConcat` handler now rejects the syntactically-obvious string `++` (string literal / String-returning builtin call / nested string `++`, via `is_string_concat_operand`) with a loud `UnsupportedFeature` error instead of emitting garbage — a *sound-rejection* heuristic (never misfires on list `++`; deliberately incomplete on pure var-var string `++`, which slice 8b closes). *Gates:* G1 builds; guard verified (string `++` -> loud error; list `++` + slices 1-7 fixtures still compile; full `run_codegen_wasm_tests.sh` green incl. `list_concat`). Tests: `test/test_e2e.ml` group "E2E String-wall slice 8 guard". Design: `proposals/DESIGN-string-concat.adoc` (merged #574). NEXT: slice 8b — type-directed string-`++` lowering (`ExprStringConcat` elaboration + byte-concat), making `++` correct + complete.
+| F+ | TODO | Slice 8b — *type-directed string `++` lowering* (the full fix): elaborate string `++` into an internal byte-concat node (typecheck already decides string-vs-array per node) and lower via the two-copy-loop idiom; replaces the 8a guard; closes the var-var gap. Then: *`slice`*-on-string negative-index residual (mostly covered by the shipped `string_sub`); *`float_to_string`* stays host-side (float-wall); *`string_get`/`int_to_char`* deferred (out-of-range raise). Then the *effect-codegen wall* (module-state / `Date.now` / `Console.log`) — the migration's other compiler half. See `proposals/DESIGN-string-concat.adoc` §"Implementation plan". *String slices 1-7 + 8a-guard landed.*
 | Ω | TODO (access-gated) | Cutover + ReScript extinction.
 |===
 

test/test_e2e.ml

@@ -4030,6 +4030,47 @@ let stringwall_i2s_tests = [
   Alcotest.test_case "int_to_string(INT_MIN)" `Quick test_stringwall_i2s_intmin;
 ]
 
+(* ---- PHASE-F string-wall slice 8 (guard half): string `++` rejection ----
+
+   String `++` reaching the wasm backend's *list*-concat lowering silently
+   miscompiles (the source string's [len][utf8] bytes are copied as i32
+   elements; "ab" ++ "cd" yields byte 2 = 2 instead of 'c' — see
+   proposals/DESIGN-string-concat.adoc). Until the type-directed lowering
+   lands, codegen rejects the syntactically-obvious string `++` with a loud
+   error. These pin that the guard fires for string `++` and does NOT fire for
+   list `++` (no false positive). *)
+
+let string_contains s sub =
+  let n = String.length s and m = String.length sub in
+  let rec go i = i + m <= n && (String.sub s i m = sub || go (i + 1)) in
+  go 0
+
+let test_stringwall_concat_guard_rejects_string () =
+  let prog = Parse_driver.parse_string ~file:"<stringwall>"
+    "fn f() -> Int { string_char_code_at(\"ab\" ++ \"cd\", 0) }" in
+  match wasm_codegen prog with
+  | Error msg when string_contains msg "concatenation" -> ()
+  | Error msg -> Alcotest.failf "expected the slice-8 string-++ guard, got: %s" msg
+  | Ok _ ->
+    Alcotest.fail
+      "string `++` must be rejected by the slice-8 guard (the list-concat \
+       lowering would silently miscompile it), not compiled to wasm"
+
+let test_stringwall_concat_guard_allows_list () =
+  (* The guard must not misfire on genuine list `++`. *)
+  let prog = Parse_driver.parse_string ~file:"<stringwall>"
+    "fn f() -> Int { let xs = [1, 2] ++ [3]; 0 }" in
+  match wasm_codegen prog with
+  | Ok _ -> ()
+  | Error msg -> Alcotest.failf "list `++` must still compile (guard false positive): %s" msg
+
+let stringwall_concat_guard_tests = [
+  Alcotest.test_case "string `++` is rejected (not silently miscompiled)"
+    `Quick test_stringwall_concat_guard_rejects_string;
+  Alcotest.test_case "list `++` still compiles (no guard false positive)"
+    `Quick test_stringwall_concat_guard_allows_list;
+]
+
 (* ---- STDLIB-04b: Throws extern `error<T>` (Refs #329) ----
 
    `error<T>(msg: String) -> T / Throws` was declared in
@@ -5144,6 +5185,7 @@ let tests =
     ("E2E String-wall slice 5 (trim)", stringwall_trim_tests);
     ("E2E String-wall slice 6 (string_find)", stringwall_find_tests);
     ("E2E String-wall slice 7 (int_to_string)", stringwall_i2s_tests);
+    ("E2E String-wall slice 8 guard (string ++ rejection)", stringwall_concat_guard_tests);
     ("E2E STDLIB-04b error #329", stdlib_04b_error_tests);
     ("E2E Vscode Bindings",      vscode_bindings_tests);
     ("E2E Array Type Sugar",     array_type_tests);