diff --git a/examples/negative-indices.ilo b/examples/negative-indices.ilo new file mode 100644 index 00000000..8695eb58 --- /dev/null +++ b/examples/negative-indices.ilo @@ -0,0 +1,35 @@ +-- Negative indices on slice-shaped builtins count from the end of the +-- list or text, matching Python semantics already in place for `at xs i`. +-- +-- slc xs s e accepts negative s and e. `slc xs -1 (len xs)` is the +-- last element as a 1-element list; `slc xs 0 -1` drops +-- the last element; bounds clamp, never wrap. +-- take n xs with n<0 keeps all but the last |n|. Equivalent to +-- Python's xs[:n]. `take -1 [10,20,30]` is [10,20]. +-- drop n xs with n<0 keeps only the last |n|. Equivalent to +-- Python's xs[n:]. `drop -1 [10,20,30]` is [30]. +-- +-- Closes the quant-trader fencepost (`@i 1..np` loops produce np-1 results, +-- so the equity curve ends at length np; `at eq np` errored, and the +-- workaround was `npm=- np 1`). With negative indices everywhere, the same +-- pattern is `slc eq 0 -1` — no separate binding, no off-by-one risk. + +last-element xs:L n>L n;slc xs -1 (len xs) +drop-last xs:L n>L n;slc xs 0 -1 +keep-last-two xs:L n>L n;slc xs -2 (len xs) +all-but-last xs:L n>L n;take -1 xs +only-last-two xs:L n>L n;drop -2 xs +trim-edges s:t>t;slc s 1 -1 + +-- run: last-element [10,20,30,40,50] +-- out: [50] +-- run: drop-last [10,20,30,40,50] +-- out: [10, 20, 30, 40] +-- run: keep-last-two [10,20,30,40,50] +-- out: [40, 50] +-- run: all-but-last [10,20,30,40,50] +-- out: [10, 20, 30, 40] +-- run: only-last-two [10,20,30,40,50] +-- out: [40, 50] +-- run: trim-edges "(quoted)" +-- out: quoted diff --git a/src/builtins.rs b/src/builtins.rs index 951a5abd..41363289 100644 --- a/src/builtins.rs +++ b/src/builtins.rs @@ -556,6 +556,57 @@ pub(crate) fn char_at_signed(s: &str, raw_idx: i64) -> CharAtResult { } } +/// Resolve a Python-style signed slice bound against `len`. +/// +/// - `raw >= 0`: clamp to `[0, len]`. +/// - `raw < 0`: treat as `len + raw`, then clamp to `[0, len]`. So `-1` on a +/// length-5 list becomes index `4` (the last element); `-5` becomes `0`; +/// `-99` clamps to `0`. +/// +/// Returned index is always in `[0, len]`, so callers can use it directly +/// as a slice bound without further checks. Matches the semantics already +/// applied to `at`'s negative index handling (`adjusted = if i < 0 { i + len } +/// else { i }`) — see `Builtin::At` in the tree-walker and `OP_AT` in the VM. +#[inline] +pub(crate) fn resolve_slice_bound(raw: i64, len: usize) -> usize { + let len_i = len as i64; + let adjusted = if raw < 0 { raw + len_i } else { raw }; + adjusted.clamp(0, len_i) as usize +} + +/// Resolve `take n xs` against `len`, returning the prefix length to retain. +/// +/// - `n >= 0`: take the first `min(n, len)` elements. +/// - `n < 0`: take all but the last `|n|`. Equivalent to Python's `xs[:n]`. +/// `take -1 [1,2,3]` returns `[1,2]`; `take -len xs` returns `[]`; `n` more +/// negative than `-len` clamps to `0` (empty). +#[inline] +pub(crate) fn resolve_take_count(n: i64, len: usize) -> usize { + if n >= 0 { + (n as usize).min(len) + } else { + let adjusted = (len as i64) + n; + adjusted.max(0) as usize + } +} + +/// Resolve `drop n xs` against `len`, returning the prefix length to skip. +/// +/// - `n >= 0`: skip the first `min(n, len)` elements. +/// - `n < 0`: keep only the last `|n|`, i.e. skip the leading `len - |n|`. +/// Equivalent to Python's `xs[n:]`. `drop -1 [1,2,3]` returns `[3]`; +/// `drop -len xs` returns the full list; `n` more negative than `-len` +/// clamps to `0` (returns the full list). +#[inline] +pub(crate) fn resolve_drop_count(n: i64, len: usize) -> usize { + if n >= 0 { + (n as usize).min(len) + } else { + let adjusted = (len as i64) + n; + adjusted.max(0) as usize + } +} + #[cfg(test)] mod tests { use super::*; @@ -684,6 +735,83 @@ mod tests { assert_eq!(Builtin::from_name(""), None); } + #[test] + fn resolve_slice_bound_positive_and_clamps() { + // Within range: returned as-is. + assert_eq!(resolve_slice_bound(0, 5), 0); + assert_eq!(resolve_slice_bound(3, 5), 3); + assert_eq!(resolve_slice_bound(5, 5), 5); + // Past len: clamps up to len (matches existing slc behaviour). + assert_eq!(resolve_slice_bound(99, 5), 5); + } + + #[test] + fn resolve_slice_bound_negative_python_style() { + // -1 is the last index; -len is 0; beyond -len clamps to 0. + assert_eq!(resolve_slice_bound(-1, 5), 4); + assert_eq!(resolve_slice_bound(-5, 5), 0); + assert_eq!(resolve_slice_bound(-99, 5), 0); + } + + #[test] + fn resolve_slice_bound_empty_list() { + // len=0 makes every bound clamp to 0 — slc of an empty list always + // returns empty, never errors. The fencepost-trap case in the + // quant-trader run. + assert_eq!(resolve_slice_bound(0, 0), 0); + assert_eq!(resolve_slice_bound(-1, 0), 0); + assert_eq!(resolve_slice_bound(99, 0), 0); + } + + #[test] + fn resolve_take_count_positive() { + assert_eq!(resolve_take_count(0, 5), 0); + assert_eq!(resolve_take_count(3, 5), 3); + assert_eq!(resolve_take_count(5, 5), 5); + assert_eq!(resolve_take_count(99, 5), 5); + } + + #[test] + fn resolve_take_count_negative_drops_tail() { + // `take -k xs` == `xs[:-k]` — keep all but the last |k|. + assert_eq!(resolve_take_count(-1, 5), 4); + assert_eq!(resolve_take_count(-4, 5), 1); + assert_eq!(resolve_take_count(-5, 5), 0); + // Beyond -len clamps to 0 (empty), matching Python's `xs[:-99]`. + assert_eq!(resolve_take_count(-99, 5), 0); + } + + #[test] + fn resolve_drop_count_positive() { + assert_eq!(resolve_drop_count(0, 5), 0); + assert_eq!(resolve_drop_count(3, 5), 3); + assert_eq!(resolve_drop_count(5, 5), 5); + assert_eq!(resolve_drop_count(99, 5), 5); + } + + #[test] + fn resolve_drop_count_negative_keeps_tail() { + // `drop -k xs` == `xs[-k:]` — discard all but the last |k|. + // Returned value is the *prefix length to skip*. + assert_eq!(resolve_drop_count(-1, 5), 4); // skip 4, keep last 1 + assert_eq!(resolve_drop_count(-4, 5), 1); // skip 1, keep last 4 + assert_eq!(resolve_drop_count(-5, 5), 0); // skip 0, keep all + // Beyond -len clamps to 0 (keep everything), matching Python `xs[-99:]`. + assert_eq!(resolve_drop_count(-99, 5), 0); + } + + #[test] + fn resolve_take_drop_empty_list() { + // Every count against len=0 must clamp to 0 — take/drop of empty + // never errors, irrespective of sign. + assert_eq!(resolve_take_count(0, 0), 0); + assert_eq!(resolve_take_count(-3, 0), 0); + assert_eq!(resolve_take_count(3, 0), 0); + assert_eq!(resolve_drop_count(0, 0), 0); + assert_eq!(resolve_drop_count(-3, 0), 0); + assert_eq!(resolve_drop_count(3, 0), 0); + } + #[test] fn tag_round_trips_for_every_builtin() { // Anchor for the OP_CALL_BUILTIN_TREE bridge: every builtin must diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index 78b60ef5..a9a61e69 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -1726,8 +1726,19 @@ fn call_function(env: &mut Env, name: &str, args: Vec) -> Result { }; } if builtin == Some(Builtin::Slc) && args.len() == 3 { - let start = match &args[1] { - Value::Number(n) => *n as usize, + // Both bounds accept negative integers Python-style: + // `slc xs -1 0` is empty, `slc xs 0 -1` drops the last element, + // `slc xs -2 (len xs)` returns the last two elements. + let start_raw = match &args[1] { + Value::Number(n) => { + if n.fract() != 0.0 { + return Err(RuntimeError::new( + "ILO-R009", + "slc: start index must be an integer".to_string(), + )); + } + *n as i64 + } other => { return Err(RuntimeError::new( "ILO-R009", @@ -1735,8 +1746,16 @@ fn call_function(env: &mut Env, name: &str, args: Vec) -> Result { )); } }; - let end = match &args[2] { - Value::Number(n) => *n as usize, + let end_raw = match &args[2] { + Value::Number(n) => { + if n.fract() != 0.0 { + return Err(RuntimeError::new( + "ILO-R009", + "slc: end index must be an integer".to_string(), + )); + } + *n as i64 + } other => { return Err(RuntimeError::new( "ILO-R009", @@ -1746,14 +1765,16 @@ fn call_function(env: &mut Env, name: &str, args: Vec) -> Result { }; return match &args[0] { Value::List(items) => { - let end = end.min(items.len()); - let start = start.min(end); + let len = items.len(); + let end = crate::builtins::resolve_slice_bound(end_raw, len); + let start = crate::builtins::resolve_slice_bound(start_raw, len).min(end); Ok(Value::List(items[start..end].to_vec())) } Value::Text(s) => { let chars: Vec = s.chars().collect(); - let end = end.min(chars.len()); - let start = start.min(end); + let len = chars.len(); + let end = crate::builtins::resolve_slice_bound(end_raw, len); + let start = crate::builtins::resolve_slice_bound(start_raw, len).min(end); Ok(Value::Text(chars[start..end].iter().collect())) } other => Err(RuntimeError::new( @@ -1763,6 +1784,8 @@ fn call_function(env: &mut Env, name: &str, args: Vec) -> Result { }; } if builtin == Some(Builtin::Take) && args.len() == 2 { + // Negative `n` means "all but the last |n|" (Python `xs[:n]`): + // `take -1 [1,2,3]` returns `[1,2]`. let n = match &args[0] { Value::Number(n) => { if n.fract() != 0.0 { @@ -1771,13 +1794,7 @@ fn call_function(env: &mut Env, name: &str, args: Vec) -> Result { "take: count must be an integer".to_string(), )); } - if *n < 0.0 { - return Err(RuntimeError::new( - "ILO-R009", - "take: count must be a non-negative integer".to_string(), - )); - } - *n as usize + *n as i64 } other => { return Err(RuntimeError::new( @@ -1788,12 +1805,12 @@ fn call_function(env: &mut Env, name: &str, args: Vec) -> Result { }; return match &args[1] { Value::List(items) => { - let end = n.min(items.len()); + let end = crate::builtins::resolve_take_count(n, items.len()); Ok(Value::List(items[..end].to_vec())) } Value::Text(s) => { let chars: Vec = s.chars().collect(); - let end = n.min(chars.len()); + let end = crate::builtins::resolve_take_count(n, chars.len()); Ok(Value::Text(chars[..end].iter().collect())) } other => Err(RuntimeError::new( @@ -1803,6 +1820,8 @@ fn call_function(env: &mut Env, name: &str, args: Vec) -> Result { }; } if builtin == Some(Builtin::Drop) && args.len() == 2 { + // Negative `n` means "keep only the last |n|" (Python `xs[n:]`): + // `drop -1 [1,2,3]` returns `[3]`. let n = match &args[0] { Value::Number(n) => { if n.fract() != 0.0 { @@ -1811,13 +1830,7 @@ fn call_function(env: &mut Env, name: &str, args: Vec) -> Result { "drop: count must be an integer".to_string(), )); } - if *n < 0.0 { - return Err(RuntimeError::new( - "ILO-R009", - "drop: count must be a non-negative integer".to_string(), - )); - } - *n as usize + *n as i64 } other => { return Err(RuntimeError::new( @@ -1828,12 +1841,12 @@ fn call_function(env: &mut Env, name: &str, args: Vec) -> Result { }; return match &args[1] { Value::List(items) => { - let start = n.min(items.len()); + let start = crate::builtins::resolve_drop_count(n, items.len()); Ok(Value::List(items[start..].to_vec())) } Value::Text(s) => { let chars: Vec = s.chars().collect(); - let start = n.min(chars.len()); + let start = crate::builtins::resolve_drop_count(n, chars.len()); Ok(Value::Text(chars[start..].iter().collect())) } other => Err(RuntimeError::new( diff --git a/src/vm/mod.rs b/src/vm/mod.rs index ff39f2bf..dcbea316 100644 --- a/src/vm/mod.rs +++ b/src/vm/mod.rs @@ -7783,6 +7783,7 @@ impl<'a> VM<'a> { } OP_SLC => { // Two-instruction sequence: OP_SLC A=result B=list C=start; data word A=end_reg + // Bounds accept negative integers Python-style (`-1` = last element). let a = ((inst >> 16) & 0xFF) as usize + base; let b = ((inst >> 8) & 0xFF) as usize + base; let c = (inst & 0xFF) as usize + base; @@ -7796,8 +7797,13 @@ impl<'a> VM<'a> { if !vc.is_number() || !vd.is_number() { vm_err!(VmError::Type("slc: indices must be numbers")); } - let start = vc.as_number() as usize; - let end = vd.as_number() as usize; + let start_f = vc.as_number(); + let end_f = vd.as_number(); + if start_f.fract() != 0.0 || end_f.fract() != 0.0 { + vm_err!(VmError::Type("slc: indices must be integers")); + } + let start_raw = start_f as i64; + let end_raw = end_f as i64; if vb.is_string() { let s = unsafe { match vb.as_heap_ref() { @@ -7806,15 +7812,18 @@ impl<'a> VM<'a> { } }; let chars: Vec = s.chars().collect(); - let end = end.min(chars.len()); - let start = start.min(end); + let len = chars.len(); + let end = crate::builtins::resolve_slice_bound(end_raw, len); + let start = crate::builtins::resolve_slice_bound(start_raw, len).min(end); let result: String = chars[start..end].iter().collect(); reg_set!(a, NanVal::heap_string(result)); } else if vb.is_heap() { match unsafe { vb.as_heap_ref() } { HeapObj::List(items) => { - let end = end.min(items.len()); - let start = start.min(end); + let len = items.len(); + let end = crate::builtins::resolve_slice_bound(end_raw, len); + let start = + crate::builtins::resolve_slice_bound(start_raw, len).min(end); let mut sliced = Vec::with_capacity(end - start); for v in &items[start..end] { v.clone_rc(); @@ -7957,6 +7966,7 @@ impl<'a> VM<'a> { reg_set!(a, nv); } OP_TAKE => { + // Negative count means "all but the last |n|" (Python `xs[:n]`). let a = ((inst >> 16) & 0xFF) as usize + base; let b = ((inst >> 8) & 0xFF) as usize + base; let c = (inst & 0xFF) as usize + base; @@ -7969,10 +7979,7 @@ impl<'a> VM<'a> { if nf.fract() != 0.0 { vm_err!(VmError::Type("take: count must be an integer")); } - if nf < 0.0 { - vm_err!(VmError::Type("take: count must be a non-negative integer")); - } - let n = nf as usize; + let n_raw = nf as i64; if vc.is_string() { let s = unsafe { match vc.as_heap_ref() { @@ -7981,13 +7988,13 @@ impl<'a> VM<'a> { } }; let chars: Vec = s.chars().collect(); - let end = n.min(chars.len()); + let end = crate::builtins::resolve_take_count(n_raw, chars.len()); let result: String = chars[..end].iter().collect(); reg_set!(a, NanVal::heap_string(result)); } else if vc.is_heap() { match unsafe { vc.as_heap_ref() } { HeapObj::List(items) => { - let end = n.min(items.len()); + let end = crate::builtins::resolve_take_count(n_raw, items.len()); let mut sliced = Vec::with_capacity(end); for v in &items[..end] { v.clone_rc(); @@ -8002,6 +8009,7 @@ impl<'a> VM<'a> { } } OP_DROP => { + // Negative count means "keep only the last |n|" (Python `xs[n:]`). let a = ((inst >> 16) & 0xFF) as usize + base; let b = ((inst >> 8) & 0xFF) as usize + base; let c = (inst & 0xFF) as usize + base; @@ -8014,10 +8022,7 @@ impl<'a> VM<'a> { if nf.fract() != 0.0 { vm_err!(VmError::Type("drop: count must be an integer")); } - if nf < 0.0 { - vm_err!(VmError::Type("drop: count must be a non-negative integer")); - } - let n = nf as usize; + let n_raw = nf as i64; if vc.is_string() { let s = unsafe { match vc.as_heap_ref() { @@ -8026,13 +8031,13 @@ impl<'a> VM<'a> { } }; let chars: Vec = s.chars().collect(); - let start = n.min(chars.len()); + let start = crate::builtins::resolve_drop_count(n_raw, chars.len()); let result: String = chars[start..].iter().collect(); reg_set!(a, NanVal::heap_string(result)); } else if vc.is_heap() { match unsafe { vc.as_heap_ref() } { HeapObj::List(items) => { - let start = n.min(items.len()); + let start = crate::builtins::resolve_drop_count(n_raw, items.len()); let mut sliced = Vec::with_capacity(items.len() - start); for v in &items[start..] { v.clone_rc(); @@ -10606,6 +10611,8 @@ pub(crate) extern "C" fn jit_rsrt(a: u64) -> u64 { #[cfg(feature = "cranelift")] #[unsafe(no_mangle)] pub(crate) extern "C" fn jit_slc(a: u64, start: u64, end: u64) -> u64 { + // Bounds accept negative integers Python-style; kept in lockstep with the + // tree-walker and OP_SLC by delegating to `resolve_slice_bound`. let vb = NanVal(a); let vc = NanVal(start); let vd = NanVal(end); @@ -10613,10 +10620,13 @@ pub(crate) extern "C" fn jit_slc(a: u64, start: u64, end: u64) -> u64 { jit_set_runtime_error(VmError::Type("slc: indices must be numbers")); return TAG_NIL; } - // OOB is deliberately clamped on tree/VM (slc is documented to saturate), - // so do not surface a runtime error for out-of-range start/end. - let s_idx = vc.as_number() as usize; - let e_idx = vd.as_number() as usize; + let s_f = vc.as_number(); + let e_f = vd.as_number(); + if s_f.fract() != 0.0 || e_f.fract() != 0.0 { + return TAG_NIL; + } + let s_raw = s_f as i64; + let e_raw = e_f as i64; if vb.is_string() { let s = unsafe { match vb.as_heap_ref() { @@ -10625,15 +10635,17 @@ pub(crate) extern "C" fn jit_slc(a: u64, start: u64, end: u64) -> u64 { } }; let chars: Vec = s.chars().collect(); - let e = e_idx.min(chars.len()); - let s = s_idx.min(e); + let len = chars.len(); + let e = crate::builtins::resolve_slice_bound(e_raw, len); + let s = crate::builtins::resolve_slice_bound(s_raw, len).min(e); return NanVal::heap_string(chars[s..e].iter().collect()).0; } if vb.is_heap() && let HeapObj::List(items) = unsafe { vb.as_heap_ref() } { - let e = e_idx.min(items.len()); - let s = s_idx.min(e); + let len = items.len(); + let e = crate::builtins::resolve_slice_bound(e_raw, len); + let s = crate::builtins::resolve_slice_bound(s_raw, len).min(e); let mut sliced = Vec::with_capacity(e - s); for v in &items[s..e] { v.clone_rc(); @@ -10725,16 +10737,18 @@ pub(crate) extern "C" fn jit_rgxsub(pat_v: u64, repl_v: u64, subj_v: u64) -> u64 #[cfg(feature = "cranelift")] #[unsafe(no_mangle)] pub(crate) extern "C" fn jit_take(n_val: u64, xs: u64) -> u64 { + // Negative count means "all but the last |n|" (Python `xs[:n]`); shares + // resolution with the tree-walker and OP_TAKE. let vn = NanVal(n_val); let vc = NanVal(xs); if !vn.is_number() { return TAG_NIL; } let nf = vn.as_number(); - if nf.fract() != 0.0 || nf < 0.0 { + if nf.fract() != 0.0 { return TAG_NIL; } - let n = nf as usize; + let n_raw = nf as i64; if vc.is_string() { let s = unsafe { match vc.as_heap_ref() { @@ -10743,13 +10757,13 @@ pub(crate) extern "C" fn jit_take(n_val: u64, xs: u64) -> u64 { } }; let chars: Vec = s.chars().collect(); - let end = n.min(chars.len()); + let end = crate::builtins::resolve_take_count(n_raw, chars.len()); return NanVal::heap_string(chars[..end].iter().collect()).0; } if vc.is_heap() && let HeapObj::List(items) = unsafe { vc.as_heap_ref() } { - let end = n.min(items.len()); + let end = crate::builtins::resolve_take_count(n_raw, items.len()); let mut sliced = Vec::with_capacity(end); for v in &items[..end] { v.clone_rc(); @@ -10763,16 +10777,18 @@ pub(crate) extern "C" fn jit_take(n_val: u64, xs: u64) -> u64 { #[cfg(feature = "cranelift")] #[unsafe(no_mangle)] pub(crate) extern "C" fn jit_drop(n_val: u64, xs: u64) -> u64 { + // Negative count means "keep only the last |n|" (Python `xs[n:]`); shares + // resolution with the tree-walker and OP_DROP. let vn = NanVal(n_val); let vc = NanVal(xs); if !vn.is_number() { return TAG_NIL; } let nf = vn.as_number(); - if nf.fract() != 0.0 || nf < 0.0 { + if nf.fract() != 0.0 { return TAG_NIL; } - let n = nf as usize; + let n_raw = nf as i64; if vc.is_string() { let s = unsafe { match vc.as_heap_ref() { @@ -10781,13 +10797,13 @@ pub(crate) extern "C" fn jit_drop(n_val: u64, xs: u64) -> u64 { } }; let chars: Vec = s.chars().collect(); - let start = n.min(chars.len()); + let start = crate::builtins::resolve_drop_count(n_raw, chars.len()); return NanVal::heap_string(chars[start..].iter().collect()).0; } if vc.is_heap() && let HeapObj::List(items) = unsafe { vc.as_heap_ref() } { - let start = n.min(items.len()); + let start = crate::builtins::resolve_drop_count(n_raw, items.len()); let mut sliced = Vec::with_capacity(items.len() - start); for v in &items[start..] { v.clone_rc(); diff --git a/tests/regression_neg_index_slice.rs b/tests/regression_neg_index_slice.rs new file mode 100644 index 00000000..695224fd --- /dev/null +++ b/tests/regression_neg_index_slice.rs @@ -0,0 +1,253 @@ +// Regression tests for Python-style negative indices on `slc`, `take`, `drop`. +// +// Background: PR #183 (2026-05-12) added negative-index support to `at xs i`. +// This change extends the same semantics to slice-shaped operators so the +// "negative index = count from the end" rule is uniform across every builtin +// that takes a position. Closes the quant-trader fencepost friction +// (assessment-feedback line 1374) and the `slc xs -np 1 np` ergonomics gap +// (line 753) by removing the workaround that bound `s=- np 1` before every +// last-element access. +// +// Coverage matrix: every engine (tree, VM, cranelift JIT) × every boundary +// case (`-len`, `-1`, `0`, `len`, beyond `len` both directions) × list and +// text. The three engines route through the shared `resolve_slice_bound` / +// `resolve_take_count` / `resolve_drop_count` helpers in `builtins.rs`, but +// the harness runs all three end-to-end to catch dispatch-layer regressions +// (e.g. the JIT helper returning `TAG_NIL` instead of unwinding into the +// shared helper). + +use std::process::Command; + +fn ilo() -> Command { + Command::new(env!("CARGO_BIN_EXE_ilo")) +} + +fn run(engine: &str, src: &str, entry: &str) -> String { + let out = ilo() + .args([src, engine, entry]) + .output() + .expect("failed to run ilo"); + assert!( + out.status.success(), + "ilo {engine} failed for `{src}`: stderr={}", + String::from_utf8_lossy(&out.stderr) + ); + String::from_utf8_lossy(&out.stdout).trim().to_string() +} + +fn check_all_engines(src: &str, entry: &str, expected: &str) { + for engine in ["--run-tree", "--run-vm"] { + assert_eq!( + run(engine, src, entry), + expected, + "engine={engine} src=`{src}` entry={entry}" + ); + } + #[cfg(feature = "cranelift")] + { + assert_eq!( + run("--run-cranelift", src, entry), + expected, + "engine=--run-cranelift src=`{src}` entry={entry}" + ); + } +} + +// ── slc: negative bounds on lists ───────────────────────────────────────── + +// `slc xs -1 (len xs)` returns the last element as a 1-element list. +#[test] +fn slc_list_neg_start_to_len() { + check_all_engines("f>L n;xs=[10,20,30,40,50];slc xs -1 5", "f", "[50]"); +} + +// `slc xs -2 (len xs)` returns the last two elements. +#[test] +fn slc_list_last_two() { + check_all_engines("f>L n;xs=[10,20,30,40,50];slc xs -2 5", "f", "[40, 50]"); +} + +// `slc xs 0 -1` drops the last element. +#[test] +fn slc_list_neg_end_drops_last() { + check_all_engines( + "f>L n;xs=[10,20,30,40,50];slc xs 0 -1", + "f", + "[10, 20, 30, 40]", + ); +} + +// `slc xs -3 -1` middle slice from negatives. +#[test] +fn slc_list_neg_both_bounds() { + check_all_engines("f>L n;xs=[10,20,30,40,50];slc xs -3 -1", "f", "[30, 40]"); +} + +// `slc xs -len 0` is empty (start clamps to 0, end clamps to 0). +#[test] +fn slc_list_neg_len_to_zero_empty() { + check_all_engines("f>L n;xs=[10,20,30,40,50];slc xs -5 0", "f", "[]"); +} + +// `slc xs -len (len xs)` is the full list. +#[test] +fn slc_list_neg_len_full() { + check_all_engines( + "f>L n;xs=[10,20,30,40,50];slc xs -5 5", + "f", + "[10, 20, 30, 40, 50]", + ); +} + +// Indices beyond `-len` clamp to 0 — never out of range, never wraps. +#[test] +fn slc_list_neg_beyond_len_clamps_to_zero() { + check_all_engines("f>L n;xs=[10,20,30,40,50];slc xs -99 -90", "f", "[]"); + check_all_engines( + "f>L n;xs=[10,20,30,40,50];slc xs -99 3", + "f", + "[10, 20, 30]", + ); +} + +// Positive end past len still clamps as before — pure regression for the +// existing happy path now that integer-validation is in place. +#[test] +fn slc_list_pos_end_past_len_clamps() { + check_all_engines("f>L n;xs=[10,20,30];slc xs 0 99", "f", "[10, 20, 30]"); +} + +// Quant-trader fencepost: previously needed `npm=- np 1;at eq npm`. Now +// `slc eq -1 np` drops the last element cleanly. +#[test] +fn slc_quant_trader_fencepost() { + check_all_engines( + "f>L n;eq=[100,101,102,103,99];slc eq 0 -1", + "f", + "[100, 101, 102, 103]", + ); +} + +// ── slc: negative bounds on text ────────────────────────────────────────── + +#[test] +fn slc_text_neg_end_drops_last_char() { + check_all_engines("f>t;slc \"hello\" 0 -1", "f", "hell"); +} + +#[test] +fn slc_text_last_three() { + check_all_engines("f>t;slc \"hello\" -3 5", "f", "llo"); +} + +#[test] +fn slc_text_neg_beyond_len_clamps() { + check_all_engines("f>t;slc \"hello\" -99 -1", "f", "hell"); +} + +// Multi-byte characters: negative slicing must operate on codepoints, not +// bytes. (The existing positive-slice path already does this; the helper +// inherits it because it works on the post-`chars().collect()` length.) +#[test] +fn slc_text_unicode_neg() { + check_all_engines("f>t;slc \"héllo\" -3 5", "f", "llo"); +} + +// ── take: negative count drops tail ─────────────────────────────────────── + +#[test] +fn take_list_neg_one_drops_last() { + check_all_engines( + "f>L n;xs=[10,20,30,40,50];take -1 xs", + "f", + "[10, 20, 30, 40]", + ); +} + +#[test] +fn take_list_neg_keeps_only_prefix() { + check_all_engines("f>L n;xs=[10,20,30,40,50];take -2 xs", "f", "[10, 20, 30]"); +} + +#[test] +fn take_list_neg_len_empty() { + check_all_engines("f>L n;xs=[10,20,30,40,50];take -5 xs", "f", "[]"); +} + +#[test] +fn take_list_neg_beyond_len_empty() { + check_all_engines("f>L n;xs=[10,20,30,40,50];take -99 xs", "f", "[]"); +} + +#[test] +fn take_text_neg_drops_tail() { + check_all_engines("f>t;take -2 \"hello\"", "f", "hel"); +} + +// ── drop: negative count keeps tail ─────────────────────────────────────── + +#[test] +fn drop_list_neg_one_keeps_last() { + check_all_engines("f>L n;xs=[10,20,30,40,50];drop -1 xs", "f", "[50]"); +} + +#[test] +fn drop_list_neg_keeps_last_two() { + check_all_engines("f>L n;xs=[10,20,30,40,50];drop -2 xs", "f", "[40, 50]"); +} + +#[test] +fn drop_list_neg_len_keeps_all() { + check_all_engines( + "f>L n;xs=[10,20,30,40,50];drop -5 xs", + "f", + "[10, 20, 30, 40, 50]", + ); +} + +#[test] +fn drop_list_neg_beyond_len_keeps_all() { + check_all_engines( + "f>L n;xs=[10,20,30,40,50];drop -99 xs", + "f", + "[10, 20, 30, 40, 50]", + ); +} + +#[test] +fn drop_text_neg_keeps_tail() { + check_all_engines("f>t;drop -3 \"hello\"", "f", "llo"); +} + +// ── boundary: positive zero still works ────────────────────────────────── + +// Make sure adding negative paths didn't break `take 0` / `drop 0` (empty +// / full respectively). These are the existing happy-path regressions. + +#[test] +fn take_list_zero_empty() { + check_all_engines("f>L n;xs=[10,20,30];take 0 xs", "f", "[]"); +} + +#[test] +fn drop_list_zero_full() { + check_all_engines("f>L n;xs=[10,20,30];drop 0 xs", "f", "[10, 20, 30]"); +} + +// ── empty-list edge cases — every negative bound must be safe ──────────── + +#[test] +fn slc_empty_list_neg_bounds() { + check_all_engines("f>L n;xs=[];slc xs -1 -1", "f", "[]"); + check_all_engines("f>L n;xs=[];slc xs -99 99", "f", "[]"); +} + +#[test] +fn take_empty_list_neg() { + check_all_engines("f>L n;xs=[];take -3 xs", "f", "[]"); +} + +#[test] +fn drop_empty_list_neg() { + check_all_engines("f>L n;xs=[];drop -3 xs", "f", "[]"); +} diff --git a/tests/regression_take_drop.rs b/tests/regression_take_drop.rs index bc3aae8b..70214bb9 100644 --- a/tests/regression_take_drop.rs +++ b/tests/regression_take_drop.rs @@ -207,102 +207,67 @@ fn drop_text_cranelift() { check_drop_text("--run-cranelift"); } -// ── negative count: error on tree/vm; nil on cranelift JIT ──────────── +// ── negative count: Python-style tail semantics across every engine ─── +// +// Pre-change behaviour: tree/VM errored "must be non-negative integer", +// Cranelift JIT silently returned nil. Both were workarounds for the +// missing tail-indexing concept. New behaviour: `take -k xs` keeps all +// but the last |k| (xs[:-k]); `drop -k xs` keeps only the last |k| +// (xs[-k:]). Deep coverage of every-engine parity for these edge cases +// lives in `regression_neg_index_slice.rs`; this file's tests just lock +// the headline cases so the stale "must be non-negative" check cannot +// silently reappear in any one engine. + const TAKE_NEG: &str = "f>L n;xs=[1,2,3];take -1 xs"; -fn check_take_neg_error(engine: &str) { - let out = ilo() - .args([TAKE_NEG, engine, "f"]) - .output() - .expect("failed to run ilo"); - assert!( - !out.status.success(), - "engine={engine}: expected error for take -1, got stdout={}", - String::from_utf8_lossy(&out.stdout) - ); - let stderr = String::from_utf8_lossy(&out.stderr); - assert!( - stderr.contains("take") || stderr.contains("non-negative") || stderr.contains("ILO-R009"), - "engine={engine}: expected take/non-negative error, got stderr={stderr}" +fn check_take_neg_python_style(engine: &str) { + assert_eq!( + run(engine, TAKE_NEG, "f"), + "[1, 2]", + "engine={engine}: expected take -1 to drop the last element" ); } #[test] fn take_negative_tree() { - check_take_neg_error("--run-tree"); + check_take_neg_python_style("--run-tree"); } #[test] fn take_negative_vm() { - check_take_neg_error("--run-vm"); + check_take_neg_python_style("--run-vm"); } #[test] #[cfg(feature = "cranelift")] fn take_negative_cranelift() { - // Cranelift JIT mirrors at/hd: returns nil on guard failure rather than erroring. - let out = ilo() - .args([TAKE_NEG, "--run-cranelift", "f"]) - .output() - .expect("failed to run ilo"); - assert!( - out.status.success(), - "cranelift: expected success returning nil for take -1, got stderr={}", - String::from_utf8_lossy(&out.stderr) - ); - let stdout = String::from_utf8_lossy(&out.stdout); - assert!( - stdout.contains("nil"), - "cranelift: expected nil, got {stdout}" - ); + check_take_neg_python_style("--run-cranelift"); } const DROP_NEG: &str = "f>L n;xs=[1,2,3];drop -1 xs"; -fn check_drop_neg_error(engine: &str) { - let out = ilo() - .args([DROP_NEG, engine, "f"]) - .output() - .expect("failed to run ilo"); - assert!( - !out.status.success(), - "engine={engine}: expected error for drop -1, got stdout={}", - String::from_utf8_lossy(&out.stdout) - ); - let stderr = String::from_utf8_lossy(&out.stderr); - assert!( - stderr.contains("drop") || stderr.contains("non-negative") || stderr.contains("ILO-R009"), - "engine={engine}: expected drop/non-negative error, got stderr={stderr}" +fn check_drop_neg_python_style(engine: &str) { + assert_eq!( + run(engine, DROP_NEG, "f"), + "[3]", + "engine={engine}: expected drop -1 to keep only the last element" ); } #[test] fn drop_negative_tree() { - check_drop_neg_error("--run-tree"); + check_drop_neg_python_style("--run-tree"); } #[test] fn drop_negative_vm() { - check_drop_neg_error("--run-vm"); + check_drop_neg_python_style("--run-vm"); } #[test] #[cfg(feature = "cranelift")] fn drop_negative_cranelift() { - let out = ilo() - .args([DROP_NEG, "--run-cranelift", "f"]) - .output() - .expect("failed to run ilo"); - assert!( - out.status.success(), - "cranelift: expected success returning nil for drop -1, got stderr={}", - String::from_utf8_lossy(&out.stderr) - ); - let stdout = String::from_utf8_lossy(&out.stdout); - assert!( - stdout.contains("nil"), - "cranelift: expected nil, got {stdout}" - ); + check_drop_neg_python_style("--run-cranelift"); } // ── type variable: take/drop preserve element type (list of text) ─────