Repository navigation
Core runtime onto the slot API (#1665 arm D B1) - #1706
Conversation
Batch B1 of the arm D step-2 design. Storage stays Value**. - eigenscript.h / eigenscript.c / trace.c leave the legacy borrow API and join tools/container_migrated_files.txt. - eigs_bool_gate moves below the container.h include and tests list_elem_type (its container_access allowlist row is removed). - The typed number readers (eigs_list_num / eigs_elem_num / eigs_opt_num) are slot-native. - Walkers migrated: ent_child / compute_entropy_impl, free_value, chan_clone_rec, values_equal_impl, value_to_string, write_value_ptr_full, eigs_arg_has_bool's nested scan. - GC edge rows for list items and dict values walk slot_is_ptr / slot_as_ptr and clear by take + slot_decref (edge count snapshotted). - Storage alloc/grow use sizeof(*storage). - Re-pinned 7 exempt shape hashes (helper closure includes a rewritten core helper; no policy change); num_read allowlist updated. Builder gates (Codex Cloud): release 4205/4205, slot-rehearsal 4206/4206, jit-smoke, jit_diff (256 programs), trace-corpus tape bytes identical to B0, bench_dmg_shape Ir +0.0051%, freestanding-check, werror_switch_check. Full ASan did not finish in the builder's window; CI's ASan shards are the leak gate. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KC99CmwatKssQYggkBCgwF
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 82556611da
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (int i = 0; i < v->data.list.count; i++) | ||
| val_decref(list_get_borrow(v, i)); | ||
| if (list_values_storage(v)) | ||
| free(list_values_storage(v)); | ||
| slot_decref(list_slot(v, i)); |
There was a problem hiding this comment.
When EIGS_SLOT_REHEARSAL is enabled, list_slot converts a stored VAL_NUM * into an immediate slot, so slot_decref is a no-op and the backing Value remains at refcount 1. Consequently, destroying numeric lists or dictionaries leaks every numeric child; the same borrowed-slot decref pattern in dict_set_hashed_raw and dict_remove also leaks replaced or removed numbers. These paths need to release the raw backing box, for example through the take helpers, before decrefing the logical slot.
Useful? React with 👍 / 👎.
| int eigs_opt_num(const Value *v, double *out, const char *who) { | ||
| if (!v || v->type == VAL_NULL) return 0; | ||
| if (v->type == VAL_NUM) { *out = VAL_NUM_RAW(v); return 1; } | ||
| eigs_num_arg(v, who); | ||
| return 0; | ||
| return eigs_slot_opt_num(eigs_container_slot((Value *)v), out, who); |
There was a problem hiding this comment.
eigs_opt_num previously treated v == NULL as an absent optional value, as its header contract specifies. The new wrapper encodes NULL as a heap-tagged slot with a zero payload, after which eigs_slot_opt_num evaluates slot_as_ptr(s)->type and segfaults; any C caller using the documented absent-value case now crashes instead of retaining its default. Keep the explicit NULL guard or translate NULL to slot_null().
Useful? React with 👍 / 👎.
| for (int i = 0; i < n; i++) { | ||
| Value *ce = chan_clone_rec(list_get_borrow(v, i), depth + 1); | ||
| EIGS_VIEW(child_view); | ||
| Value *ce = chan_clone_rec(list_get_view(v, i, &child_view), depth + 1); |
There was a problem hiding this comment.
When a channel payload has a numeric element directly below a container at depth 64, list_get_view creates a stack-backed numeric facade and passes it to chan_clone_rec at depth 65. The function's earlier depth fallback then increfs and returns that facade before reaching the VAL_NUM clone case, so the caller stores a stack pointer and the cleanup guard aborts with numeric view refcount moved; a chain of 65 nested lists ending in a number reproduces this through val_clone_for_send. Handle numeric views before the depth fallback, or otherwise ensure the fallback never retains a facade.
Useful? React with 👍 / 👎.
| for (int i = 0; i < v->data.dict.count; i++) { | ||
| /* Keys have table/private-node ownership; do not free individually. */ | ||
| val_decref(dict_value_get_borrow(v, i)); | ||
| slot_decref(dict_value_slot(v, i)); |
There was a problem hiding this comment.
eigs_container_slot leaves stored VAL_NULL and VAL_BOOL singletons encoded as heap slots, so the newly introduced slot_decref calls bypass val_decref's guard that makes those singletons immortal. Appending a singleton does not increase its refcount, but destroying each containing list or dictionary now subtracts one, as do the migrated replacement, removal, and GC paths; after enough drops, free_value attempts to free a static singleton. Encode these values as slot_null()/slot_from_bool() or preserve the immortal-type check in slot refcounting.
Useful? React with 👍 / 👎.
2 participants
Part of #1665: batch B1 of the arm D step-2 design. Storage stays
Value**.What it changes
eigenscript.h,eigenscript.candtrace.cleave the old borrow API and jointools/container_migrated_files.txt. The gate now forbids the old API in them:files=3 violations=0.eigs_bool_gatemoves below thecontainer.hinclude and testslist_elem_type. Its allowlist row is removed:container-access: examined=25 allowed=25.eigs_list_num,eigs_elem_num,eigs_opt_num) are slot-native, which takes their 129+ callers allocation-free after the flip.ent_child/compute_entropy_impl;free_valueandchan_clone_rec;values_equal_implandvalue_to_string;write_value_ptr_full;eigs_arg_has_bool's nested scan.slot_decref.Gates
Builder (Codex Cloud):
jit-smokejit_difffreestanding-check,werror_switch_checkLocal, on main with Stale-read detectors for the surviving pools: ASan poisoning + make jit-checked (#1665) #1705: the build passes with no warnings, and the container, num-read, shape-contract and changelog gates pass.
Full ASan: didn't finish within the builder's window, so CI's ASan shards are the leak gate for this PR.
Left for later batches
eigs_json_encode_value,desc_isolate_constandsandbox_value_has_callablelive inbuiltins.cand move with B3/B4.🤖 Generated with Claude Code
https://claude.ai/code/session_01KC99CmwatKssQYggkBCgwF