Skip to content

Commit ed4881a

Browse files
committed
fix(module_loader): revert #138 imported-type carry — fixes Deno-ESM duplicate constructor
Making CI standalone let `build` run for the first time (it had been startup_failing), which surfaced a latent regression from #138: flatten_imports carried imported public TopType decls into prog_decls, but the non-Wasm backends emit Option/Result constructors from a built-in runtime preamble, so the prelude types' Some/None/Ok/Err got declared twice. Running the emitted Deno-ESM module under node failed with `SyntaxError: Identifier 'Some' has already been declared` (tests/codegen-deno: http_fetch and others). Revert the flatten_imports type-carrying. #138's load-bearing fix — Wasm constructor tags via Codegen.gen_imports, which consumes the un-flattened prog — is unaffected, and the flat backends keep emitting prelude constructors via their preamble, so imported Option/Some/None/Ok/Err still lower on every backend. Verified: dune test 458 green (incl. the #138 Wasm test); tools/run_codegen_deno_tests.sh all harnesses pass under node; run_codegen_wasm_tests.sh + face-transformer tests green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lz7pRcec2Z3tVtaAhvB3M8
1 parent 8af6c5d commit ed4881a

1 file changed

Lines changed: 13 additions & 58 deletions

File tree

lib/module_loader.ml

Lines changed: 13 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -345,61 +345,16 @@ let flatten_imports (loader : t) (prog : program) : program =
345345
) select
346346
) prog.prog_imports
347347
in
348-
(* #138: type declarations are a SEPARATE namespace from fn/const bindings,
349-
so they get their own dedup table — a local `fn Foo` must not suppress an
350-
imported `type Foo`, and vice versa. Imported public types are inlined too
351-
so the prog_decls-iterating backends (Deno / JS / Julia / C / Rust / ...)
352-
register their constructors exactly as they would for a local type;
353-
without this, applying an imported constructor (`Some`/`None` from
354-
`use prelude::{Option, Some, None}`) can reach codegen with no type in
355-
scope. Local types win over imported ones, and a type reachable through
356-
more than one path is carried only once. The Wasm backend gets the same
357-
registration natively in [Codegen.gen_imports]. *)
358-
let local_type_names =
359-
List.filter_map (function
360-
| TopType td -> Some td.td_name.name
361-
| _ -> None
362-
) prog.prog_decls
363-
in
364-
let type_already_in = Hashtbl.create 16 in
365-
List.iter (fun n -> Hashtbl.add type_already_in n ()) local_type_names;
366-
let imported_types =
367-
List.concat_map (fun imp ->
368-
let path_strs path = List.map (fun (id : ident) -> id.name) path in
369-
let mod_path = match imp with
370-
| ImportSimple (p, _) | ImportList (p, _) | ImportGlob p -> path_strs p
371-
in
372-
match Hashtbl.find_opt loader.loaded mod_path with
373-
| None -> []
374-
| Some lm ->
375-
let public_types = List.filter_map (function
376-
| TopType td when td.td_vis = Public || td.td_vis = PubCrate -> Some td
377-
| _ -> None
378-
) lm.mod_program.prog_decls in
379-
(* `use M::{..}` selects a type when the list names the type itself or
380-
any of its constructors; `use M` / `use M::*` bring all public
381-
types (the resolver still gates what is referenceable). *)
382-
let selected = match imp with
383-
| ImportGlob _ | ImportSimple _ -> public_types
384-
| ImportList (_, items) ->
385-
let wanted = List.map (fun (it : import_item) -> it.ii_name.name) items in
386-
List.filter (fun td ->
387-
List.mem td.td_name.name wanted ||
388-
(match td.td_body with
389-
| TyEnum variants ->
390-
List.exists (fun vd -> List.mem vd.vd_name.name wanted) variants
391-
| _ -> false)
392-
) public_types
393-
in
394-
List.filter_map (fun td ->
395-
if Hashtbl.mem type_already_in td.td_name.name then None
396-
else begin
397-
Hashtbl.add type_already_in td.td_name.name ();
398-
Some (TopType td)
399-
end
400-
) selected
401-
) prog.prog_imports
402-
in
403-
(* Types precede imported fns/consts and all local decls so the single-pass
404-
codegen registers an imported type before any function that uses it. *)
405-
{ prog with prog_decls = imported_types @ imported_decls @ prog.prog_decls }
348+
(* #138 follow-up: imported TYPE decls are intentionally NOT inlined here.
349+
An earlier #138 revision carried imported public [TopType]s so the
350+
prog_decls-iterating backends could register their constructors — but the
351+
non-Wasm backends (Deno-ESM / JS / Julia / C / Rust / ...) already emit
352+
the Option/Result constructors from a built-in runtime preamble, so
353+
carrying the prelude types declared `Some`/`None`/`Ok`/`Err` twice
354+
(`SyntaxError: Identifier 'Some' has already been declared` when the
355+
emitted Deno-ESM module is run under node). The Wasm backend learns
356+
imported variant tags natively via [Codegen.gen_imports] (it consumes the
357+
original, un-flattened [prog]); the other backends rely on their preamble.
358+
Re-introducing type-carrying for *user-defined* cross-module enums would
359+
need per-backend constructor dedup first. *)
360+
{ prog with prog_decls = imported_decls @ prog.prog_decls }

0 commit comments

Comments
 (0)