Skip to content

Commit 2aa00ff

Browse files
claudehyperpolymath
authored andcommitted
fix(codegen): uniform heap representation for variants — fixes mixed-rep match
Zero-argument variants (None, plain enum cases) were emitted as raw i32 tags while argument-carrying variants (Some(x), Ok/Err) were heap pointers to [tag][fields]. A `match` over a value that can be either form (Option/Result) mis-read one as the other: `match None { Some(v) => v, None => d }` dereferenced the raw tag as a pointer in the Some(v) arm and returned garbage instead of d (#607). Box zero-arg variants as a heap [tag] too, so EVERY variant value is uniformly a pointer; the zero-arg match arm now dereferences [ptr+0] for the tag, symmetric with construction and with the args path. Touches the construction sites (bare-ident ExprVar, ExprVariant) and the zero-arg PatCon match. Verified under node: unwrap_or(None,99)=99 and unwrap_or(Some 7,99)=7 (the None case was 0 before); Result Ok/Err and plain multi-constructor enums unchanged. dune test 461 green; adds tests/codegen/mixed_variant_match — a runtime regression (main()==9907) executed by run_codegen_wasm_tests.sh. Closes the mixed-representation match item in #607. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lz7pRcec2Z3tVtaAhvB3M8
1 parent 0f8e6c5 commit 2aa00ff

3 files changed

Lines changed: 78 additions & 5 deletions

File tree

lib/codegen.ml

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -518,6 +518,26 @@ let async_transform_hook
518518
: (context -> expr -> (context * instr list) result option) ref
519519
= ref (fun _ _ -> None)
520520

521+
(* #607: box a zero-argument variant as a heap [tag] so that EVERY variant
522+
value is uniformly a heap pointer (args-variants are [tag][field...]).
523+
Previously zero-arg variants were raw i32 tags while args-variants were
524+
pointers, so a `match` over a value that can be either form (Option/Result)
525+
mis-read one as the other — e.g. `match None { Some(v) => v, None => d }`
526+
dereferenced the raw tag `1` as a pointer and returned garbage instead of
527+
`d`. Leaves the variant pointer on the stack. The match side
528+
(gen_pattern's zero-arg PatCon) dereferences `[ptr+0]` to read the tag, to
529+
stay symmetric with this representation. *)
530+
let gen_box_zero_arg_variant (ctx : context) (tag : int) : (context * instr list) =
531+
let (ctx1, alloc_code) = gen_heap_alloc ctx 4 in
532+
let (ctx2, ptr) = alloc_local ctx1 "__zvariant_ptr" in
533+
(ctx2,
534+
alloc_code @ [
535+
LocalTee ptr;
536+
I32Const (Int32.of_int tag);
537+
I32Store (2, 0);
538+
LocalGet ptr;
539+
])
540+
521541
(** Generate code for an expression, returning instructions and updated context *)
522542
let rec gen_expr (ctx : context) (expr : expr) : (context * instr list) result =
523543
match expr with
@@ -536,7 +556,10 @@ let rec gen_expr (ctx : context) (expr : expr) : (context * instr list) result =
536556
arm body of the form `Uninitialised => Initialised` fails with
537557
UnboundVariable even though the parser accepts it. *)
538558
begin match List.assoc_opt id.name ctx.variant_tags with
539-
| Some tag -> Ok (ctx, [I32Const (Int32.of_int tag)])
559+
| Some tag ->
560+
(* #607: heap-box the zero-arg variant (uniform pointer rep). *)
561+
let (ctx_box, box_code) = gen_box_zero_arg_variant ctx tag in
562+
Ok (ctx_box, box_code)
540563
| None ->
541564
(* Top-level const bindings are stored in func_indices with a
542565
negative sentinel: actual global index = -(k+1). *)
@@ -2067,14 +2090,16 @@ let rec gen_expr (ctx : context) (expr : expr) : (context * instr list) result =
20672090
(* For now, use variant name directly to find tag *)
20682091
begin match List.assoc_opt variant_name.name ctx.variant_tags with
20692092
| Some tag ->
2070-
(* Zero-argument variant: just return the tag as an integer *)
2071-
Ok (ctx, [I32Const (Int32.of_int tag)])
2093+
(* #607: heap-box the zero-arg variant (uniform pointer rep). *)
2094+
let (ctx_box, box_code) = gen_box_zero_arg_variant ctx tag in
2095+
Ok (ctx_box, box_code)
20722096
| None ->
20732097
(* Tag not found - assign a new sequential tag based on name *)
20742098
(* This is a fallback for when type declarations aren't processed *)
20752099
let tag = List.length ctx.variant_tags in
20762100
let ctx' = { ctx with variant_tags = (variant_name.name, tag) :: ctx.variant_tags } in
2077-
Ok (ctx', [I32Const (Int32.of_int tag)])
2101+
let (ctx_box, box_code) = gen_box_zero_arg_variant ctx' tag in
2102+
Ok (ctx_box, box_code)
20782103
end
20792104

20802105
| ExprRowRestrict (base, _field) ->
@@ -2204,8 +2229,12 @@ and gen_pattern (ctx : context) (scrutinee_local : int) (pat : pattern)
22042229
(* Zero-argument constructor: compare scrutinee to tag *)
22052230
begin match List.assoc_opt con.name ctx.variant_tags with
22062231
| Some tag ->
2232+
(* #607: zero-arg variants are heap-boxed as [tag]; deref [ptr+0] to
2233+
read the tag, symmetric with construction (and with the args path
2234+
below, which also loads the tag from offset 0). *)
22072235
let test_code = [
2208-
LocalGet scrutinee_local; (* Get scrutinee (should be tag) *)
2236+
LocalGet scrutinee_local; (* variant pointer *)
2237+
I32Load (2, 0); (* load tag from [ptr+0] *)
22092238
I32Const (Int32.of_int tag); (* Expected tag *)
22102239
I32Eq (* Compare *)
22112240
] in
@@ -2216,6 +2245,7 @@ and gen_pattern (ctx : context) (scrutinee_local : int) (pat : pattern)
22162245
let ctx' = { ctx with variant_tags = (con.name, tag) :: ctx.variant_tags } in
22172246
let test_code = [
22182247
LocalGet scrutinee_local;
2248+
I32Load (2, 0);
22192249
I32Const (Int32.of_int tag);
22202250
I32Eq
22212251
] in
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// SPDX-License-Identifier: MPL-2.0
2+
// #607 regression: a `match` over a value that can be a zero-argument variant
3+
// (`Non`) or an argument-carrying variant (`Som`) must read the tag the same
4+
// way for both. Before the fix, zero-arg variants were raw i32 tags while
5+
// args-variants were heap pointers, so the `Non` value (raw tag) was
6+
// dereferenced as a pointer in the `Som(v)` arm and matched the wrong arm —
7+
// `unwrap_or(Non, 99)` returned garbage instead of 99.
8+
//
9+
// Self-contained (declares its own enum, no stdlib import) so the wasm-codegen
10+
// harness needs no AFFINESCRIPT_STDLIB.
11+
module mixed_variant_match;
12+
13+
type Opt = Som(Int) | Non
14+
15+
fn unwrap_or(o: Opt, d: Int) -> Int {
16+
match o {
17+
Som(v) => v,
18+
Non => d,
19+
}
20+
}
21+
22+
// Encodes two cases in one result: the zero-arg arm (Non -> 99) and the
23+
// args arm (Som(7) -> 7). Expected: 99*100 + 7 = 9907.
24+
pub fn main() -> Int {
25+
unwrap_or(Non, 99) * 100 + unwrap_or(Som(7), 5)
26+
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// SPDX-License-Identifier: MPL-2.0
2+
// #607: zero-arg variant (Non) vs args-variant (Som) must match correctly.
3+
// Before the variant-representation fix, `unwrap_or(Non, 99)` returned garbage
4+
// (the raw tag was dereferenced as a pointer), so main() was not 9907.
5+
import assert from 'node:assert/strict';
6+
import { readFile } from 'node:fs/promises';
7+
8+
const buf = await readFile('./tests/codegen/mixed_variant_match.wasm');
9+
const imports = { wasi_snapshot_preview1: { fd_write: () => 0 } };
10+
const inst = (await WebAssembly.instantiate(buf, imports)).instance;
11+
12+
const r = inst.exports.main();
13+
assert.equal(
14+
r, 9907,
15+
`unwrap_or(Non,99)*100 + unwrap_or(Som 7,5) should be 9907 (Non->99, Som->7), got ${r}`,
16+
);
17+
console.log('test_mixed_variant_match.mjs OK');

0 commit comments

Comments
 (0)