From 97d1d3024e34a3783bf4cbcf985ef8c7fc728607 Mon Sep 17 00:00:00 2001 From: "shoney.arickathil" Date: Fri, 14 Aug 2026 17:28:50 +0200 Subject: [PATCH] fix: nullable scalars need their own nil word; EQS accepts nil MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by running the compiled log-watcher, not by reading code: the supervisor rejected every cron line ("malformed schedule: * * * * *") because a `*` field expands to 0 and `?Int`'s nil was also 0, so `a == nil` was true for a real value. Both log-watcher subcommands now behave: `watch` alerts on a live file, `run` reports SCHEDULE /var/log/backup.log: * * * * *. corpus 71/0, woc 565/0, wovm gates green. - a nullable SCALAR (?Int/?Bool/?Timestamp/?Id) spells nil as WO_NIL_SCALAR (-2^62), not the zero word. Heap-shaped optionals keep 0 — a null pointer is unambiguous. The value is -2^62 and NOT INT64_MIN on purpose: the compiler's integers are OCaml's 63-bit natives, so INT64_MIN is not expressible there (and `min_int * 2` silently wraps to 0 — the first attempt did exactly that) - the class table marks such fields (WOB_FIELD_NIL_SCALAR in field_class), so the runtime writes the right absence where it produces absence itself: json.decode leaving a key absent or seeing `null`, and parse_int on unparseable input (so parse_int("0") is now distinguishable from a failure). json.encode renders a nil scalar as JSON null - emit.ml: `nil` takes its word from its destination (annotation, field, return type); a comparison against `nil` emits the literal with the other operand's type, so ?scalar compares against the sentinel and ?heap against 0 - vm.c: EQS accepts a nil operand — two `?Text` values compare with it, and the answer is "both absent is equal, one absent is not". Trapping there made `a != b` on optionals unusable (it was trapping BOUNDS "null text" in the supervisor's rescan). A non-nil operand must still be a real Text - docs: both normative docs now state the heap-vs-scalar nil split and the EQS rule; the stale duplicate vm_unwind comment is gone Co-Authored-By: Claude Opus 5 (1M context) --- compiler/src/emit.ml | 89 +++++++++++++++++++++----- docs/plan/oop-vm/00-wob-format.md | 4 ++ docs/plan/oop-vm/08-builtin-surface.md | 6 +- runtime/src/builtin.c | 6 +- runtime/src/json.c | 15 ++++- runtime/src/loader.c | 3 +- runtime/src/vm.c | 23 +++---- runtime/src/wob.h | 14 ++++ 8 files changed, 123 insertions(+), 37 deletions(-) diff --git a/compiler/src/emit.ml b/compiler/src/emit.ml index 66c303e..1483307 100644 --- a/compiler/src/emit.ml +++ b/compiler/src/emit.ml @@ -1312,9 +1312,30 @@ let check_field_idx (p : pctx) (f : fstate) (pos : Ast.pos) (v : int) : int = Both exist so json.encode/json.decode can work off metadata instead of per-type generated code. *) let wob_field_json_raw = 0xFFFFFFFE +let wob_field_nil_scalar = 0xFFFFFFFD + +(* nil for a nullable SCALAR is not the zero word: `0` is a real Int, and the + driving workload stores it in a `?Int` (a cron `*` field expands to `0`), so + absence needs a value no plain Int will ever hold. runtime/src/wob.h's + WO_NIL_SCALAR = INT64_MIN. Heap-shaped optionals keep the zero word — a null + pointer is unambiguous. *) +(* -(2^62). NOT INT64_MIN: OCaml's native int is 63-bit, so INT64_MIN cannot be + written here at all (and `min_int * 2` silently wraps to 0 — the bug this + comment exists to prevent recurring). Mirrors runtime/src/wob.h's + WO_NIL_SCALAR exactly. *) +let nil_scalar_word = -4611686018427387904 + +(* is this a `?scalar` — an optional whose representation is a plain register, + so its nil has to be the sentinel rather than the zero word? *) +let is_nullable_scalar (p : pctx) (ty : Ast.field_ty) : bool = + match ty with + | Ast.Nullable inner -> field_kind p inner = 0 (* WO_K_SCALAR *) + | _ -> false let field_class_meta (p : pctx) (ty : Ast.field_ty) : int = let name_of t = match t with Ast.Scalar n -> Some n | _ -> None in + if is_nullable_scalar p ty then wob_field_nil_scalar + else match unwrap ty with | Ast.Scalar n when n = Types.json_value_type -> wob_field_json_raw | Ast.Scalar n -> ( match class_of_name p n with Some cid -> cid | None -> wob_none) @@ -1349,9 +1370,17 @@ let rec emit_expr (p : pctx) (f : fstate) (v : views) ~(dst : int) ?expected (e | IntLit n -> put f (ins_abx op_loadk dst (check_bx p f e.pos "constant" (const_int p n))) | BoolLit b -> put f (ins_abx op_loadk dst (check_bx p f e.pos "constant" (const_int p (if b then 1 else 0)))) | StrLit s -> put f (ins_abx op_loadk dst (check_bx p f e.pos "constant" (const_text p s))) - (* haxe-parity Task 6: `nil` is the zero word, whatever `?T` it stands - in for (docs/plan/oop-vm/08-builtin-surface.md's `?T` section). *) - | NilLit -> put f (ins_abx op_loadk dst (const_int p 0)) + (* haxe-parity Task 6: `nil` is the zero word for a heap-shaped `?T` and the + WO_NIL_SCALAR sentinel for a `?scalar` — see nil_scalar_word. The + destination decides: a written annotation, the field being built, or the + enclosing method's return type. With no destination at all, the zero word + is the safe answer (a heap slot). *) + | NilLit -> + let dest = match expected with Some _ -> expected | None -> f.f_ret in + let word = + match dest with Some t when is_nullable_scalar p t -> nil_scalar_word | _ -> 0 + in + put f (ins_abx op_loadk dst (check_bx p f e.pos "constant" (const_int p word))) (* Container literals lower to exactly what `multi_new()`/`map_new()` lower to — the element kinds are the destination's, never guessed (docs/plan/oop-vm/08-builtin-surface.md) — plus one `multi_push` per @@ -1594,6 +1623,23 @@ and emit_binary (p : pctx) (f : fstate) (v : views) ~(dst : int) (op : Ast.binop let b = emit_operand p f v r in put f (ins_abc o dst b a) in + (* `x == nil` / `nil == x`: the literal takes ITS destination type from the + other operand, so the sentinel-vs-zero choice matches what x actually + holds. *) + let nil_compare_into p f v ~dst o (l : Ast.expr) (r : Ast.expr) : unit = + let other = if is_nil_lit l then r else l in + let nil_e = if is_nil_lit l then l else r in + let a = emit_operand p f v other in + let bt = ty_of_expr p f other in + let save = f.f_temp in + let b = alloc_temp p f pos in + (match bt with + | Some t -> emit_expr p f v ~dst:b ~expected:t nil_e + | None -> emit_expr p f v ~dst:b nil_e); + put f (ins_abc o dst a b); + f.f_temp <- save + in + let nil_compare o = nil_compare_into p f v ~dst o l r in match op with | Add -> simple op_add | Sub -> simple op_sub @@ -1605,25 +1651,29 @@ and emit_binary (p : pctx) (f : fstate) (v : views) ~(dst : int) (op : Ast.binop | Gt -> swapped op_lt | Ge -> swapped op_le (* A comparison against `nil` is a WORD compare, never a content compare: - nil is the zero word, and EQS would dereference it as a `wo_str*` (the - VM's str_check traps on that, so an `x != nil` guard would trap instead - of answering). Text-vs-Text still uses EQS. *) + EQS would dereference nil as a `wo_str*` (the VM's str_check traps on + that, so an `x != nil` guard would trap instead of answering). The literal + `nil` is emitted with the OTHER side's declared type as its destination, + so a `?scalar` compares against the sentinel and a heap optional against + zero. Text-vs-Text still uses EQS. *) | Eq -> - if is_nil_lit l || is_nil_lit r then simple op_eq + if is_nil_lit l || is_nil_lit r then nil_compare op_eq else if is_text p f l || is_text p f r then simple op_eqs else simple op_eq | Ne -> (* no NE opcode in the v1 set: `a != b` is `(a == b) == 0`. The format doc governs, so this is a lowering, not a new opcode. *) - let a = emit_operand p f v l in - let b = emit_operand p f v r in let t = alloc_temp p f pos in - put f - (ins_abc - (if is_nil_lit l || is_nil_lit r then op_eq - else if is_text p f l || is_text p f r then op_eqs - else op_eq) - t a b); + (if is_nil_lit l || is_nil_lit r then begin + let save = f.f_temp in + f.f_temp <- t + 1; + nil_compare_into p f v ~dst:t op_eq l r; + f.f_temp <- save + end + else + let a = emit_operand p f v l in + let b = emit_operand p f v r in + put f (ins_abc (if is_text p f l || is_text p f r then op_eqs else op_eq) t a b)); let z = alloc_temp p f pos in put f (ins_abx op_loadk z (check_bx p f pos "constant" (const_int p 0))); put f (ins_abc op_eq dst t z) @@ -2162,8 +2212,13 @@ and emit_default_value (p : pctx) (f : fstate) ~(dst : int) ~(fty : Ast.field_ty | Ast.DefaultNow -> put f (ins_abc op_builtin dst dst b_now) | Ast.DefaultOpaque toks -> ( match List.map (fun (t : Token.t) -> t.Token.kind) toks with - (* `= nil` — the zero word, whatever `?T` the field is *) - | [ Token.KwNil ] -> put f (ins_abx op_loadk dst (const_int p 0)) + (* `= nil` — the field's own nil word (zero for a heap shape, the sentinel + for a `?scalar`) *) + | [ Token.KwNil ] -> + put f + (ins_abx op_loadk dst + (check_bx p f pos "constant" + (const_int p (if is_nullable_scalar p fty then nil_scalar_word else 0)))) (* `= {}` — a fresh empty container of the field's own declared type, the same rule `[]` above follows *) | [ Token.LBrace; Token.RBrace ] -> ( diff --git a/docs/plan/oop-vm/00-wob-format.md b/docs/plan/oop-vm/00-wob-format.md index 3dcfa17..883ed83 100644 --- a/docs/plan/oop-vm/00-wob-format.md +++ b/docs/plan/oop-vm/00-wob-format.md @@ -62,6 +62,10 @@ The metadata exists for exactly one reason: `json.encode`/`json.decode` are runt - **the OS half** — fs.exists/list/stat/read_all/read_at/append, time.sleep/local/iso, env.get/stopping, net.listen/accept/read/write/close, proc.run. Ids 40–56; `runtime/src/sysio.c`. A member that returns a record takes that record's **class id as its last argument**, so the VM allocates what it fills without knowing any source type name. - **json** — encode (value + the value's static kind), decode (text + the class id to build). Ids 57–58; `runtime/src/json.c`. Decode yields the zero word on malformed input rather than trapping, which is what makes `json.decode(t) as T` a checked decode. +**`?T` and nil.** A heap-shaped optional (`?Text`, `?Rec`, `?multi`, `?map`, `?@gc`) stores what `T` stores and spells nil as the **zero word** — every per-kind drop plan already ignores a zero slot, so `?T`'s field kind is `T`'s. A **nullable scalar** (`?Int`, `?Bool`, `?Timestamp`, `?Id`) cannot: `0` is a perfectly good `Int`, and real programs store it in a `?Int`. Its nil is therefore `WO_NIL_SCALAR` = −2^62 (not `INT64_MIN`: the compiler's own integers are 63-bit, so that value is not expressible on the emitting side). Such a field is marked `WOB_FIELD_NIL_SCALAR` in `field_class[i]`, which is how the runtime knows to write that word where it must produce absence itself — today only `json.decode` leaving a key absent, and `parse_int` on unparseable input. + +`EQS` accepts a nil operand for the same reason: two `?Text` values compare with it, and the answer is "both absent is equal, one absent is not". A non-nil operand must still be a real Text. + **Trap codes:** DIV0, BORROW, STACK, OOM, DB, BOUNDS, KEY, EXPLICIT, IO (a syscall the source cannot prevent said no — errno's message rides in the error record). ## Enum payload variants (haxe-parity compiler Task 4) diff --git a/docs/plan/oop-vm/08-builtin-surface.md b/docs/plan/oop-vm/08-builtin-surface.md index 556e63a..918f038 100644 --- a/docs/plan/oop-vm/08-builtin-surface.md +++ b/docs/plan/oop-vm/08-builtin-surface.md @@ -38,7 +38,7 @@ maps to one `BUILTIN` id of the format doc. | `substr(t, start, len)` | `substr` | 3 | fresh `Text`, clamped (never traps) | | `trim(t)` / `to_lower(t)` | same | 1 | fresh `Text` | | `char_of(b)` | `char_of` | 1 | fresh one-byte `Text` | -| `parse_int(t)` | `parse_int` | 1 | `?Int` — an unparseable text is `0`, which is how `?Int` spells nil | +| `parse_int(t)` | `parse_int` | 1 | `?Int` — an unparseable text yields nil (`WO_NIL_SCALAR`), so `parse_int("0")` and a failed parse are distinguishable | | `split(t, sep)` / `split_ws(t)` | same | 2 / 1 | fresh `multi Text` | | `join(m, sep)` | `join` | 2 | fresh `Text` from a `multi Text` | | `slice(m, from, to)` | `slice` | 3 | fresh `multi` over `[from, to)`; `Text` elements are COPIED, so slice and source never both own one value | @@ -228,7 +228,9 @@ interpolation — the haxe keyword verdict table's low-risk batch, added ## `?T` -A nullable field stores exactly what `T` stores and spells nil as `0`. +A nullable **heap-shaped** field (`?Text`, `?Rec`, `?multi`, `?map`, `?@gc`) stores exactly what `T` stores and spells nil as `0`. A nullable **scalar** (`?Int`, `?Bool`, `?Timestamp`, `?Id`) spells nil as `WO_NIL_SCALAR` (−2^62) instead, because `0` is a real `Int` a program legitimately stores in a `?Int` — the driving workload does exactly that (a cron `*` field expands to `0`). The class table marks such a field so the runtime can write absence itself where it must (`json.decode` on an absent key, `parse_int` on unparseable input); see [`00-wob-format.md`](00-wob-format.md). + +The rest of this section describes the heap-shaped case. The v1 format has no kind byte for it (field kinds run `0..5`; the loader rejects `6`), and it needs none: every per-kind drop plan already ignores a zero slot. `?T`'s field kind is therefore `T`'s. Note the consequence diff --git a/runtime/src/builtin.c b/runtime/src/builtin.c index 5e9ee54..d0eb36b 100644 --- a/runtime/src/builtin.c +++ b/runtime/src/builtin.c @@ -367,8 +367,8 @@ int wo_builtin(wo_vm *vm, uint64_t *R, uint32_t ins, const char **msg) { R[A] = (uint64_t)(uintptr_t)out; return 0; } - case WO_B_PARSE_INT: { /* optional-shaped: unparseable is 0, which is - * exactly how a `?Int` spells nil */ + case WO_B_PARSE_INT: { /* optional-shaped: unparseable is WO_NIL_SCALAR, + * how a nullable scalar spells nil (wob.h) */ wo_str *s = native_check(R[B], WO_CLS_STR, msg); if (!s) return WO_T_BOUNDS; uint32_t i = 0; @@ -381,7 +381,7 @@ int wo_builtin(wo_vm *vm, uint64_t *R, uint32_t ins, const char **msg) { acc = acc * 10 + (s->data[i++] - '0'); digits++; } - R[A] = digits ? (uint64_t)(neg ? -acc : acc) : 0; + R[A] = digits ? (uint64_t)(neg ? -acc : acc) : WO_NIL_SCALAR; return 0; } case WO_B_SPLIT: diff --git a/runtime/src/json.c b/runtime/src/json.c index 9a91a39..8af705e 100644 --- a/runtime/src/json.c +++ b/runtime/src/json.c @@ -125,7 +125,9 @@ static void enc_value(jbuf *b, const wo_module *mod, uint64_t v, uint8_t kind, u } switch (kind) { case WO_K_SCALAR: - jb_int(b, (int64_t)v); + /* a nullable scalar holding its nil word is JSON null, not a number */ + if (fclass == WOB_FIELD_NIL_SCALAR && v == WO_NIL_SCALAR) jb_put(b, "null", 4); + else jb_int(b, (int64_t)v); return; case WO_K_TEXT: { const wo_str *s = (const wo_str *)(uintptr_t)v; @@ -307,6 +309,12 @@ static int jparse_object(jp *j, uint32_t class_id, uint64_t *out) { wo_hdr *o = wo_obj_new(j->rt, class_id); if (!o) return -1; uint64_t *fs = wo_fields(o); + /* NEW zeroes every slot, which is nil for a heap-shaped field but a real + `0` for a scalar one — so a nullable scalar starts at its own nil word + (wob.h's WO_NIL_SCALAR) and stays there if the object omits the key. */ + for (uint32_t i = 0; i < c->field_cnt; i++) + if (c->field_class && c->field_class[i] == WOB_FIELD_NIL_SCALAR) + fs[i] = WO_NIL_SCALAR; jskip_ws(j); if (j->p >= j->end || *j->p != '{') { wo_drop_obj(j->rt, o); @@ -387,8 +395,9 @@ static int jparse_value(jp *j, uint8_t kind, uint32_t fclass, uint32_t felem, ui return 0; } char c = *j->p; - if (c == 'n') { /* null: the zero word, for every kind */ - return jskip_value(j) == 0 ? (*out = 0, 0) : -1; + if (c == 'n') { /* null: this field's own nil word */ + uint64_t nilw = fclass == WOB_FIELD_NIL_SCALAR ? WO_NIL_SCALAR : 0; + return jskip_value(j) == 0 ? (*out = nilw, 0) : -1; } if (c == '{') { if ((kind == WO_K_OWNED || kind == WO_K_GCREF) && fclass < j->mod->class_cnt) diff --git a/runtime/src/loader.c b/runtime/src/loader.c index 3146ffd..6b13611 100644 --- a/runtime/src/loader.c +++ b/runtime/src/loader.c @@ -182,7 +182,8 @@ int wo_load_buf(wo_module *m, const uint8_t *buf, size_t len, char *err, if (nm != WOB_NONE && (nm >= m->const_cnt || m->consts[nm].tag != WOB_K_TEXT)) BAIL("class %u field %u: bad name constant", (unsigned)i, (unsigned)j); uint32_t fc = m->metapool[meta_pool + fcnt + j]; - if (fc != WOB_NONE && fc != WOB_FIELD_JSON_RAW && fc >= kcnt) + if (fc != WOB_NONE && fc != WOB_FIELD_JSON_RAW && fc != WOB_FIELD_NIL_SCALAR && + fc >= kcnt) BAIL("class %u field %u: field class out of range", (unsigned)i, (unsigned)j); } m->classes[i].name = name; diff --git a/runtime/src/vm.c b/runtime/src/vm.c index aa4405a..e81beeb 100644 --- a/runtime/src/vm.c +++ b/runtime/src/vm.c @@ -19,15 +19,6 @@ int wo_vm_init(wo_vm *vm, const wo_module *mod, size_t heap_cap) { void wo_vm_destroy(wo_vm *vm) { wo_rt_destroy(&vm->rt); } -/* Trap unwinding — the spec's "traps never leak" promise (spec §6). Walk - * frames innermost to outermost; in each, look up the drop-table entry for - * that frame's current instruction (the trap pc for the innermost frame; - * the instruction before the saved resume pc — i.e. the CALL — for outer - * frames); apply the owned mask by recursive drop and the gc mask by - * decrement, nulling registers as they go. A borrow held by a dying - * register does not block its drop — the borrower IS the dying frame. - * Window overlap is safe: a slot dropped by the callee frame is nulled, so - * an outer mask covering the same physical slot sees 0 and skips. */ /* The drop-table entry governing instruction [pc]: the last one recorded * at or before it. NULL = nothing live there. */ static const wo_dropent *vm_dropent(const wo_methodrec *me, uint32_t pc) { @@ -445,10 +436,20 @@ dispatch: NEXT(); } CASE(EQS) : { + /* Text content equality — and the one comparison that must accept a + * nil operand: two `?Text` values compare with this opcode, and the + * language's answer is "both absent is equal, one absent is not" + * (trapping instead would make `a != b` on optionals unusable). Only a + * NON-nil value still has to actually be a Text. */ + uint64_t bv = R[wo_ins_b(ins)], cv = R[wo_ins_c(ins)]; + if (!bv || !cv) { + R[wo_ins_a(ins)] = bv == cv ? 1 : 0; + NEXT(); + } const char *why; - wo_str *x = str_check(R[wo_ins_b(ins)], &why); + wo_str *x = str_check(bv, &why); if (!x) TRAPF(WO_T_BOUNDS, "%s", why); - wo_str *y = str_check(R[wo_ins_c(ins)], &why); + wo_str *y = str_check(cv, &why); if (!y) TRAPF(WO_T_BOUNDS, "%s", why); R[wo_ins_a(ins)] = wo_str_eq(x, y) ? 1 : 0; NEXT(); diff --git a/runtime/src/wob.h b/runtime/src/wob.h index 748426a..f077efb 100644 --- a/runtime/src/wob.h +++ b/runtime/src/wob.h @@ -40,6 +40,20 @@ * field_elem[i]: for a MULTI field, its element kind; for a MAP field, the * key kind in the low byte and the value kind in the next; 0 otherwise. */ #define WOB_FIELD_JSON_RAW 0xFFFFFFFEu +/* A `?Int`/`?Bool`/`?Timestamp`/`?Id` field. Absence cannot be the zero word + * for a scalar — 0 is a perfectly good Int, and the driving workload stores it + * in a `?Int` (a cron `*` field expands to `0`) — so a nullable SCALAR spells + * nil as WO_NIL_SCALAR instead. Heap-shaped optionals (`?Text`, `?Rec`, + * `?multi`, …) keep the zero word: a null pointer is unambiguous. The marker + * exists so the runtime can tell the two apart where it must write absence + * itself, which today is json.decode leaving an absent key nil. */ +#define WOB_FIELD_NIL_SCALAR 0xFFFFFFFDu + +/* nil for a nullable scalar: -(2^62). Not INT64_MIN, deliberately — the + * compiler's own integers are OCaml's 63-bit native ints, so INT64_MIN is not + * expressible on the emitting side at all. One (absurd) value is unavailable + * inside a `?Int`; that is the whole cost of the choice. */ +#define WO_NIL_SCALAR ((uint64_t)(int64_t)(-4611686018427387904LL)) /* ---- constant pool tags ---- */ #define WOB_K_INT 0u /* tag byte, then i64 */