Skip to content

fix: cranelift JIT helper nil-sweep batch 1 (lst, index, slc, jpth, listget)#264

Merged
danieljohnmorris merged 3 commits into
mainfrom
fix/jit-nil-sweep-batch1
May 14, 2026
Merged

fix: cranelift JIT helper nil-sweep batch 1 (lst, index, slc, jpth, listget)#264
danieljohnmorris merged 3 commits into
mainfrom
fix/jit-nil-sweep-batch1

Conversation

@danieljohnmorris
Copy link
Copy Markdown
Collaborator

Summary

Batch 1 of the Cranelift JIT permissive-nil sweep. Companion to #259 (batch 2: recfld / recfld_name / unwrap / mget) and #254 (the JIT_RUNTIME_ERROR TLS channel itself). These helpers used to collapse type and range failures into a silent TAG_NIL return, hiding bugs that tree/VM already diagnose. After this PR, every engine surfaces the same runtime error on the same input.

Manifesto framing: fewer engine-conditional retries. An agent that hits lst xs 999 v should learn it is out of range whether the program is interpreted or JITed. Today the tree path teaches that, the Cranelift path silently passes a stale list along, the next operation fails for a different reason, and the agent has to triangulate. This closes the loop.

Repro before / after

# Before this PR
$ ilo "f>L n;lst [1,2,3] 5 99" --run-cranelift f
[1, 2, 3]   # silent nil-sweep: returns the input list, exit 0

# Tree and VM already errored
$ ilo "f>L n;lst [1,2,3] 5 99" --run-tree f
{"code":"ILO-R009","message":"lst index 5 out of range","severity":"error"}

# After this PR (all three engines)
$ ilo "f>L n;lst [1,2,3] 5 99" --run-cranelift f
{"code":"ILO-R004","message":"lst: index out of range","severity":"error"}

Same story for xs.5 on a 3-element list, lset xs -1 v, slc xs "a" "b" (non-number indices), and jpth 42 "a" (non-string args).

What's in the diff

  1. jit: route lst/index/slc/jpth/listget failures through JIT_RUNTIME_ERROR (src/vm/mod.rs): per-helper edits.
    • jit_lst: error on OOB, negative idx, fractional idx, non-number idx, non-list. Diagnostic strings match tree/VM wording.
    • jit_index: error on OOB and non-list (verifier-blocked but defensive).
    • jit_slc: error on non-number indices and non-list/non-text args only. OOB clamping is the documented tree/VM contract and is left alone — inline comment notes the asymmetry.
    • jit_jpth: error on non-string args. Path miss still returns Err(...) as a Result value.
    • jit_listget: error on non-list collection and non-number idx. OOB stays TAG_NIL because it is the foreach loop-done sentinel.
    • Helper-level unit tests for jit_lst updated to assert TAG_NIL + the JIT_RUNTIME_ERROR cell.
  2. test + example: cross-engine regression for jit nil-sweep batch 1: new tests/regression_jit_nil_sweep_batch1.rs (28 tests across tree / VM / Cranelift) and examples/jit-nil-sweep-batch1.ilo (positive paths exercised by the examples_engines harness). Includes negative regressions for the deliberately permissive paths (slc OOB clamp on list + text, jpth path-miss returning Err) and a stale-error-leak guard for the new helpers.
  3. test: update suites that pinned the old permissive-nil contract: four pre-existing tests in eval_inline.rs, regression_dot_list_index.rs, regression_list_mutation.rs, and regression_lset_alias.rs flipped from asserting the old passthrough to asserting the new error parity.

Test plan

  • cargo build --release --features cranelift
  • cargo test --release --features cranelift (full suite green)
  • cargo clippy --release --features cranelift --all-targets -- -D warnings
  • cargo fmt --check
  • New regression file: 28 tests pass across tree, VM, Cranelift
  • examples_engines harness runs the new example across all three engines
  • Stale-error-leak guard: successful run after an erroring one in a fresh process must not inherit the TLS error

Follow-ups

  • The helpers in this batch can adopt the span-id immediate pattern (one-line iconst per call site + one extra u64 arg in the signature) when that PR lands. The span-id PR is in flight on a parallel branch; no rebase blocker either direction.
  • Batches 1 and 2 together complete the explicit permissive-nil audit. If a third batch surfaces it would be helpers found by future fuzzing or by audit of opcodes that didn't appear in the original brief.

Batch 1 of the Cranelift permissive-nil sweep, following #259 (batch 2)
and #254 (the TLS channel itself). These helpers previously collapsed
type and range failures into a silent TAG_NIL return, hiding bugs that
tree/VM already diagnose.

Per-helper decisions:
- jit_lst: error on OOB, negative idx, non-integer idx, non-number idx,
  non-list. Message text matches the tree/VM diagnostic so the
  cross-engine assertions stay tight.
- jit_index: error on OOB and on non-list. The non-list arm is
  largely verifier-blocked but kept defensive.
- jit_slc: error on non-number indices and non-list/non-text args
  only. OOB clamping is the documented contract on tree/VM (slc
  saturates), so do NOT surface a runtime error for out-of-range
  start/end. Inline comment notes the asymmetry.
- jit_jpth: error on non-string args. Path miss correctly returns
  Err(...) as a Result value, not a runtime error, so that path is
  unchanged.
- jit_listget: error on non-list collection and non-number idx.
  OOB stays TAG_NIL because it is the foreach loop-done sentinel
  that matches OP_LISTGET's fall-through-to-JMP semantic. Inline
  comment documents the split.

Existing unit tests for jit_lst that asserted the old passthrough
behaviour are updated to take the JIT_RUNTIME_ERROR cell and assert
both the TAG_NIL return and the diagnostic message.

Helpers in this batch can adopt the span-id immediate pattern when
that PR lands; the pattern is +1 u64 arg per helper + 1 iconst per
call site.
Pin parity across tree, VM, and Cranelift for the helpers in this
batch:
- jit_lst OOB / negative-index / happy
- jit_index (xs.N) OOB / happy
- slc OOB clamping (list + text) — deliberately NOT errors
- jpth path-miss returns Err(...), not a runtime error
- jpth happy path

Also pins the stale-error-leak guard from #254 for the new helpers:
a successful run after an erroring one must not inherit the TLS
error cell.

The example file exercises the positive paths so the examples_engines
harness runs them on every build. Error paths are covered by the
regression file; an example file that errors would need -- err:
annotations and adds little over the test.
Four pre-existing tests asserted the Cranelift JIT's now-removed
behaviour of silently returning nil (or the input list) on lst OOB,
lst negative idx, xs.N OOB, and lset OOB. Each one is updated to
expect the runtime error that all three engines now surface.

- eval_inline::run_default_interpreter_error: xs.0 on []
- regression_dot_list_index::dot_index_out_of_range_cranelift: xs.5
- regression_list_mutation::lst_out_of_range_cranelift +
  lst_negative_cranelift: collapsed onto the same check_oor_error /
  shared assertion shape as the tree and VM siblings.
- regression_lset_alias::lset_oob_cranelift_passthrough renamed to
  lset_oob_cranelift_errors. lset is the surface alias of lst so the
  parity must hold.

No behaviour change; these were contract pins on what is now the old
behaviour. Catching them in this PR rather than in a follow-up keeps
the suite green.
@codecov
Copy link
Copy Markdown

codecov Bot commented May 13, 2026

Codecov Report

❌ Patch coverage is 91.30435% with 4 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/vm/mod.rs 91.30% 4 Missing ⚠️

📢 Thoughts on this report? Let us know!

@danieljohnmorris danieljohnmorris merged commit a39cb40 into main May 14, 2026
5 checks passed
@danieljohnmorris danieljohnmorris deleted the fix/jit-nil-sweep-batch1 branch May 14, 2026 10:30
danieljohnmorris added a commit that referenced this pull request May 15, 2026
Group C of the JIT-helper permissive-nil sweep (batches 1/2/3/4 landed in
PRs #264, #259, #282, #281).

Helpers in this batch: jit_spl, jit_cat, jit_has, jit_range, jit_window,
jit_zip, jit_chunks, jit_enumerate, jit_setunion, jit_setinter,
jit_setdiff, jit_rev, jit_srt, jit_rsrt, jit_cumsum.

Before: every helper above silently returned TAG_NIL on the type-error
path (TAG_FALSE for the two jit_has type-mismatch arms), diverging from
the tree walker and the bytecode VM, which both raise VmError::Type with
a specific message. Agents hitting srt on a mixed-type list, or cat on a
list of numbers, got nil where they expected an error to retry off.

After: each helper signals via jit_set_runtime_error_with_span using the
same wording the VM dispatcher uses ("spl requires two strings",
"cat: list items must be text", "setop: elements must be text, number,
or bool", "srt: list must contain all numbers or all text", etc.), so
the JIT entry point synthesises a VmRuntimeError that renders with a
caret matching tree/VM diagnostics.

Mechanics:
  - extern "C" signatures grow a span_bits: u64 immediate (packed
    (start << 32) | end). 2-arg helpers become 3-arg; 1-arg helpers
    become 2-arg.
  - jit_srt and jit_rsrt now distinguish mixed-list (raises
    "...must contain all numbers or all text") from non-list/non-text
    (raises "...requires a list or text") instead of falling through
    to a single nil sentinel.
  - jit_setop_impl threads span_bits and surfaces both the per-arg
    "setop arg N requires a list" messages and the per-element
    "setop: elements must be text, number, or bool" message that the
    VM dispatcher uses.

Unit tests inside vm::tests::jit_helpers updated: the *_returns_nil and
*_returns_false assertions now assert the runtime-error cell is set and
carries the expected VmError variant. New mixed-list test for jit_srt
pins the previously-silent diverging path.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant