runtime: ValType switches go exhaustive, finishing #738 — first build caught real drift by InauguralPhysicist · Pull Request #806 · InauguralSystems/EigenScript

InauguralPhysicist · 2026-08-02T08:58:05Z

Closes #738

What

The ASTType half of #738 landed earlier (9d970d4..4d8c617); this PR finishes the issue by deleting the remaining default: arms over closed enums and writing out the enumerators they silently absorbed:

val_type_name (the issue's named example — default: return "?" is now a post-switch fallback)
the Value destructor (a missed case here was the issue's "silent leak" scenario; NUM/BUILTIN/NULL are now named as owning nothing)
values_equal, chan_clone_rec (clone-vs-share is now an explicit per-type decision)
GC_FOR_EACH_CHILD and gc_clear_node (leaf types named — the two stay in enforced lockstep when a ValType is added)
eigs_value_type (embed API), store_json_encode, both trace-tape value formatters, the observer dump, OP_SLICE's type check
one ASTType straggler: W022's binding collector (added by docs+lint: state the arity-sensitive spread rule; W022 flags silent over-arity (#733) #802 after the sweep) had reintroduced a default: arm — the exact drift mode the issue predicted, caught and cleared

Planted-fault verified: an 11th ValType fails the build at 16 sites, a 34th ASTType at 73 — each error naming the file and line that hasn't been taught about it.

Drift the enforcement immediately found

The very first -Werror=switch compile flagged real gaps:

store_json_encode did not handle VAL_BUFFER: a buffer stored through ext_store silently encodes as JSON null — data loss. Kept behavior-preserving here; filed as ext_store silently encodes VAL_BUFFER as null — buffers are lost on store round-trip #805.
the tape's full nondet formatter renders VAL_JSON_RAW/VAL_TEXT_BUILDER as opaque <heap> where the short form has <json>/<text> (latent — no current nondet builtin returns them; noted in ext_store silently encodes VAL_BUFFER as null — buffers are lost on store round-trip #805). Output kept byte-identical.

Everything else is strictly mechanical; suite output is byte-identical.

Kept their `default:` on purpose

Untrusted bytecode bytes (the leaf-accessor scan, the non-GNU dispatch arm), guarded subsets (JIT binop/cmp emitters, compound-assign tokens), and TokType switches (outside #738's named scope).

The CLAUDE.md half

The false "#709 made a new ValType a build error" claim the issue flagged was already removed by the #734 CLAUDE.md rework — nothing to correct. The property it asserted is now actually true, and .claude/rules/c-runtime-memory.md states the no-default: discipline next to the GC lockstep rule it enforces.

Gate

release suite 3364/3364
ASan/UBSan detect_leaks=1 3368/3368, leak tally 0, section [87] clean
make http / make gfx / make freestanding-check all clean

🤖 Generated with Claude Code

… caught real drift The ASTType half of #738 landed earlier (9d970d4..4d8c617); this finishes the issue. The remaining default: arms over closed enums are deleted and their absorbed enumerators written out at 12 sites: val_type_name, the Value destructor, values_equal, chan_clone_rec, GC_FOR_EACH_CHILD, gc_clear_node, eigs_value_type, store_json_encode, both trace-tape value formatters, the observer dump, and OP_SLICE's type check. A new ValType is now a build error at 16 sites (verified by planting an 11th enumerator; a planted 34th ASTType fails at 73). One ASTType straggler cleared: W022's binding collector (added by #802 AFTER the sweep) had reintroduced a default: arm — the exact drift mode the issue predicted. The first -Werror=switch compile found real drift: store_json_encode did not handle VAL_BUFFER, so buffers stored via ext_store silently encode as null (data loss — behavior preserved here, filed as #805); and the tape's full nondet formatter renders VAL_JSON_RAW/ VAL_TEXT_BUILDER as "<heap>" where the short form has "<json>"/ "<text>" (latent, noted in #805). All arms are otherwise strictly mechanical — suite output is byte-identical. Kept their default: on purpose — untrusted bytecode bytes (leaf- accessor scan, non-GNU dispatch), guarded subsets (JIT binop/cmp, compound-assign tokens), and TokType (out of #738's scope). The CLAUDE.md #709 claim the issue flagged was already removed by the #734 rework; the build-error property it asserted is now actually true, and .claude/rules/c-runtime-memory.md states the discipline. Gate: release 3364/3364; ASan/UBSan detect_leaks=1 3368/3368, leak tally 0; make http / gfx / freestanding-check clean. Closes #738 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot

Pull request overview

This PR completes the #738 sweep by removing default: arms from switches over closed enums (primarily ValType, plus one ASTType straggler), so -Werror=switch enforces exhaustiveness and catches drift at compile time across the runtime, trace, embed API, lint, and ext_store.

Changes:

Converted remaining ValType switches from default: catch-alls to explicit enumerations across runtime behavior (type names, destructor, equality, cloning, GC traversal/clearing, embedding, JSON store encoding, trace/tape formatting, observer dump, and slicing type checks).
Removed a reintroduced default: in W022’s AST binding-collector switch so new ASTType values become build errors there.
Documented the exhaustiveness enforcement in CHANGELOG.md and .claude/rules/c-runtime-memory.md.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/vm.c	Makes observer dump and `OP_SLICE_GET` type handling explicit per-`ValType`.
src/trace.c	Makes trace/tape `ValType` renderings explicit (no `default:`).
src/lint.c	Makes W022’s binding collector explicit per-`ASTType` (no `default:`).
src/ext_store.c	Makes store JSON encoding behavior explicit for previously-absorbed `ValType`s.
src/eigs_embed.c	Makes embed API `ValType`→`EigsValueType` mapping explicit for “other” types.
src/eigenscript.c	Makes core runtime `ValType` behaviors explicit (naming, free, clone/share, equality, GC macros).
CHANGELOG.md	Documents the #738 ValType exhaustiveness sweep and drift caught by enforcement.
.claude/rules/c-runtime-memory.md	Documents the “no `default:` on closed-enum switches” rule for future contributors.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+            /* Not sliceable. Enumerated rather than covered by a `default:`
+             * so -Werror=switch forces a new ValType to choose here. */
+            case VAL_NUM: case VAL_FN: case VAL_BUILTIN: case VAL_NULL:
+            case VAL_JSON_RAW: case VAL_DICT: case VAL_TEXT_BUILDER:
                rt_error(EK_TYPE, g_vm.current_line, "cannot slice %s",


+        /* VAL_NULL/VAL_FN/VAL_BUILTIN are handled by the guard above; raw
+         * JSON, text builders and buffers have no store encoding (buffers
+         * silently store as null — a data-loss gap this enumeration made
+         * visible; tracked upstream). Enumerated rather than covered by a
+         * `default:` so -Werror=switch forces a new ValType to choose its
+         * store-JSON encoding here. */


            case VAL_TEXT_BUILDER:
                snprintf(buf, nbuf, "<text-builder:%zu>", v->data.text_builder.len); break;
            case VAL_JSON_RAW: snprintf(buf, nbuf, "<json-raw>"); break;
-            default:         snprintf(buf, nbuf, "null"); break;
+            /* No `default:` — -Werror=switch (Makefile CFLAGS) forces a new
+             * ValType to choose its observer-dump rendering here. */


+        /* No `default:` — -Werror=switch (Makefile CFLAGS) forces a new
+         * ValType to choose its tape rendering here. */
    }


+  are otherwise strictly mechanical — suite output is byte-identical.
+  Legitimately open switches keep their `default:`: untrusted bytecode
+  bytes (the leaf-accessor scan, the non-GNU dispatch arm), guarded
+  subsets (JIT binop/cmparm emitters, compound-assign tokens), and


All four #739 items landed earlier (#761 trace teardown, #762 g_exit_requested, #764 task-suspend + s_osr_threshold/g_sandbox_*/ g_stream_file/g_db_conn). The two deliberate remainders resolve as: - g_db_conn: merged untested for lack of libpq; the db-extension CI job (full variant, live postgres) has been green on every run since, including PR #806 — executed and verified by CI. - g_model: stays process-wide BY DESIGN. Per-state weights multiply a transformer's memory by the number of states and no consumer forces multi-state model isolation. The issue's own standard is the g_vm_abort_flag contrast — a reasoned, documented opt-out vs a silent one — so the decision now lives at the declaration. Comment-only diff; make http compiles clean, binary unchanged. Closes #739 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…auguralSystems#743) gc_clear_node missed the VAL_FN -> compiled-chunk edge that GC_FOR_EACH_CHILD reports (body_count == -1, the OP_CLOSURE ref); the ref was only recovered later by the value destructor in the unpin pass -- a third mirror the lockstep comment never named. Both dispatches are now generated from one GC_EDGE_TABLE X-macro (one row per owned edge: walk predicate + clear action side by side), so a new owning edge out of Value/Env/Chunk is added exactly once and the two cannot drift. The value destructor's chunk_decref is documented as the non-cycle mirror. Two safety invariants of the VAL_FN chunk row are now spelled out at the table: GC_EDGE_CLEAR ignores IS_NODE, so the unconditional chunk_decref((EigsChunk *)fn.body) is type-safe ONLY because make_fn sets body = NULL (OP_CLOSURE is its sole real-chunk writer, body_count == -1) and chunk_decref(NULL) no-ops; and the `body = NULL` in that CLEAR is load-bearing -- it makes the value destructor's later chunk_decref a no-op rather than a double-decref (use-after-free). CallFrame init was open-coded at every frame push and release at every saved-frame teardown loop. All 4 push sites (jit_helper_call, vm_run_ex entry, OP_CALL, OP_DISPATCH) now use callframe_init. callframe_release is declared in vm.h (non-static) so ALL THREE saved-frame teardown loops -- task_sched_thread_free and task_do_kill (vm.c) and task_free (builtins.c) -- share the one definition of the owned-field set {env iff owns_env, chunk}; no hand-encoded CallFrame release remains at any teardown site. The live-frame POP paths (OP_RETURN, vm_error_halt, the CHECK_ERROR unwind) keep their inline drops on purpose: they also drain the operand-stack window and restore the per-frame loop-stall globals, and OP_RETURN parks reusable envs / defers the chunk ref -- a plain release would be a subset. The vm.h owned-set comment names all three teardown sites and flags those pop paths for audit. The refactor also fixes a real bug it was designed to catch: the OP_DISPATCH push never stamped saved_stall_count/saved_loop_iter, so on RETURN the interpreted-dispatch path restored those unstamped counter values instead of the correct saved ones. This restore-on-RETURN is new, corrected behavior on the dispatch path, not pre-InauguralSystems#743 parity, and a probe makes the change observable: pre-fix a dispatch-per-iteration loop's __loop_iterations__ froze at a wrong value (1) after the first dispatch RETURN; post-fix it tracks the loop (41..44). Rebased onto cd9f82e, which carries InauguralSystems#806 (the ValType switches went exhaustive, gc_clear_node and GC_FOR_EACH_CHILD among them). Both conflicts resolve to this table: it supersedes the two hand-written dispatches, and InauguralSystems#806's own edit to gc_clear_node's VAL_FN case did NOT add the missing chunk edge, so the bug this closes is still live on main. InauguralSystems#806's compile-time forcing is preserved rather than dropped: because the table's rows are selected by `_v->type == VAL_x` guards, which -Werror=switch cannot see, gc_value_is_node now carries the gate (exhaustive switch, no default: arm; a value type is a node exactly when it has rows). Verified by planting an 11th enumerator -- the build fails at gc_value_is_node, as it does at InauguralSystems#806's other 15 sites. Gates (umask 022, on the rebase onto cd9f82e): CC=gcc ./build.sh rc=0; release run_all_tests.sh 3364/3364; make asan rc=0; the suite under CI's sanitizer env (ASAN_OPTIONS=detect_leaks=1, UBSAN_OPTIONS halt_on_error + print_stacktrace) 3368/3368, zero LeakSanitizer/AddressSanitizer reports; [87] closure-cycle strict leak gate clean direct under LSan (17/17, rc=0) and [101] test_spawn_gc rc=0. The task_free conversion is proven leak-free under ASan+LSan by a probe that suspends owns_env worker frames inside a try and tears them down via task_free at exit with zero findings (byte-identical output before and after the rebase). Co-Authored-By: Kimi K3 <noreply@kimi.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

… one CallFrame init/release (#810) * gc+vm: single-definition GC edge table and CallFrame init/release (#743) gc_clear_node missed the VAL_FN -> compiled-chunk edge that GC_FOR_EACH_CHILD reports (body_count == -1, the OP_CLOSURE ref); the ref was only recovered later by the value destructor in the unpin pass -- a third mirror the lockstep comment never named. Both dispatches are now generated from one GC_EDGE_TABLE X-macro (one row per owned edge: walk predicate + clear action side by side), so a new owning edge out of Value/Env/Chunk is added exactly once and the two cannot drift. The value destructor's chunk_decref is documented as the non-cycle mirror. Two safety invariants of the VAL_FN chunk row are now spelled out at the table: GC_EDGE_CLEAR ignores IS_NODE, so the unconditional chunk_decref((EigsChunk *)fn.body) is type-safe ONLY because make_fn sets body = NULL (OP_CLOSURE is its sole real-chunk writer, body_count == -1) and chunk_decref(NULL) no-ops; and the `body = NULL` in that CLEAR is load-bearing -- it makes the value destructor's later chunk_decref a no-op rather than a double-decref (use-after-free). CallFrame init was open-coded at every frame push and release at every saved-frame teardown loop. All 4 push sites (jit_helper_call, vm_run_ex entry, OP_CALL, OP_DISPATCH) now use callframe_init. callframe_release is declared in vm.h (non-static) so ALL THREE saved-frame teardown loops -- task_sched_thread_free and task_do_kill (vm.c) and task_free (builtins.c) -- share the one definition of the owned-field set {env iff owns_env, chunk}; no hand-encoded CallFrame release remains at any teardown site. The live-frame POP paths (OP_RETURN, vm_error_halt, the CHECK_ERROR unwind) keep their inline drops on purpose: they also drain the operand-stack window and restore the per-frame loop-stall globals, and OP_RETURN parks reusable envs / defers the chunk ref -- a plain release would be a subset. The vm.h owned-set comment names all three teardown sites and flags those pop paths for audit. The refactor also fixes a real bug it was designed to catch: the OP_DISPATCH push never stamped saved_stall_count/saved_loop_iter, so on RETURN the interpreted-dispatch path restored those unstamped counter values instead of the correct saved ones. This restore-on-RETURN is new, corrected behavior on the dispatch path, not pre-#743 parity, and a probe makes the change observable: pre-fix a dispatch-per-iteration loop's __loop_iterations__ froze at a wrong value (1) after the first dispatch RETURN; post-fix it tracks the loop (41..44). Rebased onto cd9f82e, which carries #806 (the ValType switches went exhaustive, gc_clear_node and GC_FOR_EACH_CHILD among them). Both conflicts resolve to this table: it supersedes the two hand-written dispatches, and #806's own edit to gc_clear_node's VAL_FN case did NOT add the missing chunk edge, so the bug this closes is still live on main. #806's compile-time forcing is preserved rather than dropped: because the table's rows are selected by `_v->type == VAL_x` guards, which -Werror=switch cannot see, gc_value_is_node now carries the gate (exhaustive switch, no default: arm; a value type is a node exactly when it has rows). Verified by planting an 11th enumerator -- the build fails at gc_value_is_node, as it does at #806's other 15 sites. Gates (umask 022, on the rebase onto cd9f82e): CC=gcc ./build.sh rc=0; release run_all_tests.sh 3364/3364; make asan rc=0; the suite under CI's sanitizer env (ASAN_OPTIONS=detect_leaks=1, UBSAN_OPTIONS halt_on_error + print_stacktrace) 3368/3368, zero LeakSanitizer/AddressSanitizer reports; [87] closure-cycle strict leak gate clean direct under LSan (17/17, rc=0) and [101] test_spawn_gc rc=0. The task_free conversion is proven leak-free under ASan+LSan by a probe that suspends owns_env worker frames inside a try and tears them down via task_free at exit with zero findings (byte-identical output before and after the rebase). Co-Authored-By: Kimi K3 <noreply@kimi.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * tests: pin the OP_DISPATCH loop-counter stamping (#743) Lands per review on #810: the collector half of #743 cannot be pinned (the value destructor masks the missed edge, which is why the planted-enumerator probe is the evidence there), but the loop-counter half is user-visible and red on pre-fix main, so it gets a regression test. Three checks in section [68], mid-file with the other OP_DISPATCH pins; the runner derives its tally from the file's own "Tests: N" line, so no run_all_tests.sh change is needed. The pin is on the DELTA, not on a count. Measuring first showed __loop_iterations__ is a cumulative process-wide counter, not a per-loop one: on this box a dispatch-per-iteration loop reads 105 where the plain loop before it read 88, because the reading carries every earlier loop's iterations. So "the dispatch loop's counter equals its no-dispatch twin's counter" is not true as a raw-value comparison in a program with more than one loop -- what holds is that each loop CONTRIBUTES exactly its own iteration count. Both loops are asserted against that invariant, comparing each loop's delta with a count it keeps by hand, so the pin survives any retuning of the convergence heuristic. Hardcoding the number would have been wrong anyway: the reviewer's box prints 34 where this one prints 17. Two shapes the test needs to stay honest. A third check asserts the dispatch loop ran more than one iteration -- without it a single-iteration loop would satisfy the delta check even with the counter frozen at 1, and the pin would be vacuous. And a warm-up loop establishes the baseline the two measured loops subtract, because __loop_iterations__ is an undefined variable until some loop has run (the pre-fix binary is what surfaced that: it failed on the baseline read before ever reaching the assertion). Mutation-proved rather than assumed. Against a release build of upstream/main at cd9f82e (pre-fix, built in a worktree): "FAIL: dispatch loop contributes its own iteration count", Dispatch Tests: 17/18, rc=1. Against this branch: 18/18, "All tests passed", rc=0. Exactly one check flips -- the plain-loop and non-vacuity checks pass on both, as they should. Gates (umask 022): CC=gcc ./build.sh rc=0, release run_all_tests.sh 3367/3367; make asan rc=0, suite under CI's sanitizer env (ASAN_OPTIONS=detect_leaks=1, UBSAN_OPTIONS halt_on_error + print_stacktrace) 3371/3371 with zero LeakSanitizer/AddressSanitizer reports and no leak-tally note; test_dispatch.eigs run directly under ASan+LSan rc=0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Kimi K3 <noreply@kimi.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings August 2, 2026 08:58

Copilot started reviewing on behalf of InauguralPhysicist August 2, 2026 08:58 View session

Copilot AI reviewed Aug 2, 2026

View reviewed changes

InauguralPhysicist merged commit 9945c52 into main Aug 2, 2026
19 checks passed

InauguralPhysicist deleted the fix-738-valtype-switch-exhaustive branch August 2, 2026 09:14

InauguralPhysicist mentioned this pull request Aug 2, 2026

Multi-state boundary: trace.c is entirely process-global and ext_http's per-connection worker calls trace_shutdown(), destroying the process tape on every request #739

Closed

Nitjsefnie mentioned this pull request Aug 2, 2026

fix(gc,vm): generate the two lockstep dispatches from one edge table; one CallFrame init/release #810

Merged

5 tasks

Nitjsefnie mentioned this pull request Aug 2, 2026

fix(store): round-trip VAL_BUFFER through ext_store instead of encoding it as null #814

Open

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

runtime: ValType switches go exhaustive, finishing #738 — first build caught real drift - #806

runtime: ValType switches go exhaustive, finishing #738 — first build caught real drift#806
InauguralPhysicist merged 1 commit into
mainfrom
fix-738-valtype-switch-exhaustive

InauguralPhysicist commented Aug 2, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

InauguralPhysicist commented Aug 2, 2026

What

Drift the enforcement immediately found

Kept their default: on purpose

The CLAUDE.md half

Gate

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Kept their `default:` on purpose