Skip to content

Commit a3e0661

Browse files
claudehyperpolymath
authored andcommitted
fix(js-codegen): don't re-declare preamble Option/Result constructors
Mirror of the Deno-ESM fix (#606) for the JS backend, which shares the same Some/None/Ok/Err runtime preamble. A program that declares `type Option` / `type Result` (e.g. stdlib/prelude.affine) re-emitted those consts from the TyEnum lowering, redeclaring them (SyntaxError under node). Skip the preamble-provided constructors; user-defined enums are unaffected. The C backend does NOT share this bug — it emits a tag-enum plus distinct constructor functions (TAG_Some / Some()), with no Some/None preamble — so #607's "JS/C" item is JS-only. Adds a JS-path regression guard alongside the existing Deno one. Verified: stdlib/prelude.affine -> JS has a single `const Some` and loads under node; dune test 460 green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lz7pRcec2Z3tVtaAhvB3M8
1 parent eae71fd commit a3e0661

2 files changed

Lines changed: 51 additions & 3 deletions

File tree

lib/js_codegen.ml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -535,7 +535,15 @@ let gen_type_decl ctx (td : type_decl) : unit =
535535
and `Type::Variant` references both work. Structs and aliases are erased. *)
536536
match td.td_body with
537537
| TyEnum variants ->
538+
(* The runtime preamble already declares Some/None/Ok/Err; re-emitting
539+
them for a program that declares `type Option`/`type Result` (e.g.
540+
stdlib/prelude.affine) redeclares the same const (SyntaxError under
541+
node). Skip the preamble-provided constructors. Mirrors the Deno-ESM
542+
fix (#606). *)
543+
let preamble_ctors = [ "Some"; "None"; "Ok"; "Err" ] in
538544
List.iter (fun (vd : variant_decl) ->
545+
if List.mem vd.vd_name.name preamble_ctors then ()
546+
else begin
539547
let name = mangle vd.vd_name.name in
540548
let arity = List.length vd.vd_fields in
541549
if arity = 0 then
@@ -550,6 +558,7 @@ let gen_type_decl ctx (td : type_decl) : unit =
550558
(Printf.sprintf "const %s = (%s) => ({ tag: \"%s\", values: [%s] });"
551559
name (String.concat ", " params)
552560
vd.vd_name.name (String.concat ", " params))
561+
end
553562
) variants;
554563
emit ctx "\n"
555564
| TyStruct _ | TyAlias _ ->

test/test_stdlib_aot.ml

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,31 @@ let pipeline_to_deno (prog : Ast.program) : (string, string) result =
6161
| Error e -> Error (Printf.sprintf "deno-codegen: %s" e)
6262
| Ok js -> Ok js)))
6363

64+
(** Same pipeline, JS (non-ESM) backend. Shares the Some/None/Ok/Err runtime
65+
preamble with Deno-ESM, so it has the same duplicate-constructor surface. *)
66+
let pipeline_to_js (prog : Ast.program) : (string, string) result =
67+
let ld = loader () in
68+
match Resolve.resolve_program_with_loader prog ld with
69+
| Error (e, sp) ->
70+
Error (Printf.sprintf "resolve: %s @ %s"
71+
(Resolve.show_resolve_error e) (Span.show sp))
72+
| Ok (rctx, itc) ->
73+
(match
74+
Typecheck.check_program
75+
~import_types:itc.Typecheck.name_types rctx.symbols prog
76+
with
77+
| Error e ->
78+
Error (Printf.sprintf "typecheck: %s" (Typecheck.format_type_error e))
79+
| Ok _ ->
80+
(match Borrow.check_program rctx.symbols prog with
81+
| Error e ->
82+
Error (Printf.sprintf "borrow: %s" (Borrow.format_borrow_error e))
83+
| Ok () ->
84+
let flat = Module_loader.flatten_imports ld prog in
85+
(match Js_codegen.codegen_js flat rctx.symbols with
86+
| Error e -> Error (Printf.sprintf "js-codegen: %s" e)
87+
| Ok js -> Ok js)))
88+
6489
(** Full AOT pipeline to the core-Wasm backend: resolve -> typecheck ->
6590
borrow -> [Codegen.generate_module] (loader-aware). Mirrors
6691
[pipeline_to_deno] but targets the backend whose cross-module constructor
@@ -262,12 +287,26 @@ let test_deno_no_duplicate_option_ctor () =
262287
"`const Some` declared exactly once (preamble only, not re-emitted)"
263288
1 (count_substr "const Some" js))
264289

265-
let deno_dup_ctor_tests =
290+
let test_js_no_duplicate_option_ctor () =
291+
match Parse_driver.parse_string ~file:"<localopt>" local_option_src with
292+
| exception e ->
293+
Alcotest.failf "local-option parse raised: %s" (Printexc.to_string e)
294+
| prog ->
295+
(match pipeline_to_js prog with
296+
| Error m -> Alcotest.failf "js codegen failed: %s" m
297+
| Ok js ->
298+
Alcotest.(check int)
299+
"`const Some` declared exactly once (preamble only, not re-emitted)"
300+
1 (count_substr "const Some" js))
301+
302+
let dup_ctor_tests =
266303
[ Alcotest.test_case "declared Option does not duplicate preamble ctor (Deno)"
267-
`Quick test_deno_no_duplicate_option_ctor ]
304+
`Quick test_deno_no_duplicate_option_ctor;
305+
Alcotest.test_case "declared Option does not duplicate preamble ctor (JS)"
306+
`Quick test_js_no_duplicate_option_ctor ]
268307

269308
let tests =
270309
[ ("STAGE-A AOT smoke (#136)", aot_smoke_tests);
271310
("STAGE-A multi-module integration (#137)", integration_tests);
272311
("cross-module constructor linking, Wasm (#138)", xmod_constructor_tests);
273-
("Deno-ESM no duplicate Option/Result constructor", deno_dup_ctor_tests) ]
312+
("Deno-ESM / JS no duplicate Option/Result constructor", dup_ctor_tests) ]

0 commit comments

Comments
 (0)