flan/docs/REVIEW-production-readiness.md

13 KiB

Production-readiness review — 2026-09-17

Three review lanes (C runtime, compiler robustness, tooling/UX) plus a suite run on the merged tree (e9d0b99, 232 checks, 0 failures, @x86 104 MATCH / 0 DIFFER). This file is written to be implemented from: each item says what is wrong, where, and what the fix is. Items marked (known) are already recorded in NEXT.md/FIX.org and are listed so the ranking is complete, not because they are news.

The verdict in one paragraph. The development loop is the most finished part of the project and the shipping loop is the least. A solo developer in Emacs on this machine can build a small raylib game today and the experience is genuinely good — the module system, the Emacs client's failure handling, the import-c header diffing, the diagnostics (a real span-and-notes system, ~365 located refusal sites, zero warning suppressions), and full LLVM/x86 backend parity are production-grade. Nearly every gap sits at the boundary where a second person, a second machine, or a shipped binary appears — plus three memory-safety holes in the runtime that nothing currently mentions.


Tier 1 — correctness blockers

These produce silent wrong values or memory corruption in programs that look fine.

1.1 Every number→string conversion aliases one static buffer (known, unenforced)

runtime/flan_rt.c:217-218flan_i64_to_bytes / flan_f64_to_bytes / flan_u64_to_bytes all return {scratch, len} into one shared static char scratch[64], and emit.ml:2266-2270 never copies the bytes out. Holding two results — (vec-push! v (str i)) in a loop, a (str n) stored in a struct — silently reads clobbered bytes. No crash, no diagnostic, so sanitizers never see it. NEXT.md's "Sharp edges" records it; the prelude's append-i64! family works around it; nothing enforces it. Fix to decide, then do: either make the runtime conversions allocate from context/temp (semantic change, kills the hazard everywhere), or have the checker type these results as a distinct short-lived slice that may not be stored or outlive the next conversion. The first is simpler and matches the Odin idiom already adopted for arenas.

1.2 cap * size overflow at container growth

runtime/flan_rt.c:1336 (Vec), :1574 (Pool, cap*size + cap*sslot). flan_vec_reserve (:1374) forwards arbitrary n and the 1<<40 clamp at :1333 is bypassed when want exceeds it (cap = want; break;). A wrap to a small positive allocates a tiny block while v->cap stores the unwrapped value; the next push memcpys far past the block (:1386). Same family, lower reach: flan_over_budget (:785), flan_map_block_size (:2080). Fix: __builtin_mul_overflow (or size > INT64_MAX/cap) at every grow/reserve site; overflow reports as StorageExhausted like any other allocation failure. Small, mechanical.

1.3 Map has no removal

runtime/flan_rt.c:1788, :2439 — deferred, no tombstones. A Map you cannot delete a key from is a daily-use gap, not an edge case (entity tables, caches). Fix: Robin Hood backward-shift deletion (no tombstones needed with the existing cache-line-run layout), a map-remove! builtin through check/emit/x86, tests on both backends. Medium-sized, self-contained.

1.4 The dev allocation registry is read cross-thread with no synchronisation

Writer: game thread inside every alloc/free (runtime/flan_dev.c:1130, :1180). Reader: the agent's listener thread (vendor/agent/flan_agent.c:1081, :1146) on a running program — the reg verb has no stopped-gate, unlike the frame chain. No seqlock, no atomics, and flan_reg_compact (flan_dev.c:1099) memsets and reinserts the whole table mid-scan. A torn (type, typelen) pair is an out-of-bounds read in the listener. Ranked Tier 1 because the dev loop is the project's priority. Fix: either gate the reg verb on stopped (matching the frame chain — smallest change), or give the registry the same per-slot seqlock the watch table already has (flan_dev.c:380-870 is the template in the same file).

1.5 (addr (.field x)) on an Option — confirm, then fix

FIX.org records Tast.Addr (Tast.Pfield ...) failing on both backends (working route: Prim (AddrOf, [Field ...])). check.ml:4715 builds Tast.Addr from user-writable (addr <place>) and check.ml:2968 builds Pfield from (.field x), so the combination is expressible today; neither emit.ml:1371-1375 nor x86.ml:2070 has an Option arm. Fix: first write the failing program to confirm reachability from source; then either lower Addr(Pfield) through the AddrOf route in check.ml, or add the Option arm to both backends. If unreachable from source, add the refusal-by-name that the house rule requires.


Tier 2 — the install and shipping story

One coherent problem: the compiler runs only from its checkout, and its output runs only on this machine. This is the single largest thing between "the author's language" and "a language someone else can try."

2.1 There is no install path

dune-project is two stanzas — no (package ...), so dune install cannot work; the documented way to run the compiler is dune exec. Worse, the README's suggested workaround (copy the binary onto PATH) silently breaks the flagship feature: the merged flan dev needs flan.cmxa + flan.a beside the binary (lib/dev.ml:3198-3203) and ocamlfind on PATH at runtime (lib/dev.ml:2898). The failure message names the escape hatches (FLAN_LIBDIR, --two-process) but neither README nor emacs/MANUAL.md mentions them. Fix: a (package) stanza with install rules that place flan.cmxa/flan.a where the binary's own lookup finds them; document FLAN_LIBDIR; make the README's install section truthful about what flan dev needs.

2.2 A built game runs only on machines configured like this one

No -static anywhere in lib/build.ml; vendor/raylib/link names -l:libraylib.so.550 by exact soname (a Fedora-ism — no unversioned symlink). The binary is otherwise genuinely standalone (the C runtime is embedded in the compiler, lib/dune:25-41 — right design). Fix: a flan build --static (or bundled-raylib) option, and per-target link lines that do not hardcode one distro's soname.

2.3 The env-var surface is real and entirely undocumented

FLAN_CLANG, FLAN_EMCC, FLAN_LD, FLAN_LLC, FLAN_WASM_SYSROOT, FLAN_WASM_BUILTINS, FLAN_CACHE_DIR, FLAN_OCAMLFIND, FLAN_LIBDIR, FLAN_RAYLIB_WEB — none in the README. Also undocumented: the live loop needs llc/ld at an LLVM version matching clang (lib/build.ml:886-962), and a mismatch breaks C-c C-c while flan build keeps working. Fix: an env-var table in the README, and a sentence about the llc/clang version coupling.


Tier 3 — the standard library

The containers, strings/UTF-8, sequences, random, and printing layers are decent. The gaps:

  • No clock of any kind — no now, no monotonic time, no sleep, not in prelude or runtime. A raylib game gets time from raylib; a non-graphical tool cannot time anything. Blocker for the "or tool" half of day-to-day use. A time/sleep builtin pair backed by clock_gettime is small.
  • Math is five f32 functions (lib/prelude.ml:624-677: sqrt, sin, cos, atan2, pow). No tan/asin/acos/log/exp/fmod/abs/hypot, no f64 variants, no PI constant.
  • File IO is whole-file only (slurp/barf). No streaming, stdin, directory listing, metadata, delete/rename/mkdir.
  • No env vars, no process spawn. argv and exit are the whole OS surface.
  • The prelude is an OCaml string literal (lib/prelude.ml:32) — cannot be read as Flan, extended, or replaced without rebuilding the compiler. Its own docstring promises a core: package "at milestone 3" that does not exist. The import-c header-diffing is a strong mitigation (users can bind libc and be told when they get it wrong), but the promised core: migration is the structural fix.

Tier 4 — robustness and polish (small, high-value)

  • Sys_error uncaught in the CLIflan check nosuch.flanFatal error: exception Sys_error(...). The daemon already has the arm (lib/dev.ml:2613-2621); copy it into bin/main.ml:8-32. One line. Add a Not_found backstop arm at the same time — nothing reaches it today, but the failure would be a message-less Fatal error: exception Not_found.
  • Three tests leave Fatal error: exception Flan.Loc.Error(_) on stderr (test_acceptance, test_session, test_web; suite still passes). Some spawned compiler process dies without going through the error printer — locate the spawn (grep the dune log attribution), and either wrap it or assert on the formatted message instead.
  • abort() in the dev runtimeflan_dev.c:47-50, :103: reload-name-table exhaustion, intern OOM, and "global changed size" kill the game instead of signalling. Against the grain of everything else in the runtime; route through flan_error.
  • Six trap paths bypass flan_exit_hook and end a merged flan dev session (flan_restart_fail, flan_restart_args_fail, flan_restart_unarmed, flan_transfer_fail, flan_null_alloc_fail, flan_free_all_failrt_die). Bounds and arithmetic already park via the break-loop hook; these should too.
  • Unchecked malloc in flan_argv (flan_rt.c:162) — the one in the file.
  • v->gen stale-slice word is maintained and never consulted (flan_rt.c:1347) — either implement the check or delete the field before someone trusts it.
  • flan_slurp_into conflates elements and bytes and skips flan_vec_check (flan_rt.c:2746) — safe only because check.ml pins slurp to (Vec u8); a latent trap.
  • flan run swallows build flags as program arguments (bin/main.ml:634-651) — flan run game.flan --debug hands --debug to the game. Filter or refuse by name like every other subcommand. Also: no -O control anywhere (Build.default pins -O2; --debug is the only route to -O0).
  • main signature errors print <unknown>:0:0 (check.ml:6033, :6038) — env.locs already holds the decl location; use the existing find_opt idiom.
  • flan_shim_cstr accepts embedded NULs that flan_path_cstr refuses (lib/shim.ml:310 vs flan_rt.c:2639) — pick one policy.
  • No -Wall -Wextra on the runtime's C compile (lib/build.ml:827, :1069).
  • Package visibility — everything in a package is public except main (lib/load.ml:25-31); rl/get-color-raw is the recorded symptom.
  • Emacs client: 30s hard deadline with no retry on long builds (flan-dev.el:155-170); accept-process-output loops can freeze Emacs up to 60s on a hung daemon (:521, :576-600); no package headers, so not installable off MELPA or by path alone.
  • No CI — README states it openly and records two silent-failure incidents. @checks exists; a workflow that runs dune build @checks on push is the whole job. Note @x86 parity is only under @checks, so routine dune test does not protect parity.

Documentation corrections (cheap, decision-relevant)

  • docs/DISCUSS.md:762 is stale and argues the opposite of the truth: it lists the condition/restart family under "no plan" for the x86 backend; x86.ml:1587-1615 lowers all of it and the survey shows 104/104 parity. Anyone reading it for a production decision concludes wrongly.
  • README documents 4 of 11 subcommands — import-c/generate-c, the most valuable undocumented feature, are missing (README.md:105-119 vs bin/main.ml).
  • lib/prelude.ml:8-10 promises the nonexistent core: package.
  • Root clutter: working artifacts (MY-NOTES.org, plan.org, committed binaries, sand.js/sand.wasm, old-ocaml/) a newcomer must ignore.

Deliberately out of scope — do not pick these up from this report

  • drop / recursive teardown — parked on worktree-agent-a18e9e62485eaedb5 with docs/handoffs/HANDOFF-drop.md; the arena route replaced it (FIX.org item 4, merged).
  • JavaScript backend — held (FIX.org item 6).
  • wasm32/browser — explicitly deprioritised; the ILP32 (size_t) truncations and the emscripten gaps are recorded here but not queued.
  • macOS/Windows portability (aligned_alloc, MSG_NOSIGNAL, __atomic_* vs stdatomic.h, long ftell 2 GiB cap) — real, recorded, not current-machine problems.
  • Known and accepted: seqlock memcpy formal-UB (correctly fenced, retry-bounded); one-.so-leak-per-reload (cells hold module text addresses by design); the arena route's compile-time→runtime-trap trade (stated in FIX.org); string-literal write-through (waiting on provenance, plan.org decision #3).

Suggested order

  1. Quick wins, one sitting: Sys_error + Not_found arms; flan_argv malloc check; main-signature locations; flan run flag filtering; DISCUSS.md:762 correction; README subcommand + env-var tables.
  2. Runtime correctness: mul-overflow guards (1.2), registry sync (1.4), scratch-buffer decision + fix (1.1), map-remove! (1.3), dev-runtime aborts → signals.
  3. Confirm and fix Addr(Pfield) on Option (1.5); locate the stderr Loc.Error fatals.
  4. Install story (2.1, 2.3), then shipping (2.2).
  5. Stdlib: clock first, then math, then IO/env — each is independent.
  6. CI: dune build @checks on push.