From abb6d0f99550ff7b1d4789efce95755f3ac68f3c Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Thu, 27 Aug 2026 13:37:36 +0100 Subject: [PATCH 1/2] =?UTF-8?q?fix(codegen):=20test=20nested=20constructor?= =?UTF-8?q?=20patterns=20=E2=80=94=20three=20backends=20emitted=20the=20sa?= =?UTF-8?q?me=20guard?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #731. A match whose arms differed only in a NESTED constructor emitted identical guards, so every arm after the first was unreachable and the first arm's body ran for all of them. It type-checked; only the emitted code was wrong. | PatCon (id, _) -> scrut ^ ".tag === " ^ ... ^ the sub-patterns, discarded WIDER THAN THE ISSUE SAID. I filed #731 against the Deno-ESM backend. It is in THREE: lib/codegen_deno.ml:1222 Deno-ESM lib/js_codegen.ml:379 plain JS lib/lua_codegen.ml:102 Lua Each has its own gen_pattern_test with the same defect. Checked the rest: c_codegen, codegen_gc, wasm_backend and native_backend do not share this lowering path. WHY IT STAYED INVISIBLE. gen_pattern_bindings in every one of the three was ALREADY descending correctly, binding through .value / .values[i]. So the bound variables landed on the right values and the output looked entirely plausible -- it just took the wrong branch. Only the TEST was truncated to the outermost constructor. before: if (__scrut.tag === "Some") if (__scrut.tag === "Some") <- identical after: if (__scrut.tag === "Some" && __scrut.value.tag === "Circle") if (__scrut.tag === "Some" && __scrut.value.tag === "Square") VERIFIED BY EXECUTION, not by reading the output: Circle(1) -> 1 (expect 1) was 1 Square(1) -> 1001 (expect 1001) was 1 The fix mirrors gen_pattern_bindings exactly in each backend -- .value for arity 1, .values[i] otherwise -- so test and binding paths cannot drift apart. Sub-patterns that test "true" (a variable or wildcard) are dropped from the conjunction, so guards read "tag === X && value.tag === Y" rather than trailing a string of "&& true". WHY THIS MATTERED NOW. Found while hand-porting the first complete .affine file in metadatastician/stapeln, where a JFloat id returned Ok(2.7) from a function declared -> Result: a Float escaping into an Int position, i.e. the emitted program violating the signature the checker had accepted. Nested patterns are not an edge case -- they are the ordinary shape of decoders, of Option/Result over a sum type, and of every TEA update function. The ReScript -> AffineScript campaign covers ~3,996 files across ~80 repos, and until this landed any ported file using them could pass check, pass review, and run wrong. Self-merged under the owner's standing --admin grant. --- lib/codegen_deno.ml | 25 ++++++++++++++++++++++++- lib/js_codegen.ml | 21 ++++++++++++++++++--- lib/lua_codegen.ml | 18 +++++++++++++++++- 3 files changed, 59 insertions(+), 5 deletions(-) diff --git a/lib/codegen_deno.ml b/lib/codegen_deno.ml index 4678f0b5..2a289331 100644 --- a/lib/codegen_deno.ml +++ b/lib/codegen_deno.ml @@ -1256,7 +1256,30 @@ and gen_pattern_test scrut pat = match pat with | PatWildcard _ | PatVar _ -> "true" | PatLit lit -> scrut ^ " === " ^ gen_literal lit - | PatCon (id, _) -> scrut ^ ".tag === " ^ Printf.sprintf "%S" id.name + (* Descend into the sub-patterns. This used to be [PatCon (id, _)], testing + only the OUTERMOST tag and discarding the arguments -- so [Some(Circle(n))] + and [Some(Square(n))] emitted the SAME guard, the second arm was + unreachable, and the first arm's body ran for both. It type-checked; only + the emitted JavaScript was wrong, which is the worst place for it to be. + + The paths mirror gen_pattern_bindings below, which was already descending + correctly -- that asymmetry is why the bug was invisible: bindings landed + on the right values, so the output looked plausible. *) + | PatCon (id, args) -> + let tag_test = scrut ^ ".tag === " ^ Printf.sprintf "%S" id.name in + let sub_tests = + match args with + | [] -> [] + | [single] -> [gen_pattern_test (scrut ^ ".value") single] + | many -> + List.mapi (fun i p -> + gen_pattern_test + (scrut ^ ".values[" ^ string_of_int i ^ "]") p) many + in + (* A variable or wildcard sub-pattern tests "true"; dropping those keeps + the guard readable rather than "tag === X && true && true". *) + let meaningful = List.filter (fun s -> s <> "true") sub_tests in + String.concat " && " (tag_test :: meaningful) | PatTuple pats -> let conds = List.mapi (fun i p -> gen_pattern_test (scrut ^ "[" ^ string_of_int i ^ "]") p) pats in diff --git a/lib/js_codegen.ml b/lib/js_codegen.ml index adb80a67..e80c0bf2 100644 --- a/lib/js_codegen.ml +++ b/lib/js_codegen.ml @@ -376,9 +376,24 @@ and gen_pattern_test scrut pat = match pat with | PatWildcard _ | PatVar _ -> "true" | PatLit lit -> scrut ^ " === " ^ gen_literal lit - | PatCon (id, _) -> - (* Tagged-union variant: { tag: "Some", value: ... } *) - scrut ^ ".tag === " ^ Printf.sprintf "%S" id.name + (* Tagged-union variant: { tag: "Some", value: ... } + Sub-patterns MUST be tested too. This used to discard [args], so + [Some(Circle(n))] and [Some(Square(n))] produced the same guard and the + second arm was unreachable -- the first arm's body ran for both. Paths + mirror gen_pattern_bindings: .value for arity 1, .values[i] otherwise. *) + | PatCon (id, args) -> + let tag_test = scrut ^ ".tag === " ^ Printf.sprintf "%S" id.name in + let sub_tests = + match args with + | [] -> [] + | [single] -> [gen_pattern_test (scrut ^ ".value") single] + | many -> + List.mapi (fun i p -> + gen_pattern_test + (scrut ^ ".values[" ^ string_of_int i ^ "]") p) many + in + String.concat " && " + (tag_test :: List.filter (fun s -> s <> "true") sub_tests) | PatTuple pats -> let conds = List.mapi (fun i p -> gen_pattern_test (scrut ^ "[" ^ string_of_int i ^ "]") p diff --git a/lib/lua_codegen.ml b/lib/lua_codegen.ml index 3533c59e..8e7e165d 100644 --- a/lib/lua_codegen.ml +++ b/lib/lua_codegen.ml @@ -99,7 +99,23 @@ and gen_pattern_test scrut pat = match pat with | PatWildcard _ | PatVar _ -> "true" | PatLit lit -> Printf.sprintf "%s == %s" scrut (gen_lit lit) - | PatCon (id, _) -> Printf.sprintf "%s.tag == %S" scrut id.name + (* Sub-patterns must be tested, not discarded -- see codegen_deno.ml. Paths + mirror gen_pattern_bindings below: .value for arity 1, .values[i] else. + Lua indexes from 1, and the bindings walker uses the same expression, so + the two stay in step. *) + | PatCon (id, args) -> + let tag_test = Printf.sprintf "%s.tag == %S" scrut id.name in + let sub_tests = + match args with + | [] -> [] + | [single] -> [gen_pattern_test (scrut ^ ".value") single] + | many -> + List.mapi (fun i p -> + gen_pattern_test + (Printf.sprintf "%s.values[%d]" scrut i) p) many + in + String.concat " and " + (tag_test :: List.filter (fun s -> s <> "true") sub_tests) | PatTuple _ -> "true" (* arity match by structure, not tag *) | PatRecord _ -> "true" | PatAs (_, p) -> gen_pattern_test scrut p From eaf4f4912f74d32c6837035927b8028faa471416 Mon Sep 17 00:00:00 2001 From: "Jonathan D.A. Jewell" <6759885+hyperpolymath@users.noreply.github.com> Date: Thu, 27 Aug 2026 13:41:19 +0100 Subject: [PATCH 2/2] test(codegen): regression guard for nested constructor patterns (#731) The 534-test suite had a nested-TUPLE pattern test but none for nested CONSTRUCTORS on the JS-family backends, which is why #731 survived. This adds one for the Deno-ESM and plain-JS paths. Verified to be a real guard, not decoration: reverting the PatCon arm in codegen_deno.ml turns exactly this test red (1 failure, named), and restoring it returns the suite to green. The assertion is that the inner constructor appears in a GUARD. Asserting on bindings would prove nothing -- gen_pattern_bindings was already descending correctly, and that asymmetry is precisely what hid the bug. --- test/test_stdlib_aot.ml | 70 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 69 insertions(+), 1 deletion(-) diff --git a/test/test_stdlib_aot.ml b/test/test_stdlib_aot.ml index de7a381f..fc90b2fa 100644 --- a/test/test_stdlib_aot.ml +++ b/test/test_stdlib_aot.ml @@ -340,9 +340,77 @@ let tuple_pattern_tests = [ Alcotest.test_case "nested (literal/var) tuple patterns -> Wasm" `Quick test_nested_tuple_patterns_wasm ] +(* ---- Nested CONSTRUCTOR patterns must be discriminated (#731) ------------- + + Regression guard. gen_pattern_test discarded PatCon's sub-patterns, so arms + differing only in a NESTED constructor emitted IDENTICAL guards: + + if (__scrut.tag === "Some") <- Some(Circle(n)) + if (__scrut.tag === "Some") <- Some(Square(n)) unreachable + + Every arm after the first was dead and the first arm's body ran for all of + them. It type-checked; only the emitted code was wrong, so nothing in the + compiler caught it -- and neither did this file, which had a nested-TUPLE + pattern test but none for nested CONSTRUCTORS on the JS-family backends. + + Verified to fail without the fix: reverting the PatCon arm in + codegen_deno.ml turns exactly this test red. *) +let nested_ctor_src = {| +module nestedctor; +use prelude::{ Option, Some, None }; + +pub type Shape = Circle(Int) | Square(Int) + +pub fn describe(s: Option) -> Int { + match s { + Some(Circle(n)) => n, + Some(Square(n)) => n + 1000, + None => -1, + } +} +|} + +let check_nested_ctor_guards (backend : string) (js : string) = + (* The inner constructor must appear in a GUARD, not merely in a binding -- + bindings were already descending correctly, which is what made the bug + invisible. *) + Alcotest.(check bool) + (backend ^ ": guard discriminates the inner Circle") + true (count_substr "tag === \"Circle\"" js > 0 + || count_substr "tag == \"Circle\"" js > 0); + Alcotest.(check bool) + (backend ^ ": guard discriminates the inner Square") + true (count_substr "tag === \"Square\"" js > 0 + || count_substr "tag == \"Square\"" js > 0) + +let test_deno_nested_ctor_guards () = + match Parse_driver.parse_string ~file:"" nested_ctor_src with + | exception e -> + Alcotest.failf "nested-ctor parse raised: %s" (Printexc.to_string e) + | prog -> + (match pipeline_to_deno prog with + | Error m -> Alcotest.failf "deno codegen failed: %s" m + | Ok js -> check_nested_ctor_guards "Deno-ESM" js) + +let test_js_nested_ctor_guards () = + match Parse_driver.parse_string ~file:"" nested_ctor_src with + | exception e -> + Alcotest.failf "nested-ctor parse raised: %s" (Printexc.to_string e) + | prog -> + (match pipeline_to_js prog with + | Error m -> Alcotest.failf "js codegen failed: %s" m + | Ok js -> check_nested_ctor_guards "JS" js) + +let nested_ctor_tests = + [ Alcotest.test_case "nested constructor patterns are discriminated (Deno)" + `Quick test_deno_nested_ctor_guards; + Alcotest.test_case "nested constructor patterns are discriminated (JS)" + `Quick test_js_nested_ctor_guards ] + let tests = [ ("STAGE-A AOT smoke (#136)", aot_smoke_tests); ("STAGE-A multi-module integration (#137)", integration_tests); ("cross-module constructor linking, Wasm (#138)", xmod_constructor_tests); ("Deno-ESM / JS no duplicate Option/Result constructor", dup_ctor_tests); - ("Wasm nested tuple patterns", tuple_pattern_tests) ] + ("Wasm nested tuple patterns", tuple_pattern_tests); + ("Nested constructor patterns discriminated (#731)", nested_ctor_tests) ]