negative indices on slc / take / drop (parity with at)#266
Merged
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
resolve_slice_bound, resolve_take_count, resolve_drop_count: small inline helpers that take (raw_i64, len_usize) and return the resolved prefix position. Negative inputs count from the end and clamp at -len, matching the semantics PR #183 already established for `at xs i`. Splitting these out of the engine call sites lets the tree-walker, VM, and Cranelift JIT helpers all dispatch through the same logic — no chance of one backend silently diverging on a `-len` edge case. 8 unit tests covering positive, -1, -len, beyond-len, len=0 boundaries.
slc accepts negative start and end (each resolved against the input length), matching `at xs -1` from PR #183. take and drop accept negative counts: `take -k xs` keeps all but the last k, `drop -k xs` keeps only the last k. Both equivalent to Python's xs[:-k] and xs[-k:]. slc also picks up the same fract!=0 integer-validation check that take and drop already had — previously slc silently cast non-integer floats to usize. Brings the three slice builtins to parity with each other and with at. Routes through the resolve_* helpers in builtins.rs so the rule is defined once and reused by VM + JIT.
VM bytecode handlers and Cranelift JIT helpers (jit_slc, jit_take, jit_drop) now route through the resolve_* helpers, picking up negative counts and bounds. OP_SLC adopts the same fract!=0 integer check the others had — silent float casting on slc indices was the last divergent behaviour between slc and take/drop. Refreshes the stale "must be a non-negative integer" assertions in regression_take_drop.rs: those tests asserted the old VM/tree error and the cranelift TAG_NIL workaround, which all three engines have now stopped doing. New asserts lock the python-style outcome on every engine — same shape across the board, no engine-skip drift.
regression_neg_index_slice.rs: 28 tests covering slc/take/drop with -1, -len, beyond-len bounds on lists and text; the quant-trader fencepost scenario as a named test; the empty-list edge case (every negative bound must be a no-op on len=0); and unicode codepoint slicing to confirm `-3` on text counts codepoints, not bytes. Every test runs on tree, VM, and (under --features cranelift) JIT. examples/negative-indices.ilo: six worked patterns that the engine harness exercises. Documents the canonical idioms — slc xs -1 (len xs) for the last element, slc xs 0 -1 for drop-last, take/drop with negative counts — so an agent encountering the pattern in future has a working reference in-tree.
7d7d813 to
ababe74
Compare
5 tasks
danieljohnmorris
added a commit
that referenced
this pull request
May 15, 2026
- slc / take / drop accept negative indices counting from end (bounds clamp), matching at xs i. Closes the quant-trader fencepost and the slc xs -np 1 np ergonomics gap (#266). - Map keys are typed: text or integer. mset m 7 v and mget m 7 work directly, no str conversion. Int(1) and Text("1") are distinct. Float keys floor to i64; jdmp stringifies numeric keys for JSON (#267). - Add map / flt / fld to the builtin reference. All HOFs (map, flt, fld, srt, grp, uniqby, partition, flatmap) now work cross-engine on tree, VM, Cranelift JIT, and AOT (#274 #277 #278 #279 #280 #283). - New Inline lambdas subsection: Phase 1 literals are cross-engine, Phase 2 closure capture is tree-only with automatic fallthrough surfacing ILO-R012 on VM and Cranelift (#265 #284). - AOT-compiled binaries from ilo compile now strip the top-level ~/^ wrapper byte-for-byte the same as in-process runners (#281).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR #183 added Python-style negative indices to
at xs ilast week. This PR extends the same rule to the slice-shaped builtins so "negative index = count from the end" is uniform across every position-taking primitive:slc xs s e— negativesandeboth resolved againstlen, clamped to[0, len].slc xs -1 (len xs)is the last element as a 1-element list;slc xs 0 -1drops the last element.take n xs— negativenmeans "all but the last|n|", i.e. Python'sxs[:n].take -1 [1,2,3]returns[1,2].drop n xs— negativenmeans "keep only the last|n|", i.e. Python'sxs[n:].drop -1 [1,2,3]returns[3].Closes the quant-trader fencepost (
at eq npILO-R009 because@i 1..nploops producenp-1results), and the olderslc xs -np 1 npergonomics gap. One rule everywhere; nolast xsbuiltin (the token-economics math doesn't justify a new primitive whenat xs -1is 3 tokens and already documented inexamples/at-indexing.ilo).Repro
Before:
After:
Bounds clamp; never wrap, never error on
-99.What's in the diff
Four causally-linked commits:
resolve_slice_bound,resolve_take_count,resolve_drop_countdefined once with 8 unit tests covering positive /-1/-len/ beyond-len / empty boundaries. Lets the three engines dispatch through identical logic, same pattern PR at: Python-style negative indexing #183 established withchar_at_signed.slc,take,droprouted through the helpers.slcpicks up the samefract != 0integer checktake/dropandatalready had (it previously cast floats silently).regression_take_drop.rsasserts that locked in the old "non-negative" errors plus the cranelift TAG_NIL workaround.regression_neg_index_slice.rsexercising the matrix (3 engines ×-1/-len/beyond-len/0/len× list/text) plus the quant-trader fencepost as a named test, plusexamples/negative-indices.iloso the engine harness exercises six canonical patterns and agents have a worked reference in-tree.One deliberate tightening worth flagging:
slcindices now requirefract == 0. Previously the tree-walker and VM both silently cast non-integer floats like1.5tousize. This bringsslcto parity withtake,drop, andat, and the diagnostic is precise ("slc: start index must be an integer"). Vanishingly unlikely to break a real program but called out so anyone hitting it sees the deliberate boundary.Test plan
cargo test --release --features cranelift— full suite green (the one transient AOT-test flake is pre-existing — thevm::compile_cranelift::tests::aot_*tests share/tmp/ilo_test_aot_*filenames; passes in isolation).regression_neg_index_slice.rscover slc / take / drop on lists and text, including-1,-len, beyond--len, unicode codepoints, empty-list edge case, and the quant-trader fencepost.regression_take_drop.rslocks the new behaviour across all three engines (previously asserted error on tree/VM and nil on cranelift — that drift is gone).examples/negative-indices.ilocovers six idioms exercised byexamples_engines.rson every engine.cargo clippy --release --features cranelift --all-targets -- -D warningsclean.cargo fmt --checkclean.Follow-ups
slc xs -np 1 npparse friction in the assessment doc (line 753 ofilo_assessment_feedback.md) is mostly a-nplexer/parser issue separate from the slc semantics fixed here. Now that the semantics are right, that entry's workaround pattern goes away: a user can writeslc xs 0 -1directly without bindings=- np 1first.