From 5b0a680c85bd29a91724a833bb8951978e5a1027 Mon Sep 17 00:00:00 2001 From: Sean Parent Date: Tue, 7 Jul 2026 12:25:38 -0700 Subject: [PATCH 01/29] docs(specs): add design spec for CEL tuple types Covers tuple literals, .N indexing, the in-place type-erased runtime representation on RawStack, and a generalized signature-matching mechanism for interop with user-supplied Rust operations. --- .../specs/2026-07-07-cel-tuples-design.md | 244 ++++++++++++++++++ 1 file changed, 244 insertions(+) create mode 100644 docs/superpowers/specs/2026-07-07-cel-tuples-design.md diff --git a/docs/superpowers/specs/2026-07-07-cel-tuples-design.md b/docs/superpowers/specs/2026-07-07-cel-tuples-design.md new file mode 100644 index 0000000..69ea059 --- /dev/null +++ b/docs/superpowers/specs/2026-07-07-cel-tuples-design.md @@ -0,0 +1,244 @@ +# Design: Tuple Types in CEL + +**Date:** 2026-07-07 +**Branch:** `worktree-cel-tuples` + +## Summary + +Add tuple literals (`("Hello", 42)`) and `.N` field indexing to CEL, backed by a +type-erased, in-place runtime representation on `RawStack` — no heap allocation for +the tuple value itself. Interop with user-supplied Rust operations that take or +return a tuple goes through a generalization of the existing built-in operator +dispatch (`OpSignature`/`BuiltinScope`), bridging to a concrete `repr(C)` Rust type +(e.g. `CStackList<...>`) rather than a plain Rust tuple, because plain Rust tuples +have unspecified (`repr(Rust)`) layout and can't be soundly reinterpreted from raw +bytes. + +This spec covers the interpreted `DynSegment` backend only — the only backend that +exists today (see `docs/VISION.md`). First-class functions, arrays/vecs, and +method-call syntax are related future directions but out of scope here; the +metadata introduced by this design (`AssociatedType`) is shaped so those features +can reuse it later. + +## Background + +Three existing pieces of the runtime are directly relevant: + +- **`AssociatedType`** (`cel-runtime/src/dyn_segment.rs`) already exists as a + placeholder — "reserved for parse-time call checking and richer error reporting + ... tuple elements ... first-class functions" — but today carries only + `type_id`/`type_name`/`associated: Vec`, with no offset or + dropper. It's unused. +- **`CStackList`** (`cel-runtime/src/c_stack_list.rs`) is a `repr(C)`, + tail-first cons-list with typenum-based indexing and existing conversions + to/from real Rust tuples (`IntoCStackList`). A test in that module proves + transmute-compatibility with a `#[repr(C)]` struct of matching field order. +- **`op1`/`op2`/`op3`** on `DynSegment` are Rust-generic over any concrete + `'static` type known at the call site (e.g. the hand-written signature tables + in `cel-parser/src/op_table.rs`). CEL tuple element types are discovered only + at *parse time*, from arbitrary script text, with unbounded shape combinations + — there is no finite set of `(A, B, ...)` combinations to pre-enumerate in Rust + source the way `op_table.rs` enumerates `u8 + u8`, `u8 + u16`, etc. Tuple + construction and indexing therefore can't be ordinary generic ops; they need a + new runtime, byte-level, type-erased primitive that carries its own metadata — + which is exactly what `AssociatedType` was reserved for. + +A key correctness constraint (caught during design review): a tuple's internal +layout (the offsets between its elements) must depend only on the tuple's own +element types, never on how much was already on the ambient stack when +construction started. Naively pushing elements with `RawStack::push`'s ordinary +alignment (relative to the *current* stack length) would give a layout that +varies with stack depth — unusable for matching against a fixed concrete +`repr(C)` type at interop boundaries. + +## Grammar + +``` +tuple_or_group = ")" + | or_expression ["," [ or_expression { "," or_expression } ]] ")" +``` + +Extends the existing `(` handling in `is_primary_expression`: + +- `()` → unit (unchanged). +- `(x)` → grouping (unchanged) — a single expression with no comma is never a + tuple. +- `(x,)` → 1-tuple. The trailing comma is *mandatory* here, exactly as in Rust, + because `(x)` already means grouping — there's no ambiguity-free way to spell a + 1-tuple otherwise. +- `(x, y)`, `(x, y, z)`, ... → tuple, arity ≥ 2. **No trailing comma allowed** in + this branch: inside the `{ "," or_expression }` loop, a comma always means + "another element follows," never "maybe end." This keeps every decision point + resolvable with a single token of lookahead (the same reason `parameter_list` + already disallows trailing commas today) and avoids the "did we just consume a + separator or a terminator" ambiguity a fully-optional trailing comma would + introduce. + +Indexing extends `is_postfix_expression`: + +``` +postfix_expression = primary_expression { "(" parameter_list ")" | "." unsuffixed_integer }. +``` + +`.N` requires the top-of-stack entry to carry tuple metadata (see below) with at +least `N + 1` elements. Because tuple shape is always known at parse time, an +out-of-range or non-tuple `.N` is a **parse-time** error, never a runtime one. + +Method-call syntax (`.name(...)`) is explicitly out of scope — VISION.md lists +"method calls" as a separate future direction. + +## Runtime Representation + +Extend `AssociatedType` with the two fields it's missing: + +```rust +pub struct AssociatedType { + pub type_id: TypeId, + pub type_name: Cow<'static, str>, + pub offset: usize, // new: byte offset from the tuple's own base + pub dropper: fn(&mut RawStack), // new: reuses the same per-type dropper push_type generates + pub associated: Vec, +} +``` + +A tuple's own `StackInfo` entry uses a marker type for `type_id` +(`TypeId::of::()`) — the tuple's real type identity is the *ordered +sequence* of element `AssociatedType`s, not one flat `TypeId`, so flat-`TypeId` +comparisons (`pop_types`, `call0::()`, etc.) must special-case `DynTuple` and +compare the `associated` shape instead when they encounter one. + +This is deliberately the same shape a later first-class-function design could +reuse for argument-list metadata, or an array design for a repeated-element +descriptor — but neither is built now. + +## Construction + +Parse each element normally — this is already exactly how `parameter_list` +evaluates call arguments today, nothing new there. By the closing `)`, all `N` +element types/sizes/alignments are known at parse time, so the parser computes +the *ideal* self-contained layout (offsets computed from a hypothetical zero +base, using the same `align_index` math `RawStack`/`RawSequence` already use) as +constants baked into one runtime op. That op: + +1. Reserves enough leading padding to align the tuple's start to its own max + element alignment. +2. Moves each already-evaluated element from its ambient (context-dependent) + offset to its ideal (context-independent) offset, via the new `push_raw` + primitive (below) rather than ad hoc `ptr::copy` calls. +3. Adjusts the tracked stack length and replaces the `N` individual `StackInfo` + entries with one aggregate entry carrying the `associated` list. + +No heap allocation: the repack happens in place within the existing `RawStack` +buffer. + +### New primitive: `RawStack::push_raw` + +`RawStack::push` is generic over a compile-time `T`. Construction (and +indexing, below) need a type-erased equivalent: + +```rust +/// Pushes `size` bytes from `src`, aligned to `align`, using the same +/// padding/marker-byte bookkeeping as `push::`. +/// +/// # Safety +/// `src` must be valid for `size` bytes and not overlap the stack's own buffer +/// in a way that `push::` wouldn't already guard against. +pub unsafe fn push_raw(&mut self, align: usize, size: usize, src: *const u8) -> bool +``` + +Both the construction repack and `.N` indexing route through this one +primitive instead of separate raw-pointer logic. + +## Indexing (`.N`) + +An earlier version of this design assumed elements below the target index could +be `drop_in_place`d without being removed from the stack, since dropping them +"only" wastes space. **That's wrong**: leftover bytes below the target corrupt +`RawStack::pop`'s backward marker-byte scan for whatever pops *next* — the scan +looks for a specific padding-marker pattern to compute how much padding preceded +a value, and garbage left behind by a manually-destroyed element isn't a valid +marker. Concretely, `5 + (0, 1).1` must still correctly pop `5` after evaluating +`.1`, and it wouldn't if `0`'s dead bytes were left sitting under `1`. + +The corrected algorithm pops the whole tuple and pushes just the result: + +1. Copy the target element's raw bytes (known fixed offset/size from + `associated`) into a small scratch buffer, *before* dropping anything. +2. `drop_in_place` every other element at its own known offset (the target's + bytes were only copied, not consumed, so it's skipped here). +3. Truncate the stack back to exactly where it was before the tuple started, + using the tuple's own aggregate `padding` flag — the same bookkeeping any + ordinary pop already uses, since that marker was correctly established when + the tuple was originally pushed as a unit. +4. Push the scratch bytes back via `push_raw`, computing correct + alignment/padding for the *current* (now properly restored) stack position. + +This leaves no unreclaimed dead space and keeps every subsequent pop's +marker-byte scan valid. Reclaiming space via compaction instead of a full +pop-and-repush is a possible future optimization, not needed for correctness. + +## Interop / Op Registration + +Per the "generalize existing mechanisms, don't bolt on a new one" direction: + +- **Matching:** `OpSignature` (`cel-parser/src/op_table.rs`) currently matches an + operand by one flat `TypeId`. Add a second operand-shape kind — "tuple of + `TypeId`s `[T0, T1, ...]` in order" — matched against a stack slot's + `associated` list instead of its flat `type_id`. `BuiltinScope::lookup`'s scan + extends naturally to check either kind per operand position. +- **Registration surface:** a public API alongside `push_scope` lets host code + (pm-lang or elsewhere) register `(shape: &[TypeId], op_fn)` entries scanned the + same array-scan way `BUILTINS`/`ADD_SIGNATURES` are scanned today — user + tuple-typed ops go through the same dispatch as built-ins, not a side + mechanism. +- **Bridging to a concrete Rust type:** a registrant's `op_fn` wants a genuine + concrete `T` (a `CStackList<...>` chain, or their own `#[repr(C)]` struct) to + write ordinary Rust code against — including converting to a plain Rust tuple + *inside* their own function body, which is sound there since at that point + it's an ordinary in-process value with compiler-chosen layout, not a + cross-boundary reinterpretation. One new `DynSegment` primitive, + `pop_tuple_as::()`, derives `T`'s expected element-`TypeId` shape + via the existing `ToTypeIdList` trait (already used by + `DynSegment::new::()` for argument seeding), checks it against the + runtime tuple's `associated` shape, and — if they match — does a raw byte + reinterpretation. This is sound specifically because both sides use the same + natural-alignment, declaration-order layout established above; it would *not* + be sound against a plain Rust tuple type, whose layout is unspecified. The + reverse, `push_tuple::`, builds the outgoing + `AssociatedType` list from `T`'s own `ToTypeIdList` output for ops that return + a tuple. + +This reuses `OpSignature`-style scanning, `ToTypeIdList`, and `CStackList` +rather than inventing new interop machinery. + +## Testing + +Contract-only, per this repo's convention (derived from the public +interface/grammar, not implementation internals): + +- **`cel-parser` grammar:** `()`, `(x)` vs `(x,)` vs `(x,y)`/`(x,y,z)`, + missing/extra commas, `.N` on tuples of various arities, parse-time errors for + `.N` on non-tuples and out-of-range `N`. +- **Indexing combined with another operation** — the case that caught the + original design flaw — e.g. `5 + (0, 1).1` → `6`, and similar combinations + exercising indexing followed by arithmetic/comparison ops, to confirm the + stack is left in a state subsequent ops can correctly consume. +- **`cel-runtime`:** `RawStack::push_raw` behaves like `push::` for + equivalent size/align (alignment/padding correctness, round-trip via existing + `pop`); tuple construction + `.N` indexing exercised through `DynSegment`'s + public API, including drop-count assertions (extending the existing + `DropCounter` pattern already used for `op1r`/`op2r` unwind tests) to verify + every element is dropped exactly once whether or not it's the one extracted; + `pop_tuple_as`/`push_tuple` round-trip tests against a concrete `CStackList` + shape. +- **`cel-parser` op-table:** signature matching against registered tuple-shaped + operands. + +## Out of Scope + +Deferred, per VISION.md's own separation of these as distinct future +directions: arrays/vecs, first-class functions, method-call syntax +(`.name(...)`), the compiled/macro backend, and the compaction optimization +noted above (reclaiming dead space below an extracted element instead of a full +pop-and-repush). The `AssociatedType` shape introduced here is meant to support +these later without re-deriving it. From 680db7198c3ee0a0795502748ac7b92c2cbcb23b Mon Sep 17 00:00:00 2001 From: Sean Parent Date: Tue, 7 Jul 2026 12:53:18 -0700 Subject: [PATCH 02/29] docs(specs): simplify tuple grammar to a single production MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fold the leading paren into tuple_or_group instead of splitting it across two top-level alternatives — same five cases, easier to read. --- docs/superpowers/specs/2026-07-07-cel-tuples-design.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/superpowers/specs/2026-07-07-cel-tuples-design.md b/docs/superpowers/specs/2026-07-07-cel-tuples-design.md index 69ea059..9bd03bd 100644 --- a/docs/superpowers/specs/2026-07-07-cel-tuples-design.md +++ b/docs/superpowers/specs/2026-07-07-cel-tuples-design.md @@ -54,8 +54,7 @@ varies with stack depth — unusable for matching against a fixed concrete ## Grammar ``` -tuple_or_group = ")" - | or_expression ["," [ or_expression { "," or_expression } ]] ")" +tuple_or_group = "(" [ or_expression ["," [ or_expression { "," or_expression } ]] ] ")" ``` Extends the existing `(` handling in `is_primary_expression`: From 33f6654327e6943586a1f6b783db9a92f2ba70e8 Mon Sep 17 00:00:00 2001 From: Sean Parent Date: Tue, 7 Jul 2026 13:10:46 -0700 Subject: [PATCH 03/29] docs(plans): add implementation plan for CEL tuple types Nine-task plan covering RawStack primitives (push_raw/repack/drop_sized), StackInfo/AssociatedType metadata, tuple construction and .N indexing, parser grammar, generalized tuple-shaped op registration, and the CStackList interop bridge. --- .../plans/2026-07-07-cel-tuples.md | 1685 +++++++++++++++++ 1 file changed, 1685 insertions(+) create mode 100644 docs/superpowers/plans/2026-07-07-cel-tuples.md diff --git a/docs/superpowers/plans/2026-07-07-cel-tuples.md b/docs/superpowers/plans/2026-07-07-cel-tuples.md new file mode 100644 index 0000000..da7cc0d --- /dev/null +++ b/docs/superpowers/plans/2026-07-07-cel-tuples.md @@ -0,0 +1,1685 @@ +# CEL Tuples Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add tuple literals (`("Hello", 42)`), `.N` field indexing, and a generalized interop-registration mechanism for tuple-typed user operations to CEL, per `docs/superpowers/specs/2026-07-07-cel-tuples-design.md`. + +**Architecture:** Tuples are a type-erased, in-place runtime representation on `RawStack` — no heap allocation for the tuple value itself (one small, transient, per-element scratch buffer is used during `.N` extraction; see Task 5). `StackInfo` and a new `AssociatedType` carry per-element offset/size/align/dropper metadata computed at parse time. Construction repacks already-evaluated elements into a context-independent layout; indexing pops the whole tuple and pushes back just the extracted element. Interop with user Rust code goes through a generalization of the existing `OpSignature`/`BuiltinScope` array-scan dispatch, bridging to a concrete `CStackList<...>` type. + +**Tech Stack:** Rust, `cel-runtime` (stack/segment machinery), `cel-parser` (recursive-descent parser, `proc_macro2` tokens), `anyhow` for fallible ops, existing `phf`/signature-table patterns in `cel-parser/src/op_table.rs`. + +## Global Constraints + +- Format with `cargo fmt --all` before every commit (enforced by pre-commit hook). +- Every public function needs a contract-style `///` doc comment (Summary, Preconditions as `debug_assert!`, `# Errors`/`# Safety` where applicable, Postconditions, Complexity if not O(1)) per the root `CLAUDE.md`. +- Unit tests are derived from contract/public interface only — never from implementation internals. +- No heap allocation for the tuple's *persistent* on-stack representation. A small, transient, single-element scratch buffer during `.N` extraction (Task 5) is an accepted, documented exception — not the "boxed tuple" pattern the design avoids. +- Run `cargo test --workspace` and `cargo clippy --workspace --exclude begin -- -D warnings` before the final commit of each task. + +--- + +### Task 1: `RawStack` — type-erased push and length + +**Files:** +- Modify: `cel-runtime/src/raw_stack.rs` + +**Interfaces:** +- Produces: `RawStack::len(&self) -> usize`, `unsafe fn RawStack::push_raw(&mut self, align: usize, size: usize, src: *const u8) -> bool`. + +- [ ] **Step 1: Write the failing tests** + +Add to the `tests` module in `cel-runtime/src/raw_stack.rs`: + +```rust +#[test] +fn len_reflects_pushed_bytes() { + let mut stack = RawStack::with_base_alignment(align_of::()); + assert_eq!(stack.len(), 0); + let _ = stack.push(7u32); + assert_eq!(stack.len(), size_of::()); +} + +#[test] +fn push_raw_round_trips_like_push() { + let mut stack = RawStack::with_base_alignment(align_of::()); + let padding1 = stack.push(1u8); + let value = 3.14f64; + let padding2 = + unsafe { stack.push_raw(align_of::(), size_of::(), (&value as *const f64).cast::()) }; + let popped: f64 = unsafe { stack.pop(padding2) }; + assert_eq!(popped, 3.14); + let popped_u8: u8 = unsafe { stack.pop(padding1) }; + assert_eq!(popped_u8, 1); +} + +#[test] +fn push_raw_padding_matches_typed_push() { + let mut stack_a = RawStack::with_base_alignment(align_of::()); + let _ = stack_a.push(1u8); + let padding_typed = stack_a.push(2.5f64); + + let mut stack_b = RawStack::with_base_alignment(align_of::()); + let _ = stack_b.push(1u8); + let value = 2.5f64; + let padding_raw = unsafe { + stack_b.push_raw(align_of::(), size_of::(), (&value as *const f64).cast::()) + }; + assert_eq!(padding_typed, padding_raw); +} +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `cargo test -p cel-runtime raw_stack:: -- --exact len_reflects_pushed_bytes push_raw_round_trips_like_push push_raw_padding_matches_typed_push` +Expected: FAIL with "no method named `len`" / "no method named `push_raw`". + +- [ ] **Step 3: Implement `len` and `push_raw`** + +Add to `impl RawStack` in `cel-runtime/src/raw_stack.rs`, right after `with_base_alignment`: + +```rust + /// Returns the number of bytes currently on the stack. + #[must_use] + pub fn len(&self) -> usize { + self.buffer.len() + } +``` + +Add after the existing `push` method: + +```rust + /// Pushes `size` raw bytes from `src`, aligned to `align`, using the same + /// padding/marker-byte bookkeeping as [`push`](Self::push). + /// + /// - Precondition: `align` is a power of two. + /// + /// # Safety + /// `src` must be valid for reads of `size` bytes. + pub unsafe fn push_raw(&mut self, align: usize, size: usize, src: *const u8) -> bool { + debug_assert!(align.is_power_of_two()); + let len = self.buffer.len(); + let aligned_index = align_index(align, len); + let new_len = aligned_index + size; + + self.buffer.reserve(new_len - len); + unsafe { + self.buffer.set_len(new_len); + if aligned_index - len > 0 { + self.buffer[len].write(1); + self.buffer[len + 1..aligned_index].fill(MaybeUninit::new(0)); + } + std::ptr::copy_nonoverlapping( + src, + self.buffer.as_mut_ptr().add(aligned_index).cast::(), + size, + ); + } + aligned_index - len > 0 + } +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `cargo test -p cel-runtime raw_stack::` +Expected: PASS (all tests in the module, including the 3 new ones and existing ones). + +- [ ] **Step 5: Commit** + +```bash +cargo fmt --all +git add cel-runtime/src/raw_stack.rs +git commit -m "feat(cel-runtime): add RawStack::len and push_raw" +``` + +--- + +### Task 2: `RawStack` — surgical drop/copy/truncate primitives + +**Files:** +- Modify: `cel-runtime/src/raw_stack.rs` + +**Interfaces:** +- Consumes: `RawStack::len` (Task 1). +- Produces: `unsafe fn RawStack::copy_from(&self, offset: usize, size: usize, dst: *mut u8)`, `unsafe fn RawStack::drop_at(&mut self, offset: usize, run_drop: impl FnOnce(*mut u8))`, `unsafe fn RawStack::truncate_to(&mut self, new_len: usize, padding: bool)`, `unsafe fn RawStack::drop_sized(&mut self, size: usize, padding: bool, run_drop: impl FnOnce(*mut u8))`. + +- [ ] **Step 1: Write the failing tests** + +Add to the `tests` module in `cel-runtime/src/raw_stack.rs`: + +```rust +#[test] +fn copy_from_reads_bytes_at_offset() { + let mut stack = RawStack::with_base_alignment(align_of::()); + let _ = stack.push(10u32); + let _ = stack.push(20u32); + let mut buf = [0u8; 4]; + unsafe { stack.copy_from(0, 4, buf.as_mut_ptr()) }; + assert_eq!(u32::from_ne_bytes(buf), 10); +} + +#[test] +fn drop_at_runs_destructor_without_changing_length() { + use std::sync::Arc; + use std::sync::atomic::{AtomicUsize, Ordering}; + + struct DropCounter(Arc); + impl Drop for DropCounter { + fn drop(&mut self) { + self.0.fetch_add(1, Ordering::SeqCst); + } + } + + let count = Arc::new(AtomicUsize::new(0)); + let mut stack = RawStack::with_base_alignment(align_of::()); + let _ = stack.push(DropCounter(count.clone())); + let len_before = stack.len(); + unsafe { + stack.drop_at(0, |ptr| unsafe { std::ptr::drop_in_place(ptr.cast::()) }); + } + assert_eq!(count.load(Ordering::SeqCst), 1); + assert_eq!(stack.len(), len_before); +} + +#[test] +fn truncate_to_strips_recorded_padding() { + let mut stack = RawStack::with_base_alignment(align_of::()); + let _ = stack.push(1u8); + let padding = stack.push(2.5f64); // padding == true: 7 bytes inserted before the f64 + let len_with_value = stack.len(); + unsafe { stack.truncate_to(len_with_value - size_of::(), padding) }; + assert_eq!(stack.len(), 1); // back to just the u8 +} + +#[test] +fn drop_sized_combines_drop_at_and_truncate_to() { + use std::sync::Arc; + use std::sync::atomic::{AtomicUsize, Ordering}; + + struct DropCounter(Arc); + impl Drop for DropCounter { + fn drop(&mut self) { + self.0.fetch_add(1, Ordering::SeqCst); + } + } + + let count = Arc::new(AtomicUsize::new(0)); + let mut stack = RawStack::with_base_alignment(align_of::()); + let _ = stack.push(1u8); + let padding = stack.push(DropCounter(count.clone())); + unsafe { + stack.drop_sized(size_of::(), padding, |ptr| unsafe { + std::ptr::drop_in_place(ptr.cast::()) + }); + } + assert_eq!(count.load(Ordering::SeqCst), 1); + assert_eq!(stack.len(), 1); +} +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `cargo test -p cel-runtime raw_stack:: -- --exact copy_from_reads_bytes_at_offset drop_at_runs_destructor_without_changing_length truncate_to_strips_recorded_padding drop_sized_combines_drop_at_and_truncate_to` +Expected: FAIL with "no method named" errors. + +- [ ] **Step 3: Implement the four methods** + +Add to `impl RawStack` in `cel-runtime/src/raw_stack.rs`, after `push_raw`: + +```rust + /// Copies `size` bytes starting at absolute buffer offset `offset` into `dst`. + /// + /// # Safety + /// `offset..offset + size` must be within the currently-initialized buffer; + /// `dst` must be valid for writes of `size` bytes. + pub unsafe fn copy_from(&self, offset: usize, size: usize, dst: *mut u8) { + unsafe { + std::ptr::copy_nonoverlapping(self.buffer.as_ptr().add(offset).cast::(), dst, size); + } + } + + /// Drops a value in place at absolute buffer offset `offset`, without + /// altering the stack's tracked length. + /// + /// # Safety + /// `offset` must point to a live, valid value; `run_drop` must correctly + /// run that value's destructor given a pointer to its start. + pub unsafe fn drop_at(&mut self, offset: usize, run_drop: impl FnOnce(*mut u8)) { + unsafe { run_drop(self.buffer.as_mut_ptr().add(offset).cast::()) }; + } + + /// Truncates the stack back to `new_len`, additionally stripping `padding` + /// bytes that preceded the removed region (scanned the same way + /// [`pop`](Self::pop) does). + /// + /// # Safety + /// No live (undropped) value may exist at or above `new_len`. + pub unsafe fn truncate_to(&mut self, new_len: usize, padding: bool) { + let padding_count = if padding { + self.buffer[..new_len] + .iter() + .rev() + .take_while(|&x| unsafe { x.assume_init() == 0 }) + .count() + + 1 + } else { + 0 + }; + self.buffer.truncate(new_len - padding_count); + } + + /// Drops a value of `size` bytes at the top of the stack in place, then + /// removes it (and any padding that preceded it). + /// + /// # Safety + /// The top `size` bytes (plus padding if `padding` is true) must be a + /// live, valid value; `run_drop` must correctly run its destructor given a + /// pointer to its start. + pub unsafe fn drop_sized(&mut self, size: usize, padding: bool, run_drop: impl FnOnce(*mut u8)) { + let p = self.buffer.len() - size; + unsafe { + self.drop_at(p, run_drop); + self.truncate_to(p, padding); + } + } +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `cargo test -p cel-runtime raw_stack::` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +cargo fmt --all +git add cel-runtime/src/raw_stack.rs +git commit -m "feat(cel-runtime): add RawStack drop/copy/truncate primitives" +``` + +--- + +### Task 3: `RawStack::repack` — context-independent layout construction + +**Files:** +- Modify: `cel-runtime/src/raw_stack.rs` + +**Interfaces:** +- Consumes: `RawStack::len` (Task 1). +- Produces: `unsafe fn RawStack::repack(&mut self, ambient_start: usize, dest_base: usize, total_size: usize, src_offsets: &[usize], dest_offsets: &[usize], sizes: &[usize]) -> bool`. + +- [ ] **Step 1: Write the failing test** + +Add to the `tests` module in `cel-runtime/src/raw_stack.rs`: + +```rust +#[test] +fn repack_moves_elements_to_ideal_offsets_and_reports_padding() { + // Ambient layout: [u8 @0][pad][u32 @4][u8 @8] — u8 then u32 then u8, each + // pushed with ordinary alignment relative to a 1-byte ambient start. + let mut stack = RawStack::with_base_alignment(align_of::()); + let ambient_start = stack.len(); // 0 + let _ = stack.push(0xAAu8); // element 0: ambient offset 0 + let _p1 = stack.push(0xBBBB_BBBBu32); // element 1: ambient offset 4 (1 byte padded to 4) + let _p2 = stack.push(0xCCu8); // element 2: ambient offset 8 + + // Ideal (self-contained) layout for (u8, u32, u8) from a zero base: + // offset 0 (u8), offset 4 (u32, aligned up from 1), offset 8 (u8) -> total 9, + // rounded to the tuple's own max align (4) -> total_size 12. + let src_offsets = [0usize, 4, 8]; + let dest_offsets = [0usize, 4, 8]; + let sizes = [1usize, 4, 1]; + let total_size = 12usize; + let dest_base = 0usize; // ambient_start (0) is already 4-aligned + + let padding = unsafe { + stack.repack(ambient_start, dest_base, total_size, &src_offsets, &dest_offsets, &sizes) + }; + assert!(!padding, "ambient_start was already aligned; no leading pad expected"); + assert_eq!(stack.len(), dest_base + total_size); + + let mut a = [0u8; 1]; + let mut b = [0u8; 4]; + let mut c = [0u8; 1]; + unsafe { + stack.copy_from(dest_base, 1, a.as_mut_ptr()); + stack.copy_from(dest_base + 4, 4, b.as_mut_ptr()); + stack.copy_from(dest_base + 8, 1, c.as_mut_ptr()); + } + assert_eq!(a[0], 0xAA); + assert_eq!(u32::from_ne_bytes(b), 0xBBBB_BBBB); + assert_eq!(c[0], 0xCC); +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `cargo test -p cel-runtime raw_stack::repack_moves_elements_to_ideal_offsets_and_reports_padding` +Expected: FAIL with "no method named `repack`". + +- [ ] **Step 3: Implement `repack`** + +Add to `impl RawStack` in `cel-runtime/src/raw_stack.rs`, after `drop_sized`: + +```rust + /// Repacks `sizes.len()` already-pushed values (currently at the absolute + /// byte offsets in `src_offsets`) into one contiguous, self-contained + /// region of `total_size` bytes starting at `dest_base`, placing element + /// `i` at `dest_base + dest_offsets[i]`. Adjusts the tracked length to + /// `dest_base + total_size` and returns whether leading padding was + /// inserted between `ambient_start` and `dest_base`. + /// + /// - Precondition: `src_offsets`, `dest_offsets`, and `sizes` have equal + /// length; `dest_base >= ambient_start`; the source ranges are + /// currently valid, initialized bytes. + /// + /// - Complexity: O(n) in `sizes.len()`. + /// + /// # Safety + /// The offsets and sizes must correctly describe the actual bytes in the + /// buffer; no two destination ranges may overlap. + pub unsafe fn repack( + &mut self, + ambient_start: usize, + dest_base: usize, + total_size: usize, + src_offsets: &[usize], + dest_offsets: &[usize], + sizes: &[usize], + ) -> bool { + debug_assert!(dest_base >= ambient_start); + debug_assert_eq!(src_offsets.len(), sizes.len()); + debug_assert_eq!(dest_offsets.len(), sizes.len()); + + let target_len = dest_base + total_size; + let current_len = self.buffer.len(); + let grown_len = current_len.max(target_len); + unsafe { + if grown_len > current_len { + self.buffer.reserve(grown_len - current_len); + self.buffer.set_len(grown_len); + } + let base_ptr = self.buffer.as_mut_ptr().cast::(); + for i in 0..sizes.len() { + std::ptr::copy( + base_ptr.add(src_offsets[i]), + base_ptr.add(dest_base + dest_offsets[i]), + sizes[i], + ); + } + if dest_base > ambient_start { + self.buffer[ambient_start].write(1); + self.buffer[ambient_start + 1..dest_base].fill(MaybeUninit::new(0)); + } + self.buffer.set_len(target_len); + } + dest_base > ambient_start + } +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `cargo test -p cel-runtime raw_stack::` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +cargo fmt --all +git add cel-runtime/src/raw_stack.rs +git commit -m "feat(cel-runtime): add RawStack::repack for layout-independent tuples" +``` + +--- + +### Task 4: `StackInfo`/`AssociatedType` metadata + `RawDropper`/`DynTuple` + +**Files:** +- Modify: `cel-runtime/src/dyn_segment.rs` + +**Interfaces:** +- Produces: `pub type RawDropper = unsafe fn(*mut u8, &[AssociatedType])`, `pub struct DynTuple`, `AssociatedType { type_id, type_name, offset, size, align, dropper: RawDropper, associated }`, `StackInfo { type_id, type_name, padding, size, align, raw_dropper: RawDropper, associated }` (public fields: `type_id`, `type_name`, `size`, `align`, `associated`; `padding`/`raw_dropper` stay `pub(crate)`). +- Removes: the old `type Dropper = fn(&mut RawStack);` mechanism and `StackInfo.dropper`/per-closure droppers generated in `push_type`/`to_stack_info_list` — replaced by `raw_dropper` + `RawStack::drop_sized`. + +This task is a mechanical widening: every existing behavior must stay observably identical (all current `dyn_segment.rs` tests continue passing unchanged), while adding the fields tuples need. + +- [ ] **Step 1: Write the failing tests** + +Add to the `tests` module in `cel-runtime/src/dyn_segment.rs`: + +```rust +#[test] +fn stack_info_records_size_and_align() { + let mut seg = DynSegment::new::<()>(); + seg.op0(|| 7u32); + let infos = seg.peek_stack_infos(1); + assert_eq!(infos[0].size, size_of::()); + assert_eq!(infos[0].align, align_of::()); + assert!(infos[0].associated.is_empty()); +} + +#[test] +fn associated_type_carries_offset_size_align_dropper() { + // Exercises the new AssociatedType shape directly — no runtime behavior + // yet, just the data shape this task adds. + let a = AssociatedType { + type_id: std::any::TypeId::of::(), + type_name: std::borrow::Cow::Borrowed("u32"), + offset: 4, + size: 4, + align: 4, + dropper: |ptr, _associated| unsafe { std::ptr::drop_in_place(ptr.cast::()) }, + associated: Vec::new(), + }; + assert_eq!(a.offset, 4); + assert_eq!(a.size, 4); + assert_eq!(a.align, 4); +} +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `cargo test -p cel-runtime dyn_segment:: -- --exact stack_info_records_size_and_align associated_type_carries_offset_size_align_dropper` +Expected: FAIL — `size`/`align` fields don't exist yet on `StackInfo`, and `AssociatedType`'s struct literal doesn't have `offset`/`size`/`align`/`dropper` fields yet. + +- [ ] **Step 3: Widen the types** + +Replace the `AssociatedType` struct and the `Dropper`/`StackInfo` definitions in `cel-runtime/src/dyn_segment.rs`: + +```rust +/// Drops a value in place, given a pointer to its bytes and (for tuple +/// values) its own element metadata for recursive drops. +/// +/// # Safety +/// `ptr` must point to a valid, live, properly aligned value of the type this +/// dropper was generated for; `associated` must be that same value's own +/// element list (empty for non-tuple values). +pub type RawDropper = unsafe fn(*mut u8, &[AssociatedType]); + +/// Recursive type node carrying a [`TypeId`], display name, byte layout, and +/// an in-place dropper — describes one element of a tuple (or, nested, one +/// element of a tuple element). +#[derive(Clone, Debug)] +pub struct AssociatedType { + /// Runtime type id for this node. + pub type_id: TypeId, + /// Human-readable name for error reporting (borrowed when from `type_name::()`). + pub type_name: Cow<'static, str>, + /// Byte offset from the start of the enclosing tuple. + pub offset: usize, + /// Size in bytes of this element's value. + pub size: usize, + /// Required alignment in bytes of this element's value. + pub align: usize, + /// In-place dropper for this element, callable at `base + offset`. + pub dropper: RawDropper, + /// Child types, for a nested tuple element. + pub associated: Vec, +} + +/// Marker type used as the `TypeId` for tuple aggregate stack entries. +/// +/// A tuple's real type identity is the ordered `associated` list on its +/// [`StackInfo`], not this marker's `TypeId` — comparisons that need to +/// distinguish tuple shapes must inspect `associated`, not `type_id`. +#[derive(Debug)] +pub struct DynTuple; + +/// Information about a type on the stack, including its cleanup function. +/// +/// Holds metadata for a value pushed onto the stack: runtime type id, display +/// name for errors, padding, size/alignment, an in-place dropper, and an +/// optional list of associated element types (populated for tuples). +pub struct StackInfo { + /// Runtime type id for this stack slot (e.g. for scope matching). + pub type_id: TypeId, + /// Human-readable type name for error reporting (borrowed when from `type_name::()`). + pub type_name: Cow<'static, str>, + /// Whether padding was inserted before this value for alignment. + pub(crate) padding: bool, + /// Size in bytes of this stack slot's value. + pub size: usize, + /// Required alignment in bytes of this stack slot's value. + pub align: usize, + /// In-place dropper for this value, callable at its own start address. + pub(crate) raw_dropper: RawDropper, + /// Associated element types (populated for tuples; empty otherwise). + pub associated: Vec, +} +``` + +Update `ToTypeIdList for CNil<()>` / `ToTypeIdList for CStackList`: + +```rust +impl ToTypeIdList for CNil<()> { + fn to_stack_info_list() -> Vec { + Vec::new() + } +} + +impl ToTypeIdList + for CStackList +{ + fn to_stack_info_list() -> Vec { + let mut list = T::to_stack_info_list(); + list.push(StackInfo { + type_id: TypeId::of::(), + type_name: Cow::Borrowed(std::any::type_name::()), + padding: Self::HEAD_PADDED, + size: size_of::(), + align: align_of::(), + raw_dropper: |ptr, _associated| unsafe { std::ptr::drop_in_place(ptr.cast::()) }, + associated: Vec::new(), + }); + list + } +} +``` + +Update `DynSegment::push_type`: + +```rust + /// Push type to stack and register dropper. + fn push_type(&mut self) + where + T: 'static, + { + let aligned_index = align_index(align_of::(), self.stack_index); + let padded = aligned_index != self.stack_index; + + self.stack_ids.push(StackInfo { + type_id: TypeId::of::(), + type_name: Cow::Borrowed(std::any::type_name::()), + padding: padded, + size: size_of::(), + align: align_of::(), + raw_dropper: |ptr, _associated| unsafe { std::ptr::drop_in_place(ptr.cast::()) }, + associated: Vec::new(), + }); + self.stack_index = aligned_index + size_of::(); + } +``` + +Update `capture_unwind`/`unwind_on_err`: + +```rust + /// Captures the current stack droppers for use when unwinding on error. + /// + /// - Complexity: O(n) in the current stack depth. + fn capture_unwind(&self) -> Vec<(usize, bool, RawDropper, Vec)> { + self.stack_ids + .iter() + .map(|info| (info.size, info.padding, info.raw_dropper, info.associated.clone())) + .collect() + } + + /// Runs the captured droppers in reverse order on error, then propagates the error. + fn unwind_on_err( + unwind: &[(usize, bool, RawDropper, Vec)], + stack: &mut RawStack, + result: Result, + ) -> Result { + match result { + Ok(r) => Ok(r), + Err(e) => { + for (size, padding, raw_dropper, associated) in unwind.iter().rev() { + unsafe { + stack.drop_sized(*size, *padding, |ptr| unsafe { + raw_dropper(ptr, associated) + }); + } + } + Err(e) + } + } + } +``` + +No other call sites need to change — `op0r`/`op1r`/`op2r` already just do `let unwind = self.capture_unwind();` and pass `&unwind` through, which still type-checks with the new return type. + +- [ ] **Step 4: Run tests to verify everything passes** + +Run: `cargo test -p cel-runtime` +Expected: PASS — every pre-existing test in `dyn_segment.rs` (drop-on-error, op1r/op2r unwind, etc.) plus the two new tests from Step 1. + +- [ ] **Step 5: Commit** + +```bash +cargo fmt --all +git add cel-runtime/src/dyn_segment.rs +git commit -m "refactor(cel-runtime): widen StackInfo/AssociatedType with size/align/raw_dropper" +``` + +--- + +### Task 5: Tuple construction and `.N` indexing (`DynSegment` API) + +**Files:** +- Modify: `cel-runtime/src/dyn_segment.rs` + +**Interfaces:** +- Consumes: `RawStack::{repack, copy_from, drop_at, truncate_to, push_raw, len}` (Tasks 1-3); `StackInfo`/`AssociatedType`/`RawDropper`/`DynTuple` (Task 4). +- Produces: `DynSegment::current_stack_offset(&self) -> usize`, `DynSegment::make_tuple(&mut self, n: usize, ambient_start: usize)`, `DynSegment::tuple_index(&mut self, index: usize)`, `DynSegment::peek_tuple_arity(&self) -> Option`. + +- [ ] **Step 1: Write the failing tests** + +Add to the `tests` module in `cel-runtime/src/dyn_segment.rs`: + +```rust +#[test] +fn make_tuple_then_index_each_element() { + let mut seg = DynSegment::new::<()>(); + let ambient_start = seg.current_stack_offset(); + seg.op0(|| 10u32); + seg.op0(|| "hello"); + seg.make_tuple(2, ambient_start); + assert_eq!(seg.peek_tuple_arity(), Some(2)); + + // Index element 1 first on a clone-free single segment isn't possible + // (tuple_index consumes the tuple), so build two segments to check both. + let mut seg0 = DynSegment::new::<()>(); + let ambient_start0 = seg0.current_stack_offset(); + seg0.op0(|| 10u32); + seg0.op0(|| "hello"); + seg0.make_tuple(2, ambient_start0); + seg0.tuple_index(0); + assert_eq!(seg0.call0::().unwrap(), 10); + + seg.tuple_index(1); + assert_eq!(seg.call0::<&'static str>().unwrap(), "hello"); +} + +#[test] +fn tuple_layout_is_independent_of_ambient_stack_depth() { + // (u8, u32): with nothing ahead of it vs. with a u8 already on the stack, + // internal padding between elements must be identical either way. + let mut seg_a = DynSegment::new::<()>(); + let ambient_a = seg_a.current_stack_offset(); + seg_a.op0(|| 1u8); + seg_a.op0(|| 2u32); + seg_a.make_tuple(2, ambient_a); + + let mut seg_b = DynSegment::new::<()>(); + seg_b.op0(|| 99u8); // extra value ahead, shifts ambient depth + let ambient_b = seg_b.current_stack_offset(); + seg_b.op0(|| 1u8); + seg_b.op0(|| 2u32); + seg_b.make_tuple(2, ambient_b); + + seg_a.tuple_index(1); + seg_b.tuple_index(1); + assert_eq!(seg_a.call0::().unwrap(), 2); + seg_b.op2(|_extra: u8, x: u32| x).unwrap(); + assert_eq!(seg_b.call0::().unwrap(), 2); +} + +#[test] +fn tuple_index_drops_every_other_element_exactly_once() { + use std::sync::Arc; + use std::sync::atomic::{AtomicUsize, Ordering}; + + #[derive(Clone)] + struct DropCounter(Arc); + impl Drop for DropCounter { + fn drop(&mut self) { + self.0.fetch_add(1, Ordering::SeqCst); + } + } + + let drop_count = Arc::new(AtomicUsize::new(0)); + let tracker = DropCounter(drop_count.clone()); + + let mut seg = DynSegment::new::<()>(); + let ambient_start = seg.current_stack_offset(); + seg.op0(move || tracker.clone()); + seg.op0(|| 42u32); + seg.make_tuple(2, ambient_start); + seg.tuple_index(1); // keep the u32, drop the DropCounter + + assert_eq!(seg.call0::().unwrap(), 42); + assert_eq!(drop_count.load(Ordering::SeqCst), 1); +} + +#[test] +fn tuple_index_combined_with_another_op() { + // Mirrors the spec's `5 + (0, 1).1` case: indexing must leave the stack in + // a state a subsequent op can correctly consume. + let mut seg = DynSegment::new::<()>(); + seg.op0(|| 5u32); + let ambient_start = seg.current_stack_offset(); + seg.op0(|| 0u32); + seg.op0(|| 1u32); + seg.make_tuple(2, ambient_start); + seg.tuple_index(1); + seg.op2(|a: u32, b: u32| a + b).unwrap(); + assert_eq!(seg.call0::().unwrap(), 6); +} + +#[test] +fn one_tuple_round_trips() { + let mut seg = DynSegment::new::<()>(); + let ambient_start = seg.current_stack_offset(); + seg.op0(|| 99u32); + seg.make_tuple(1, ambient_start); + assert_eq!(seg.peek_tuple_arity(), Some(1)); + seg.tuple_index(0); + assert_eq!(seg.call0::().unwrap(), 99); +} +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `cargo test -p cel-runtime dyn_segment:: -- --exact make_tuple_then_index_each_element tuple_layout_is_independent_of_ambient_stack_depth tuple_index_drops_every_other_element_exactly_once tuple_index_combined_with_another_op one_tuple_round_trips` +Expected: FAIL with "no method named `current_stack_offset`" / `make_tuple` / `tuple_index` / `peek_tuple_arity`. + +- [ ] **Step 3: Implement `current_stack_offset`, `peek_tuple_arity`, `make_tuple`, `tuple_index`** + +Add `use anyhow::anyhow;` to the top-level imports if not already present (it is not — `dyn_segment.rs` currently imports `anyhow::Result` and `anyhow::ensure`; leave those, this task doesn't need `anyhow!` yet). + +Add a free function near the top of `cel-runtime/src/dyn_segment.rs` (outside `impl DynSegment`), after the `AssociatedType`/`DynTuple` definitions from Task 4: + +```rust +/// `RawDropper` for a tuple value: drops each element at `ptr + element.offset` +/// in reverse order, recursing into nested tuples via their own droppers. +/// +/// # Safety +/// `ptr` must point to a live tuple value whose layout matches `associated`. +unsafe fn drop_tuple(ptr: *mut u8, associated: &[AssociatedType]) { + for elem in associated.iter().rev() { + unsafe { (elem.dropper)(ptr.add(elem.offset), &elem.associated) }; + } +} + +/// Extracts element `index` from the tuple currently on top of `stack`, +/// dropping every other element, leaving just the extracted value on top. +/// +/// - Complexity: O(n) in the tuple's arity. +/// +/// # Safety +/// The top `tuple_size` bytes (plus `tuple_padding` if set) of `stack` must be +/// a live tuple value whose layout matches `associated`. +unsafe fn extract_tuple_element( + stack: &mut RawStack, + tuple_size: usize, + tuple_padding: bool, + associated: &[AssociatedType], + index: usize, +) { + let tuple_base = stack.len() - tuple_size; + let target = &associated[index]; + + let mut scratch = vec![0u8; target.size]; + unsafe { + stack.copy_from(tuple_base + target.offset, target.size, scratch.as_mut_ptr()); + } + + for (i, elem) in associated.iter().enumerate().rev() { + if i == index { + continue; + } + let elem_associated = &elem.associated; + unsafe { + stack.drop_at(tuple_base + elem.offset, |ptr| unsafe { + (elem.dropper)(ptr, elem_associated) + }); + } + } + + unsafe { + stack.truncate_to(tuple_base, tuple_padding); + stack.push_raw(target.align, target.size, scratch.as_ptr()); + } +} +``` + +Add these methods to `impl DynSegment` in `cel-runtime/src/dyn_segment.rs`, near `push_type`: + +```rust + /// Returns the current parse-time stack byte offset. + /// + /// Snapshot this before parsing a tuple's first element and pass it to + /// [`make_tuple`](Self::make_tuple). + #[must_use] + pub fn current_stack_offset(&self) -> usize { + self.stack_index + } + + /// Returns the arity of the tuple on top of the stack, or `None` if the + /// top value isn't a tuple. + #[must_use] + pub fn peek_tuple_arity(&self) -> Option { + let info = self.stack_ids.last()?; + (info.type_id == TypeId::of::()).then(|| info.associated.len()) + } + + /// Collapses the top `n` stack values (pushed starting at byte offset + /// `ambient_start`, e.g. via [`current_stack_offset`](Self::current_stack_offset) + /// captured before parsing the first element) into one tuple value. + /// + /// The tuple's internal layout (offsets between elements) depends only on + /// the elements' own types — never on `ambient_start` — matching the + /// layout a `#[repr(C)]` struct with these fields, in this order, would + /// use. + /// + /// - Precondition: at least `n` values are on the stack, pushed + /// contiguously starting at `ambient_start` with no other values + /// interleaved. + /// + /// - Complexity: O(n). + pub fn make_tuple(&mut self, n: usize, ambient_start: usize) { + debug_assert!(n <= self.stack_ids.len()); + let start = self.stack_ids.len() - n; + let elems: Vec = self.stack_ids.drain(start..).collect(); + + let mut ambient_offset = ambient_start; + let mut ideal_offset = 0usize; + let mut tuple_align = 1usize; + let mut src_offsets = Vec::with_capacity(n); + let mut associated = Vec::with_capacity(n); + for elem in &elems { + ambient_offset = align_index(elem.align, ambient_offset); + ideal_offset = align_index(elem.align, ideal_offset); + tuple_align = tuple_align.max(elem.align); + + src_offsets.push(ambient_offset); + associated.push(AssociatedType { + type_id: elem.type_id, + type_name: elem.type_name.clone(), + offset: ideal_offset, + size: elem.size, + align: elem.align, + dropper: elem.raw_dropper, + associated: elem.associated.clone(), + }); + + ambient_offset += elem.size; + ideal_offset += elem.size; + } + let total_size = align_index(tuple_align, ideal_offset); + let dest_base = align_index(tuple_align, ambient_start); + + let dest_offsets: Vec = associated.iter().map(|a| a.offset).collect(); + let sizes: Vec = elems.iter().map(|e| e.size).collect(); + + self.segment.raw0_(move |stack| { + unsafe { + stack.repack(ambient_start, dest_base, total_size, &src_offsets, &dest_offsets, &sizes); + } + Ok(()) + }); + + self.stack_ids.push(StackInfo { + type_id: TypeId::of::(), + type_name: Cow::Borrowed(std::any::type_name::()), + padding: dest_base != ambient_start, + size: total_size, + align: tuple_align, + raw_dropper: drop_tuple, + associated, + }); + self.stack_index = dest_base + total_size; + } + + /// Extracts element `index` from the tuple on top of the stack, replacing + /// the whole tuple with just that element's value. + /// + /// - Precondition: the top-of-stack value is a tuple with at least + /// `index + 1` elements. + /// + /// - Complexity: O(n) in the tuple's arity. + pub fn tuple_index(&mut self, index: usize) { + let info = self + .stack_ids + .pop() + .expect("tuple_index requires a value on the stack"); + debug_assert_eq!( + info.type_id, + TypeId::of::(), + "tuple_index requires a tuple on top of the stack" + ); + debug_assert!(index < info.associated.len(), "tuple_index out of range"); + + let tuple_start = self.stack_index - info.size; + let target = info.associated[index].clone(); + let associated = info.associated.clone(); + let tuple_padding = info.padding; + let tuple_size = info.size; + + self.segment.raw0_(move |stack| { + unsafe { + extract_tuple_element(stack, tuple_size, tuple_padding, &associated, index); + } + Ok(()) + }); + + let target_start = align_index(target.align, tuple_start); + self.stack_ids.push(StackInfo { + type_id: target.type_id, + type_name: target.type_name, + padding: target_start != tuple_start, + size: target.size, + align: target.align, + raw_dropper: target.dropper, + associated: target.associated, + }); + self.stack_index = target_start + target.size; + } +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `cargo test -p cel-runtime dyn_segment::` +Expected: PASS — all 5 new tests plus every pre-existing test in the module. + +- [ ] **Step 5: Commit** + +```bash +cargo fmt --all +git add cel-runtime/src/dyn_segment.rs +git commit -m "feat(cel-runtime): add tuple construction (make_tuple) and indexing (tuple_index)" +``` + +--- + +### Task 6: Parser grammar — tuple literals + +**Files:** +- Modify: `cel-parser/src/lib.rs` + +**Interfaces:** +- Consumes: `DynSegment::{current_stack_offset, make_tuple}` (Task 5). + +- [ ] **Step 1: Write the failing tests** + +Add to the `tests` module in `cel-parser/src/lib.rs` (find the existing `#[cfg(test)] mod tests` block and add alongside the other parser tests): + +```rust +#[test] +fn unit_still_parses_as_unit() { + let mut parser = CELParser::new(OpLookup::new()); + let mut seg = parser.parse_str("()").unwrap(); + seg.call0::<()>().unwrap(); +} + +#[test] +fn single_paren_expression_is_grouping_not_tuple() { + let mut parser = CELParser::new(OpLookup::new()); + let mut seg = parser.parse_str("(1i32 + 2i32)").unwrap(); + assert_eq!(seg.call0::().unwrap(), 3); +} + +#[test] +fn one_tuple_requires_trailing_comma() { + let mut parser = CELParser::new(OpLookup::new()); + let mut seg = parser.parse_str("(1i32,)").unwrap(); + assert_eq!(seg.peek_tuple_arity(), Some(1)); + seg.tuple_index(0); + assert_eq!(seg.call0::().unwrap(), 1); +} + +#[test] +fn two_element_tuple_no_trailing_comma() { + let mut parser = CELParser::new(OpLookup::new()); + let mut seg = parser.parse_str(r#"("Hello", 42i32)"#).unwrap(); + assert_eq!(seg.peek_tuple_arity(), Some(2)); +} + +#[test] +fn trailing_comma_rejected_for_arity_two() { + let mut parser = CELParser::new(OpLookup::new()); + let result = parser.parse_str("(1i32, 2i32,)"); + assert!(result.is_err(), "trailing comma is only valid for 1-tuples"); +} + +#[test] +fn missing_comma_between_elements_is_an_error() { + let mut parser = CELParser::new(OpLookup::new()); + let result = parser.parse_str("(1i32 2i32)"); + assert!(result.is_err()); +} +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `cargo test -p cel-parser one_tuple_requires_trailing_comma two_element_tuple_no_trailing_comma trailing_comma_rejected_for_arity_two missing_comma_between_elements_is_an_error` +Expected: FAIL — `(1i32,)` and `("Hello", 42i32)` currently error ("expected closing parenthesis"), and `peek_tuple_arity`/`tuple_index` calls won't compile against the current `DynSegment` re-export until Task 5 lands (already done) — these tests specifically fail on parse behavior, not missing methods. + +- [ ] **Step 3: Implement the grammar** + +Replace the `Parenthesis` arm of `is_primary_expression` in `cel-parser/src/lib.rs`: + +```rust + Some(Token::OpenDelim { + delimiter: Delimiter::Parenthesis, + .. + }) => { + self.advance(); + // Unit expression: () + if matches!( + self.peek_token(), + Some(Token::CloseDelim { + delimiter: Delimiter::Parenthesis, + .. + }) + ) { + self.advance(); + self.context.just(()); + return Ok(true); + } + let ambient_start = self.context.current_stack_offset(); + if !self.is_or_expression()? { + return Err(self.error_at("expected expression")); + } + if matches!( + self.peek_token(), + Some(Token::CloseDelim { + delimiter: Delimiter::Parenthesis, + .. + }) + ) { + // Grouping: exactly one expression, no comma. + self.advance(); + return Ok(true); + } + if !self.is_punctuation(",") { + return Err(self.error_at("expected ',' or closing parenthesis")); + } + let mut count = 1; + if matches!( + self.peek_token(), + Some(Token::CloseDelim { + delimiter: Delimiter::Parenthesis, + .. + }) + ) { + // Single element + trailing comma: 1-tuple. + self.advance(); + self.context.make_tuple(count, ambient_start); + return Ok(true); + } + loop { + if !self.is_or_expression()? { + return Err(self.error_at("expected expression after ','")); + } + count += 1; + if matches!( + self.peek_token(), + Some(Token::CloseDelim { + delimiter: Delimiter::Parenthesis, + .. + }) + ) { + self.advance(); + break; + } + if !self.is_punctuation(",") { + return Err(self.error_at("expected ',' or closing parenthesis")); + } + } + self.context.make_tuple(count, ambient_start); + Ok(true) + } +``` + +Update the grammar doc comment at the top of `cel-parser/src/lib.rs` (in the module-level `//!` block): + +``` +//! primary_expression = literal | identifier | tuple_or_group | if_expression. +//! tuple_or_group = "(" [ or_expression ["," [ or_expression { "," or_expression } ]] ] ")". +``` + +(Replace the existing `primary_expression = literal | identifier | "(" or_expression ")" | if_expression.` line and add the new `tuple_or_group` line right after it.) + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `cargo test -p cel-parser` +Expected: PASS — all 6 new tests plus the full existing `cel-parser` suite (unchanged grouping/if/call behavior). + +- [ ] **Step 5: Commit** + +```bash +cargo fmt --all +git add cel-parser/src/lib.rs +git commit -m "feat(cel-parser): parse tuple literals" +``` + +--- + +### Task 7: Parser grammar — `.N` indexing + +**Files:** +- Modify: `cel-parser/src/lib.rs` + +**Interfaces:** +- Consumes: `DynSegment::{peek_tuple_arity, tuple_index}` (Task 5); tuple literal parsing (Task 6). + +- [ ] **Step 1: Write the failing tests** + +Add to the `tests` module in `cel-parser/src/lib.rs`: + +```rust +#[test] +fn index_first_element_of_tuple() { + let mut parser = CELParser::new(OpLookup::new()); + let mut seg = parser.parse_str("(10i32, 20i32).0").unwrap(); + assert_eq!(seg.call0::().unwrap(), 10); +} + +#[test] +fn index_second_element_of_tuple() { + let mut parser = CELParser::new(OpLookup::new()); + let mut seg = parser.parse_str("(10i32, 20i32).1").unwrap(); + assert_eq!(seg.call0::().unwrap(), 20); +} + +#[test] +fn indexing_combined_with_addition() { + let mut parser = CELParser::new(OpLookup::new()); + let mut seg = parser.parse_str("5i32 + (0i32, 1i32).1").unwrap(); + assert_eq!(seg.call0::().unwrap(), 6); +} + +#[test] +fn indexing_combined_with_addition_on_the_right() { + let mut parser = CELParser::new(OpLookup::new()); + let mut seg = parser.parse_str("(0i32, 1i32).1 + 5i32").unwrap(); + assert_eq!(seg.call0::().unwrap(), 6); +} + +#[test] +fn out_of_range_index_is_a_parse_error() { + let mut parser = CELParser::new(OpLookup::new()); + let result = parser.parse_str("(1i32, 2i32).5"); + assert!(result.is_err()); +} + +#[test] +fn indexing_a_non_tuple_is_a_parse_error() { + let mut parser = CELParser::new(OpLookup::new()); + let result = parser.parse_str("1i32.0"); + assert!(result.is_err()); +} + +#[test] +fn suffixed_index_is_a_parse_error() { + let mut parser = CELParser::new(OpLookup::new()); + let result = parser.parse_str("(1i32, 2i32).0i32"); + assert!(result.is_err()); +} +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `cargo test -p cel-parser index_first_element_of_tuple index_second_element_of_tuple indexing_combined_with_addition indexing_combined_with_addition_on_the_right out_of_range_index_is_a_parse_error indexing_a_non_tuple_is_a_parse_error suffixed_index_is_a_parse_error` +Expected: FAIL — `.N` isn't recognized as postfix syntax yet, so these parse as `1i32` followed by a stray `.0`/etc. and error or misparse. + +- [ ] **Step 3: Implement `.N` postfix indexing** + +Replace `is_postfix_expression` in `cel-parser/src/lib.rs`: + +```rust + /// `postfix_expression = primary_expression { "(" parameter_list ")" | "." unsuffixed_integer }.` + fn is_postfix_expression(&mut self) -> Result { + let start_span = self.peek_span(); + if !self.is_primary_expression()? { + return Ok(false); + } + loop { + if matches!( + self.peek_token(), + Some(Token::OpenDelim { + delimiter: Delimiter::Parenthesis, + .. + }) + ) { + self.advance(); // consume "(" + let arg_count = self.parameter_list()?; + match self.peek_token() { + Some(Token::CloseDelim { + delimiter: Delimiter::Parenthesis, + .. + }) => { + self.advance(); // consume ")" + } + _ => return Err(self.error_at("expected closing parenthesis")), + } + // Stack order is [callee, arg1, arg2, ...]; lookup peeks top (arg_count + 1) entries. + self.op_lookup.lookup( + "()", + &mut self.context, + arg_count + 1, + start_span.expect("production has token at start"), + self.last_span, + )?; + } else if self.is_punctuation(".") { + let index = match self.peek_token() { + Some(Token::Literal(CelLiteral::Int(integer))) => { + let integer = integer.clone(); + if !integer.suffix().is_empty() { + return Err(self.error_at("tuple index must be an unsuffixed integer")); + } + self.advance(); + integer.base10_parse::().map_err(|e| { + self.error_at(&format!("invalid tuple index `{integer}`: {e}")) + })? + } + _ => return Err(self.error_at("expected integer after '.'")), + }; + let arity = self + .context + .peek_tuple_arity() + .ok_or_else(|| self.error_at("'.N' requires a tuple"))?; + if index >= arity { + return Err(self.error_at(&format!( + "tuple index `{index}` out of range for tuple of arity {arity}" + ))); + } + self.context.tuple_index(index); + } else { + break; + } + } + Ok(true) + } +``` + +Update the grammar doc comment at the top of `cel-parser/src/lib.rs`: + +``` +//! postfix_expression = primary_expression { "(" parameter_list ")" | "." unsuffixed_integer }. +``` + +(Replace the existing `postfix_expression = primary_expression { "(" parameter_list ")" }.` line.) + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `cargo test -p cel-parser` +Expected: PASS — all 7 new tests plus the full existing suite. + +- [ ] **Step 5: Commit** + +```bash +cargo fmt --all +git add cel-parser/src/lib.rs +git commit -m "feat(cel-parser): parse .N tuple indexing" +``` + +--- + +### Task 8: Generalized tuple-shaped op registration + +**Files:** +- Modify: `cel-parser/src/op_table.rs` + +**Interfaces:** +- Consumes: `StackInfo.associated` (Task 4/5). +- Produces: `pub struct TupleOpSignature { name, shape, tuple_operand_index, operand_type_ids, op_fn }`, `OpLookup::register_tuple_op(&mut self, signature: TupleOpSignature)`. + +- [ ] **Step 1: Write the failing test** + +Add to the `tests` module in `cel-parser/src/op_table.rs`: + +```rust +#[test] +fn tuple_shaped_signature_matches_and_dispatches() -> Result<()> { + let mut lookup = OpLookup::new(); + lookup.register_tuple_op(TupleOpSignature { + name: "greet".to_string(), + shape: vec![TypeId::of::(), TypeId::of::()], + tuple_operand_index: 0, + operand_type_ids: vec![], + op_fn: |seg, _span| { + seg.tuple_index(1); + seg.op1(|_ignored: i32| true) + }, + }); + + let mut segment = DynSegment::new::<()>(); + let ambient_start = segment.current_stack_offset(); + segment.op0(|| "hi".to_string()); + segment.op0(|| 7i32); + segment.make_tuple(2, ambient_start); + + lookup.lookup("greet", &mut segment, 1, Span::call_site(), Span::call_site())?; + assert!(segment.call0::()?); + Ok(()) +} + +#[test] +fn tuple_shaped_signature_does_not_match_wrong_shape() { + let mut lookup = OpLookup::new(); + lookup.register_tuple_op(TupleOpSignature { + name: "greet".to_string(), + shape: vec![TypeId::of::(), TypeId::of::()], + tuple_operand_index: 0, + operand_type_ids: vec![], + op_fn: |seg, _span| { + seg.tuple_index(1); + seg.op1(|_ignored: i32| true) + }, + }); + + let mut segment = DynSegment::new::<()>(); + let ambient_start = segment.current_stack_offset(); + segment.op0(|| 1i32); + segment.op0(|| 2i32); + segment.make_tuple(2, ambient_start); + + let result = lookup.lookup("greet", &mut segment, 1, Span::call_site(), Span::call_site()); + assert!(result.is_err(), "shape (i32, i32) should not match (String, i32)"); +} +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `cargo test -p cel-parser tuple_shaped_signature_matches_and_dispatches tuple_shaped_signature_does_not_match_wrong_shape` +Expected: FAIL with "no method named `register_tuple_op`" / `TupleOpSignature` not found. + +- [ ] **Step 3: Implement `TupleOpSignature` and matching** + +Add near the top of `cel-parser/src/op_table.rs`, after the `OpFn` type alias: + +```rust +/// A signature for an operator/function whose selected operand is a tuple. +/// +/// Matches when the operand at `tuple_operand_index` (0-based, in the same +/// stack order [`DynSegment::peek_stack_infos`] returns) is a tuple whose +/// element `TypeId`s equal `shape`, in order, and every other peeked operand's +/// flat `TypeId` equals the corresponding entry in `operand_type_ids` (the +/// entry at `tuple_operand_index` in `operand_type_ids` is ignored). +pub struct TupleOpSignature { + /// Operator/function name this signature is registered under. + pub name: String, + /// Expected element `TypeId`s, in order, for the tuple-shaped operand. + pub shape: Vec, + /// Which peeked operand position must be the tuple. + pub tuple_operand_index: usize, + /// Flat `TypeId`s expected for the non-tuple operands, in stack order + /// (the `tuple_operand_index` entry is ignored). + pub operand_type_ids: Vec, + /// Function that pushes the operation onto the segment. + pub op_fn: OpFn, +} +``` + +Add `tuple_signatures: Vec` to `OpLookup`, its constructor, and a matching method + call site: + +```rust +pub struct OpLookup { + scopes: Vec, + builtin_scope: BuiltinScope, + tuple_signatures: Vec, +} + +impl OpLookup { + pub fn new() -> Self { + OpLookup { + scopes: Vec::new(), + builtin_scope: BuiltinScope, + tuple_signatures: Vec::new(), + } + } + + /// Registers a tuple-shaped operator signature, matched by element + /// `TypeId` sequence the same way built-in operators are matched by flat + /// `TypeId`. + pub fn register_tuple_op(&mut self, signature: TupleOpSignature) { + self.tuple_signatures.push(signature); + } + + /// Attempts to find and apply a registered tuple-shaped signature. + /// + /// Returns `Ok(true)` if found and applied, `Ok(false)` if not found. + /// + /// - Complexity: O(s) where s is the number of registered tuple signatures. + fn lookup_tuple_signature( + &self, + name: &str, + segment: &mut DynSegment, + num_operands: usize, + span: SourceSpan, + ) -> Result { + let stack_infos = segment.peek_stack_infos(num_operands); + for sig in &self.tuple_signatures { + if sig.name != name || sig.tuple_operand_index >= stack_infos.len() { + continue; + } + let tuple_info = &stack_infos[sig.tuple_operand_index]; + let shape_matches = tuple_info.associated.len() == sig.shape.len() + && tuple_info + .associated + .iter() + .zip(&sig.shape) + .all(|(a, t)| a.type_id == *t); + if !shape_matches { + continue; + } + let others_match = stack_infos.iter().enumerate().all(|(i, info)| { + i == sig.tuple_operand_index || info.type_id == sig.operand_type_ids[i] + }); + if others_match { + (sig.op_fn)(segment, span)?; + return Ok(true); + } + } + Ok(false) + } +``` + +In `OpLookup::lookup`, insert the new check between the custom-scope loop and the built-in scope lookup: + +```rust + for scope in self.scopes.iter().rev() { + match scope(name, segment, num_operands, source_span) { + Ok(true) => return Ok(()), + Ok(false) => {} + Err(e) => return Err(crate::ParseError::new_range(e.to_string(), start, end)), + } + } + + match self.lookup_tuple_signature(name, segment, num_operands, source_span) { + Ok(true) => return Ok(()), + Ok(false) => {} + Err(e) => { + return Err(crate::ParseError::new_range( + format!("operation error: {}", e), + start, + end, + )); + } + } + + match self + .builtin_scope + .lookup(name, segment, num_operands, source_span) + { + // ... unchanged from here +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `cargo test -p cel-parser op_table::` +Expected: PASS — both new tests plus the full existing `op_table.rs` suite (built-in operator dispatch is untouched, since `tuple_signatures` is empty by default). + +- [ ] **Step 5: Commit** + +```bash +cargo fmt --all +git add cel-parser/src/op_table.rs +git commit -m "feat(cel-parser): generalize op dispatch for tuple-shaped operands" +``` + +--- + +### Task 9: `pop_tuple_as`/`push_tuple` — sound `CStackList` bridge + +**Files:** +- Modify: `cel-runtime/src/dyn_segment.rs` + +**Interfaces:** +- Consumes: `ToTypeIdList` (existing), `List` (existing), `StackInfo`/`AssociatedType` (Task 4). +- Produces: `DynSegment::pop_tuple_as::(&mut self) -> anyhow::Result<()>`, `DynSegment::push_tuple::(&mut self)`. + +- [ ] **Step 1: Write the failing tests** + +Add to the `tests` module in `cel-runtime/src/dyn_segment.rs`: + +```rust +#[test] +fn push_tuple_then_pop_tuple_as_round_trips() -> Result<(), anyhow::Error> { + let mut seg = DynSegment::new::<()>(); + // Build a concrete CStackList>> by pushing + // fields in declaration order (NOT via into_c_stack_list, which reverses + // order — see pop_tuple_as's doc comment). `CNil`'s inner field is + // private, so build the empty base via the public `IntoCStackList` + // conversion on `()` rather than the tuple-struct constructor. + seg.op0(|| CStackList(().into_c_stack_list(), 7u32).push("hi")); + seg.push_tuple::>>>(); + assert_eq!(seg.peek_tuple_arity(), Some(2)); + + seg.pop_tuple_as::>>>()?; + let result = seg.call0::>>>()?; + assert_eq!(result.head(), &"hi"); + assert_eq!(result.tail().head(), &7u32); + Ok(()) +} + +#[test] +fn pop_tuple_as_rejects_shape_mismatch() { + let mut seg = DynSegment::new::<()>(); + let ambient_start = seg.current_stack_offset(); + seg.op0(|| 1u32); + seg.op0(|| 2u32); + seg.make_tuple(2, ambient_start); + + let result = seg.pop_tuple_as::>>>(); + assert!(result.is_err(), "(u32, u32) should not match (u32, &str)"); +} +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `cargo test -p cel-runtime push_tuple_then_pop_tuple_as_round_trips pop_tuple_as_rejects_shape_mismatch` +Expected: FAIL with "no method named `push_tuple`" / `pop_tuple_as`. + +- [ ] **Step 3: Implement `pop_tuple_as` and `push_tuple`** + +Add `use anyhow::anyhow;` to the imports at the top of `cel-runtime/src/dyn_segment.rs` (alongside the existing `use anyhow::Result;` / `use anyhow::ensure;`). + +Add these methods to `impl DynSegment`: + +```rust + /// Reinterprets the tuple on top of the stack as a concrete `L` + /// (typically a `CStackList<...>` chain), replacing its `StackInfo` with + /// `L`'s. No bytes move: both sides already use the same + /// natural-alignment, declaration-order layout, so this is a relabel, not + /// a copy. + /// + /// - Precondition: `L` was assembled via sequential `.push()` calls in + /// the same field order as the tuple (not via `into_c_stack_list()` on + /// a same-order plain tuple, which reverses element order). + /// + /// # Errors + /// Returns an error if the top of stack isn't a tuple, or its element + /// `TypeId`s (in order) don't match `L`'s. + pub fn pop_tuple_as(&mut self) -> Result<()> { + let info = self + .stack_ids + .last() + .ok_or_else(|| anyhow!("pop_tuple_as: stack is empty"))?; + ensure!( + info.type_id == TypeId::of::(), + "pop_tuple_as: top of stack is not a tuple" + ); + let expected: Vec = L::to_stack_info_list().iter().map(|s| s.type_id).collect(); + let actual: Vec = info.associated.iter().map(|a| a.type_id).collect(); + ensure!( + expected == actual, + "pop_tuple_as: tuple element types do not match `{}`", + std::any::type_name::() + ); + debug_assert_eq!(info.size, size_of::()); + debug_assert_eq!(info.align, align_of::()); + + let info = self.stack_ids.last_mut().expect("checked above"); + info.type_id = TypeId::of::(); + info.type_name = Cow::Borrowed(std::any::type_name::()); + info.raw_dropper = |ptr, _associated| unsafe { std::ptr::drop_in_place(ptr.cast::()) }; + info.associated = Vec::new(); + Ok(()) + } + + /// Relabels the concrete `L` value on top of the stack as a tuple, + /// exposing its elements for `.N` indexing and tuple-shaped op matching. + /// No bytes move — see [`pop_tuple_as`](Self::pop_tuple_as) for why this + /// is sound. + /// + /// - Precondition: the top of the stack currently holds a value of type + /// `L`, assembled via sequential `.push()` calls (not + /// `into_c_stack_list()` on a same-order plain tuple). + pub fn push_tuple(&mut self) { + let info = self + .stack_ids + .last_mut() + .expect("push_tuple requires a value on the stack"); + debug_assert_eq!( + info.type_id, + TypeId::of::(), + "push_tuple: top of stack is not the expected type" + ); + let element_infos = L::to_stack_info_list(); + let mut offset = 0usize; + let associated = element_infos + .iter() + .map(|elem_info| { + offset = align_index(elem_info.align, offset); + let a = AssociatedType { + type_id: elem_info.type_id, + type_name: elem_info.type_name.clone(), + offset, + size: elem_info.size, + align: elem_info.align, + dropper: elem_info.raw_dropper, + associated: elem_info.associated.clone(), + }; + offset += elem_info.size; + a + }) + .collect(); + info.type_id = TypeId::of::(); + info.type_name = Cow::Borrowed(std::any::type_name::()); + info.raw_dropper = drop_tuple; + info.associated = associated; + } +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `cargo test -p cel-runtime` +Expected: PASS — both new tests plus the entire `cel-runtime` suite. + +- [ ] **Step 5: Run full workspace verification** + +Run: +```bash +cargo test --workspace +cargo test --doc --workspace +cargo clippy --workspace --exclude begin -- -D warnings +cargo clippy -p begin --no-default-features -- -D warnings +``` +Expected: all PASS with zero warnings. + +- [ ] **Step 6: Commit** + +```bash +cargo fmt --all +git add cel-runtime/src/dyn_segment.rs +git commit -m "feat(cel-runtime): add pop_tuple_as/push_tuple CStackList bridge" +``` + +--- + +## Self-Review Notes + +- **Spec coverage:** Grammar (Tasks 6-7), runtime representation / `AssociatedType` (Task 4), construction (Task 5/`make_tuple`), indexing incl. the corrected pop-and-repush algorithm (Task 5/`tuple_index`), `push_raw`/`repack` primitives (Tasks 1-3), interop registration (Task 8), `CStackList` bridge (Task 9), and the combined-indexing-with-another-op test the spec calls out explicitly (Task 5's `tuple_index_combined_with_another_op` and Task 7's `indexing_combined_with_addition*`) are all covered. +- **Deferred per spec's "Out of Scope":** arrays/vecs, first-class functions, method-call syntax, the compiled/macro backend, and reclaiming dead space via compaction (Task 5 pops-and-repushes the whole tuple instead) are intentionally not tasks here. +- **Type consistency:** `make_tuple(n, ambient_start)` / `tuple_index(index)` / `current_stack_offset()` / `peek_tuple_arity()` signatures introduced in Task 5 are used identically (same names, same parameter order) in Tasks 6-9. `RawDropper`/`AssociatedType`/`StackInfo` field names introduced in Task 4 are used consistently through Tasks 5 and 9. From 5e4527031f8d4ad4dd117b5c909c24caf6281130 Mon Sep 17 00:00:00 2001 From: Sean Parent Date: Tue, 7 Jul 2026 15:20:08 -0700 Subject: [PATCH 04/29] feat(cel-runtime): add RawStack::len and push_raw --- cel-runtime/src/raw_stack.rs | 402 +++++++++++++++++++++-------------- 1 file changed, 241 insertions(+), 161 deletions(-) diff --git a/cel-runtime/src/raw_stack.rs b/cel-runtime/src/raw_stack.rs index 0b4eab4..dcfd6a3 100644 --- a/cel-runtime/src/raw_stack.rs +++ b/cel-runtime/src/raw_stack.rs @@ -1,161 +1,241 @@ -use crate::memory::align_index; -use crate::raw_vec::RawVec; -use std::mem::MaybeUninit; -use std::mem::size_of; - -/// A simple raw stack that stores values as raw bytes. Each value is naturally aligned given the -/// base alignment of the stack, which is the maximum alignment of any value stored in the stack. -#[derive(Debug)] -pub struct RawStack { - buffer: RawVec, -} - -impl RawStack { - /// Creates a new `RawStack` with base alignment. - /// - /// # Examples - /// - /// ``` - /// use cel_runtime::raw_stack::RawStack; - /// let stack = RawStack::with_base_alignment(align_of::()); - /// ``` - #[must_use] - pub fn with_base_alignment(base_alignment: usize) -> Self { - RawStack { - buffer: RawVec::with_base_alignment(base_alignment), - } - } - - /// Pushes a value of type `T` onto the stack. - /// - /// The value is stored as raw bytes in the internal buffer. The pushed value must be - /// later popped using the correct type. - /// - /// # Type Parameters - /// - /// * `T`: The type of the value to push. - /// - /// # Examples - /// - /// ``` - /// use cel_runtime::raw_stack::RawStack; - /// let mut stack = RawStack::with_base_alignment(align_of::()); - /// let _ = stack.push(42u32); - /// ``` - /// - /// # Complexity - /// - /// The function has an amortized O(1) time complexity. - pub fn push(&mut self, value: T) -> bool { - let len = self.buffer.len(); - let aligned_index = align_index(align_of::(), len); - let new_len = aligned_index + size_of::(); - - self.buffer.reserve(new_len - len); - unsafe { - self.buffer.set_len(new_len); - if aligned_index - len > 0 { - // write a 1 in the first padding byte and 0 in the rest - self.buffer[len].write(1); - self.buffer[len + 1..aligned_index].fill(MaybeUninit::new(0)); - } - - std::ptr::write( - self.buffer.as_mut_ptr().add(aligned_index).cast::(), - value, - ); - } - aligned_index - len > 0 - } - - /// Pops a value of type `T` from the stack. Does not change the stack capacity. - /// - /// # Safety - /// - /// The type `T` must be the same type as the value on the top of the stack. - /// Incorrect usage can lead to undefined behavior. - /// - /// # Type Parameters - /// - /// * `T`: The type of the value to pop. - /// - /// # Examples - /// - /// ``` - /// use cel_runtime::raw_stack::RawStack; - /// let mut stack = RawStack::with_base_alignment(align_of::()); - /// let padding = stack.push(100u32); - /// let value: u32 = unsafe { stack.pop(padding) }; - /// ``` - pub unsafe fn pop(&mut self, padding: bool) -> T { - let p: usize = self.buffer.len() - size_of::(); - let result = unsafe { std::ptr::read(self.buffer.as_ptr().add(p).cast::()) }; - // count the number of trailing 0s in the buffer before the result - let padding_count = if padding { - self.buffer[..p] - .iter() - .rev() - .take_while(|&x| unsafe { x.assume_init() == 0 }) - .count() - + 1 - } else { - 0 - }; - self.buffer.truncate(p - padding_count); - result - } - - /// Pops a value of type `T` from the stack and drops it. - /// - /// # Safety - /// - /// The type `T` must be the same type as the value on the top of the stack. - /// Incorrect usage can lead to undefined behavior. - /// - /// # Note - /// - /// This cannot use `drop_in_place` because the type may not be aligned. - pub unsafe fn drop(&mut self, padding: bool) { - unsafe { self.pop::(padding) }; - } -} - -/* Test module */ -#[cfg(test)] -mod tests { - use super::*; - use std::cmp::max; - - #[test] - fn push_pop_u32() { - let mut stack = RawStack::with_base_alignment(align_of::()); - let padding = stack.push(10u32); - let result: u32 = unsafe { stack.pop(padding) }; - assert_eq!(result, 10); - } - - #[test] - fn multiple_push_pop() { - let mut stack = RawStack::with_base_alignment(align_of::()); - let padding1 = stack.push(1u32); - let padding2 = stack.push(2u32); - let padding3 = stack.push(3u32); - let v3: u32 = unsafe { stack.pop(padding3) }; - let v2: u32 = unsafe { stack.pop(padding2) }; - let v1: u32 = unsafe { stack.pop(padding1) }; - assert_eq!(v1, 1); - assert_eq!(v2, 2); - assert_eq!(v3, 3); - } - - #[test] - fn push_pop_different_types() { - let mut stack = RawStack::with_base_alignment(max(align_of::(), align_of::())); - let padding1 = stack.push(42u32); - let padding2 = stack.push(3.14f64); - let value_f: f64 = unsafe { stack.pop(padding2) }; - let value_u: u32 = unsafe { stack.pop(padding1) }; - assert_eq!(value_f, 3.14); - assert_eq!(value_u, 42); - } -} +use crate::memory::align_index; +use crate::raw_vec::RawVec; +use std::mem::MaybeUninit; +use std::mem::size_of; + +/// A simple raw stack that stores values as raw bytes. Each value is naturally aligned given the +/// base alignment of the stack, which is the maximum alignment of any value stored in the stack. +#[derive(Debug)] +pub struct RawStack { + buffer: RawVec, +} + +impl RawStack { + /// Creates a new `RawStack` with base alignment. + /// + /// # Examples + /// + /// ``` + /// use cel_runtime::raw_stack::RawStack; + /// let stack = RawStack::with_base_alignment(align_of::()); + /// ``` + #[must_use] + pub fn with_base_alignment(base_alignment: usize) -> Self { + RawStack { + buffer: RawVec::with_base_alignment(base_alignment), + } + } + + /// Returns the number of bytes currently on the stack. + #[must_use] + pub fn len(&self) -> usize { + self.buffer.len() + } + + /// Pushes a value of type `T` onto the stack. + /// + /// The value is stored as raw bytes in the internal buffer. The pushed value must be + /// later popped using the correct type. + /// + /// # Type Parameters + /// + /// * `T`: The type of the value to push. + /// + /// # Examples + /// + /// ``` + /// use cel_runtime::raw_stack::RawStack; + /// let mut stack = RawStack::with_base_alignment(align_of::()); + /// let _ = stack.push(42u32); + /// ``` + /// + /// # Complexity + /// + /// The function has an amortized O(1) time complexity. + pub fn push(&mut self, value: T) -> bool { + let len = self.buffer.len(); + let aligned_index = align_index(align_of::(), len); + let new_len = aligned_index + size_of::(); + + self.buffer.reserve(new_len - len); + unsafe { + self.buffer.set_len(new_len); + if aligned_index - len > 0 { + // write a 1 in the first padding byte and 0 in the rest + self.buffer[len].write(1); + self.buffer[len + 1..aligned_index].fill(MaybeUninit::new(0)); + } + + std::ptr::write( + self.buffer.as_mut_ptr().add(aligned_index).cast::(), + value, + ); + } + aligned_index - len > 0 + } + + /// Pushes `size` raw bytes from `src`, aligned to `align`, using the same + /// padding/marker-byte bookkeeping as [`push`](Self::push). + /// + /// - Precondition: `align` is a power of two. + /// + /// # Safety + /// `src` must be valid for reads of `size` bytes. + pub unsafe fn push_raw(&mut self, align: usize, size: usize, src: *const u8) -> bool { + debug_assert!(align.is_power_of_two()); + let len = self.buffer.len(); + let aligned_index = align_index(align, len); + let new_len = aligned_index + size; + + self.buffer.reserve(new_len - len); + unsafe { + self.buffer.set_len(new_len); + if aligned_index - len > 0 { + self.buffer[len].write(1); + self.buffer[len + 1..aligned_index].fill(MaybeUninit::new(0)); + } + std::ptr::copy_nonoverlapping( + src, + self.buffer.as_mut_ptr().add(aligned_index).cast::(), + size, + ); + } + aligned_index - len > 0 + } + + /// Pops a value of type `T` from the stack. Does not change the stack capacity. + /// + /// # Safety + /// + /// The type `T` must be the same type as the value on the top of the stack. + /// Incorrect usage can lead to undefined behavior. + /// + /// # Type Parameters + /// + /// * `T`: The type of the value to pop. + /// + /// # Examples + /// + /// ``` + /// use cel_runtime::raw_stack::RawStack; + /// let mut stack = RawStack::with_base_alignment(align_of::()); + /// let padding = stack.push(100u32); + /// let value: u32 = unsafe { stack.pop(padding) }; + /// ``` + pub unsafe fn pop(&mut self, padding: bool) -> T { + let p: usize = self.buffer.len() - size_of::(); + let result = unsafe { std::ptr::read(self.buffer.as_ptr().add(p).cast::()) }; + // count the number of trailing 0s in the buffer before the result + let padding_count = if padding { + self.buffer[..p] + .iter() + .rev() + .take_while(|&x| unsafe { x.assume_init() == 0 }) + .count() + + 1 + } else { + 0 + }; + self.buffer.truncate(p - padding_count); + result + } + + /// Pops a value of type `T` from the stack and drops it. + /// + /// # Safety + /// + /// The type `T` must be the same type as the value on the top of the stack. + /// Incorrect usage can lead to undefined behavior. + /// + /// # Note + /// + /// This cannot use `drop_in_place` because the type may not be aligned. + pub unsafe fn drop(&mut self, padding: bool) { + unsafe { self.pop::(padding) }; + } +} + +/* Test module */ +#[cfg(test)] +mod tests { + use super::*; + use std::cmp::max; + + #[test] + fn push_pop_u32() { + let mut stack = RawStack::with_base_alignment(align_of::()); + let padding = stack.push(10u32); + let result: u32 = unsafe { stack.pop(padding) }; + assert_eq!(result, 10); + } + + #[test] + fn multiple_push_pop() { + let mut stack = RawStack::with_base_alignment(align_of::()); + let padding1 = stack.push(1u32); + let padding2 = stack.push(2u32); + let padding3 = stack.push(3u32); + let v3: u32 = unsafe { stack.pop(padding3) }; + let v2: u32 = unsafe { stack.pop(padding2) }; + let v1: u32 = unsafe { stack.pop(padding1) }; + assert_eq!(v1, 1); + assert_eq!(v2, 2); + assert_eq!(v3, 3); + } + + #[test] + fn push_pop_different_types() { + let mut stack = RawStack::with_base_alignment(max(align_of::(), align_of::())); + let padding1 = stack.push(42u32); + let padding2 = stack.push(3.14f64); + let value_f: f64 = unsafe { stack.pop(padding2) }; + let value_u: u32 = unsafe { stack.pop(padding1) }; + assert_eq!(value_f, 3.14); + assert_eq!(value_u, 42); + } + + #[test] + fn len_reflects_pushed_bytes() { + let mut stack = RawStack::with_base_alignment(align_of::()); + assert_eq!(stack.len(), 0); + let _ = stack.push(7u32); + assert_eq!(stack.len(), size_of::()); + } + + #[test] + fn push_raw_round_trips_like_push() { + let mut stack = RawStack::with_base_alignment(align_of::()); + let padding1 = stack.push(1u8); + let value = 3.14f64; + let padding2 = unsafe { + stack.push_raw( + align_of::(), + size_of::(), + (&value as *const f64).cast::(), + ) + }; + let popped: f64 = unsafe { stack.pop(padding2) }; + assert_eq!(popped, 3.14); + let popped_u8: u8 = unsafe { stack.pop(padding1) }; + assert_eq!(popped_u8, 1); + } + + #[test] + fn push_raw_padding_matches_typed_push() { + let mut stack_a = RawStack::with_base_alignment(align_of::()); + let _ = stack_a.push(1u8); + let padding_typed = stack_a.push(2.5f64); + + let mut stack_b = RawStack::with_base_alignment(align_of::()); + let _ = stack_b.push(1u8); + let value = 2.5f64; + let padding_raw = unsafe { + stack_b.push_raw( + align_of::(), + size_of::(), + (&value as *const f64).cast::(), + ) + }; + assert_eq!(padding_typed, padding_raw); + } +} From 12c2a789c53c7e1ab6c83244d3d0f1cb5e5f94f9 Mon Sep 17 00:00:00 2001 From: Sean Parent Date: Tue, 7 Jul 2026 15:28:59 -0700 Subject: [PATCH 05/29] fix(cel-runtime): preserve CRLF line endings and address review findings for RawStack - Restore CRLF line endings in raw_stack.rs (previous commit accidentally rewrote the whole file to LF, inflating the diff). - Add #[allow(clippy::len_without_is_empty)] to RawStack::len (RawStack is stack machinery, not a general collection; an is_empty() method would be unused). - Document in push_raw's # Safety section that src must not overlap the stack's internal buffer, matching the copy_nonoverlapping precondition. --- cel-runtime/src/raw_stack.rs | 484 ++++++++++++++++++----------------- 1 file changed, 243 insertions(+), 241 deletions(-) diff --git a/cel-runtime/src/raw_stack.rs b/cel-runtime/src/raw_stack.rs index dcfd6a3..bad2272 100644 --- a/cel-runtime/src/raw_stack.rs +++ b/cel-runtime/src/raw_stack.rs @@ -1,241 +1,243 @@ -use crate::memory::align_index; -use crate::raw_vec::RawVec; -use std::mem::MaybeUninit; -use std::mem::size_of; - -/// A simple raw stack that stores values as raw bytes. Each value is naturally aligned given the -/// base alignment of the stack, which is the maximum alignment of any value stored in the stack. -#[derive(Debug)] -pub struct RawStack { - buffer: RawVec, -} - -impl RawStack { - /// Creates a new `RawStack` with base alignment. - /// - /// # Examples - /// - /// ``` - /// use cel_runtime::raw_stack::RawStack; - /// let stack = RawStack::with_base_alignment(align_of::()); - /// ``` - #[must_use] - pub fn with_base_alignment(base_alignment: usize) -> Self { - RawStack { - buffer: RawVec::with_base_alignment(base_alignment), - } - } - - /// Returns the number of bytes currently on the stack. - #[must_use] - pub fn len(&self) -> usize { - self.buffer.len() - } - - /// Pushes a value of type `T` onto the stack. - /// - /// The value is stored as raw bytes in the internal buffer. The pushed value must be - /// later popped using the correct type. - /// - /// # Type Parameters - /// - /// * `T`: The type of the value to push. - /// - /// # Examples - /// - /// ``` - /// use cel_runtime::raw_stack::RawStack; - /// let mut stack = RawStack::with_base_alignment(align_of::()); - /// let _ = stack.push(42u32); - /// ``` - /// - /// # Complexity - /// - /// The function has an amortized O(1) time complexity. - pub fn push(&mut self, value: T) -> bool { - let len = self.buffer.len(); - let aligned_index = align_index(align_of::(), len); - let new_len = aligned_index + size_of::(); - - self.buffer.reserve(new_len - len); - unsafe { - self.buffer.set_len(new_len); - if aligned_index - len > 0 { - // write a 1 in the first padding byte and 0 in the rest - self.buffer[len].write(1); - self.buffer[len + 1..aligned_index].fill(MaybeUninit::new(0)); - } - - std::ptr::write( - self.buffer.as_mut_ptr().add(aligned_index).cast::(), - value, - ); - } - aligned_index - len > 0 - } - - /// Pushes `size` raw bytes from `src`, aligned to `align`, using the same - /// padding/marker-byte bookkeeping as [`push`](Self::push). - /// - /// - Precondition: `align` is a power of two. - /// - /// # Safety - /// `src` must be valid for reads of `size` bytes. - pub unsafe fn push_raw(&mut self, align: usize, size: usize, src: *const u8) -> bool { - debug_assert!(align.is_power_of_two()); - let len = self.buffer.len(); - let aligned_index = align_index(align, len); - let new_len = aligned_index + size; - - self.buffer.reserve(new_len - len); - unsafe { - self.buffer.set_len(new_len); - if aligned_index - len > 0 { - self.buffer[len].write(1); - self.buffer[len + 1..aligned_index].fill(MaybeUninit::new(0)); - } - std::ptr::copy_nonoverlapping( - src, - self.buffer.as_mut_ptr().add(aligned_index).cast::(), - size, - ); - } - aligned_index - len > 0 - } - - /// Pops a value of type `T` from the stack. Does not change the stack capacity. - /// - /// # Safety - /// - /// The type `T` must be the same type as the value on the top of the stack. - /// Incorrect usage can lead to undefined behavior. - /// - /// # Type Parameters - /// - /// * `T`: The type of the value to pop. - /// - /// # Examples - /// - /// ``` - /// use cel_runtime::raw_stack::RawStack; - /// let mut stack = RawStack::with_base_alignment(align_of::()); - /// let padding = stack.push(100u32); - /// let value: u32 = unsafe { stack.pop(padding) }; - /// ``` - pub unsafe fn pop(&mut self, padding: bool) -> T { - let p: usize = self.buffer.len() - size_of::(); - let result = unsafe { std::ptr::read(self.buffer.as_ptr().add(p).cast::()) }; - // count the number of trailing 0s in the buffer before the result - let padding_count = if padding { - self.buffer[..p] - .iter() - .rev() - .take_while(|&x| unsafe { x.assume_init() == 0 }) - .count() - + 1 - } else { - 0 - }; - self.buffer.truncate(p - padding_count); - result - } - - /// Pops a value of type `T` from the stack and drops it. - /// - /// # Safety - /// - /// The type `T` must be the same type as the value on the top of the stack. - /// Incorrect usage can lead to undefined behavior. - /// - /// # Note - /// - /// This cannot use `drop_in_place` because the type may not be aligned. - pub unsafe fn drop(&mut self, padding: bool) { - unsafe { self.pop::(padding) }; - } -} - -/* Test module */ -#[cfg(test)] -mod tests { - use super::*; - use std::cmp::max; - - #[test] - fn push_pop_u32() { - let mut stack = RawStack::with_base_alignment(align_of::()); - let padding = stack.push(10u32); - let result: u32 = unsafe { stack.pop(padding) }; - assert_eq!(result, 10); - } - - #[test] - fn multiple_push_pop() { - let mut stack = RawStack::with_base_alignment(align_of::()); - let padding1 = stack.push(1u32); - let padding2 = stack.push(2u32); - let padding3 = stack.push(3u32); - let v3: u32 = unsafe { stack.pop(padding3) }; - let v2: u32 = unsafe { stack.pop(padding2) }; - let v1: u32 = unsafe { stack.pop(padding1) }; - assert_eq!(v1, 1); - assert_eq!(v2, 2); - assert_eq!(v3, 3); - } - - #[test] - fn push_pop_different_types() { - let mut stack = RawStack::with_base_alignment(max(align_of::(), align_of::())); - let padding1 = stack.push(42u32); - let padding2 = stack.push(3.14f64); - let value_f: f64 = unsafe { stack.pop(padding2) }; - let value_u: u32 = unsafe { stack.pop(padding1) }; - assert_eq!(value_f, 3.14); - assert_eq!(value_u, 42); - } - - #[test] - fn len_reflects_pushed_bytes() { - let mut stack = RawStack::with_base_alignment(align_of::()); - assert_eq!(stack.len(), 0); - let _ = stack.push(7u32); - assert_eq!(stack.len(), size_of::()); - } - - #[test] - fn push_raw_round_trips_like_push() { - let mut stack = RawStack::with_base_alignment(align_of::()); - let padding1 = stack.push(1u8); - let value = 3.14f64; - let padding2 = unsafe { - stack.push_raw( - align_of::(), - size_of::(), - (&value as *const f64).cast::(), - ) - }; - let popped: f64 = unsafe { stack.pop(padding2) }; - assert_eq!(popped, 3.14); - let popped_u8: u8 = unsafe { stack.pop(padding1) }; - assert_eq!(popped_u8, 1); - } - - #[test] - fn push_raw_padding_matches_typed_push() { - let mut stack_a = RawStack::with_base_alignment(align_of::()); - let _ = stack_a.push(1u8); - let padding_typed = stack_a.push(2.5f64); - - let mut stack_b = RawStack::with_base_alignment(align_of::()); - let _ = stack_b.push(1u8); - let value = 2.5f64; - let padding_raw = unsafe { - stack_b.push_raw( - align_of::(), - size_of::(), - (&value as *const f64).cast::(), - ) - }; - assert_eq!(padding_typed, padding_raw); - } -} +use crate::memory::align_index; +use crate::raw_vec::RawVec; +use std::mem::MaybeUninit; +use std::mem::size_of; + +/// A simple raw stack that stores values as raw bytes. Each value is naturally aligned given the +/// base alignment of the stack, which is the maximum alignment of any value stored in the stack. +#[derive(Debug)] +pub struct RawStack { + buffer: RawVec, +} + +impl RawStack { + /// Creates a new `RawStack` with base alignment. + /// + /// # Examples + /// + /// ``` + /// use cel_runtime::raw_stack::RawStack; + /// let stack = RawStack::with_base_alignment(align_of::()); + /// ``` + #[must_use] + pub fn with_base_alignment(base_alignment: usize) -> Self { + RawStack { + buffer: RawVec::with_base_alignment(base_alignment), + } + } + + /// Returns the number of bytes currently on the stack. + #[must_use] + #[allow(clippy::len_without_is_empty)] + pub fn len(&self) -> usize { + self.buffer.len() + } + + /// Pushes a value of type `T` onto the stack. + /// + /// The value is stored as raw bytes in the internal buffer. The pushed value must be + /// later popped using the correct type. + /// + /// # Type Parameters + /// + /// * `T`: The type of the value to push. + /// + /// # Examples + /// + /// ``` + /// use cel_runtime::raw_stack::RawStack; + /// let mut stack = RawStack::with_base_alignment(align_of::()); + /// let _ = stack.push(42u32); + /// ``` + /// + /// # Complexity + /// + /// The function has an amortized O(1) time complexity. + pub fn push(&mut self, value: T) -> bool { + let len = self.buffer.len(); + let aligned_index = align_index(align_of::(), len); + let new_len = aligned_index + size_of::(); + + self.buffer.reserve(new_len - len); + unsafe { + self.buffer.set_len(new_len); + if aligned_index - len > 0 { + // write a 1 in the first padding byte and 0 in the rest + self.buffer[len].write(1); + self.buffer[len + 1..aligned_index].fill(MaybeUninit::new(0)); + } + + std::ptr::write( + self.buffer.as_mut_ptr().add(aligned_index).cast::(), + value, + ); + } + aligned_index - len > 0 + } + + /// Pushes `size` raw bytes from `src`, aligned to `align`, using the same + /// padding/marker-byte bookkeeping as [`push`](Self::push). + /// + /// - Precondition: `align` is a power of two. + /// + /// # Safety + /// `src` must be valid for reads of `size` bytes, and must not overlap the + /// stack's internal buffer. + pub unsafe fn push_raw(&mut self, align: usize, size: usize, src: *const u8) -> bool { + debug_assert!(align.is_power_of_two()); + let len = self.buffer.len(); + let aligned_index = align_index(align, len); + let new_len = aligned_index + size; + + self.buffer.reserve(new_len - len); + unsafe { + self.buffer.set_len(new_len); + if aligned_index - len > 0 { + self.buffer[len].write(1); + self.buffer[len + 1..aligned_index].fill(MaybeUninit::new(0)); + } + std::ptr::copy_nonoverlapping( + src, + self.buffer.as_mut_ptr().add(aligned_index).cast::(), + size, + ); + } + aligned_index - len > 0 + } + + /// Pops a value of type `T` from the stack. Does not change the stack capacity. + /// + /// # Safety + /// + /// The type `T` must be the same type as the value on the top of the stack. + /// Incorrect usage can lead to undefined behavior. + /// + /// # Type Parameters + /// + /// * `T`: The type of the value to pop. + /// + /// # Examples + /// + /// ``` + /// use cel_runtime::raw_stack::RawStack; + /// let mut stack = RawStack::with_base_alignment(align_of::()); + /// let padding = stack.push(100u32); + /// let value: u32 = unsafe { stack.pop(padding) }; + /// ``` + pub unsafe fn pop(&mut self, padding: bool) -> T { + let p: usize = self.buffer.len() - size_of::(); + let result = unsafe { std::ptr::read(self.buffer.as_ptr().add(p).cast::()) }; + // count the number of trailing 0s in the buffer before the result + let padding_count = if padding { + self.buffer[..p] + .iter() + .rev() + .take_while(|&x| unsafe { x.assume_init() == 0 }) + .count() + + 1 + } else { + 0 + }; + self.buffer.truncate(p - padding_count); + result + } + + /// Pops a value of type `T` from the stack and drops it. + /// + /// # Safety + /// + /// The type `T` must be the same type as the value on the top of the stack. + /// Incorrect usage can lead to undefined behavior. + /// + /// # Note + /// + /// This cannot use `drop_in_place` because the type may not be aligned. + pub unsafe fn drop(&mut self, padding: bool) { + unsafe { self.pop::(padding) }; + } +} + +/* Test module */ +#[cfg(test)] +mod tests { + use super::*; + use std::cmp::max; + + #[test] + fn push_pop_u32() { + let mut stack = RawStack::with_base_alignment(align_of::()); + let padding = stack.push(10u32); + let result: u32 = unsafe { stack.pop(padding) }; + assert_eq!(result, 10); + } + + #[test] + fn multiple_push_pop() { + let mut stack = RawStack::with_base_alignment(align_of::()); + let padding1 = stack.push(1u32); + let padding2 = stack.push(2u32); + let padding3 = stack.push(3u32); + let v3: u32 = unsafe { stack.pop(padding3) }; + let v2: u32 = unsafe { stack.pop(padding2) }; + let v1: u32 = unsafe { stack.pop(padding1) }; + assert_eq!(v1, 1); + assert_eq!(v2, 2); + assert_eq!(v3, 3); + } + + #[test] + fn push_pop_different_types() { + let mut stack = RawStack::with_base_alignment(max(align_of::(), align_of::())); + let padding1 = stack.push(42u32); + let padding2 = stack.push(3.14f64); + let value_f: f64 = unsafe { stack.pop(padding2) }; + let value_u: u32 = unsafe { stack.pop(padding1) }; + assert_eq!(value_f, 3.14); + assert_eq!(value_u, 42); + } + + #[test] + fn len_reflects_pushed_bytes() { + let mut stack = RawStack::with_base_alignment(align_of::()); + assert_eq!(stack.len(), 0); + let _ = stack.push(7u32); + assert_eq!(stack.len(), size_of::()); + } + + #[test] + fn push_raw_round_trips_like_push() { + let mut stack = RawStack::with_base_alignment(align_of::()); + let padding1 = stack.push(1u8); + let value = 3.14f64; + let padding2 = unsafe { + stack.push_raw( + align_of::(), + size_of::(), + (&value as *const f64).cast::(), + ) + }; + let popped: f64 = unsafe { stack.pop(padding2) }; + assert_eq!(popped, 3.14); + let popped_u8: u8 = unsafe { stack.pop(padding1) }; + assert_eq!(popped_u8, 1); + } + + #[test] + fn push_raw_padding_matches_typed_push() { + let mut stack_a = RawStack::with_base_alignment(align_of::()); + let _ = stack_a.push(1u8); + let padding_typed = stack_a.push(2.5f64); + + let mut stack_b = RawStack::with_base_alignment(align_of::()); + let _ = stack_b.push(1u8); + let value = 2.5f64; + let padding_raw = unsafe { + stack_b.push_raw( + align_of::(), + size_of::(), + (&value as *const f64).cast::(), + ) + }; + assert_eq!(padding_typed, padding_raw); + } +} From a57e874920466e179c83899fbed20b5507e5cc72 Mon Sep 17 00:00:00 2001 From: Sean Parent Date: Tue, 7 Jul 2026 15:54:17 -0700 Subject: [PATCH 06/29] feat(cel-runtime): add RawStack drop/copy/truncate primitives Co-Authored-By: Claude Sonnet 5 --- cel-runtime/src/raw_stack.rs | 613 +++++++++++++++++++++-------------- 1 file changed, 370 insertions(+), 243 deletions(-) diff --git a/cel-runtime/src/raw_stack.rs b/cel-runtime/src/raw_stack.rs index bad2272..3007373 100644 --- a/cel-runtime/src/raw_stack.rs +++ b/cel-runtime/src/raw_stack.rs @@ -1,243 +1,370 @@ -use crate::memory::align_index; -use crate::raw_vec::RawVec; -use std::mem::MaybeUninit; -use std::mem::size_of; - -/// A simple raw stack that stores values as raw bytes. Each value is naturally aligned given the -/// base alignment of the stack, which is the maximum alignment of any value stored in the stack. -#[derive(Debug)] -pub struct RawStack { - buffer: RawVec, -} - -impl RawStack { - /// Creates a new `RawStack` with base alignment. - /// - /// # Examples - /// - /// ``` - /// use cel_runtime::raw_stack::RawStack; - /// let stack = RawStack::with_base_alignment(align_of::()); - /// ``` - #[must_use] - pub fn with_base_alignment(base_alignment: usize) -> Self { - RawStack { - buffer: RawVec::with_base_alignment(base_alignment), - } - } - - /// Returns the number of bytes currently on the stack. - #[must_use] - #[allow(clippy::len_without_is_empty)] - pub fn len(&self) -> usize { - self.buffer.len() - } - - /// Pushes a value of type `T` onto the stack. - /// - /// The value is stored as raw bytes in the internal buffer. The pushed value must be - /// later popped using the correct type. - /// - /// # Type Parameters - /// - /// * `T`: The type of the value to push. - /// - /// # Examples - /// - /// ``` - /// use cel_runtime::raw_stack::RawStack; - /// let mut stack = RawStack::with_base_alignment(align_of::()); - /// let _ = stack.push(42u32); - /// ``` - /// - /// # Complexity - /// - /// The function has an amortized O(1) time complexity. - pub fn push(&mut self, value: T) -> bool { - let len = self.buffer.len(); - let aligned_index = align_index(align_of::(), len); - let new_len = aligned_index + size_of::(); - - self.buffer.reserve(new_len - len); - unsafe { - self.buffer.set_len(new_len); - if aligned_index - len > 0 { - // write a 1 in the first padding byte and 0 in the rest - self.buffer[len].write(1); - self.buffer[len + 1..aligned_index].fill(MaybeUninit::new(0)); - } - - std::ptr::write( - self.buffer.as_mut_ptr().add(aligned_index).cast::(), - value, - ); - } - aligned_index - len > 0 - } - - /// Pushes `size` raw bytes from `src`, aligned to `align`, using the same - /// padding/marker-byte bookkeeping as [`push`](Self::push). - /// - /// - Precondition: `align` is a power of two. - /// - /// # Safety - /// `src` must be valid for reads of `size` bytes, and must not overlap the - /// stack's internal buffer. - pub unsafe fn push_raw(&mut self, align: usize, size: usize, src: *const u8) -> bool { - debug_assert!(align.is_power_of_two()); - let len = self.buffer.len(); - let aligned_index = align_index(align, len); - let new_len = aligned_index + size; - - self.buffer.reserve(new_len - len); - unsafe { - self.buffer.set_len(new_len); - if aligned_index - len > 0 { - self.buffer[len].write(1); - self.buffer[len + 1..aligned_index].fill(MaybeUninit::new(0)); - } - std::ptr::copy_nonoverlapping( - src, - self.buffer.as_mut_ptr().add(aligned_index).cast::(), - size, - ); - } - aligned_index - len > 0 - } - - /// Pops a value of type `T` from the stack. Does not change the stack capacity. - /// - /// # Safety - /// - /// The type `T` must be the same type as the value on the top of the stack. - /// Incorrect usage can lead to undefined behavior. - /// - /// # Type Parameters - /// - /// * `T`: The type of the value to pop. - /// - /// # Examples - /// - /// ``` - /// use cel_runtime::raw_stack::RawStack; - /// let mut stack = RawStack::with_base_alignment(align_of::()); - /// let padding = stack.push(100u32); - /// let value: u32 = unsafe { stack.pop(padding) }; - /// ``` - pub unsafe fn pop(&mut self, padding: bool) -> T { - let p: usize = self.buffer.len() - size_of::(); - let result = unsafe { std::ptr::read(self.buffer.as_ptr().add(p).cast::()) }; - // count the number of trailing 0s in the buffer before the result - let padding_count = if padding { - self.buffer[..p] - .iter() - .rev() - .take_while(|&x| unsafe { x.assume_init() == 0 }) - .count() - + 1 - } else { - 0 - }; - self.buffer.truncate(p - padding_count); - result - } - - /// Pops a value of type `T` from the stack and drops it. - /// - /// # Safety - /// - /// The type `T` must be the same type as the value on the top of the stack. - /// Incorrect usage can lead to undefined behavior. - /// - /// # Note - /// - /// This cannot use `drop_in_place` because the type may not be aligned. - pub unsafe fn drop(&mut self, padding: bool) { - unsafe { self.pop::(padding) }; - } -} - -/* Test module */ -#[cfg(test)] -mod tests { - use super::*; - use std::cmp::max; - - #[test] - fn push_pop_u32() { - let mut stack = RawStack::with_base_alignment(align_of::()); - let padding = stack.push(10u32); - let result: u32 = unsafe { stack.pop(padding) }; - assert_eq!(result, 10); - } - - #[test] - fn multiple_push_pop() { - let mut stack = RawStack::with_base_alignment(align_of::()); - let padding1 = stack.push(1u32); - let padding2 = stack.push(2u32); - let padding3 = stack.push(3u32); - let v3: u32 = unsafe { stack.pop(padding3) }; - let v2: u32 = unsafe { stack.pop(padding2) }; - let v1: u32 = unsafe { stack.pop(padding1) }; - assert_eq!(v1, 1); - assert_eq!(v2, 2); - assert_eq!(v3, 3); - } - - #[test] - fn push_pop_different_types() { - let mut stack = RawStack::with_base_alignment(max(align_of::(), align_of::())); - let padding1 = stack.push(42u32); - let padding2 = stack.push(3.14f64); - let value_f: f64 = unsafe { stack.pop(padding2) }; - let value_u: u32 = unsafe { stack.pop(padding1) }; - assert_eq!(value_f, 3.14); - assert_eq!(value_u, 42); - } - - #[test] - fn len_reflects_pushed_bytes() { - let mut stack = RawStack::with_base_alignment(align_of::()); - assert_eq!(stack.len(), 0); - let _ = stack.push(7u32); - assert_eq!(stack.len(), size_of::()); - } - - #[test] - fn push_raw_round_trips_like_push() { - let mut stack = RawStack::with_base_alignment(align_of::()); - let padding1 = stack.push(1u8); - let value = 3.14f64; - let padding2 = unsafe { - stack.push_raw( - align_of::(), - size_of::(), - (&value as *const f64).cast::(), - ) - }; - let popped: f64 = unsafe { stack.pop(padding2) }; - assert_eq!(popped, 3.14); - let popped_u8: u8 = unsafe { stack.pop(padding1) }; - assert_eq!(popped_u8, 1); - } - - #[test] - fn push_raw_padding_matches_typed_push() { - let mut stack_a = RawStack::with_base_alignment(align_of::()); - let _ = stack_a.push(1u8); - let padding_typed = stack_a.push(2.5f64); - - let mut stack_b = RawStack::with_base_alignment(align_of::()); - let _ = stack_b.push(1u8); - let value = 2.5f64; - let padding_raw = unsafe { - stack_b.push_raw( - align_of::(), - size_of::(), - (&value as *const f64).cast::(), - ) - }; - assert_eq!(padding_typed, padding_raw); - } -} +use crate::memory::align_index; +use crate::raw_vec::RawVec; +use std::mem::MaybeUninit; +use std::mem::size_of; + +/// A simple raw stack that stores values as raw bytes. Each value is naturally aligned given the +/// base alignment of the stack, which is the maximum alignment of any value stored in the stack. +#[derive(Debug)] +pub struct RawStack { + buffer: RawVec, +} + +impl RawStack { + /// Creates a new `RawStack` with base alignment. + /// + /// # Examples + /// + /// ``` + /// use cel_runtime::raw_stack::RawStack; + /// let stack = RawStack::with_base_alignment(align_of::()); + /// ``` + #[must_use] + pub fn with_base_alignment(base_alignment: usize) -> Self { + RawStack { + buffer: RawVec::with_base_alignment(base_alignment), + } + } + + /// Returns the number of bytes currently on the stack. + #[must_use] + #[allow(clippy::len_without_is_empty)] + pub fn len(&self) -> usize { + self.buffer.len() + } + + /// Pushes a value of type `T` onto the stack. + /// + /// The value is stored as raw bytes in the internal buffer. The pushed value must be + /// later popped using the correct type. + /// + /// # Type Parameters + /// + /// * `T`: The type of the value to push. + /// + /// # Examples + /// + /// ``` + /// use cel_runtime::raw_stack::RawStack; + /// let mut stack = RawStack::with_base_alignment(align_of::()); + /// let _ = stack.push(42u32); + /// ``` + /// + /// # Complexity + /// + /// The function has an amortized O(1) time complexity. + pub fn push(&mut self, value: T) -> bool { + let len = self.buffer.len(); + let aligned_index = align_index(align_of::(), len); + let new_len = aligned_index + size_of::(); + + self.buffer.reserve(new_len - len); + unsafe { + self.buffer.set_len(new_len); + if aligned_index - len > 0 { + // write a 1 in the first padding byte and 0 in the rest + self.buffer[len].write(1); + self.buffer[len + 1..aligned_index].fill(MaybeUninit::new(0)); + } + + std::ptr::write( + self.buffer.as_mut_ptr().add(aligned_index).cast::(), + value, + ); + } + aligned_index - len > 0 + } + + /// Pushes `size` raw bytes from `src`, aligned to `align`, using the same + /// padding/marker-byte bookkeeping as [`push`](Self::push). + /// + /// - Precondition: `align` is a power of two. + /// + /// # Safety + /// `src` must be valid for reads of `size` bytes. + pub unsafe fn push_raw(&mut self, align: usize, size: usize, src: *const u8) -> bool { + debug_assert!(align.is_power_of_two()); + let len = self.buffer.len(); + let aligned_index = align_index(align, len); + let new_len = aligned_index + size; + + self.buffer.reserve(new_len - len); + unsafe { + self.buffer.set_len(new_len); + if aligned_index - len > 0 { + self.buffer[len].write(1); + self.buffer[len + 1..aligned_index].fill(MaybeUninit::new(0)); + } + std::ptr::copy_nonoverlapping( + src, + self.buffer.as_mut_ptr().add(aligned_index).cast::(), + size, + ); + } + aligned_index - len > 0 + } + + /// Copies `size` bytes starting at absolute buffer offset `offset` into `dst`. + /// + /// # Safety + /// `offset..offset + size` must be within the currently-initialized buffer; + /// `dst` must be valid for writes of `size` bytes. + pub unsafe fn copy_from(&self, offset: usize, size: usize, dst: *mut u8) { + unsafe { + std::ptr::copy_nonoverlapping(self.buffer.as_ptr().add(offset).cast::(), dst, size); + } + } + + /// Drops a value in place at absolute buffer offset `offset`, without + /// altering the stack's tracked length. + /// + /// # Safety + /// `offset` must point to a live, valid value; `run_drop` must correctly + /// run that value's destructor given a pointer to its start. + pub unsafe fn drop_at(&mut self, offset: usize, run_drop: impl FnOnce(*mut u8)) { + unsafe { run_drop(self.buffer.as_mut_ptr().add(offset).cast::()) }; + } + + /// Truncates the stack back to `new_len`, additionally stripping `padding` + /// bytes that preceded the removed region (scanned the same way + /// [`pop`](Self::pop) does). + /// + /// # Safety + /// No live (undropped) value may exist at or above `new_len`. + pub unsafe fn truncate_to(&mut self, new_len: usize, padding: bool) { + let padding_count = if padding { + self.buffer[..new_len] + .iter() + .rev() + .take_while(|&x| unsafe { x.assume_init() == 0 }) + .count() + + 1 + } else { + 0 + }; + self.buffer.truncate(new_len - padding_count); + } + + /// Drops a value of `size` bytes at the top of the stack in place, then + /// removes it (and any padding that preceded it). + /// + /// # Safety + /// The top `size` bytes (plus padding if `padding` is true) must be a + /// live, valid value; `run_drop` must correctly run its destructor given a + /// pointer to its start. + pub unsafe fn drop_sized( + &mut self, + size: usize, + padding: bool, + run_drop: impl FnOnce(*mut u8), + ) { + let p = self.buffer.len() - size; + unsafe { + self.drop_at(p, run_drop); + self.truncate_to(p, padding); + } + } + /// # Safety + /// + /// The type `T` must be the same type as the value on the top of the stack. + /// Incorrect usage can lead to undefined behavior. + /// + /// # Type Parameters + /// + /// * `T`: The type of the value to pop. + /// + /// # Examples + /// + /// ``` + /// use cel_runtime::raw_stack::RawStack; + /// let mut stack = RawStack::with_base_alignment(align_of::()); + /// let padding = stack.push(100u32); + /// let value: u32 = unsafe { stack.pop(padding) }; + /// ``` + pub unsafe fn pop(&mut self, padding: bool) -> T { + let p: usize = self.buffer.len() - size_of::(); + let result = unsafe { std::ptr::read(self.buffer.as_ptr().add(p).cast::()) }; + // count the number of trailing 0s in the buffer before the result + let padding_count = if padding { + self.buffer[..p] + .iter() + .rev() + .take_while(|&x| unsafe { x.assume_init() == 0 }) + .count() + + 1 + } else { + 0 + }; + self.buffer.truncate(p - padding_count); + result + } + + /// Pops a value of type `T` from the stack and drops it. + /// + /// # Safety + /// + /// The type `T` must be the same type as the value on the top of the stack. + /// Incorrect usage can lead to undefined behavior. + /// + /// # Note + /// + /// This cannot use `drop_in_place` because the type may not be aligned. + pub unsafe fn drop(&mut self, padding: bool) { + unsafe { self.pop::(padding) }; + } +} + +/* Test module */ +#[cfg(test)] +mod tests { + use super::*; + use std::cmp::max; + + #[test] + fn push_pop_u32() { + let mut stack = RawStack::with_base_alignment(align_of::()); + let padding = stack.push(10u32); + let result: u32 = unsafe { stack.pop(padding) }; + assert_eq!(result, 10); + } + + #[test] + fn multiple_push_pop() { + let mut stack = RawStack::with_base_alignment(align_of::()); + let padding1 = stack.push(1u32); + let padding2 = stack.push(2u32); + let padding3 = stack.push(3u32); + let v3: u32 = unsafe { stack.pop(padding3) }; + let v2: u32 = unsafe { stack.pop(padding2) }; + let v1: u32 = unsafe { stack.pop(padding1) }; + assert_eq!(v1, 1); + assert_eq!(v2, 2); + assert_eq!(v3, 3); + } + + #[test] + fn push_pop_different_types() { + let mut stack = RawStack::with_base_alignment(max(align_of::(), align_of::())); + let padding1 = stack.push(42u32); + let padding2 = stack.push(3.14f64); + let value_f: f64 = unsafe { stack.pop(padding2) }; + let value_u: u32 = unsafe { stack.pop(padding1) }; + assert_eq!(value_f, 3.14); + assert_eq!(value_u, 42); + } + + #[test] + fn len_reflects_pushed_bytes() { + let mut stack = RawStack::with_base_alignment(align_of::()); + assert_eq!(stack.len(), 0); + let _ = stack.push(7u32); + assert_eq!(stack.len(), size_of::()); + } + + #[test] + fn push_raw_round_trips_like_push() { + let mut stack = RawStack::with_base_alignment(align_of::()); + let padding1 = stack.push(1u8); + let value = 3.14f64; + let padding2 = unsafe { + stack.push_raw( + align_of::(), + size_of::(), + (&value as *const f64).cast::(), + ) + }; + let popped: f64 = unsafe { stack.pop(padding2) }; + assert_eq!(popped, 3.14); + let popped_u8: u8 = unsafe { stack.pop(padding1) }; + assert_eq!(popped_u8, 1); + } + + #[test] + fn push_raw_padding_matches_typed_push() { + let mut stack_a = RawStack::with_base_alignment(align_of::()); + let _ = stack_a.push(1u8); + let padding_typed = stack_a.push(2.5f64); + + let mut stack_b = RawStack::with_base_alignment(align_of::()); + let _ = stack_b.push(1u8); + let value = 2.5f64; + let padding_raw = unsafe { + stack_b.push_raw( + align_of::(), + size_of::(), + (&value as *const f64).cast::(), + ) + }; + assert_eq!(padding_typed, padding_raw); + } + + #[test] + fn copy_from_reads_bytes_at_offset() { + let mut stack = RawStack::with_base_alignment(align_of::()); + let _ = stack.push(10u32); + let _ = stack.push(20u32); + let mut buf = [0u8; 4]; + unsafe { stack.copy_from(0, 4, buf.as_mut_ptr()) }; + assert_eq!(u32::from_ne_bytes(buf), 10); + } + + #[test] + fn drop_at_runs_destructor_without_changing_length() { + use std::sync::Arc; + use std::sync::atomic::{AtomicUsize, Ordering}; + + struct DropCounter(Arc); + impl Drop for DropCounter { + fn drop(&mut self) { + self.0.fetch_add(1, Ordering::SeqCst); + } + } + + let count = Arc::new(AtomicUsize::new(0)); + let mut stack = RawStack::with_base_alignment(align_of::()); + let _ = stack.push(DropCounter(count.clone())); + let len_before = stack.len(); + unsafe { + stack.drop_at(0, |ptr| unsafe { + std::ptr::drop_in_place(ptr.cast::()) + }); + } + assert_eq!(count.load(Ordering::SeqCst), 1); + assert_eq!(stack.len(), len_before); + } + + #[test] + fn truncate_to_strips_recorded_padding() { + let mut stack = RawStack::with_base_alignment(align_of::()); + let _ = stack.push(1u8); + let padding = stack.push(2.5f64); // padding == true: 7 bytes inserted before the f64 + let len_with_value = stack.len(); + unsafe { stack.truncate_to(len_with_value - size_of::(), padding) }; + assert_eq!(stack.len(), 1); // back to just the u8 + } + + #[test] + fn drop_sized_combines_drop_at_and_truncate_to() { + use std::sync::Arc; + use std::sync::atomic::{AtomicUsize, Ordering}; + + struct DropCounter(Arc); + impl Drop for DropCounter { + fn drop(&mut self) { + self.0.fetch_add(1, Ordering::SeqCst); + } + } + + let count = Arc::new(AtomicUsize::new(0)); + let mut stack = RawStack::with_base_alignment(align_of::()); + let _ = stack.push(1u8); + let padding = stack.push(DropCounter(count.clone())); + unsafe { + stack.drop_sized(size_of::(), padding, |ptr| unsafe { + std::ptr::drop_in_place(ptr.cast::()) + }); + } + assert_eq!(count.load(Ordering::SeqCst), 1); + assert_eq!(stack.len(), 1); + } +} From 976ccbf6b0b77f7c84905835e8c86fdd6ebe1e79 Mon Sep 17 00:00:00 2001 From: Sean Parent Date: Tue, 7 Jul 2026 15:59:27 -0700 Subject: [PATCH 07/29] feat(cel-runtime): add RawStack drop/copy/truncate primitives --- cel-runtime/src/raw_stack.rs | 742 ++++++++++++++++++----------------- 1 file changed, 372 insertions(+), 370 deletions(-) diff --git a/cel-runtime/src/raw_stack.rs b/cel-runtime/src/raw_stack.rs index 3007373..e4e8b87 100644 --- a/cel-runtime/src/raw_stack.rs +++ b/cel-runtime/src/raw_stack.rs @@ -1,370 +1,372 @@ -use crate::memory::align_index; -use crate::raw_vec::RawVec; -use std::mem::MaybeUninit; -use std::mem::size_of; - -/// A simple raw stack that stores values as raw bytes. Each value is naturally aligned given the -/// base alignment of the stack, which is the maximum alignment of any value stored in the stack. -#[derive(Debug)] -pub struct RawStack { - buffer: RawVec, -} - -impl RawStack { - /// Creates a new `RawStack` with base alignment. - /// - /// # Examples - /// - /// ``` - /// use cel_runtime::raw_stack::RawStack; - /// let stack = RawStack::with_base_alignment(align_of::()); - /// ``` - #[must_use] - pub fn with_base_alignment(base_alignment: usize) -> Self { - RawStack { - buffer: RawVec::with_base_alignment(base_alignment), - } - } - - /// Returns the number of bytes currently on the stack. - #[must_use] - #[allow(clippy::len_without_is_empty)] - pub fn len(&self) -> usize { - self.buffer.len() - } - - /// Pushes a value of type `T` onto the stack. - /// - /// The value is stored as raw bytes in the internal buffer. The pushed value must be - /// later popped using the correct type. - /// - /// # Type Parameters - /// - /// * `T`: The type of the value to push. - /// - /// # Examples - /// - /// ``` - /// use cel_runtime::raw_stack::RawStack; - /// let mut stack = RawStack::with_base_alignment(align_of::()); - /// let _ = stack.push(42u32); - /// ``` - /// - /// # Complexity - /// - /// The function has an amortized O(1) time complexity. - pub fn push(&mut self, value: T) -> bool { - let len = self.buffer.len(); - let aligned_index = align_index(align_of::(), len); - let new_len = aligned_index + size_of::(); - - self.buffer.reserve(new_len - len); - unsafe { - self.buffer.set_len(new_len); - if aligned_index - len > 0 { - // write a 1 in the first padding byte and 0 in the rest - self.buffer[len].write(1); - self.buffer[len + 1..aligned_index].fill(MaybeUninit::new(0)); - } - - std::ptr::write( - self.buffer.as_mut_ptr().add(aligned_index).cast::(), - value, - ); - } - aligned_index - len > 0 - } - - /// Pushes `size` raw bytes from `src`, aligned to `align`, using the same - /// padding/marker-byte bookkeeping as [`push`](Self::push). - /// - /// - Precondition: `align` is a power of two. - /// - /// # Safety - /// `src` must be valid for reads of `size` bytes. - pub unsafe fn push_raw(&mut self, align: usize, size: usize, src: *const u8) -> bool { - debug_assert!(align.is_power_of_two()); - let len = self.buffer.len(); - let aligned_index = align_index(align, len); - let new_len = aligned_index + size; - - self.buffer.reserve(new_len - len); - unsafe { - self.buffer.set_len(new_len); - if aligned_index - len > 0 { - self.buffer[len].write(1); - self.buffer[len + 1..aligned_index].fill(MaybeUninit::new(0)); - } - std::ptr::copy_nonoverlapping( - src, - self.buffer.as_mut_ptr().add(aligned_index).cast::(), - size, - ); - } - aligned_index - len > 0 - } - - /// Copies `size` bytes starting at absolute buffer offset `offset` into `dst`. - /// - /// # Safety - /// `offset..offset + size` must be within the currently-initialized buffer; - /// `dst` must be valid for writes of `size` bytes. - pub unsafe fn copy_from(&self, offset: usize, size: usize, dst: *mut u8) { - unsafe { - std::ptr::copy_nonoverlapping(self.buffer.as_ptr().add(offset).cast::(), dst, size); - } - } - - /// Drops a value in place at absolute buffer offset `offset`, without - /// altering the stack's tracked length. - /// - /// # Safety - /// `offset` must point to a live, valid value; `run_drop` must correctly - /// run that value's destructor given a pointer to its start. - pub unsafe fn drop_at(&mut self, offset: usize, run_drop: impl FnOnce(*mut u8)) { - unsafe { run_drop(self.buffer.as_mut_ptr().add(offset).cast::()) }; - } - - /// Truncates the stack back to `new_len`, additionally stripping `padding` - /// bytes that preceded the removed region (scanned the same way - /// [`pop`](Self::pop) does). - /// - /// # Safety - /// No live (undropped) value may exist at or above `new_len`. - pub unsafe fn truncate_to(&mut self, new_len: usize, padding: bool) { - let padding_count = if padding { - self.buffer[..new_len] - .iter() - .rev() - .take_while(|&x| unsafe { x.assume_init() == 0 }) - .count() - + 1 - } else { - 0 - }; - self.buffer.truncate(new_len - padding_count); - } - - /// Drops a value of `size` bytes at the top of the stack in place, then - /// removes it (and any padding that preceded it). - /// - /// # Safety - /// The top `size` bytes (plus padding if `padding` is true) must be a - /// live, valid value; `run_drop` must correctly run its destructor given a - /// pointer to its start. - pub unsafe fn drop_sized( - &mut self, - size: usize, - padding: bool, - run_drop: impl FnOnce(*mut u8), - ) { - let p = self.buffer.len() - size; - unsafe { - self.drop_at(p, run_drop); - self.truncate_to(p, padding); - } - } - /// # Safety - /// - /// The type `T` must be the same type as the value on the top of the stack. - /// Incorrect usage can lead to undefined behavior. - /// - /// # Type Parameters - /// - /// * `T`: The type of the value to pop. - /// - /// # Examples - /// - /// ``` - /// use cel_runtime::raw_stack::RawStack; - /// let mut stack = RawStack::with_base_alignment(align_of::()); - /// let padding = stack.push(100u32); - /// let value: u32 = unsafe { stack.pop(padding) }; - /// ``` - pub unsafe fn pop(&mut self, padding: bool) -> T { - let p: usize = self.buffer.len() - size_of::(); - let result = unsafe { std::ptr::read(self.buffer.as_ptr().add(p).cast::()) }; - // count the number of trailing 0s in the buffer before the result - let padding_count = if padding { - self.buffer[..p] - .iter() - .rev() - .take_while(|&x| unsafe { x.assume_init() == 0 }) - .count() - + 1 - } else { - 0 - }; - self.buffer.truncate(p - padding_count); - result - } - - /// Pops a value of type `T` from the stack and drops it. - /// - /// # Safety - /// - /// The type `T` must be the same type as the value on the top of the stack. - /// Incorrect usage can lead to undefined behavior. - /// - /// # Note - /// - /// This cannot use `drop_in_place` because the type may not be aligned. - pub unsafe fn drop(&mut self, padding: bool) { - unsafe { self.pop::(padding) }; - } -} - -/* Test module */ -#[cfg(test)] -mod tests { - use super::*; - use std::cmp::max; - - #[test] - fn push_pop_u32() { - let mut stack = RawStack::with_base_alignment(align_of::()); - let padding = stack.push(10u32); - let result: u32 = unsafe { stack.pop(padding) }; - assert_eq!(result, 10); - } - - #[test] - fn multiple_push_pop() { - let mut stack = RawStack::with_base_alignment(align_of::()); - let padding1 = stack.push(1u32); - let padding2 = stack.push(2u32); - let padding3 = stack.push(3u32); - let v3: u32 = unsafe { stack.pop(padding3) }; - let v2: u32 = unsafe { stack.pop(padding2) }; - let v1: u32 = unsafe { stack.pop(padding1) }; - assert_eq!(v1, 1); - assert_eq!(v2, 2); - assert_eq!(v3, 3); - } - - #[test] - fn push_pop_different_types() { - let mut stack = RawStack::with_base_alignment(max(align_of::(), align_of::())); - let padding1 = stack.push(42u32); - let padding2 = stack.push(3.14f64); - let value_f: f64 = unsafe { stack.pop(padding2) }; - let value_u: u32 = unsafe { stack.pop(padding1) }; - assert_eq!(value_f, 3.14); - assert_eq!(value_u, 42); - } - - #[test] - fn len_reflects_pushed_bytes() { - let mut stack = RawStack::with_base_alignment(align_of::()); - assert_eq!(stack.len(), 0); - let _ = stack.push(7u32); - assert_eq!(stack.len(), size_of::()); - } - - #[test] - fn push_raw_round_trips_like_push() { - let mut stack = RawStack::with_base_alignment(align_of::()); - let padding1 = stack.push(1u8); - let value = 3.14f64; - let padding2 = unsafe { - stack.push_raw( - align_of::(), - size_of::(), - (&value as *const f64).cast::(), - ) - }; - let popped: f64 = unsafe { stack.pop(padding2) }; - assert_eq!(popped, 3.14); - let popped_u8: u8 = unsafe { stack.pop(padding1) }; - assert_eq!(popped_u8, 1); - } - - #[test] - fn push_raw_padding_matches_typed_push() { - let mut stack_a = RawStack::with_base_alignment(align_of::()); - let _ = stack_a.push(1u8); - let padding_typed = stack_a.push(2.5f64); - - let mut stack_b = RawStack::with_base_alignment(align_of::()); - let _ = stack_b.push(1u8); - let value = 2.5f64; - let padding_raw = unsafe { - stack_b.push_raw( - align_of::(), - size_of::(), - (&value as *const f64).cast::(), - ) - }; - assert_eq!(padding_typed, padding_raw); - } - - #[test] - fn copy_from_reads_bytes_at_offset() { - let mut stack = RawStack::with_base_alignment(align_of::()); - let _ = stack.push(10u32); - let _ = stack.push(20u32); - let mut buf = [0u8; 4]; - unsafe { stack.copy_from(0, 4, buf.as_mut_ptr()) }; - assert_eq!(u32::from_ne_bytes(buf), 10); - } - - #[test] - fn drop_at_runs_destructor_without_changing_length() { - use std::sync::Arc; - use std::sync::atomic::{AtomicUsize, Ordering}; - - struct DropCounter(Arc); - impl Drop for DropCounter { - fn drop(&mut self) { - self.0.fetch_add(1, Ordering::SeqCst); - } - } - - let count = Arc::new(AtomicUsize::new(0)); - let mut stack = RawStack::with_base_alignment(align_of::()); - let _ = stack.push(DropCounter(count.clone())); - let len_before = stack.len(); - unsafe { - stack.drop_at(0, |ptr| unsafe { - std::ptr::drop_in_place(ptr.cast::()) - }); - } - assert_eq!(count.load(Ordering::SeqCst), 1); - assert_eq!(stack.len(), len_before); - } - - #[test] - fn truncate_to_strips_recorded_padding() { - let mut stack = RawStack::with_base_alignment(align_of::()); - let _ = stack.push(1u8); - let padding = stack.push(2.5f64); // padding == true: 7 bytes inserted before the f64 - let len_with_value = stack.len(); - unsafe { stack.truncate_to(len_with_value - size_of::(), padding) }; - assert_eq!(stack.len(), 1); // back to just the u8 - } - - #[test] - fn drop_sized_combines_drop_at_and_truncate_to() { - use std::sync::Arc; - use std::sync::atomic::{AtomicUsize, Ordering}; - - struct DropCounter(Arc); - impl Drop for DropCounter { - fn drop(&mut self) { - self.0.fetch_add(1, Ordering::SeqCst); - } - } - - let count = Arc::new(AtomicUsize::new(0)); - let mut stack = RawStack::with_base_alignment(align_of::()); - let _ = stack.push(1u8); - let padding = stack.push(DropCounter(count.clone())); - unsafe { - stack.drop_sized(size_of::(), padding, |ptr| unsafe { - std::ptr::drop_in_place(ptr.cast::()) - }); - } - assert_eq!(count.load(Ordering::SeqCst), 1); - assert_eq!(stack.len(), 1); - } -} +use crate::memory::align_index; +use crate::raw_vec::RawVec; +use std::mem::MaybeUninit; +use std::mem::size_of; + +/// A simple raw stack that stores values as raw bytes. Each value is naturally aligned given the +/// base alignment of the stack, which is the maximum alignment of any value stored in the stack. +#[derive(Debug)] +pub struct RawStack { + buffer: RawVec, +} + +impl RawStack { + /// Creates a new `RawStack` with base alignment. + /// + /// # Examples + /// + /// ``` + /// use cel_runtime::raw_stack::RawStack; + /// let stack = RawStack::with_base_alignment(align_of::()); + /// ``` + #[must_use] + pub fn with_base_alignment(base_alignment: usize) -> Self { + RawStack { + buffer: RawVec::with_base_alignment(base_alignment), + } + } + + /// Returns the number of bytes currently on the stack. + #[must_use] + #[allow(clippy::len_without_is_empty)] + pub fn len(&self) -> usize { + self.buffer.len() + } + + /// Pushes a value of type `T` onto the stack. + /// + /// The value is stored as raw bytes in the internal buffer. The pushed value must be + /// later popped using the correct type. + /// + /// # Type Parameters + /// + /// * `T`: The type of the value to push. + /// + /// # Examples + /// + /// ``` + /// use cel_runtime::raw_stack::RawStack; + /// let mut stack = RawStack::with_base_alignment(align_of::()); + /// let _ = stack.push(42u32); + /// ``` + /// + /// # Complexity + /// + /// The function has an amortized O(1) time complexity. + pub fn push(&mut self, value: T) -> bool { + let len = self.buffer.len(); + let aligned_index = align_index(align_of::(), len); + let new_len = aligned_index + size_of::(); + + self.buffer.reserve(new_len - len); + unsafe { + self.buffer.set_len(new_len); + if aligned_index - len > 0 { + // write a 1 in the first padding byte and 0 in the rest + self.buffer[len].write(1); + self.buffer[len + 1..aligned_index].fill(MaybeUninit::new(0)); + } + + std::ptr::write( + self.buffer.as_mut_ptr().add(aligned_index).cast::(), + value, + ); + } + aligned_index - len > 0 + } + + /// Pushes `size` raw bytes from `src`, aligned to `align`, using the same + /// padding/marker-byte bookkeeping as [`push`](Self::push). + /// + /// - Precondition: `align` is a power of two. + /// + /// # Safety + /// `src` must be valid for reads of `size` bytes, and must not overlap the + /// stack's internal buffer. + pub unsafe fn push_raw(&mut self, align: usize, size: usize, src: *const u8) -> bool { + debug_assert!(align.is_power_of_two()); + let len = self.buffer.len(); + let aligned_index = align_index(align, len); + let new_len = aligned_index + size; + + self.buffer.reserve(new_len - len); + unsafe { + self.buffer.set_len(new_len); + if aligned_index - len > 0 { + self.buffer[len].write(1); + self.buffer[len + 1..aligned_index].fill(MaybeUninit::new(0)); + } + std::ptr::copy_nonoverlapping( + src, + self.buffer.as_mut_ptr().add(aligned_index).cast::(), + size, + ); + } + aligned_index - len > 0 + } + + /// Copies `size` bytes starting at absolute buffer offset `offset` into `dst`. + /// + /// # Safety + /// `offset..offset + size` must be within the currently-initialized buffer; + /// `dst` must be valid for writes of `size` bytes. + pub unsafe fn copy_from(&self, offset: usize, size: usize, dst: *mut u8) { + unsafe { + std::ptr::copy_nonoverlapping(self.buffer.as_ptr().add(offset).cast::(), dst, size); + } + } + + /// Drops a value in place at absolute buffer offset `offset`, without + /// altering the stack's tracked length. + /// + /// # Safety + /// `offset` must point to a live, valid value; `run_drop` must correctly + /// run that value's destructor given a pointer to its start. + pub unsafe fn drop_at(&mut self, offset: usize, run_drop: impl FnOnce(*mut u8)) { + unsafe { run_drop(self.buffer.as_mut_ptr().add(offset).cast::()) }; + } + + /// Truncates the stack back to `new_len`, additionally stripping `padding` + /// bytes that preceded the removed region (scanned the same way + /// [`pop`](Self::pop) does). + /// + /// # Safety + /// No live (undropped) value may exist at or above `new_len`. + pub unsafe fn truncate_to(&mut self, new_len: usize, padding: bool) { + let padding_count = if padding { + self.buffer[..new_len] + .iter() + .rev() + .take_while(|&x| unsafe { x.assume_init() == 0 }) + .count() + + 1 + } else { + 0 + }; + self.buffer.truncate(new_len - padding_count); + } + + /// Drops a value of `size` bytes at the top of the stack in place, then + /// removes it (and any padding that preceded it). + /// + /// # Safety + /// The top `size` bytes (plus padding if `padding` is true) must be a + /// live, valid value; `run_drop` must correctly run its destructor given a + /// pointer to its start. + pub unsafe fn drop_sized( + &mut self, + size: usize, + padding: bool, + run_drop: impl FnOnce(*mut u8), + ) { + let p = self.buffer.len() - size; + unsafe { + self.drop_at(p, run_drop); + self.truncate_to(p, padding); + } + } + + /// Pops a value of type `T` from the stack. Does not change the stack capacity. + /// + /// # Safety + /// + /// The type `T` must be the same type as the value on the top of the stack. + /// Incorrect usage can lead to undefined behavior. + /// + /// # Type Parameters + /// + /// * `T`: The type of the value to pop. + /// + /// # Examples + /// + /// ``` + /// use cel_runtime::raw_stack::RawStack; + /// let mut stack = RawStack::with_base_alignment(align_of::()); + /// let padding = stack.push(100u32); + /// let value: u32 = unsafe { stack.pop(padding) }; + /// ``` + pub unsafe fn pop(&mut self, padding: bool) -> T { + let p: usize = self.buffer.len() - size_of::(); + let result = unsafe { std::ptr::read(self.buffer.as_ptr().add(p).cast::()) }; + // count the number of trailing 0s in the buffer before the result + let padding_count = if padding { + self.buffer[..p] + .iter() + .rev() + .take_while(|&x| unsafe { x.assume_init() == 0 }) + .count() + + 1 + } else { + 0 + }; + self.buffer.truncate(p - padding_count); + result + } + + /// Pops a value of type `T` from the stack and drops it. + /// + /// # Safety + /// + /// The type `T` must be the same type as the value on the top of the stack. + /// Incorrect usage can lead to undefined behavior. + /// + /// # Note + /// + /// This cannot use `drop_in_place` because the type may not be aligned. + pub unsafe fn drop(&mut self, padding: bool) { + unsafe { self.pop::(padding) }; + } +} + +/* Test module */ +#[cfg(test)] +mod tests { + use super::*; + use std::cmp::max; + + #[test] + fn push_pop_u32() { + let mut stack = RawStack::with_base_alignment(align_of::()); + let padding = stack.push(10u32); + let result: u32 = unsafe { stack.pop(padding) }; + assert_eq!(result, 10); + } + + #[test] + fn multiple_push_pop() { + let mut stack = RawStack::with_base_alignment(align_of::()); + let padding1 = stack.push(1u32); + let padding2 = stack.push(2u32); + let padding3 = stack.push(3u32); + let v3: u32 = unsafe { stack.pop(padding3) }; + let v2: u32 = unsafe { stack.pop(padding2) }; + let v1: u32 = unsafe { stack.pop(padding1) }; + assert_eq!(v1, 1); + assert_eq!(v2, 2); + assert_eq!(v3, 3); + } + + #[test] + fn push_pop_different_types() { + let mut stack = RawStack::with_base_alignment(max(align_of::(), align_of::())); + let padding1 = stack.push(42u32); + let padding2 = stack.push(3.14f64); + let value_f: f64 = unsafe { stack.pop(padding2) }; + let value_u: u32 = unsafe { stack.pop(padding1) }; + assert_eq!(value_f, 3.14); + assert_eq!(value_u, 42); + } + + #[test] + fn len_reflects_pushed_bytes() { + let mut stack = RawStack::with_base_alignment(align_of::()); + assert_eq!(stack.len(), 0); + let _ = stack.push(7u32); + assert_eq!(stack.len(), size_of::()); + } + + #[test] + fn push_raw_round_trips_like_push() { + let mut stack = RawStack::with_base_alignment(align_of::()); + let padding1 = stack.push(1u8); + let value = 3.14f64; + let padding2 = unsafe { + stack.push_raw( + align_of::(), + size_of::(), + (&value as *const f64).cast::(), + ) + }; + let popped: f64 = unsafe { stack.pop(padding2) }; + assert_eq!(popped, 3.14); + let popped_u8: u8 = unsafe { stack.pop(padding1) }; + assert_eq!(popped_u8, 1); + } + + #[test] + fn push_raw_padding_matches_typed_push() { + let mut stack_a = RawStack::with_base_alignment(align_of::()); + let _ = stack_a.push(1u8); + let padding_typed = stack_a.push(2.5f64); + + let mut stack_b = RawStack::with_base_alignment(align_of::()); + let _ = stack_b.push(1u8); + let value = 2.5f64; + let padding_raw = unsafe { + stack_b.push_raw( + align_of::(), + size_of::(), + (&value as *const f64).cast::(), + ) + }; + assert_eq!(padding_typed, padding_raw); + } + + #[test] + fn copy_from_reads_bytes_at_offset() { + let mut stack = RawStack::with_base_alignment(align_of::()); + let _ = stack.push(10u32); + let _ = stack.push(20u32); + let mut buf = [0u8; 4]; + unsafe { stack.copy_from(0, 4, buf.as_mut_ptr()) }; + assert_eq!(u32::from_ne_bytes(buf), 10); + } + + #[test] + fn drop_at_runs_destructor_without_changing_length() { + use std::sync::Arc; + use std::sync::atomic::{AtomicUsize, Ordering}; + + struct DropCounter(Arc); + impl Drop for DropCounter { + fn drop(&mut self) { + self.0.fetch_add(1, Ordering::SeqCst); + } + } + + let count = Arc::new(AtomicUsize::new(0)); + let mut stack = RawStack::with_base_alignment(align_of::()); + let _ = stack.push(DropCounter(count.clone())); + let len_before = stack.len(); + unsafe { + stack.drop_at(0, |ptr| std::ptr::drop_in_place(ptr.cast::())); + } + assert_eq!(count.load(Ordering::SeqCst), 1); + assert_eq!(stack.len(), len_before); + } + + #[test] + fn truncate_to_strips_recorded_padding() { + let mut stack = RawStack::with_base_alignment(align_of::()); + let _ = stack.push(1u8); + let padding = stack.push(2.5f64); // padding == true: 7 bytes inserted before the f64 + let len_with_value = stack.len(); + unsafe { stack.truncate_to(len_with_value - size_of::(), padding) }; + assert_eq!(stack.len(), 1); // back to just the u8 + } + + #[test] + fn drop_sized_combines_drop_at_and_truncate_to() { + use std::sync::Arc; + use std::sync::atomic::{AtomicUsize, Ordering}; + + struct DropCounter(Arc); + impl Drop for DropCounter { + fn drop(&mut self) { + self.0.fetch_add(1, Ordering::SeqCst); + } + } + + let count = Arc::new(AtomicUsize::new(0)); + let mut stack = RawStack::with_base_alignment(align_of::()); + let _ = stack.push(1u8); + let padding = stack.push(DropCounter(count.clone())); + unsafe { + stack.drop_sized(size_of::(), padding, |ptr| { + std::ptr::drop_in_place(ptr.cast::()) + }); + } + assert_eq!(count.load(Ordering::SeqCst), 1); + assert_eq!(stack.len(), 1); + } +} From 1f941911323262b387fd82e555d1aca4dda977c2 Mon Sep 17 00:00:00 2001 From: Sean Parent Date: Tue, 7 Jul 2026 16:04:14 -0700 Subject: [PATCH 08/29] fix(cel-runtime): add missing overlap doc and debug_asserts on Task 2 primitives copy_from's safety doc was missing the dst-non-overlap clause (uses copy_nonoverlapping); copy_from/truncate_to/drop_sized had no debug_assert! for their arithmetic preconditions, unlike sibling push_raw. Addresses Task 2 review findings. --- cel-runtime/src/raw_stack.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cel-runtime/src/raw_stack.rs b/cel-runtime/src/raw_stack.rs index e4e8b87..df1d003 100644 --- a/cel-runtime/src/raw_stack.rs +++ b/cel-runtime/src/raw_stack.rs @@ -109,8 +109,10 @@ impl RawStack { /// /// # Safety /// `offset..offset + size` must be within the currently-initialized buffer; - /// `dst` must be valid for writes of `size` bytes. + /// `dst` must be valid for writes of `size` bytes and must not overlap the + /// stack's internal buffer. pub unsafe fn copy_from(&self, offset: usize, size: usize, dst: *mut u8) { + debug_assert!(offset + size <= self.buffer.len()); unsafe { std::ptr::copy_nonoverlapping(self.buffer.as_ptr().add(offset).cast::(), dst, size); } @@ -133,6 +135,7 @@ impl RawStack { /// # Safety /// No live (undropped) value may exist at or above `new_len`. pub unsafe fn truncate_to(&mut self, new_len: usize, padding: bool) { + debug_assert!(new_len <= self.buffer.len()); let padding_count = if padding { self.buffer[..new_len] .iter() @@ -159,6 +162,7 @@ impl RawStack { padding: bool, run_drop: impl FnOnce(*mut u8), ) { + debug_assert!(size <= self.buffer.len()); let p = self.buffer.len() - size; unsafe { self.drop_at(p, run_drop); From 72446d4c8a185c8ce1e93beb96b36a73ec9393e8 Mon Sep 17 00:00:00 2001 From: Sean Parent Date: Tue, 7 Jul 2026 16:07:42 -0700 Subject: [PATCH 09/29] feat(cel-runtime): add RawStack::repack for layout-independent tuples --- cel-runtime/src/raw_stack.rs | 102 +++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+) diff --git a/cel-runtime/src/raw_stack.rs b/cel-runtime/src/raw_stack.rs index df1d003..d4de5d7 100644 --- a/cel-runtime/src/raw_stack.rs +++ b/cel-runtime/src/raw_stack.rs @@ -170,6 +170,60 @@ impl RawStack { } } + /// Repacks `sizes.len()` already-pushed values (currently at the absolute + /// byte offsets in `src_offsets`) into one contiguous, self-contained + /// region of `total_size` bytes starting at `dest_base`, placing element + /// `i` at `dest_base + dest_offsets[i]`. Adjusts the tracked length to + /// `dest_base + total_size` and returns whether leading padding was + /// inserted between `ambient_start` and `dest_base`. + /// + /// - Precondition: `src_offsets`, `dest_offsets`, and `sizes` have equal + /// length; `dest_base >= ambient_start`; the source ranges are + /// currently valid, initialized bytes. + /// + /// - Complexity: O(n) in `sizes.len()`. + /// + /// # Safety + /// The offsets and sizes must correctly describe the actual bytes in the + /// buffer; no two destination ranges may overlap. + pub unsafe fn repack( + &mut self, + ambient_start: usize, + dest_base: usize, + total_size: usize, + src_offsets: &[usize], + dest_offsets: &[usize], + sizes: &[usize], + ) -> bool { + debug_assert!(dest_base >= ambient_start); + debug_assert_eq!(src_offsets.len(), sizes.len()); + debug_assert_eq!(dest_offsets.len(), sizes.len()); + + let target_len = dest_base + total_size; + let current_len = self.buffer.len(); + let grown_len = current_len.max(target_len); + unsafe { + if grown_len > current_len { + self.buffer.reserve(grown_len - current_len); + self.buffer.set_len(grown_len); + } + let base_ptr = self.buffer.as_mut_ptr().cast::(); + for i in 0..sizes.len() { + std::ptr::copy( + base_ptr.add(src_offsets[i]), + base_ptr.add(dest_base + dest_offsets[i]), + sizes[i], + ); + } + if dest_base > ambient_start { + self.buffer[ambient_start].write(1); + self.buffer[ambient_start + 1..dest_base].fill(MaybeUninit::new(0)); + } + self.buffer.set_len(target_len); + } + dest_base > ambient_start + } + /// Pops a value of type `T` from the stack. Does not change the stack capacity. /// /// # Safety @@ -373,4 +427,52 @@ mod tests { assert_eq!(count.load(Ordering::SeqCst), 1); assert_eq!(stack.len(), 1); } + + #[test] + fn repack_moves_elements_to_ideal_offsets_and_reports_padding() { + // Ambient layout: [u8 @0][pad][u32 @4][u8 @8] — u8 then u32 then u8, each + // pushed with ordinary alignment relative to a 1-byte ambient start. + let mut stack = RawStack::with_base_alignment(align_of::()); + let ambient_start = stack.len(); // 0 + let _ = stack.push(0xAAu8); // element 0: ambient offset 0 + let _p1 = stack.push(0xBBBB_BBBBu32); // element 1: ambient offset 4 (1 byte padded to 4) + let _p2 = stack.push(0xCCu8); // element 2: ambient offset 8 + + // Ideal (self-contained) layout for (u8, u32, u8) from a zero base: + // offset 0 (u8), offset 4 (u32, aligned up from 1), offset 8 (u8) -> total 9, + // rounded to the tuple's own max align (4) -> total_size 12. + let src_offsets = [0usize, 4, 8]; + let dest_offsets = [0usize, 4, 8]; + let sizes = [1usize, 4, 1]; + let total_size = 12usize; + let dest_base = 0usize; // ambient_start (0) is already 4-aligned + + let padding = unsafe { + stack.repack( + ambient_start, + dest_base, + total_size, + &src_offsets, + &dest_offsets, + &sizes, + ) + }; + assert!( + !padding, + "ambient_start was already aligned; no leading pad expected" + ); + assert_eq!(stack.len(), dest_base + total_size); + + let mut a = [0u8; 1]; + let mut b = [0u8; 4]; + let mut c = [0u8; 1]; + unsafe { + stack.copy_from(dest_base, 1, a.as_mut_ptr()); + stack.copy_from(dest_base + 4, 4, b.as_mut_ptr()); + stack.copy_from(dest_base + 8, 1, c.as_mut_ptr()); + } + assert_eq!(a[0], 0xAA); + assert_eq!(u32::from_ne_bytes(b), 0xBBBB_BBBB); + assert_eq!(c[0], 0xCC); + } } From 8867dec44517f74eb50c359f922fe26de66139ca Mon Sep 17 00:00:00 2001 From: Sean Parent Date: Tue, 7 Jul 2026 16:16:40 -0700 Subject: [PATCH 10/29] fix(cel-runtime): process repack elements in reverse index order Forward-order copying could overwrite a later element's not-yet-read source bytes whenever dest_base > ambient_start (i.e. almost any tuple whose first element's alignment is smaller than a later element's), silently corrupting tuple contents. Reverse order is provably safe: given dest_base >= ambient_start and same-order src/dest layouts, each element's destination is always at or after every earlier element's source end. Adds a regression test that fails under the old ordering and passes under the fix. --- cel-runtime/src/raw_stack.rs | 72 +++++++++++++++++++++++++++++++++++- 1 file changed, 70 insertions(+), 2 deletions(-) diff --git a/cel-runtime/src/raw_stack.rs b/cel-runtime/src/raw_stack.rs index d4de5d7..a1cc432 100644 --- a/cel-runtime/src/raw_stack.rs +++ b/cel-runtime/src/raw_stack.rs @@ -185,7 +185,14 @@ impl RawStack { /// /// # Safety /// The offsets and sizes must correctly describe the actual bytes in the - /// buffer; no two destination ranges may overlap. + /// buffer; no two destination ranges may overlap. `src_offsets` and + /// `dest_offsets` must be in the same relative element order (element `i`'s + /// source and destination must both be the `i`-th non-overlapping range in + /// their respective layouts) — this method does not reorder elements, only + /// re-pads between them. Given that and `dest_base >= ambient_start`, each + /// element's destination start is guaranteed to be at or after every + /// earlier element's source end, which is what makes processing in reverse + /// index order below safe against clobbering not-yet-read source bytes. pub unsafe fn repack( &mut self, ambient_start: usize, @@ -208,7 +215,11 @@ impl RawStack { self.buffer.set_len(grown_len); } let base_ptr = self.buffer.as_mut_ptr().cast::(); - for i in 0..sizes.len() { + // Process highest index first: each element's destination is + // provably at or after every earlier element's source end (see + // `# Safety` above), so this order never overwrites source bytes + // an earlier iteration still needs to read. + for i in (0..sizes.len()).rev() { std::ptr::copy( base_ptr.add(src_offsets[i]), base_ptr.add(dest_base + dest_offsets[i]), @@ -475,4 +486,61 @@ mod tests { assert_eq!(u32::from_ne_bytes(b), 0xBBBB_BBBB); assert_eq!(c[0], 0xCC); } + + #[test] + fn repack_shifts_right_without_corrupting_unread_source_bytes() { + // A misaligned ambient_start forces dest_base > ambient_start, which + // means the destination of the *first* tuple element can land inside + // the *source* range of a *later*, not-yet-copied element. Processing + // elements low-index-first would silently corrupt that later element's + // bytes before they're read; this test fails under that ordering and + // passes under the correct (reverse) ordering. + let mut stack = RawStack::with_base_alignment(align_of::()); + let _ = stack.push(0xFFu8); // sentinel, ambient offset 0, before the tuple + let ambient_start = stack.len(); // 1: misaligned relative to u32's 4-byte align + let _ = stack.push(0xDDu8); // element 0: ambient offset 1 + let _ = stack.push(0x1122_3344u32); // element 1: ambient offset 4 (3 bytes padded) + + // Ideal (u8, u32) layout from zero: offset 0 (u8), offset 4 (u32) -> total 8. + let src_offsets = [1usize, 4]; + let dest_offsets = [0usize, 4]; + let sizes = [1usize, 4]; + let total_size = 8usize; + let dest_base = 4usize; // align_index(4, ambient_start=1) == 4, so this shifts right + + let padding = unsafe { + stack.repack( + ambient_start, + dest_base, + total_size, + &src_offsets, + &dest_offsets, + &sizes, + ) + }; + assert!( + padding, + "ambient_start (1) is not 4-aligned; a leading pad is expected" + ); + assert_eq!(stack.len(), dest_base + total_size); + + let mut sentinel = [0u8; 1]; + let mut a = [0u8; 1]; + let mut b = [0u8; 4]; + unsafe { + stack.copy_from(0, 1, sentinel.as_mut_ptr()); + stack.copy_from(dest_base, 1, a.as_mut_ptr()); + stack.copy_from(dest_base + 4, 4, b.as_mut_ptr()); + } + assert_eq!( + sentinel[0], 0xFF, + "bytes before the tuple must be untouched" + ); + assert_eq!(a[0], 0xDD); + assert_eq!( + u32::from_ne_bytes(b), + 0x1122_3344, + "the u32's bytes must survive the repack uncorrupted" + ); + } } From cecc9d580bea2f54ff1ba7ab7307d0af8dd7743b Mon Sep 17 00:00:00 2001 From: Sean Parent Date: Tue, 7 Jul 2026 16:27:19 -0700 Subject: [PATCH 11/29] refactor(cel-runtime): widen StackInfo/AssociatedType with size/align/raw_dropper --- cel-runtime/src/dyn_segment.rs | 121 +++++++++++++++++++++++++-------- 1 file changed, 93 insertions(+), 28 deletions(-) diff --git a/cel-runtime/src/dyn_segment.rs b/cel-runtime/src/dyn_segment.rs index 1de6108..c0973c4 100644 --- a/cel-runtime/src/dyn_segment.rs +++ b/cel-runtime/src/dyn_segment.rs @@ -33,24 +33,49 @@ impl Drop for DynCallGuard { } } -/// Recursive type node carrying a [`TypeId`], display name, and optional associated types. +/// Drops a value in place, given a pointer to its bytes and (for tuple +/// values) its own element metadata for recursive drops. /// -/// Used for function parameter/return types, tuple elements, and similar structure. -/// Not yet used; reserved for parse-time call checking and richer error reporting. +/// # Safety +/// `ptr` must point to a valid, live, properly aligned value of the type this +/// dropper was generated for; `associated` must be that same value's own +/// element list (empty for non-tuple values). +pub type RawDropper = unsafe fn(*mut u8, &[AssociatedType]); + +/// Recursive type node carrying a [`TypeId`], display name, byte layout, and +/// an in-place dropper — describes one element of a tuple (or, nested, one +/// element of a tuple element). #[derive(Clone, Debug)] pub struct AssociatedType { /// Runtime type id for this node. pub type_id: TypeId, /// Human-readable name for error reporting (borrowed when from `type_name::()`). pub type_name: Cow<'static, str>, - /// Child types (e.g. function parameters, tuple elements). + /// Byte offset from the start of the enclosing tuple. + pub offset: usize, + /// Size in bytes of this element's value. + pub size: usize, + /// Required alignment in bytes of this element's value. + pub align: usize, + /// In-place dropper for this element, callable at `base + offset`. + pub dropper: RawDropper, + /// Child types, for a nested tuple element. pub associated: Vec, } +/// Marker type used as the `TypeId` for tuple aggregate stack entries. +/// +/// A tuple's real type identity is the ordered `associated` list on its +/// [`StackInfo`], not this marker's `TypeId` — comparisons that need to +/// distinguish tuple shapes must inspect `associated`, not `type_id`. +#[derive(Debug)] +pub struct DynTuple; + /// Information about a type on the stack, including its cleanup function. /// -/// Holds metadata for a value pushed onto the stack: runtime type id, display name -/// for errors, padding, dropper, and an optional list of associated types. +/// Holds metadata for a value pushed onto the stack: runtime type id, display +/// name for errors, padding, size/alignment, an in-place dropper, and an +/// optional list of associated element types (populated for tuples). pub struct StackInfo { /// Runtime type id for this stack slot (e.g. for scope matching). pub type_id: TypeId, @@ -58,9 +83,13 @@ pub struct StackInfo { pub type_name: Cow<'static, str>, /// Whether padding was inserted before this value for alignment. pub(crate) padding: bool, - /// Dropper used when unwinding the stack. - dropper: Dropper, - /// Associated types (e.g. function params, tuple elements). Unused for now. + /// Size in bytes of this stack slot's value. + pub size: usize, + /// Required alignment in bytes of this stack slot's value. + pub align: usize, + /// In-place dropper for this value, callable at its own start address. + pub(crate) raw_dropper: RawDropper, + /// Associated element types (populated for tuples; empty otherwise). pub associated: Vec, } @@ -91,21 +120,15 @@ impl ToTypeIdList type_id: TypeId::of::(), type_name: Cow::Borrowed(std::any::type_name::()), padding: Self::HEAD_PADDED, - dropper: |stack| unsafe { stack.drop::(Self::HEAD_PADDED) }, + size: size_of::(), + align: align_of::(), + raw_dropper: |ptr, _associated| unsafe { std::ptr::drop_in_place(ptr.cast::()) }, associated: Vec::new(), }); list } } -/// A type-checked wrapper around [`RawSegment`] that maintains a stack of type information -/// to ensure type safety during operation execution. -/// -/// [`DynSegment`] tracks the types of values on the stack at compile time and verifies -/// that operations receive arguments of the correct type. This prevents type mismatches -/// that could occur when using [`RawSegment`] directly. -type Dropper = fn(&mut RawStack); - /// A dynamic segment that provides runtime type checking for stack operations. /// /// This struct wraps a [`RawSegment`] and maintains type information about the stack @@ -205,11 +228,9 @@ impl DynSegment { type_id: TypeId::of::(), type_name: Cow::Borrowed(std::any::type_name::()), padding: padded, - dropper: if padded { - |stack| unsafe { stack.drop::(true) } - } else { - |stack| unsafe { stack.drop::(false) } - }, + size: size_of::(), + align: align_of::(), + raw_dropper: |ptr, _associated| unsafe { std::ptr::drop_in_place(ptr.cast::()) }, associated: Vec::new(), }); self.stack_index = aligned_index + size_of::(); @@ -230,17 +251,33 @@ impl DynSegment { /// Captures the current stack droppers for use when unwinding on error. /// /// - Complexity: O(n) in the current stack depth. - fn capture_unwind(&self) -> Vec { - self.stack_ids.iter().map(|info| info.dropper).collect() + fn capture_unwind(&self) -> Vec<(usize, bool, RawDropper, Vec)> { + self.stack_ids + .iter() + .map(|info| { + ( + info.size, + info.padding, + info.raw_dropper, + info.associated.clone(), + ) + }) + .collect() } /// Runs the captured droppers in reverse order on error, then propagates the error. - fn unwind_on_err(unwind: &[Dropper], stack: &mut RawStack, result: Result) -> Result { + fn unwind_on_err( + unwind: &[(usize, bool, RawDropper, Vec)], + stack: &mut RawStack, + result: Result, + ) -> Result { match result { Ok(r) => Ok(r), Err(e) => { - for dropper in unwind.iter().rev() { - dropper(stack); + for (size, padding, raw_dropper, associated) in unwind.iter().rev() { + unsafe { + stack.drop_sized(*size, *padding, |ptr| raw_dropper(ptr, associated)); + } } Err(e) } @@ -899,4 +936,32 @@ mod tests { ); Ok(()) } + + #[test] + fn stack_info_records_size_and_align() { + let mut seg = DynSegment::new::<()>(); + seg.op0(|| 7u32); + let infos = seg.peek_stack_infos(1); + assert_eq!(infos[0].size, size_of::()); + assert_eq!(infos[0].align, align_of::()); + assert!(infos[0].associated.is_empty()); + } + + #[test] + fn associated_type_carries_offset_size_align_dropper() { + // Exercises the new AssociatedType shape directly — no runtime behavior + // yet, just the data shape this task adds. + let a = AssociatedType { + type_id: std::any::TypeId::of::(), + type_name: std::borrow::Cow::Borrowed("u32"), + offset: 4, + size: 4, + align: 4, + dropper: |ptr, _associated| unsafe { std::ptr::drop_in_place(ptr.cast::()) }, + associated: Vec::new(), + }; + assert_eq!(a.offset, 4); + assert_eq!(a.size, 4); + assert_eq!(a.align, 4); + } } From e6061bdfe0d6fff4e1e83f8c093a9e10a1bb4626 Mon Sep 17 00:00:00 2001 From: Sean Parent Date: Tue, 7 Jul 2026 16:37:49 -0700 Subject: [PATCH 12/29] feat(cel-runtime): add tuple construction (make_tuple) and indexing (tuple_index) --- cel-runtime/src/dyn_segment.rs | 293 +++++++++++++++++++++++++++++++++ 1 file changed, 293 insertions(+) diff --git a/cel-runtime/src/dyn_segment.rs b/cel-runtime/src/dyn_segment.rs index c0973c4..d8ba067 100644 --- a/cel-runtime/src/dyn_segment.rs +++ b/cel-runtime/src/dyn_segment.rs @@ -71,6 +71,62 @@ pub struct AssociatedType { #[derive(Debug)] pub struct DynTuple; +/// `RawDropper` for a tuple value: drops each element at `ptr + element.offset` +/// in reverse order, recursing into nested tuples via their own droppers. +/// +/// # Safety +/// `ptr` must point to a live tuple value whose layout matches `associated`. +unsafe fn drop_tuple(ptr: *mut u8, associated: &[AssociatedType]) { + for elem in associated.iter().rev() { + unsafe { (elem.dropper)(ptr.add(elem.offset), &elem.associated) }; + } +} + +/// Extracts element `index` from the tuple currently on top of `stack`, +/// dropping every other element, leaving just the extracted value on top. +/// +/// - Complexity: O(n) in the tuple's arity. +/// +/// # Safety +/// The top `tuple_size` bytes (plus `tuple_padding` if set) of `stack` must be +/// a live tuple value whose layout matches `associated`. +unsafe fn extract_tuple_element( + stack: &mut RawStack, + tuple_size: usize, + tuple_padding: bool, + associated: &[AssociatedType], + index: usize, +) { + let tuple_base = stack.len() - tuple_size; + let target = &associated[index]; + + let mut scratch = vec![0u8; target.size]; + unsafe { + stack.copy_from( + tuple_base + target.offset, + target.size, + scratch.as_mut_ptr(), + ); + } + + for (i, elem) in associated.iter().enumerate().rev() { + if i == index { + continue; + } + let elem_associated = &elem.associated; + unsafe { + stack.drop_at(tuple_base + elem.offset, |ptr| { + (elem.dropper)(ptr, elem_associated) + }); + } + } + + unsafe { + stack.truncate_to(tuple_base, tuple_padding); + stack.push_raw(target.align, target.size, scratch.as_ptr()); + } +} + /// Information about a type on the stack, including its cleanup function. /// /// Holds metadata for a value pushed onto the stack: runtime type id, display @@ -236,6 +292,143 @@ impl DynSegment { self.stack_index = aligned_index + size_of::(); } + /// Returns the current parse-time stack byte offset. + /// + /// Snapshot this before parsing a tuple's first element and pass it to + /// [`make_tuple`](Self::make_tuple). + #[must_use] + pub fn current_stack_offset(&self) -> usize { + self.stack_index + } + + /// Returns the arity of the tuple on top of the stack, or `None` if the + /// top value isn't a tuple. + #[must_use] + pub fn peek_tuple_arity(&self) -> Option { + let info = self.stack_ids.last()?; + (info.type_id == TypeId::of::()).then_some(info.associated.len()) + } + + /// Collapses the top `n` stack values (pushed starting at byte offset + /// `ambient_start`, e.g. via [`current_stack_offset`](Self::current_stack_offset) + /// captured before parsing the first element) into one tuple value. + /// + /// The tuple's internal layout (offsets between elements) depends only on + /// the elements' own types — never on `ambient_start` — matching the + /// layout a `#[repr(C)]` struct with these fields, in this order, would + /// use. + /// + /// - Precondition: at least `n` values are on the stack, pushed + /// contiguously starting at `ambient_start` with no other values + /// interleaved. + /// + /// - Complexity: O(n). + pub fn make_tuple(&mut self, n: usize, ambient_start: usize) { + debug_assert!(n <= self.stack_ids.len()); + let start = self.stack_ids.len() - n; + let elems: Vec = self.stack_ids.drain(start..).collect(); + + let mut ambient_offset = ambient_start; + let mut ideal_offset = 0usize; + let mut tuple_align = 1usize; + let mut src_offsets = Vec::with_capacity(n); + let mut associated = Vec::with_capacity(n); + for elem in &elems { + ambient_offset = align_index(elem.align, ambient_offset); + ideal_offset = align_index(elem.align, ideal_offset); + tuple_align = tuple_align.max(elem.align); + + src_offsets.push(ambient_offset); + associated.push(AssociatedType { + type_id: elem.type_id, + type_name: elem.type_name.clone(), + offset: ideal_offset, + size: elem.size, + align: elem.align, + dropper: elem.raw_dropper, + associated: elem.associated.clone(), + }); + + ambient_offset += elem.size; + ideal_offset += elem.size; + } + let total_size = align_index(tuple_align, ideal_offset); + let dest_base = align_index(tuple_align, ambient_start); + + let dest_offsets: Vec = associated.iter().map(|a| a.offset).collect(); + let sizes: Vec = elems.iter().map(|e| e.size).collect(); + + self.segment.raw0_(move |stack| { + unsafe { + stack.repack( + ambient_start, + dest_base, + total_size, + &src_offsets, + &dest_offsets, + &sizes, + ); + } + Ok(()) + }); + + self.stack_ids.push(StackInfo { + type_id: TypeId::of::(), + type_name: Cow::Borrowed(std::any::type_name::()), + padding: dest_base != ambient_start, + size: total_size, + align: tuple_align, + raw_dropper: drop_tuple, + associated, + }); + self.stack_index = dest_base + total_size; + } + + /// Extracts element `index` from the tuple on top of the stack, replacing + /// the whole tuple with just that element's value. + /// + /// - Precondition: the top-of-stack value is a tuple with at least + /// `index + 1` elements. + /// + /// - Complexity: O(n) in the tuple's arity. + pub fn tuple_index(&mut self, index: usize) { + let info = self + .stack_ids + .pop() + .expect("tuple_index requires a value on the stack"); + debug_assert_eq!( + info.type_id, + TypeId::of::(), + "tuple_index requires a tuple on top of the stack" + ); + debug_assert!(index < info.associated.len(), "tuple_index out of range"); + + let tuple_start = self.stack_index - info.size; + let target = info.associated[index].clone(); + let associated = info.associated.clone(); + let tuple_padding = info.padding; + let tuple_size = info.size; + + self.segment.raw0_(move |stack| { + unsafe { + extract_tuple_element(stack, tuple_size, tuple_padding, &associated, index); + } + Ok(()) + }); + + let target_start = align_index(target.align, tuple_start); + self.stack_ids.push(StackInfo { + type_id: target.type_id, + type_name: target.type_name, + padding: target_start != tuple_start, + size: target.size, + align: target.align, + raw_dropper: target.dropper, + associated: target.associated, + }); + self.stack_index = target_start + target.size; + } + /// Returns the padding flags for the top N entries of the type stack. /// /// - Complexity: O(N). @@ -964,4 +1157,104 @@ mod tests { assert_eq!(a.size, 4); assert_eq!(a.align, 4); } + + #[test] + fn make_tuple_then_index_each_element() { + let mut seg = DynSegment::new::<()>(); + let ambient_start = seg.current_stack_offset(); + seg.op0(|| 10u32); + seg.op0(|| "hello"); + seg.make_tuple(2, ambient_start); + assert_eq!(seg.peek_tuple_arity(), Some(2)); + + // Index element 1 first on a clone-free single segment isn't possible + // (tuple_index consumes the tuple), so build two segments to check both. + let mut seg0 = DynSegment::new::<()>(); + let ambient_start0 = seg0.current_stack_offset(); + seg0.op0(|| 10u32); + seg0.op0(|| "hello"); + seg0.make_tuple(2, ambient_start0); + seg0.tuple_index(0); + assert_eq!(seg0.call0::().unwrap(), 10); + + seg.tuple_index(1); + assert_eq!(seg.call0::<&'static str>().unwrap(), "hello"); + } + + #[test] + fn tuple_layout_is_independent_of_ambient_stack_depth() { + // (u8, u32): with nothing ahead of it vs. with a u8 already on the stack, + // internal padding between elements must be identical either way. + let mut seg_a = DynSegment::new::<()>(); + let ambient_a = seg_a.current_stack_offset(); + seg_a.op0(|| 1u8); + seg_a.op0(|| 2u32); + seg_a.make_tuple(2, ambient_a); + + let mut seg_b = DynSegment::new::<()>(); + seg_b.op0(|| 99u8); // extra value ahead, shifts ambient depth + let ambient_b = seg_b.current_stack_offset(); + seg_b.op0(|| 1u8); + seg_b.op0(|| 2u32); + seg_b.make_tuple(2, ambient_b); + + seg_a.tuple_index(1); + seg_b.tuple_index(1); + assert_eq!(seg_a.call0::().unwrap(), 2); + seg_b.op2(|_extra: u8, x: u32| x).unwrap(); + assert_eq!(seg_b.call0::().unwrap(), 2); + } + + #[test] + fn tuple_index_drops_every_other_element_exactly_once() { + use std::sync::Arc; + use std::sync::atomic::{AtomicUsize, Ordering}; + + #[derive(Clone)] + struct DropCounter(Arc); + impl Drop for DropCounter { + fn drop(&mut self) { + self.0.fetch_add(1, Ordering::SeqCst); + } + } + + let drop_count = Arc::new(AtomicUsize::new(0)); + let tracker = DropCounter(drop_count.clone()); + + let mut seg = DynSegment::new::<()>(); + let ambient_start = seg.current_stack_offset(); + seg.op0(move || tracker.clone()); + seg.op0(|| 42u32); + seg.make_tuple(2, ambient_start); + seg.tuple_index(1); // keep the u32, drop the DropCounter + + assert_eq!(seg.call0::().unwrap(), 42); + assert_eq!(drop_count.load(Ordering::SeqCst), 1); + } + + #[test] + fn tuple_index_combined_with_another_op() { + // Mirrors the spec's `5 + (0, 1).1` case: indexing must leave the stack in + // a state a subsequent op can correctly consume. + let mut seg = DynSegment::new::<()>(); + seg.op0(|| 5u32); + let ambient_start = seg.current_stack_offset(); + seg.op0(|| 0u32); + seg.op0(|| 1u32); + seg.make_tuple(2, ambient_start); + seg.tuple_index(1); + seg.op2(|a: u32, b: u32| a + b).unwrap(); + assert_eq!(seg.call0::().unwrap(), 6); + } + + #[test] + fn one_tuple_round_trips() { + let mut seg = DynSegment::new::<()>(); + let ambient_start = seg.current_stack_offset(); + seg.op0(|| 99u32); + seg.make_tuple(1, ambient_start); + assert_eq!(seg.peek_tuple_arity(), Some(1)); + seg.tuple_index(0); + assert_eq!(seg.call0::().unwrap(), 99); + } } From aa12e85717665e247b1536b567d701127dd7e3b4 Mon Sep 17 00:00:00 2001 From: Sean Parent Date: Tue, 7 Jul 2026 16:50:27 -0700 Subject: [PATCH 13/29] fix(cel-runtime): fix tuple_index padding/offset bookkeeping for leading-padded tuples MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit extract_tuple_element previously truncated past the tuple's own leading pad (using tuple_padding) and then re-pushed the extracted element, but tuple_index's parse-time bookkeeping computed the result's padding flag and stack_index relative to tuple_start (dest_base) instead of the true post-truncation base — a mismatch whenever the tuple had leading padding, corrupting subsequent pops/unwinds once the extracted element's alignment was smaller than the tuple's own alignment. Fix: never strip the tuple's own leading pad during indexing. tuple_base is already aligned for every element inside the tuple (the tuple's alignment is the max of all elements'), so truncating only down to tuple_base and re-pushing the target from there introduces no new padding — the extracted element simply inherits the tuple's own leading-padding relationship unchanged. Adds a regression test with an element whose alignment is smaller than the tuple's own, which fails under the old bookkeeping and passes now. --- cel-runtime/src/dyn_segment.rs | 54 ++++++++++++++++++++++++++++------ 1 file changed, 45 insertions(+), 9 deletions(-) diff --git a/cel-runtime/src/dyn_segment.rs b/cel-runtime/src/dyn_segment.rs index d8ba067..f9f59e9 100644 --- a/cel-runtime/src/dyn_segment.rs +++ b/cel-runtime/src/dyn_segment.rs @@ -85,20 +85,26 @@ unsafe fn drop_tuple(ptr: *mut u8, associated: &[AssociatedType]) { /// Extracts element `index` from the tuple currently on top of `stack`, /// dropping every other element, leaving just the extracted value on top. /// +/// Never strips the tuple's own leading padding (if any): the extracted +/// element inherits that same leading-padding relationship unchanged, since +/// `tuple_base` is by construction already aligned for every element inside +/// the tuple (the tuple's own alignment is the max of all its elements'), so +/// re-pushing the target from `tuple_base` introduces no padding of its own. +/// /// - Complexity: O(n) in the tuple's arity. /// /// # Safety -/// The top `tuple_size` bytes (plus `tuple_padding` if set) of `stack` must be -/// a live tuple value whose layout matches `associated`. +/// The top `tuple_size` bytes of `stack` must be a live tuple value whose +/// layout matches `associated`. unsafe fn extract_tuple_element( stack: &mut RawStack, tuple_size: usize, - tuple_padding: bool, associated: &[AssociatedType], index: usize, ) { let tuple_base = stack.len() - tuple_size; let target = &associated[index]; + debug_assert!(tuple_base.is_multiple_of(target.align)); let mut scratch = vec![0u8; target.size]; unsafe { @@ -122,8 +128,14 @@ unsafe fn extract_tuple_element( } unsafe { - stack.truncate_to(tuple_base, tuple_padding); - stack.push_raw(target.align, target.size, scratch.as_ptr()); + // padding=false: this truncates only down to tuple_base, never past + // the tuple's own leading pad (see doc comment above). + stack.truncate_to(tuple_base, false); + let repushed_padding = stack.push_raw(target.align, target.size, scratch.as_ptr()); + debug_assert!( + !repushed_padding, + "tuple_base is already aligned for every element inside the tuple" + ); } } @@ -403,6 +415,11 @@ impl DynSegment { ); debug_assert!(index < info.associated.len(), "tuple_index out of range"); + // tuple_start is the tuple's own aligned base (dest_base from + // make_tuple), already a multiple of every element's alignment. The + // extracted element inherits the tuple's own leading-padding + // relationship unchanged — see extract_tuple_element's doc comment + // for why no realignment is needed or performed. let tuple_start = self.stack_index - info.size; let target = info.associated[index].clone(); let associated = info.associated.clone(); @@ -411,22 +428,21 @@ impl DynSegment { self.segment.raw0_(move |stack| { unsafe { - extract_tuple_element(stack, tuple_size, tuple_padding, &associated, index); + extract_tuple_element(stack, tuple_size, &associated, index); } Ok(()) }); - let target_start = align_index(target.align, tuple_start); self.stack_ids.push(StackInfo { type_id: target.type_id, type_name: target.type_name, - padding: target_start != tuple_start, + padding: tuple_padding, size: target.size, align: target.align, raw_dropper: target.dropper, associated: target.associated, }); - self.stack_index = target_start + target.size; + self.stack_index = tuple_start + target.size; } /// Returns the padding flags for the top N entries of the type stack. @@ -1247,6 +1263,26 @@ mod tests { assert_eq!(seg.call0::().unwrap(), 6); } + #[test] + fn tuple_index_result_inherits_leading_padding_when_element_align_is_smaller() { + // Regression test: index element 0 (u32, align 4) out of a leading- + // padded (u32, u64) tuple (tuple_align 8) at a misaligned ambient + // start. The extracted element's align (4) is smaller than the + // tuple's own align (8), which previously caused the result's + // padding flag and stack_index bookkeeping to disagree with the + // actual runtime layout (see plan/task-5 review history) — popping a + // deeper sentinel afterward would silently read the wrong bytes. + let mut seg = DynSegment::new::<()>(); + seg.op0(|| 0xEEu8); // sentinel, ambient offset 0 + let ambient_start = seg.current_stack_offset(); // 1: misaligned for align-8 tuple + seg.op0(|| 0xAABB_CCDDu32); // element 0 + seg.op0(|| 0x1122_3344_5566_7788u64); // element 1 + seg.make_tuple(2, ambient_start); + seg.tuple_index(0); + seg.op2(|_sentinel: u8, x: u32| x).unwrap(); + assert_eq!(seg.call0::().unwrap(), 0xAABB_CCDD); + } + #[test] fn one_tuple_round_trips() { let mut seg = DynSegment::new::<()>(); From cc572727efaeef23f68babc40fec49130217a5db Mon Sep 17 00:00:00 2001 From: Sean Parent Date: Tue, 7 Jul 2026 16:56:42 -0700 Subject: [PATCH 14/29] test(cel-runtime): assert the deeper (bug-sensitive) value in tuple padding tests Both tuple_layout_is_independent_of_ambient_stack_depth and tuple_index_result_inherits_leading_padding_when_element_align_is_smaller were asserting on the just-extracted element, whose own read doesn't depend on the padding/offset bookkeeping the tests exist to guard. Assert on the deeper (pre-tuple) sentinel instead, which does. --- cel-runtime/src/dyn_segment.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/cel-runtime/src/dyn_segment.rs b/cel-runtime/src/dyn_segment.rs index f9f59e9..b9567d9 100644 --- a/cel-runtime/src/dyn_segment.rs +++ b/cel-runtime/src/dyn_segment.rs @@ -1217,8 +1217,12 @@ mod tests { seg_a.tuple_index(1); seg_b.tuple_index(1); assert_eq!(seg_a.call0::().unwrap(), 2); - seg_b.op2(|_extra: u8, x: u32| x).unwrap(); - assert_eq!(seg_b.call0::().unwrap(), 2); + // Return the deeper (pre-tuple) value, not the just-extracted one: the + // extracted u32's own read doesn't depend on its padding flag being + // correct (it's computed from the live buffer length), but correctly + // recovering `extra` underneath it does. + seg_b.op2(|extra: u8, _x: u32| extra).unwrap(); + assert_eq!(seg_b.call0::().unwrap(), 99); } #[test] @@ -1279,8 +1283,11 @@ mod tests { seg.op0(|| 0x1122_3344_5566_7788u64); // element 1 seg.make_tuple(2, ambient_start); seg.tuple_index(0); - seg.op2(|_sentinel: u8, x: u32| x).unwrap(); - assert_eq!(seg.call0::().unwrap(), 0xAABB_CCDD); + // Return the deeper sentinel, not the just-extracted u32: recovering + // it correctly is exactly what depends on the fixed padding/offset + // bookkeeping (the u32's own read would succeed even under the bug). + seg.op2(|sentinel: u8, _x: u32| sentinel).unwrap(); + assert_eq!(seg.call0::().unwrap(), 0xEE); } #[test] From c26cb165ea251271da5ef539d85d001cb79bd503 Mon Sep 17 00:00:00 2001 From: Sean Parent Date: Tue, 7 Jul 2026 17:04:23 -0700 Subject: [PATCH 15/29] feat(cel-parser): parse tuple literals --- cel-parser/src/lib.rs | 102 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 95 insertions(+), 7 deletions(-) diff --git a/cel-parser/src/lib.rs b/cel-parser/src/lib.rs index a3f94b7..7027d5c 100644 --- a/cel-parser/src/lib.rs +++ b/cel-parser/src/lib.rs @@ -26,7 +26,8 @@ //! multiplicative_expression = unary_expression { ("*" | "/" | "%") unary_expression }. //! unary_expression = (("-" | "!") unary_expression) | postfix_expression. //! postfix_expression = primary_expression { "(" parameter_list ")" }. -//! primary_expression = literal | identifier | "(" or_expression ")" | if_expression. +//! primary_expression = literal | identifier | tuple_or_group | if_expression. +//! tuple_or_group = "(" [ or_expression ["," [ or_expression { "," or_expression } ]] ] ")". //! if_expression = "if" or_expression "{" or_expression "}" [ "else" ( "{" or_expression "}" | if_expression ) ]. //! parameter_list = [ or_expression { "," or_expression } ]. //! ``` @@ -852,16 +853,20 @@ impl CELParser { Ok(count) } - /// `primary_expression = literal | identifier | "(" or_expression ")" | if_expression.` + /// `primary_expression = literal | identifier | tuple_or_group | if_expression.` + /// `tuple_or_group = "(" [ or_expression ["," [ or_expression { "," or_expression } ]] ] ")".` /// /// Dispatches to [`is_if_expression`](Self::is_if_expression) when the `if` keyword is seen. + /// `()` parses as unit, `(expr)` as grouping, `(expr,)` as a 1-tuple, and + /// `(expr, expr, ...)` as an n-tuple. /// /// # Errors /// /// Returns an error if: /// - A literal value cannot be parsed (e.g., integer out of range). /// - An identifier is not found in the op lookup table. - /// - A parenthesized expression is malformed or missing its closing `)`. + /// - A parenthesized expression or tuple literal is malformed, has a missing or + /// misplaced comma, or is missing its closing `)`. /// - An `if` expression fails to parse. fn is_primary_expression(&mut self) -> Result { match self.peek_token() { @@ -902,19 +907,58 @@ impl CELParser { self.context.just(()); return Ok(true); } + let ambient_start = self.context.current_stack_offset(); if !self.is_or_expression()? { return Err(self.error_at("expected expression")); } - match self.peek_token() { + if matches!( + self.peek_token(), Some(Token::CloseDelim { delimiter: Delimiter::Parenthesis, .. - }) => { + }) + ) { + // Grouping: exactly one expression, no comma. + self.advance(); + return Ok(true); + } + if !self.is_punctuation(",") { + return Err(self.error_at("expected ',' or closing parenthesis")); + } + let mut count = 1; + if matches!( + self.peek_token(), + Some(Token::CloseDelim { + delimiter: Delimiter::Parenthesis, + .. + }) + ) { + // Single element + trailing comma: 1-tuple. + self.advance(); + self.context.make_tuple(count, ambient_start); + return Ok(true); + } + loop { + if !self.is_or_expression()? { + return Err(self.error_at("expected expression after ','")); + } + count += 1; + if matches!( + self.peek_token(), + Some(Token::CloseDelim { + delimiter: Delimiter::Parenthesis, + .. + }) + ) { self.advance(); - Ok(true) + break; + } + if !self.is_punctuation(",") { + return Err(self.error_at("expected ',' or closing parenthesis")); } - _ => Err(self.error_at("expected closing parenthesis")), } + self.context.make_tuple(count, ambient_start); + Ok(true) } _ => Ok(false), } @@ -1128,6 +1172,50 @@ mod tests { assert!(result.is_ok()); } + #[test] + fn unit_still_parses_as_unit() { + let mut parser = CELParser::new(OpLookup::new()); + let mut seg = parser.parse_str("()").unwrap(); + seg.call0::<()>().unwrap(); + } + + #[test] + fn single_paren_expression_is_grouping_not_tuple() { + let mut parser = CELParser::new(OpLookup::new()); + let mut seg = parser.parse_str("(1i32 + 2i32)").unwrap(); + assert_eq!(seg.call0::().unwrap(), 3); + } + + #[test] + fn one_tuple_requires_trailing_comma() { + let mut parser = CELParser::new(OpLookup::new()); + let mut seg = parser.parse_str("(1i32,)").unwrap(); + assert_eq!(seg.peek_tuple_arity(), Some(1)); + seg.tuple_index(0); + assert_eq!(seg.call0::().unwrap(), 1); + } + + #[test] + fn two_element_tuple_no_trailing_comma() { + let mut parser = CELParser::new(OpLookup::new()); + let mut seg = parser.parse_str(r#"("Hello", 42i32)"#).unwrap(); + assert_eq!(seg.peek_tuple_arity(), Some(2)); + } + + #[test] + fn trailing_comma_rejected_for_arity_two() { + let mut parser = CELParser::new(OpLookup::new()); + let result = parser.parse_str("(1i32, 2i32,)"); + assert!(result.is_err(), "trailing comma is only valid for 1-tuples"); + } + + #[test] + fn missing_comma_between_elements_is_an_error() { + let mut parser = CELParser::new(OpLookup::new()); + let result = parser.parse_str("(1i32 2i32)"); + assert!(result.is_err()); + } + #[test] fn complex_expression() { let mut parser = CELParser::new(OpLookup::new()); From 05baca4152bea3b1b233b1ca79a172e8e02de916 Mon Sep 17 00:00:00 2001 From: Sean Parent Date: Tue, 7 Jul 2026 20:17:34 -0700 Subject: [PATCH 16/29] feat(cel-parser): parse .N tuple indexing --- cel-parser/src/lib.rs | 125 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 101 insertions(+), 24 deletions(-) diff --git a/cel-parser/src/lib.rs b/cel-parser/src/lib.rs index 7027d5c..8ad4db0 100644 --- a/cel-parser/src/lib.rs +++ b/cel-parser/src/lib.rs @@ -25,7 +25,7 @@ //! additive_expression = multiplicative_expression { ("+" | "-") multiplicative_expression }. //! multiplicative_expression = unary_expression { ("*" | "/" | "%") unary_expression }. //! unary_expression = (("-" | "!") unary_expression) | postfix_expression. -//! postfix_expression = primary_expression { "(" parameter_list ")" }. +//! postfix_expression = primary_expression { "(" parameter_list ")" | "." unsuffixed_integer }. //! primary_expression = literal | identifier | tuple_or_group | if_expression. //! tuple_or_group = "(" [ or_expression ["," [ or_expression { "," or_expression } ]] ] ")". //! if_expression = "if" or_expression "{" or_expression "}" [ "else" ( "{" or_expression "}" | if_expression ) ]. @@ -800,38 +800,66 @@ impl CELParser { } } - /// `postfix_expression = primary_expression { "(" parameter_list ")" }.` + /// `postfix_expression = primary_expression { "(" parameter_list ")" | "." unsuffixed_integer }.` fn is_postfix_expression(&mut self) -> Result { let start_span = self.peek_span(); if !self.is_primary_expression()? { return Ok(false); } - while matches!( - self.peek_token(), - Some(Token::OpenDelim { - delimiter: Delimiter::Parenthesis, - .. - }) - ) { - self.advance(); // consume "(" - let arg_count = self.parameter_list()?; - match self.peek_token() { - Some(Token::CloseDelim { + loop { + if matches!( + self.peek_token(), + Some(Token::OpenDelim { delimiter: Delimiter::Parenthesis, .. - }) => { - self.advance(); // consume ")" + }) + ) { + self.advance(); // consume "(" + let arg_count = self.parameter_list()?; + match self.peek_token() { + Some(Token::CloseDelim { + delimiter: Delimiter::Parenthesis, + .. + }) => { + self.advance(); // consume ")" + } + _ => return Err(self.error_at("expected closing parenthesis")), + } + // Stack order is [callee, arg1, arg2, ...]; lookup peeks top (arg_count + 1) entries. + self.op_lookup.lookup( + "()", + &mut self.context, + arg_count + 1, + start_span.expect("production has token at start"), + self.last_span, + )?; + } else if self.is_punctuation(".") { + let index = match self.peek_token() { + Some(Token::Literal(CelLiteral::Int(integer))) => { + let integer = integer.clone(); + if !integer.suffix().is_empty() { + return Err(self.error_at("tuple index must be an unsuffixed integer")); + } + self.advance(); + integer.base10_parse::().map_err(|e| { + self.error_at(&format!("invalid tuple index `{integer}`: {e}")) + })? + } + _ => return Err(self.error_at("expected integer after '.'")), + }; + let arity = self + .context + .peek_tuple_arity() + .ok_or_else(|| self.error_at("'.N' requires a tuple"))?; + if index >= arity { + return Err(self.error_at(&format!( + "tuple index `{index}` out of range for tuple of arity {arity}" + ))); } - _ => return Err(self.error_at("expected closing parenthesis")), + self.context.tuple_index(index); + } else { + break; } - // Stack order is [callee, arg1, arg2, ...]; lookup peeks top (arg_count + 1) entries. - self.op_lookup.lookup( - "()", - &mut self.context, - arg_count + 1, - start_span.expect("production has token at start"), - self.last_span, - )?; } Ok(true) } @@ -1216,6 +1244,55 @@ mod tests { assert!(result.is_err()); } + #[test] + fn index_first_element_of_tuple() { + let mut parser = CELParser::new(OpLookup::new()); + let mut seg = parser.parse_str("(10i32, 20i32).0").unwrap(); + assert_eq!(seg.call0::().unwrap(), 10); + } + + #[test] + fn index_second_element_of_tuple() { + let mut parser = CELParser::new(OpLookup::new()); + let mut seg = parser.parse_str("(10i32, 20i32).1").unwrap(); + assert_eq!(seg.call0::().unwrap(), 20); + } + + #[test] + fn indexing_combined_with_addition() { + let mut parser = CELParser::new(OpLookup::new()); + let mut seg = parser.parse_str("5i32 + (0i32, 1i32).1").unwrap(); + assert_eq!(seg.call0::().unwrap(), 6); + } + + #[test] + fn indexing_combined_with_addition_on_the_right() { + let mut parser = CELParser::new(OpLookup::new()); + let mut seg = parser.parse_str("(0i32, 1i32).1 + 5i32").unwrap(); + assert_eq!(seg.call0::().unwrap(), 6); + } + + #[test] + fn out_of_range_index_is_a_parse_error() { + let mut parser = CELParser::new(OpLookup::new()); + let result = parser.parse_str("(1i32, 2i32).5"); + assert!(result.is_err()); + } + + #[test] + fn indexing_a_non_tuple_is_a_parse_error() { + let mut parser = CELParser::new(OpLookup::new()); + let result = parser.parse_str("1i32.0"); + assert!(result.is_err()); + } + + #[test] + fn suffixed_index_is_a_parse_error() { + let mut parser = CELParser::new(OpLookup::new()); + let result = parser.parse_str("(1i32, 2i32).0i32"); + assert!(result.is_err()); + } + #[test] fn complex_expression() { let mut parser = CELParser::new(OpLookup::new()); From 97469db346ccb17347af51dbc365fa11b2bdf092 Mon Sep 17 00:00:00 2001 From: Sean Parent Date: Tue, 7 Jul 2026 20:24:25 -0700 Subject: [PATCH 17/29] feat(cel-parser): generalize op dispatch for tuple-shaped operands --- cel-parser/src/op_table.rs | 144 +++++++++++++++++++++++++++++++++++++ 1 file changed, 144 insertions(+) diff --git a/cel-parser/src/op_table.rs b/cel-parser/src/op_table.rs index bf0a0b7..e804a4a 100644 --- a/cel-parser/src/op_table.rs +++ b/cel-parser/src/op_table.rs @@ -56,6 +56,27 @@ fn span_err(_span: SourceSpan, e: anyhow::Error) -> anyhow::Error { /// have no state. pub type OpFn = fn(&mut DynSegment, SourceSpan) -> Result<()>; +/// A signature for an operator/function whose selected operand is a tuple. +/// +/// Matches when the operand at `tuple_operand_index` (0-based, in the same +/// stack order [`DynSegment::peek_stack_infos`] returns) is a tuple whose +/// element `TypeId`s equal `shape`, in order, and every other peeked operand's +/// flat `TypeId` equals the corresponding entry in `operand_type_ids` (the +/// entry at `tuple_operand_index` in `operand_type_ids` is ignored). +pub struct TupleOpSignature { + /// Operator/function name this signature is registered under. + pub name: String, + /// Expected element `TypeId`s, in order, for the tuple-shaped operand. + pub shape: Vec, + /// Which peeked operand position must be the tuple. + pub tuple_operand_index: usize, + /// Flat `TypeId`s expected for the non-tuple operands, in stack order + /// (the `tuple_operand_index` entry is ignored). + pub operand_type_ids: Vec, + /// Function that pushes the operation onto the segment. + pub op_fn: OpFn, +} + /// A scope function that attempts to resolve and apply an operation. /// /// Receives the operation name, the segment, the number of operands on top of the stack, @@ -968,6 +989,7 @@ impl BuiltinScope { pub struct OpLookup { scopes: Vec, builtin_scope: BuiltinScope, + tuple_signatures: Vec, } impl OpLookup { @@ -984,7 +1006,53 @@ impl OpLookup { OpLookup { scopes: Vec::new(), builtin_scope: BuiltinScope, + tuple_signatures: Vec::new(), + } + } + + /// Registers a tuple-shaped operator signature, matched by element + /// `TypeId` sequence the same way built-in operators are matched by flat + /// `TypeId`. + pub fn register_tuple_op(&mut self, signature: TupleOpSignature) { + self.tuple_signatures.push(signature); + } + + /// Attempts to find and apply a registered tuple-shaped signature. + /// + /// Returns `Ok(true)` if found and applied, `Ok(false)` if not found. + /// + /// - Complexity: O(s) where s is the number of registered tuple signatures. + fn lookup_tuple_signature( + &self, + name: &str, + segment: &mut DynSegment, + num_operands: usize, + span: SourceSpan, + ) -> Result { + let stack_infos = segment.peek_stack_infos(num_operands); + for sig in &self.tuple_signatures { + if sig.name != name || sig.tuple_operand_index >= stack_infos.len() { + continue; + } + let tuple_info = &stack_infos[sig.tuple_operand_index]; + let shape_matches = tuple_info.associated.len() == sig.shape.len() + && tuple_info + .associated + .iter() + .zip(&sig.shape) + .all(|(a, t)| a.type_id == *t); + if !shape_matches { + continue; + } + let others_match = stack_infos.iter().enumerate().all(|(i, info)| { + i == sig.tuple_operand_index || info.type_id == sig.operand_type_ids[i] + }); + if others_match { + (sig.op_fn)(segment, span)?; + return Ok(true); + } } + Ok(false) } /// Pushes a new scope onto the stack. @@ -1051,6 +1119,18 @@ impl OpLookup { } } + match self.lookup_tuple_signature(name, segment, num_operands, source_span) { + Ok(true) => return Ok(()), + Ok(false) => {} + Err(e) => { + return Err(crate::ParseError::new_range( + format!("operation error: {}", e), + start, + end, + )); + } + } + match self .builtin_scope .lookup(name, segment, num_operands, source_span) @@ -1523,4 +1603,68 @@ mod tests { "expected caret marker in rendered output, got: {rendered}" ); } + + #[test] + fn tuple_shaped_signature_matches_and_dispatches() -> Result<()> { + let mut lookup = OpLookup::new(); + lookup.register_tuple_op(TupleOpSignature { + name: "greet".to_string(), + shape: vec![TypeId::of::(), TypeId::of::()], + tuple_operand_index: 0, + operand_type_ids: vec![], + op_fn: |seg, _span| { + seg.tuple_index(1); + seg.op1(|_ignored: i32| true) + }, + }); + + let mut segment = DynSegment::new::<()>(); + let ambient_start = segment.current_stack_offset(); + segment.op0(|| "hi".to_string()); + segment.op0(|| 7i32); + segment.make_tuple(2, ambient_start); + + lookup.lookup( + "greet", + &mut segment, + 1, + Span::call_site(), + Span::call_site(), + )?; + assert!(segment.call0::()?); + Ok(()) + } + + #[test] + fn tuple_shaped_signature_does_not_match_wrong_shape() { + let mut lookup = OpLookup::new(); + lookup.register_tuple_op(TupleOpSignature { + name: "greet".to_string(), + shape: vec![TypeId::of::(), TypeId::of::()], + tuple_operand_index: 0, + operand_type_ids: vec![], + op_fn: |seg, _span| { + seg.tuple_index(1); + seg.op1(|_ignored: i32| true) + }, + }); + + let mut segment = DynSegment::new::<()>(); + let ambient_start = segment.current_stack_offset(); + segment.op0(|| 1i32); + segment.op0(|| 2i32); + segment.make_tuple(2, ambient_start); + + let result = lookup.lookup( + "greet", + &mut segment, + 1, + Span::call_site(), + Span::call_site(), + ); + assert!( + result.is_err(), + "shape (i32, i32) should not match (String, i32)" + ); + } } From 578a40e19b43e61a2f7c0fde76e8fba24d4ac660 Mon Sep 17 00:00:00 2001 From: Sean Parent Date: Tue, 7 Jul 2026 20:30:24 -0700 Subject: [PATCH 18/29] fix(cel-parser): make TupleOpSignature operand matching bounds-safe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit others_match indexed operand_type_ids[i] directly, which panics if a registered TupleOpSignature's operand_type_ids is shorter than num_operands at a non-tuple position — reachable by any external registrant (TupleOpSignature/register_tuple_op are pub, unlike the private, macro-constructed OpSignature). Use .get(i) so a missing entry simply fails to match instead of crashing the parser. --- cel-parser/src/op_table.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/cel-parser/src/op_table.rs b/cel-parser/src/op_table.rs index e804a4a..8f3ec4d 100644 --- a/cel-parser/src/op_table.rs +++ b/cel-parser/src/op_table.rs @@ -62,7 +62,12 @@ pub type OpFn = fn(&mut DynSegment, SourceSpan) -> Result<()>; /// stack order [`DynSegment::peek_stack_infos`] returns) is a tuple whose /// element `TypeId`s equal `shape`, in order, and every other peeked operand's /// flat `TypeId` equals the corresponding entry in `operand_type_ids` (the -/// entry at `tuple_operand_index` in `operand_type_ids` is ignored). +/// entry at `tuple_operand_index` in `operand_type_ids` is never read — it's +/// safe to omit trailing entries, including the whole vector, when +/// `tuple_operand_index` is the only or highest operand position). +/// +/// A non-tuple operand position with no corresponding `operand_type_ids` +/// entry (i.e. out of bounds) simply never matches, rather than panicking. pub struct TupleOpSignature { /// Operator/function name this signature is registered under. pub name: String, @@ -1045,7 +1050,7 @@ impl OpLookup { continue; } let others_match = stack_infos.iter().enumerate().all(|(i, info)| { - i == sig.tuple_operand_index || info.type_id == sig.operand_type_ids[i] + i == sig.tuple_operand_index || sig.operand_type_ids.get(i) == Some(&info.type_id) }); if others_match { (sig.op_fn)(segment, span)?; From 5328554549671f79b0c5b6aacaf2a1e2046938c3 Mon Sep 17 00:00:00 2001 From: Sean Parent Date: Tue, 7 Jul 2026 20:35:57 -0700 Subject: [PATCH 19/29] feat(cel-runtime): add pop_tuple_as/push_tuple CStackList bridge --- cel-runtime/src/dyn_segment.rs | 115 +++++++++++++++++++++++++++++++++ 1 file changed, 115 insertions(+) diff --git a/cel-runtime/src/dyn_segment.rs b/cel-runtime/src/dyn_segment.rs index b9567d9..3c960ae 100644 --- a/cel-runtime/src/dyn_segment.rs +++ b/cel-runtime/src/dyn_segment.rs @@ -5,6 +5,7 @@ use crate::raw_segment::RawSegment; use crate::raw_stack::RawStack; use crate::{CStackListHeadLimit, CStackListHeadPadded, ReverseList}; use anyhow::Result; +use anyhow::anyhow; use anyhow::ensure; use std::any::{Any, TypeId}; use std::borrow::Cow; @@ -881,6 +882,89 @@ impl DynSegment { } unsafe { self.segment.call1(arg) } } + + /// Reinterprets the tuple on top of the stack as a concrete `L` + /// (typically a `CStackList<...>` chain), replacing its `StackInfo` with + /// `L`'s. No bytes move: both sides already use the same + /// natural-alignment, declaration-order layout, so this is a relabel, not + /// a copy. + /// + /// - Precondition: `L` was assembled via sequential `.push()` calls in + /// the same field order as the tuple (not via `into_c_stack_list()` on + /// a same-order plain tuple, which reverses element order). + /// + /// # Errors + /// Returns an error if the top of stack isn't a tuple, or its element + /// `TypeId`s (in order) don't match `L`'s. + pub fn pop_tuple_as(&mut self) -> Result<()> { + let info = self + .stack_ids + .last() + .ok_or_else(|| anyhow!("pop_tuple_as: stack is empty"))?; + ensure!( + info.type_id == TypeId::of::(), + "pop_tuple_as: top of stack is not a tuple" + ); + let expected: Vec = L::to_stack_info_list().iter().map(|s| s.type_id).collect(); + let actual: Vec = info.associated.iter().map(|a| a.type_id).collect(); + ensure!( + expected == actual, + "pop_tuple_as: tuple element types do not match `{}`", + std::any::type_name::() + ); + debug_assert_eq!(info.size, size_of::()); + debug_assert_eq!(info.align, align_of::()); + + let info = self.stack_ids.last_mut().expect("checked above"); + info.type_id = TypeId::of::(); + info.type_name = Cow::Borrowed(std::any::type_name::()); + info.raw_dropper = |ptr, _associated| unsafe { std::ptr::drop_in_place(ptr.cast::()) }; + info.associated = Vec::new(); + Ok(()) + } + + /// Relabels the concrete `L` value on top of the stack as a tuple, + /// exposing its elements for `.N` indexing and tuple-shaped op matching. + /// No bytes move — see [`pop_tuple_as`](Self::pop_tuple_as) for why this + /// is sound. + /// + /// - Precondition: the top of the stack currently holds a value of type + /// `L`, assembled via sequential `.push()` calls (not + /// `into_c_stack_list()` on a same-order plain tuple). + pub fn push_tuple(&mut self) { + let info = self + .stack_ids + .last_mut() + .expect("push_tuple requires a value on the stack"); + debug_assert_eq!( + info.type_id, + TypeId::of::(), + "push_tuple: top of stack is not the expected type" + ); + let element_infos = L::to_stack_info_list(); + let mut offset = 0usize; + let associated = element_infos + .iter() + .map(|elem_info| { + offset = align_index(elem_info.align, offset); + let a = AssociatedType { + type_id: elem_info.type_id, + type_name: elem_info.type_name.clone(), + offset, + size: elem_info.size, + align: elem_info.align, + dropper: elem_info.raw_dropper, + associated: elem_info.associated.clone(), + }; + offset += elem_info.size; + a + }) + .collect(); + info.type_id = TypeId::of::(); + info.type_name = Cow::Borrowed(std::any::type_name::()); + info.raw_dropper = drop_tuple; + info.associated = associated; + } } #[cfg(test)] @@ -1300,4 +1384,35 @@ mod tests { seg.tuple_index(0); assert_eq!(seg.call0::().unwrap(), 99); } + + #[test] + fn push_tuple_then_pop_tuple_as_round_trips() -> Result<(), anyhow::Error> { + let mut seg = DynSegment::new::<()>(); + // Build a concrete CStackList>> by pushing + // fields in declaration order (NOT via into_c_stack_list, which reverses + // order — see pop_tuple_as's doc comment). `CNil`'s inner field is + // private, so build the empty base via the public `IntoCStackList` + // conversion on `()` rather than the tuple-struct constructor. + seg.op0(|| CStackList(().into_c_stack_list(), 7u32).push("hi")); + seg.push_tuple::>>>(); + assert_eq!(seg.peek_tuple_arity(), Some(2)); + + seg.pop_tuple_as::>>>()?; + let result = seg.call0::>>>()?; + assert_eq!(result.head(), &"hi"); + assert_eq!(result.tail().head(), &7u32); + Ok(()) + } + + #[test] + fn pop_tuple_as_rejects_shape_mismatch() { + let mut seg = DynSegment::new::<()>(); + let ambient_start = seg.current_stack_offset(); + seg.op0(|| 1u32); + seg.op0(|| 2u32); + seg.make_tuple(2, ambient_start); + + let result = seg.pop_tuple_as::>>>(); + assert!(result.is_err(), "(u32, u32) should not match (u32, &str)"); + } } From 86f6f965bdd5be2a7546e15246675581803ada40 Mon Sep 17 00:00:00 2001 From: Sean Parent Date: Tue, 7 Jul 2026 21:33:56 -0700 Subject: [PATCH 20/29] fix(cel-runtime): match erased-tuple layout to CStackList's nested cons-cell layout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit make_tuple and push_tuple computed a tuple's per-element offsets using a flat #[repr(C)] struct formula, but CStackList is a *nested* cons cell where each level pads itself up to the max alignment of everything nested inside it so far before the next field is appended. For shapes where a higher-alignment element is followed by two or more lower-alignment elements (e.g. (u32, u8, u8): flat gives offsets [0,4,5]/size 8, CStackList's real layout gives [0,4,8]/size 12), the two formulas diverge, making pop_tuple_as/push_tuple relabel bytes at the wrong offsets — an out-of-bounds read/write only caught by a debug-only size assertion in pop_tuple_as, and not caught at all in push_tuple. Fix both offset-accumulation loops to additionally round the running offset up to the maximum alignment seen so far after placing each element, matching CStackList's real layout for every shape (not just non-decreasing-alignment ones). Leave RawStack::repack and the ambient (already-pushed) offset formula in make_tuple untouched, since neither depends on this convention. Adds round-trip tests through both bridge directions (push_tuple and pop_tuple_as) against a genuinely divergent (u32, u8, u8) shape, verified to fail under the old formula and pass under the fix. --- cel-runtime/src/dyn_segment.rs | 136 ++++++++++++++++++++++++++++++--- 1 file changed, 127 insertions(+), 9 deletions(-) diff --git a/cel-runtime/src/dyn_segment.rs b/cel-runtime/src/dyn_segment.rs index 3c960ae..466d22a 100644 --- a/cel-runtime/src/dyn_segment.rs +++ b/cel-runtime/src/dyn_segment.rs @@ -328,8 +328,13 @@ impl DynSegment { /// /// The tuple's internal layout (offsets between elements) depends only on /// the elements' own types — never on `ambient_start` — matching the - /// layout a `#[repr(C)]` struct with these fields, in this order, would - /// use. + /// layout that [`CStackList`]'s own nested `#[repr(C)]` cons cells + /// produce when built via sequential `.push()` calls in the same + /// declaration order: each element is placed at its own alignment, then + /// the running offset is padded up to the maximum alignment of every + /// element seen so far (not just the tuple's overall alignment) before + /// the next element is placed, mirroring how each nested cons cell pads + /// itself to its own alignment before the next field is appended. /// /// - Precondition: at least `n` values are on the stack, pushed /// contiguously starting at `ambient_start` with no other values @@ -342,30 +347,41 @@ impl DynSegment { let elems: Vec = self.stack_ids.drain(start..).collect(); let mut ambient_offset = ambient_start; - let mut ideal_offset = 0usize; + let mut offset = 0usize; let mut tuple_align = 1usize; let mut src_offsets = Vec::with_capacity(n); let mut associated = Vec::with_capacity(n); for elem in &elems { + // ambient_offset tracks where this element already sits on the + // ambient RawStack from ordinary sequential pushes — a plain + // flat layout, unrelated to CStackList's nested convention. ambient_offset = align_index(elem.align, ambient_offset); - ideal_offset = align_index(elem.align, ideal_offset); + src_offsets.push(ambient_offset); + ambient_offset += elem.size; + + // offset tracks the element's position in the tuple's canonical + // (CStackList-matching) layout: place at this element's own + // alignment, then pad up to the running max alignment so far. + offset = align_index(elem.align, offset); tuple_align = tuple_align.max(elem.align); - src_offsets.push(ambient_offset); associated.push(AssociatedType { type_id: elem.type_id, type_name: elem.type_name.clone(), - offset: ideal_offset, + offset, size: elem.size, align: elem.align, dropper: elem.raw_dropper, associated: elem.associated.clone(), }); - ambient_offset += elem.size; - ideal_offset += elem.size; + offset += elem.size; + offset = align_index(tuple_align, offset); } - let total_size = align_index(tuple_align, ideal_offset); + // The last iteration's rounding already used the full tuple_align + // (tuple_align has accumulated every element's alignment by then), + // so `offset` is already the tuple's correct total size. + let total_size = offset; let dest_base = align_index(tuple_align, ambient_start); let dest_offsets: Vec = associated.iter().map(|a| a.offset).collect(); @@ -943,10 +959,12 @@ impl DynSegment { ); let element_infos = L::to_stack_info_list(); let mut offset = 0usize; + let mut align_so_far = 1usize; let associated = element_infos .iter() .map(|elem_info| { offset = align_index(elem_info.align, offset); + align_so_far = align_so_far.max(elem_info.align); let a = AssociatedType { type_id: elem_info.type_id, type_name: elem_info.type_name.clone(), @@ -957,6 +975,7 @@ impl DynSegment { associated: elem_info.associated.clone(), }; offset += elem_info.size; + offset = align_index(align_so_far, offset); a }) .collect(); @@ -1415,4 +1434,103 @@ mod tests { let result = seg.pop_tuple_as::>>>(); assert!(result.is_err(), "(u32, u32) should not match (u32, &str)"); } + + /// `(u32, u8, u8)` shape used to distinguish the correct CStackList-nested + /// layout (offsets `[0, 4, 8]`, size 12) from the old flat `#[repr(C)]` + /// struct formula (offsets `[0, 4, 5]`, size 8): a higher-alignment + /// element (`u32`) is followed by two lower-alignment elements (`u8`, + /// `u8`), which is exactly the case the two formulas disagree on. + type DivergentAlignmentShape = CStackList>>>; + + fn build_divergent_alignment_shape() -> DivergentAlignmentShape { + // Declaration order (u32, u8, u8): elem0 = u32 is pushed first (innermost + // tail), elem2 = the second u8 is pushed last (outermost head). + CStackList( + CStackList(CStackList(().into_c_stack_list(), 0xAAu32), 0xBBu8), + 0xCCu8, + ) + } + + #[test] + fn make_tuple_then_index_last_element_of_divergent_alignment_shape() { + // Confirms element 2 reads back correctly via make_tuple's own + // internally consistent repack. This alone can't distinguish offset 8 + // from offset 5 (make_tuple's write and read paths both use the same + // formula) — see `push_tuple_round_trips_divergent_alignment_shape` + // below for the test that actually exercises the real CStackList + // layout and would fail under the old (flat) formula. + let mut seg = DynSegment::new::<()>(); + let ambient_start = seg.current_stack_offset(); + seg.op0(|| 0xAAu32); + seg.op0(|| 0xBBu8); + seg.op0(|| 0xCCu8); + seg.make_tuple(3, ambient_start); + assert_eq!(seg.peek_tuple_arity(), Some(3)); + seg.tuple_index(2); + assert_eq!(seg.call0::().unwrap(), 0xCCu8); + } + + #[test] + fn push_tuple_round_trips_divergent_alignment_shape() { + // Round-trips a real `CStackList>>>` value (memory layout produced by rustc itself, not by + // our offset formula) through `push_tuple`, then indexes each + // element. This test fails under the old flat-struct offset formula + // (which computes offset 5 for the last element, when the real + // struct places it at offset 8) and passes under the fix. + let sample = build_divergent_alignment_shape(); + // Confirm construction order really is (u32, u8, u8) in declaration + // order (outermost push is last / head — see pop_tuple_as's doc + // comment for this convention). + assert_eq!(*sample.head(), 0xCCu8); + assert_eq!(*sample.tail().head(), 0xBBu8); + assert_eq!(*sample.tail().tail().head(), 0xAAu32); + + let mut seg = DynSegment::new::<()>(); + seg.op0(build_divergent_alignment_shape); + seg.push_tuple::(); + assert_eq!(seg.peek_tuple_arity(), Some(3)); + + // tuple_index consumes the tuple, so index each element in its own + // segment (mirrors make_tuple_then_index_each_element's pattern). + let mut seg0 = DynSegment::new::<()>(); + seg0.op0(build_divergent_alignment_shape); + seg0.push_tuple::(); + seg0.tuple_index(0); + assert_eq!(seg0.call0::().unwrap(), 0xAAu32); + + let mut seg1 = DynSegment::new::<()>(); + seg1.op0(build_divergent_alignment_shape); + seg1.push_tuple::(); + seg1.tuple_index(1); + assert_eq!(seg1.call0::().unwrap(), 0xBBu8); + + seg.tuple_index(2); + assert_eq!(seg.call0::().unwrap(), 0xCCu8); + } + + #[test] + fn make_tuple_then_pop_tuple_as_round_trips_divergent_alignment_shape() + -> Result<(), anyhow::Error> { + // Closes the loop on the other bridge direction: build via + // make_tuple (which now computes the CStackList-matching layout), + // relabel with pop_tuple_as, and confirm the resulting concrete + // value's fields (read by rustc's own layout, via .head()/.tail()) + // agree with what was pushed for every element, including the + // divergent-alignment last element. + let mut seg = DynSegment::new::<()>(); + let ambient_start = seg.current_stack_offset(); + seg.op0(|| 0xAAu32); + seg.op0(|| 0xBBu8); + seg.op0(|| 0xCCu8); + seg.make_tuple(3, ambient_start); + assert_eq!(seg.peek_tuple_arity(), Some(3)); + + seg.pop_tuple_as::()?; + let result = seg.call0::()?; + assert_eq!(*result.tail().tail().head(), 0xAAu32); + assert_eq!(*result.tail().head(), 0xBBu8); + assert_eq!(*result.head(), 0xCCu8); + Ok(()) + } } From f37778533414329bd8b48d0e1f9c6a1ae338aae9 Mon Sep 17 00:00:00 2001 From: Sean Parent Date: Tue, 7 Jul 2026 21:34:04 -0700 Subject: [PATCH 21/29] fix(cel-parser): parse chained .N.M tuple indexing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Source text like .0.1 (intended as two chained tuple-index operations, e.g. (t).0.1) tokenizes as Punct('.') followed by a single Lit::Float token 0.1 — Rust's own lexer maximally munches the digits after the second '.'. is_postfix_expression only handled Token::Literal(Int(..)) after a '.', so it rejected any chained index with "expected integer after '.'", even though the grammar's repetition and the tuple design spec both intend .0.1 to work. Detect the float case and split its base10_digits() (e.g. "0.1") on the decimal point back into two integer indices, applying each in sequence via a new apply_tuple_index helper shared with the existing single-index path. A suffix on the whole float token (e.g. 0.1i32) is still rejected by the existing unsuffixed-integer rule. --- cel-parser/src/lib.rs | 94 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 81 insertions(+), 13 deletions(-) diff --git a/cel-parser/src/lib.rs b/cel-parser/src/lib.rs index 8ad4db0..aa8ab14 100644 --- a/cel-parser/src/lib.rs +++ b/cel-parser/src/lib.rs @@ -801,6 +801,13 @@ impl CELParser { } /// `postfix_expression = primary_expression { "(" parameter_list ")" | "." unsuffixed_integer }.` + /// + /// The repetition allows chained indices (`t.0.1`): each `"." unsuffixed_integer` + /// is applied in turn to whatever value the previous step left on top of the + /// stack. Source text like `.0.1` tokenizes as a single `.` followed by one + /// float literal `0.1` (Rust's own lexer maximally munches the digits after + /// the second `.`), so that case is detected and split back into its two + /// integer indices — see the `Token::Literal(CelLiteral::Float(..))` arm below. fn is_postfix_expression(&mut self) -> Result { let start_span = self.peek_span(); if !self.is_primary_expression()? { @@ -834,29 +841,43 @@ impl CELParser { self.last_span, )?; } else if self.is_punctuation(".") { - let index = match self.peek_token() { + match self.peek_token() { Some(Token::Literal(CelLiteral::Int(integer))) => { let integer = integer.clone(); if !integer.suffix().is_empty() { return Err(self.error_at("tuple index must be an unsuffixed integer")); } self.advance(); - integer.base10_parse::().map_err(|e| { + let index = integer.base10_parse::().map_err(|e| { self.error_at(&format!("invalid tuple index `{integer}`: {e}")) - })? + })?; + self.apply_tuple_index(index)?; + } + Some(Token::Literal(CelLiteral::Float(float))) => { + let float = float.clone(); + if !float.suffix().is_empty() { + return Err(self.error_at("tuple index must be an unsuffixed integer")); + } + self.advance(); + // base10_digits() returns the decimal digits with underscores + // stripped and the suffix removed, e.g. "0.1" or "10.25" — + // splitting on the (always-present) '.' recovers the two + // chained integer indices. + let digits = float.base10_digits(); + let (first, second) = digits + .split_once('.') + .expect("float literal digits always contain a decimal point"); + let first_index = first.parse::().map_err(|e| { + self.error_at(&format!("invalid tuple index `{first}`: {e}")) + })?; + let second_index = second.parse::().map_err(|e| { + self.error_at(&format!("invalid tuple index `{second}`: {e}")) + })?; + self.apply_tuple_index(first_index)?; + self.apply_tuple_index(second_index)?; } _ => return Err(self.error_at("expected integer after '.'")), - }; - let arity = self - .context - .peek_tuple_arity() - .ok_or_else(|| self.error_at("'.N' requires a tuple"))?; - if index >= arity { - return Err(self.error_at(&format!( - "tuple index `{index}` out of range for tuple of arity {arity}" - ))); } - self.context.tuple_index(index); } else { break; } @@ -864,6 +885,26 @@ impl CELParser { Ok(true) } + /// Applies a single `.N` tuple-index operation to the value currently on + /// top of the stack, replacing it with element `index`. + /// + /// # Errors + /// Returns an error if the top of stack isn't a tuple, or if `index` is + /// out of range for its arity. + fn apply_tuple_index(&mut self, index: usize) -> Result<()> { + let arity = self + .context + .peek_tuple_arity() + .ok_or_else(|| self.error_at("'.N' requires a tuple"))?; + if index >= arity { + return Err(self.error_at(&format!( + "tuple index `{index}` out of range for tuple of arity {arity}" + ))); + } + self.context.tuple_index(index); + Ok(()) + } + /// `parameter_list = [ or_expression { "," or_expression } ].` /// /// Returns the argument count. @@ -1293,6 +1334,33 @@ mod tests { assert!(result.is_err()); } + #[test] + fn chained_tuple_index_into_nested_tuple() { + // `.1` selects the inner tuple `(10i32, 20i32)`, then `.0` selects its + // first element. Source text `.1.0` tokenizes as a single float + // literal `1.0`, which must be split back into the chained indices + // `1` then `0` — using a shape/value where applying the indices to + // the wrong operand or in the wrong order gives a different (wrong) + // answer than 10 (e.g. swapping order would try `.0` on an i32 and + // fail to parse; picking element 0 first would return "a" or 10 + // depending on order, not confusably 10 either way, so pick values + // that make a mix-up obvious). + let mut parser = CELParser::new(OpLookup::new()); + let mut seg = parser + .parse_str(r#"("a", (10i32, 20i32)).1.0"#) + .expect("chained .1.0 index should parse"); + assert_eq!(seg.call0::().unwrap(), 10); + } + + #[test] + fn chained_tuple_index_suffixed_second_part_is_a_parse_error() { + // The suffix lands on the whole `0.1i32` float token; the existing + // unsuffixed-integer rule must still reject it. + let mut parser = CELParser::new(OpLookup::new()); + let result = parser.parse_str("(1i32, 2i32).0.1i32"); + assert!(result.is_err()); + } + #[test] fn complex_expression() { let mut parser = CELParser::new(OpLookup::new()); From 51f7b7a84fd3ef714ecf0295dc8a1ae2d48c1c54 Mon Sep 17 00:00:00 2001 From: Sean Parent Date: Tue, 7 Jul 2026 21:43:26 -0700 Subject: [PATCH 22/29] fix(cel-parser): don't panic on scientific-notation float after chained tuple index The chained .N.M indexing fix assumed a float token's base10_digits() always contains a '.', splitting on it via .expect(). Scientific notation (e.g. `1e2`) normalizes to digits with no '.' at all, so `t.1e2` panicked instead of returning a parse error -- a crash reachable from ordinary malformed script text, violating this crate's no-panics-on-malformed-input contract. Return a graceful ParseError instead, checked before consuming the token so the error span still points at it. --- cel-parser/src/lib.rs | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/cel-parser/src/lib.rs b/cel-parser/src/lib.rs index aa8ab14..4421985 100644 --- a/cel-parser/src/lib.rs +++ b/cel-parser/src/lib.rs @@ -858,15 +858,21 @@ impl CELParser { if !float.suffix().is_empty() { return Err(self.error_at("tuple index must be an unsuffixed integer")); } - self.advance(); // base10_digits() returns the decimal digits with underscores - // stripped and the suffix removed, e.g. "0.1" or "10.25" — - // splitting on the (always-present) '.' recovers the two - // chained integer indices. + // stripped and the suffix removed, e.g. "0.1" or "10.25" for + // ordinary decimal floats — splitting on '.' recovers the two + // chained integer indices. Scientific-notation floats (e.g. + // `1e2`) normalize to digits with no '.' at all; reject those + // as a parse error (checked before advancing, so the error + // span still points at the float token) rather than assuming + // a '.' is always present. let digits = float.base10_digits(); - let (first, second) = digits - .split_once('.') - .expect("float literal digits always contain a decimal point"); + let Some((first, second)) = digits.split_once('.') else { + return Err(self.error_at( + "tuple index chain must use decimal notation (e.g. `.0.1`)", + )); + }; + self.advance(); let first_index = first.parse::().map_err(|e| { self.error_at(&format!("invalid tuple index `{first}`: {e}")) })?; @@ -1361,6 +1367,16 @@ mod tests { assert!(result.is_err()); } + #[test] + fn chained_tuple_index_scientific_notation_is_a_parse_error_not_a_panic() { + // `1e2` normalizes to digits with no '.' at all (scientific notation), + // unlike ordinary decimal floats like `0.1` — must be a graceful parse + // error, not a panic on the assumption that '.' is always present. + let mut parser = CELParser::new(OpLookup::new()); + let result = parser.parse_str("(1i32, 2i32).1e2"); + assert!(result.is_err()); + } + #[test] fn complex_expression() { let mut parser = CELParser::new(OpLookup::new()); From 9f7e71f0ce3d4cae9db610c8393b8d1905338fb1 Mon Sep 17 00:00:00 2001 From: Sean Parent Date: Wed, 8 Jul 2026 13:09:15 -0700 Subject: [PATCH 23/29] feat: add playground feature to multiple Cargo.toml files and update VSCode settings --- .vscode/settings.json | 2 +- CLAUDE.md | 5 +++++ begin/Cargo.toml | 1 + cel-parser/Cargo.toml | 1 + cel-parser/src/lib.rs | 2 +- cel-rs-macros/Cargo.toml | 3 +++ pm-lang/Cargo.toml | 3 +++ property-model/Cargo.toml | 3 +++ xtask/Cargo.toml | 3 +++ 9 files changed, 21 insertions(+), 2 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 08eda63..372f6ab 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -10,5 +10,5 @@ "concatenative", "Peekable" ], - "rust-analyzer.cargo.features": "all" + "rust-analyzer.cargo.features": ["playground"] } \ No newline at end of file diff --git a/CLAUDE.md b/CLAUDE.md index 5a9bf06..409c323 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -58,6 +58,11 @@ Never commit directly to `main`. Before creating a PR, run the full check suite locally — every command in the Commands section above, including both clippy invocations (workspace and begin). +`cargo build --workspace` and `cargo test --workspace` must produce zero compiler +warnings — clippy's `-D warnings` does not catch everything a plain build/test compile +can warn about (e.g. an unused `mut`). Read the build/test output and fix any warnings +before opening the PR. + ## Project Status This project has not been released yet and has no clients. The API is not stable and may change at diff --git a/begin/Cargo.toml b/begin/Cargo.toml index 275c3c4..a4e7c19 100644 --- a/begin/Cargo.toml +++ b/begin/Cargo.toml @@ -8,6 +8,7 @@ description = "Cross-platform property model development environment" default = ["desktop"] desktop = ["dioxus/desktop"] web = ["dioxus/web"] +playground = [] [dependencies] dioxus = { version = "0.7.9", features = [] } diff --git a/cel-parser/Cargo.toml b/cel-parser/Cargo.toml index 23635ab..0b459c0 100644 --- a/cel-parser/Cargo.toml +++ b/cel-parser/Cargo.toml @@ -16,6 +16,7 @@ once_cell = "1.19" [features] default = ["span-diagnostics"] span-diagnostics = [] +playground = [] [lints] workspace = true diff --git a/cel-parser/src/lib.rs b/cel-parser/src/lib.rs index 4421985..d59c987 100644 --- a/cel-parser/src/lib.rs +++ b/cel-parser/src/lib.rs @@ -1273,7 +1273,7 @@ mod tests { #[test] fn two_element_tuple_no_trailing_comma() { let mut parser = CELParser::new(OpLookup::new()); - let mut seg = parser.parse_str(r#"("Hello", 42i32)"#).unwrap(); + let seg: DynSegment = parser.parse_str(r#"("Hello", 42i32)"#).unwrap(); assert_eq!(seg.peek_tuple_arity(), Some(2)); } diff --git a/cel-rs-macros/Cargo.toml b/cel-rs-macros/Cargo.toml index 3200854..b5dbc3e 100644 --- a/cel-rs-macros/Cargo.toml +++ b/cel-rs-macros/Cargo.toml @@ -16,5 +16,8 @@ cel-parser = { path = "../cel-parser" } [dev-dependencies] trybuild = "1.0" +[features] +playground = [] + [lints] workspace = true diff --git a/pm-lang/Cargo.toml b/pm-lang/Cargo.toml index c1af4d0..d19900c 100644 --- a/pm-lang/Cargo.toml +++ b/pm-lang/Cargo.toml @@ -13,5 +13,8 @@ syn = { version = "2.0", features = ["extra-traits", "parsing"] } anyhow = "1.0" indexmap = "2" +[features] +playground = [] + [lints] workspace = true diff --git a/property-model/Cargo.toml b/property-model/Cargo.toml index 9d15547..3e39486 100644 --- a/property-model/Cargo.toml +++ b/property-model/Cargo.toml @@ -8,5 +8,8 @@ description = "Runtime library for property model constraint graphs" slotmap = "1.0" anyhow = "1.0" +[features] +playground = [] + [lints] workspace = true diff --git a/xtask/Cargo.toml b/xtask/Cargo.toml index bad7139..2b89ae0 100644 --- a/xtask/Cargo.toml +++ b/xtask/Cargo.toml @@ -12,3 +12,6 @@ path = "src/main.rs" ureq = "3" toml = "1" serde = { version = "1", features = ["derive"] } + +[features] +playground = [] From 349ac1344ce4a57a666d0582d8b443908c7a82ea Mon Sep 17 00:00:00 2001 From: Sean Parent Date: Wed, 8 Jul 2026 14:02:55 -0700 Subject: [PATCH 24/29] Refactor RawStack to use MaybeUninit for raw byte operations - Updated the `push_raw` and `copy_from` methods to accept and operate on `MaybeUninit` instead of `u8` to handle potential uninitialized bytes safely. - Adjusted the corresponding tests to ensure compatibility with the new type. - Improved safety by ensuring that raw byte operations do not inadvertently read uninitialized memory. --- cel-parser/src/lib.rs | 65 ++ cel-parser/src/op_table.rs | 46 +- cel-runtime/src/dyn_segment.rs | 66 +- cel-runtime/src/raw_stack.rs | 1105 ++++++++++++++++---------------- 4 files changed, 715 insertions(+), 567 deletions(-) diff --git a/cel-parser/src/lib.rs b/cel-parser/src/lib.rs index d59c987..bf26f59 100644 --- a/cel-parser/src/lib.rs +++ b/cel-parser/src/lib.rs @@ -1298,6 +1298,71 @@ mod tests { assert_eq!(seg.call0::().unwrap(), 10); } + #[test] + fn tuple_element_can_be_arithmetic_expression() { + let mut parser = CELParser::new(OpLookup::new()); + let mut seg = parser.parse_str("(1i32 + 2i32, 3i32).0").unwrap(); + assert_eq!(seg.call0::().unwrap(), 3); + } + + #[test] + fn tuple_second_element_can_be_arithmetic_expression() { + let mut parser = CELParser::new(OpLookup::new()); + let mut seg = parser.parse_str("(1i32 + 2i32, 3i32).1").unwrap(); + assert_eq!(seg.call0::().unwrap(), 3); + } + + #[test] + fn tuple_ambient_start_correct_after_sibling_expression() { + // Regression test: a fully-evaluated sibling subexpression earlier in + // the same additive chain must not shift where the following tuple + // literal thinks its elements land on the real stack. + let mut parser = CELParser::new(OpLookup::new()); + let mut seg = parser + .parse_str("(1i32 + 2i32) + 3i32 + (4i32, 5i32).0") + .unwrap(); + assert_eq!(seg.call0::().unwrap(), 10); + } + + #[test] + fn tuple_index_inside_if_then_branch() { + // Regression test: `join2` pops the condition bool before running + // the chosen fragment, so a tuple literal inside that fragment must + // compute its layout as if that pop already happened. + let mut parser = CELParser::new(OpLookup::new()); + let mut seg = parser + .parse_str("if true { (10i32, 20i32).1 } else { 0i32 }") + .unwrap(); + assert_eq!(seg.call0::().unwrap(), 20); + } + + #[test] + fn tuple_index_inside_if_else_branch() { + let mut parser = CELParser::new(OpLookup::new()); + let mut seg = parser + .parse_str("if false { 0i32 } else { (10i32, 20i32).1 }") + .unwrap(); + assert_eq!(seg.call0::().unwrap(), 20); + } + + #[test] + fn tuple_index_inside_and_rhs() { + let mut parser = CELParser::new(OpLookup::new()); + let mut seg = parser + .parse_str("(1i32, 2i32).1 == 2i32 && (10i32, 20i32).1 == 20i32") + .unwrap(); + assert!(seg.call0::().unwrap()); + } + + #[test] + fn tuple_index_inside_or_rhs() { + let mut parser = CELParser::new(OpLookup::new()); + let mut seg = parser + .parse_str("(1i32, 2i32).1 == 99i32 || (10i32, 20i32).1 == 20i32") + .unwrap(); + assert!(seg.call0::().unwrap()); + } + #[test] fn index_second_element_of_tuple() { let mut parser = CELParser::new(OpLookup::new()); diff --git a/cel-parser/src/op_table.rs b/cel-parser/src/op_table.rs index 8f3ec4d..f67653a 100644 --- a/cel-parser/src/op_table.rs +++ b/cel-parser/src/op_table.rs @@ -27,7 +27,7 @@ //! or masking the shift count (release). use anyhow::{Result, anyhow}; -use cel_runtime::DynSegment; +use cel_runtime::{DynSegment, DynTuple}; use once_cell::sync::Lazy; use phf::phf_map; use std::any::TypeId; @@ -62,12 +62,13 @@ pub type OpFn = fn(&mut DynSegment, SourceSpan) -> Result<()>; /// stack order [`DynSegment::peek_stack_infos`] returns) is a tuple whose /// element `TypeId`s equal `shape`, in order, and every other peeked operand's /// flat `TypeId` equals the corresponding entry in `operand_type_ids` (the -/// entry at `tuple_operand_index` in `operand_type_ids` is never read — it's -/// safe to omit trailing entries, including the whole vector, when -/// `tuple_operand_index` is the only or highest operand position). +/// entry at `tuple_operand_index` in `operand_type_ids` is never read). /// -/// A non-tuple operand position with no corresponding `operand_type_ids` -/// entry (i.e. out of bounds) simply never matches, rather than panicking. +/// `operand_type_ids` must have an entry for every non-tuple operand +/// position: a missing entry (out of bounds, including an entirely empty +/// vector when there are non-tuple operands) simply never matches, rather +/// than panicking — it is only safe to omit the whole vector when +/// `tuple_operand_index` is the *only* operand position. pub struct TupleOpSignature { /// Operator/function name this signature is registered under. pub name: String, @@ -1040,7 +1041,8 @@ impl OpLookup { continue; } let tuple_info = &stack_infos[sig.tuple_operand_index]; - let shape_matches = tuple_info.associated.len() == sig.shape.len() + let shape_matches = tuple_info.type_id == TypeId::of::() + && tuple_info.associated.len() == sig.shape.len() && tuple_info .associated .iter() @@ -1672,4 +1674,34 @@ mod tests { "shape (i32, i32) should not match (String, i32)" ); } + + #[test] + fn tuple_shaped_signature_with_empty_shape_does_not_match_non_tuple() { + // Regression test: a 0-element `shape` must only match an actual + // 0-arity tuple, not any non-tuple operand (which also reports an + // empty `associated` list). + let mut lookup = OpLookup::new(); + lookup.register_tuple_op(TupleOpSignature { + name: "unit_greet".to_string(), + shape: vec![], + tuple_operand_index: 0, + operand_type_ids: vec![], + op_fn: |seg, _span| seg.op1(|_ignored: i32| true), + }); + + let mut segment = DynSegment::new::<()>(); + segment.op0(|| 42i32); + + let result = lookup.lookup( + "unit_greet", + &mut segment, + 1, + Span::call_site(), + Span::call_site(), + ); + assert!( + result.is_err(), + "empty-shape tuple signature must not match a non-tuple operand" + ); + } } diff --git a/cel-runtime/src/dyn_segment.rs b/cel-runtime/src/dyn_segment.rs index 466d22a..0e81205 100644 --- a/cel-runtime/src/dyn_segment.rs +++ b/cel-runtime/src/dyn_segment.rs @@ -11,6 +11,7 @@ use std::any::{Any, TypeId}; use std::borrow::Cow; use std::cell::Cell; use std::cmp::max; +use std::mem::MaybeUninit; thread_local! { // Safety: valid only during the execution of `call_dyn` on this thread. @@ -107,7 +108,12 @@ unsafe fn extract_tuple_element( let target = &associated[index]; debug_assert!(tuple_base.is_multiple_of(target.align)); - let mut scratch = vec![0u8; target.size]; + // MaybeUninit, not u8: `target`'s bytes may include its own interior + // padding, which is itself uninitialized — reading it into a `Vec` + // (whose elements must always be valid, initialized `u8`s) would be + // undefined behavior even though these bytes are never inspected, only + // moved. + let mut scratch: Vec> = vec![MaybeUninit::uninit(); target.size]; unsafe { stack.copy_from( tuple_base + target.offset, @@ -228,7 +234,13 @@ pub struct DynSegment { /// Type names for each argument slot, for error reporting (parallel to `argument_ids`). pub(crate) argument_names: Vec>, pub(crate) stack_ids: Vec, - stack_index: usize, + /// Fixed byte offset `stack_ids[0]` is laid out relative to; established + /// once at construction (post-argument space for a full segment, or the + /// as-if-already-popped ambient offset for a fragment — see + /// [`new_fragment`](Self::new_fragment)). The current top-of-stack offset + /// is always recomputed from this plus `stack_ids`, never cached, so it + /// can never drift out of sync after ops consume stack entries. + base_stack_index: usize, } impl DynSegment { @@ -244,20 +256,32 @@ impl DynSegment { argument_ids: stack_ids.iter().map(|s| s.type_id).collect(), argument_names: stack_ids.iter().map(|s| s.type_name.clone()).collect(), stack_ids, - stack_index: size_of::>(), + base_stack_index: size_of::>(), } } /// Create a DynSegment that is a fragment of a larger segment, it may /// be used to implement conditional execution. + /// + /// - Precondition: the top of the stack currently holds the condition + /// value that [`join2`](Self::join2) will pop before this fragment's + /// ops run — the fragment's own local offsets are computed as if that + /// pop had already happened, matching the layout the fragment will + /// actually see at execution time. (`join2` independently rejects a + /// non-`bool` condition with an `Err`, so a mismatched condition type + /// at this point is a pending parse error, not a violated invariant.) #[must_use] pub fn new_fragment(&self) -> Self { + debug_assert!( + !self.stack_ids.is_empty(), + "new_fragment requires a condition value on top of the stack" + ); DynSegment { segment: RawSegment::new(), argument_ids: Vec::new(), argument_names: Vec::new(), stack_ids: Vec::new(), - stack_index: self.stack_index, + base_stack_index: self.stack_offset_after(self.stack_ids.len().saturating_sub(1)), } } @@ -285,13 +309,33 @@ impl DynSegment { Ok(()) } + /// Computes the top-of-stack byte offset after the first `count` entries + /// of `stack_ids`, replaying each entry's own alignment/size from + /// `base_stack_index`. + /// + /// Recomputing on demand (rather than caching a running total) keeps this + /// correct after any operation that removes entries from `stack_ids` + /// (e.g. [`pop_types`](Self::pop_types)), since there is no cached value + /// that could fall out of sync with the actual entries left on the stack. + /// + /// - Complexity: O(count). + fn stack_offset_after(&self, count: usize) -> usize { + let mut offset = self.base_stack_index; + for info in &self.stack_ids[..count] { + offset = align_index(info.align, offset); + offset += info.size; + } + offset + } + /// Push type to stack and register dropper. fn push_type(&mut self) where T: 'static, { - let aligned_index = align_index(align_of::(), self.stack_index); - let padded = aligned_index != self.stack_index; + let current = self.stack_offset_after(self.stack_ids.len()); + let aligned_index = align_index(align_of::(), current); + let padded = aligned_index != current; self.stack_ids.push(StackInfo { type_id: TypeId::of::(), @@ -302,7 +346,6 @@ impl DynSegment { raw_dropper: |ptr, _associated| unsafe { std::ptr::drop_in_place(ptr.cast::()) }, associated: Vec::new(), }); - self.stack_index = aligned_index + size_of::(); } /// Returns the current parse-time stack byte offset. @@ -311,7 +354,7 @@ impl DynSegment { /// [`make_tuple`](Self::make_tuple). #[must_use] pub fn current_stack_offset(&self) -> usize { - self.stack_index + self.stack_offset_after(self.stack_ids.len()) } /// Returns the arity of the tuple on top of the stack, or `None` if the @@ -410,7 +453,6 @@ impl DynSegment { raw_dropper: drop_tuple, associated, }); - self.stack_index = dest_base + total_size; } /// Extracts element `index` from the tuple on top of the stack, replacing @@ -432,12 +474,9 @@ impl DynSegment { ); debug_assert!(index < info.associated.len(), "tuple_index out of range"); - // tuple_start is the tuple's own aligned base (dest_base from - // make_tuple), already a multiple of every element's alignment. The - // extracted element inherits the tuple's own leading-padding + // The extracted element inherits the tuple's own leading-padding // relationship unchanged — see extract_tuple_element's doc comment // for why no realignment is needed or performed. - let tuple_start = self.stack_index - info.size; let target = info.associated[index].clone(); let associated = info.associated.clone(); let tuple_padding = info.padding; @@ -459,7 +498,6 @@ impl DynSegment { raw_dropper: target.dropper, associated: target.associated, }); - self.stack_index = tuple_start + target.size; } /// Returns the padding flags for the top N entries of the type stack. diff --git a/cel-runtime/src/raw_stack.rs b/cel-runtime/src/raw_stack.rs index a1cc432..0ceb4e1 100644 --- a/cel-runtime/src/raw_stack.rs +++ b/cel-runtime/src/raw_stack.rs @@ -1,546 +1,559 @@ -use crate::memory::align_index; -use crate::raw_vec::RawVec; -use std::mem::MaybeUninit; -use std::mem::size_of; - -/// A simple raw stack that stores values as raw bytes. Each value is naturally aligned given the -/// base alignment of the stack, which is the maximum alignment of any value stored in the stack. -#[derive(Debug)] -pub struct RawStack { - buffer: RawVec, -} - -impl RawStack { - /// Creates a new `RawStack` with base alignment. - /// - /// # Examples - /// - /// ``` - /// use cel_runtime::raw_stack::RawStack; - /// let stack = RawStack::with_base_alignment(align_of::()); - /// ``` - #[must_use] - pub fn with_base_alignment(base_alignment: usize) -> Self { - RawStack { - buffer: RawVec::with_base_alignment(base_alignment), - } - } - - /// Returns the number of bytes currently on the stack. - #[must_use] - #[allow(clippy::len_without_is_empty)] - pub fn len(&self) -> usize { - self.buffer.len() - } - - /// Pushes a value of type `T` onto the stack. - /// - /// The value is stored as raw bytes in the internal buffer. The pushed value must be - /// later popped using the correct type. - /// - /// # Type Parameters - /// - /// * `T`: The type of the value to push. - /// - /// # Examples - /// - /// ``` - /// use cel_runtime::raw_stack::RawStack; - /// let mut stack = RawStack::with_base_alignment(align_of::()); - /// let _ = stack.push(42u32); - /// ``` - /// - /// # Complexity - /// - /// The function has an amortized O(1) time complexity. - pub fn push(&mut self, value: T) -> bool { - let len = self.buffer.len(); - let aligned_index = align_index(align_of::(), len); - let new_len = aligned_index + size_of::(); - - self.buffer.reserve(new_len - len); - unsafe { - self.buffer.set_len(new_len); - if aligned_index - len > 0 { - // write a 1 in the first padding byte and 0 in the rest - self.buffer[len].write(1); - self.buffer[len + 1..aligned_index].fill(MaybeUninit::new(0)); - } - - std::ptr::write( - self.buffer.as_mut_ptr().add(aligned_index).cast::(), - value, - ); - } - aligned_index - len > 0 - } - - /// Pushes `size` raw bytes from `src`, aligned to `align`, using the same - /// padding/marker-byte bookkeeping as [`push`](Self::push). - /// - /// - Precondition: `align` is a power of two. - /// - /// # Safety - /// `src` must be valid for reads of `size` bytes, and must not overlap the - /// stack's internal buffer. - pub unsafe fn push_raw(&mut self, align: usize, size: usize, src: *const u8) -> bool { - debug_assert!(align.is_power_of_two()); - let len = self.buffer.len(); - let aligned_index = align_index(align, len); - let new_len = aligned_index + size; - - self.buffer.reserve(new_len - len); - unsafe { - self.buffer.set_len(new_len); - if aligned_index - len > 0 { - self.buffer[len].write(1); - self.buffer[len + 1..aligned_index].fill(MaybeUninit::new(0)); - } - std::ptr::copy_nonoverlapping( - src, - self.buffer.as_mut_ptr().add(aligned_index).cast::(), - size, - ); - } - aligned_index - len > 0 - } - - /// Copies `size` bytes starting at absolute buffer offset `offset` into `dst`. - /// - /// # Safety - /// `offset..offset + size` must be within the currently-initialized buffer; - /// `dst` must be valid for writes of `size` bytes and must not overlap the - /// stack's internal buffer. - pub unsafe fn copy_from(&self, offset: usize, size: usize, dst: *mut u8) { - debug_assert!(offset + size <= self.buffer.len()); - unsafe { - std::ptr::copy_nonoverlapping(self.buffer.as_ptr().add(offset).cast::(), dst, size); - } - } - - /// Drops a value in place at absolute buffer offset `offset`, without - /// altering the stack's tracked length. - /// - /// # Safety - /// `offset` must point to a live, valid value; `run_drop` must correctly - /// run that value's destructor given a pointer to its start. - pub unsafe fn drop_at(&mut self, offset: usize, run_drop: impl FnOnce(*mut u8)) { - unsafe { run_drop(self.buffer.as_mut_ptr().add(offset).cast::()) }; - } - - /// Truncates the stack back to `new_len`, additionally stripping `padding` - /// bytes that preceded the removed region (scanned the same way - /// [`pop`](Self::pop) does). - /// - /// # Safety - /// No live (undropped) value may exist at or above `new_len`. - pub unsafe fn truncate_to(&mut self, new_len: usize, padding: bool) { - debug_assert!(new_len <= self.buffer.len()); - let padding_count = if padding { - self.buffer[..new_len] - .iter() - .rev() - .take_while(|&x| unsafe { x.assume_init() == 0 }) - .count() - + 1 - } else { - 0 - }; - self.buffer.truncate(new_len - padding_count); - } - - /// Drops a value of `size` bytes at the top of the stack in place, then - /// removes it (and any padding that preceded it). - /// - /// # Safety - /// The top `size` bytes (plus padding if `padding` is true) must be a - /// live, valid value; `run_drop` must correctly run its destructor given a - /// pointer to its start. - pub unsafe fn drop_sized( - &mut self, - size: usize, - padding: bool, - run_drop: impl FnOnce(*mut u8), - ) { - debug_assert!(size <= self.buffer.len()); - let p = self.buffer.len() - size; - unsafe { - self.drop_at(p, run_drop); - self.truncate_to(p, padding); - } - } - - /// Repacks `sizes.len()` already-pushed values (currently at the absolute - /// byte offsets in `src_offsets`) into one contiguous, self-contained - /// region of `total_size` bytes starting at `dest_base`, placing element - /// `i` at `dest_base + dest_offsets[i]`. Adjusts the tracked length to - /// `dest_base + total_size` and returns whether leading padding was - /// inserted between `ambient_start` and `dest_base`. - /// - /// - Precondition: `src_offsets`, `dest_offsets`, and `sizes` have equal - /// length; `dest_base >= ambient_start`; the source ranges are - /// currently valid, initialized bytes. - /// - /// - Complexity: O(n) in `sizes.len()`. - /// - /// # Safety - /// The offsets and sizes must correctly describe the actual bytes in the - /// buffer; no two destination ranges may overlap. `src_offsets` and - /// `dest_offsets` must be in the same relative element order (element `i`'s - /// source and destination must both be the `i`-th non-overlapping range in - /// their respective layouts) — this method does not reorder elements, only - /// re-pads between them. Given that and `dest_base >= ambient_start`, each - /// element's destination start is guaranteed to be at or after every - /// earlier element's source end, which is what makes processing in reverse - /// index order below safe against clobbering not-yet-read source bytes. - pub unsafe fn repack( - &mut self, - ambient_start: usize, - dest_base: usize, - total_size: usize, - src_offsets: &[usize], - dest_offsets: &[usize], - sizes: &[usize], - ) -> bool { - debug_assert!(dest_base >= ambient_start); - debug_assert_eq!(src_offsets.len(), sizes.len()); - debug_assert_eq!(dest_offsets.len(), sizes.len()); - - let target_len = dest_base + total_size; - let current_len = self.buffer.len(); - let grown_len = current_len.max(target_len); - unsafe { - if grown_len > current_len { - self.buffer.reserve(grown_len - current_len); - self.buffer.set_len(grown_len); - } - let base_ptr = self.buffer.as_mut_ptr().cast::(); - // Process highest index first: each element's destination is - // provably at or after every earlier element's source end (see - // `# Safety` above), so this order never overwrites source bytes - // an earlier iteration still needs to read. - for i in (0..sizes.len()).rev() { - std::ptr::copy( - base_ptr.add(src_offsets[i]), - base_ptr.add(dest_base + dest_offsets[i]), - sizes[i], - ); - } - if dest_base > ambient_start { - self.buffer[ambient_start].write(1); - self.buffer[ambient_start + 1..dest_base].fill(MaybeUninit::new(0)); - } - self.buffer.set_len(target_len); - } - dest_base > ambient_start - } - - /// Pops a value of type `T` from the stack. Does not change the stack capacity. - /// - /// # Safety - /// - /// The type `T` must be the same type as the value on the top of the stack. - /// Incorrect usage can lead to undefined behavior. - /// - /// # Type Parameters - /// - /// * `T`: The type of the value to pop. - /// - /// # Examples - /// - /// ``` - /// use cel_runtime::raw_stack::RawStack; - /// let mut stack = RawStack::with_base_alignment(align_of::()); - /// let padding = stack.push(100u32); - /// let value: u32 = unsafe { stack.pop(padding) }; - /// ``` - pub unsafe fn pop(&mut self, padding: bool) -> T { - let p: usize = self.buffer.len() - size_of::(); - let result = unsafe { std::ptr::read(self.buffer.as_ptr().add(p).cast::()) }; - // count the number of trailing 0s in the buffer before the result - let padding_count = if padding { - self.buffer[..p] - .iter() - .rev() - .take_while(|&x| unsafe { x.assume_init() == 0 }) - .count() - + 1 - } else { - 0 - }; - self.buffer.truncate(p - padding_count); - result - } - - /// Pops a value of type `T` from the stack and drops it. - /// - /// # Safety - /// - /// The type `T` must be the same type as the value on the top of the stack. - /// Incorrect usage can lead to undefined behavior. - /// - /// # Note - /// - /// This cannot use `drop_in_place` because the type may not be aligned. - pub unsafe fn drop(&mut self, padding: bool) { - unsafe { self.pop::(padding) }; - } -} - -/* Test module */ -#[cfg(test)] -mod tests { - use super::*; - use std::cmp::max; - - #[test] - fn push_pop_u32() { - let mut stack = RawStack::with_base_alignment(align_of::()); - let padding = stack.push(10u32); - let result: u32 = unsafe { stack.pop(padding) }; - assert_eq!(result, 10); - } - - #[test] - fn multiple_push_pop() { - let mut stack = RawStack::with_base_alignment(align_of::()); - let padding1 = stack.push(1u32); - let padding2 = stack.push(2u32); - let padding3 = stack.push(3u32); - let v3: u32 = unsafe { stack.pop(padding3) }; - let v2: u32 = unsafe { stack.pop(padding2) }; - let v1: u32 = unsafe { stack.pop(padding1) }; - assert_eq!(v1, 1); - assert_eq!(v2, 2); - assert_eq!(v3, 3); - } - - #[test] - fn push_pop_different_types() { - let mut stack = RawStack::with_base_alignment(max(align_of::(), align_of::())); - let padding1 = stack.push(42u32); - let padding2 = stack.push(3.14f64); - let value_f: f64 = unsafe { stack.pop(padding2) }; - let value_u: u32 = unsafe { stack.pop(padding1) }; - assert_eq!(value_f, 3.14); - assert_eq!(value_u, 42); - } - - #[test] - fn len_reflects_pushed_bytes() { - let mut stack = RawStack::with_base_alignment(align_of::()); - assert_eq!(stack.len(), 0); - let _ = stack.push(7u32); - assert_eq!(stack.len(), size_of::()); - } - - #[test] - fn push_raw_round_trips_like_push() { - let mut stack = RawStack::with_base_alignment(align_of::()); - let padding1 = stack.push(1u8); - let value = 3.14f64; - let padding2 = unsafe { - stack.push_raw( - align_of::(), - size_of::(), - (&value as *const f64).cast::(), - ) - }; - let popped: f64 = unsafe { stack.pop(padding2) }; - assert_eq!(popped, 3.14); - let popped_u8: u8 = unsafe { stack.pop(padding1) }; - assert_eq!(popped_u8, 1); - } - - #[test] - fn push_raw_padding_matches_typed_push() { - let mut stack_a = RawStack::with_base_alignment(align_of::()); - let _ = stack_a.push(1u8); - let padding_typed = stack_a.push(2.5f64); - - let mut stack_b = RawStack::with_base_alignment(align_of::()); - let _ = stack_b.push(1u8); - let value = 2.5f64; - let padding_raw = unsafe { - stack_b.push_raw( - align_of::(), - size_of::(), - (&value as *const f64).cast::(), - ) - }; - assert_eq!(padding_typed, padding_raw); - } - - #[test] - fn copy_from_reads_bytes_at_offset() { - let mut stack = RawStack::with_base_alignment(align_of::()); - let _ = stack.push(10u32); - let _ = stack.push(20u32); - let mut buf = [0u8; 4]; - unsafe { stack.copy_from(0, 4, buf.as_mut_ptr()) }; - assert_eq!(u32::from_ne_bytes(buf), 10); - } - - #[test] - fn drop_at_runs_destructor_without_changing_length() { - use std::sync::Arc; - use std::sync::atomic::{AtomicUsize, Ordering}; - - struct DropCounter(Arc); - impl Drop for DropCounter { - fn drop(&mut self) { - self.0.fetch_add(1, Ordering::SeqCst); - } - } - - let count = Arc::new(AtomicUsize::new(0)); - let mut stack = RawStack::with_base_alignment(align_of::()); - let _ = stack.push(DropCounter(count.clone())); - let len_before = stack.len(); - unsafe { - stack.drop_at(0, |ptr| std::ptr::drop_in_place(ptr.cast::())); - } - assert_eq!(count.load(Ordering::SeqCst), 1); - assert_eq!(stack.len(), len_before); - } - - #[test] - fn truncate_to_strips_recorded_padding() { - let mut stack = RawStack::with_base_alignment(align_of::()); - let _ = stack.push(1u8); - let padding = stack.push(2.5f64); // padding == true: 7 bytes inserted before the f64 - let len_with_value = stack.len(); - unsafe { stack.truncate_to(len_with_value - size_of::(), padding) }; - assert_eq!(stack.len(), 1); // back to just the u8 - } - - #[test] - fn drop_sized_combines_drop_at_and_truncate_to() { - use std::sync::Arc; - use std::sync::atomic::{AtomicUsize, Ordering}; - - struct DropCounter(Arc); - impl Drop for DropCounter { - fn drop(&mut self) { - self.0.fetch_add(1, Ordering::SeqCst); - } - } - - let count = Arc::new(AtomicUsize::new(0)); - let mut stack = RawStack::with_base_alignment(align_of::()); - let _ = stack.push(1u8); - let padding = stack.push(DropCounter(count.clone())); - unsafe { - stack.drop_sized(size_of::(), padding, |ptr| { - std::ptr::drop_in_place(ptr.cast::()) - }); - } - assert_eq!(count.load(Ordering::SeqCst), 1); - assert_eq!(stack.len(), 1); - } - - #[test] - fn repack_moves_elements_to_ideal_offsets_and_reports_padding() { - // Ambient layout: [u8 @0][pad][u32 @4][u8 @8] — u8 then u32 then u8, each - // pushed with ordinary alignment relative to a 1-byte ambient start. - let mut stack = RawStack::with_base_alignment(align_of::()); - let ambient_start = stack.len(); // 0 - let _ = stack.push(0xAAu8); // element 0: ambient offset 0 - let _p1 = stack.push(0xBBBB_BBBBu32); // element 1: ambient offset 4 (1 byte padded to 4) - let _p2 = stack.push(0xCCu8); // element 2: ambient offset 8 - - // Ideal (self-contained) layout for (u8, u32, u8) from a zero base: - // offset 0 (u8), offset 4 (u32, aligned up from 1), offset 8 (u8) -> total 9, - // rounded to the tuple's own max align (4) -> total_size 12. - let src_offsets = [0usize, 4, 8]; - let dest_offsets = [0usize, 4, 8]; - let sizes = [1usize, 4, 1]; - let total_size = 12usize; - let dest_base = 0usize; // ambient_start (0) is already 4-aligned - - let padding = unsafe { - stack.repack( - ambient_start, - dest_base, - total_size, - &src_offsets, - &dest_offsets, - &sizes, - ) - }; - assert!( - !padding, - "ambient_start was already aligned; no leading pad expected" - ); - assert_eq!(stack.len(), dest_base + total_size); - - let mut a = [0u8; 1]; - let mut b = [0u8; 4]; - let mut c = [0u8; 1]; - unsafe { - stack.copy_from(dest_base, 1, a.as_mut_ptr()); - stack.copy_from(dest_base + 4, 4, b.as_mut_ptr()); - stack.copy_from(dest_base + 8, 1, c.as_mut_ptr()); - } - assert_eq!(a[0], 0xAA); - assert_eq!(u32::from_ne_bytes(b), 0xBBBB_BBBB); - assert_eq!(c[0], 0xCC); - } - - #[test] - fn repack_shifts_right_without_corrupting_unread_source_bytes() { - // A misaligned ambient_start forces dest_base > ambient_start, which - // means the destination of the *first* tuple element can land inside - // the *source* range of a *later*, not-yet-copied element. Processing - // elements low-index-first would silently corrupt that later element's - // bytes before they're read; this test fails under that ordering and - // passes under the correct (reverse) ordering. - let mut stack = RawStack::with_base_alignment(align_of::()); - let _ = stack.push(0xFFu8); // sentinel, ambient offset 0, before the tuple - let ambient_start = stack.len(); // 1: misaligned relative to u32's 4-byte align - let _ = stack.push(0xDDu8); // element 0: ambient offset 1 - let _ = stack.push(0x1122_3344u32); // element 1: ambient offset 4 (3 bytes padded) - - // Ideal (u8, u32) layout from zero: offset 0 (u8), offset 4 (u32) -> total 8. - let src_offsets = [1usize, 4]; - let dest_offsets = [0usize, 4]; - let sizes = [1usize, 4]; - let total_size = 8usize; - let dest_base = 4usize; // align_index(4, ambient_start=1) == 4, so this shifts right - - let padding = unsafe { - stack.repack( - ambient_start, - dest_base, - total_size, - &src_offsets, - &dest_offsets, - &sizes, - ) - }; - assert!( - padding, - "ambient_start (1) is not 4-aligned; a leading pad is expected" - ); - assert_eq!(stack.len(), dest_base + total_size); - - let mut sentinel = [0u8; 1]; - let mut a = [0u8; 1]; - let mut b = [0u8; 4]; - unsafe { - stack.copy_from(0, 1, sentinel.as_mut_ptr()); - stack.copy_from(dest_base, 1, a.as_mut_ptr()); - stack.copy_from(dest_base + 4, 4, b.as_mut_ptr()); - } - assert_eq!( - sentinel[0], 0xFF, - "bytes before the tuple must be untouched" - ); - assert_eq!(a[0], 0xDD); - assert_eq!( - u32::from_ne_bytes(b), - 0x1122_3344, - "the u32's bytes must survive the repack uncorrupted" - ); - } -} +use crate::memory::align_index; +use crate::raw_vec::RawVec; +use std::mem::MaybeUninit; +use std::mem::size_of; + +/// A simple raw stack that stores values as raw bytes. Each value is naturally aligned given the +/// base alignment of the stack, which is the maximum alignment of any value stored in the stack. +#[derive(Debug)] +pub struct RawStack { + buffer: RawVec, +} + +impl RawStack { + /// Creates a new `RawStack` with base alignment. + /// + /// # Examples + /// + /// ``` + /// use cel_runtime::raw_stack::RawStack; + /// let stack = RawStack::with_base_alignment(align_of::()); + /// ``` + #[must_use] + pub fn with_base_alignment(base_alignment: usize) -> Self { + RawStack { + buffer: RawVec::with_base_alignment(base_alignment), + } + } + + /// Returns the number of bytes currently on the stack. + #[must_use] + #[allow(clippy::len_without_is_empty)] + pub fn len(&self) -> usize { + self.buffer.len() + } + + /// Pushes a value of type `T` onto the stack. + /// + /// The value is stored as raw bytes in the internal buffer. The pushed value must be + /// later popped using the correct type. + /// + /// # Type Parameters + /// + /// * `T`: The type of the value to push. + /// + /// # Examples + /// + /// ``` + /// use cel_runtime::raw_stack::RawStack; + /// let mut stack = RawStack::with_base_alignment(align_of::()); + /// let _ = stack.push(42u32); + /// ``` + /// + /// # Complexity + /// + /// The function has an amortized O(1) time complexity. + pub fn push(&mut self, value: T) -> bool { + let len = self.buffer.len(); + let aligned_index = align_index(align_of::(), len); + let new_len = aligned_index + size_of::(); + + self.buffer.reserve(new_len - len); + unsafe { + self.buffer.set_len(new_len); + if aligned_index - len > 0 { + // write a 1 in the first padding byte and 0 in the rest + self.buffer[len].write(1); + self.buffer[len + 1..aligned_index].fill(MaybeUninit::new(0)); + } + + std::ptr::write( + self.buffer.as_mut_ptr().add(aligned_index).cast::(), + value, + ); + } + aligned_index - len > 0 + } + + /// Pushes `size` raw bytes from `src`, aligned to `align`, using the same + /// padding/marker-byte bookkeeping as [`push`](Self::push). + /// + /// `src` is typed as `MaybeUninit` rather than `u8` because the bytes + /// being copied may include a source value's interior padding, which is + /// itself uninitialized — reading it through a `u8` pointer instead would + /// be undefined behavior even though this function never inspects the + /// bytes' values. + /// + /// - Precondition: `align` is a power of two. + /// + /// # Safety + /// `src` must be valid for reads of `size` bytes, and must not overlap the + /// stack's internal buffer. + pub unsafe fn push_raw( + &mut self, + align: usize, + size: usize, + src: *const MaybeUninit, + ) -> bool { + debug_assert!(align.is_power_of_two()); + let len = self.buffer.len(); + let aligned_index = align_index(align, len); + let new_len = aligned_index + size; + + self.buffer.reserve(new_len - len); + unsafe { + self.buffer.set_len(new_len); + if aligned_index - len > 0 { + self.buffer[len].write(1); + self.buffer[len + 1..aligned_index].fill(MaybeUninit::new(0)); + } + std::ptr::copy_nonoverlapping(src, self.buffer.as_mut_ptr().add(aligned_index), size); + } + aligned_index - len > 0 + } + + /// Copies `size` bytes starting at absolute buffer offset `offset` into `dst`. + /// + /// `dst` is typed as `MaybeUninit` rather than `u8` because the bytes + /// being copied may be a value's interior padding, which is itself + /// uninitialized — reading it through a `u8` pointer instead would be + /// undefined behavior even though this function never inspects the + /// bytes' values. + /// + /// # Safety + /// `offset..offset + size` must be within the currently-initialized buffer; + /// `dst` must be valid for writes of `size` bytes and must not overlap the + /// stack's internal buffer. + pub unsafe fn copy_from(&self, offset: usize, size: usize, dst: *mut MaybeUninit) { + debug_assert!(offset + size <= self.buffer.len()); + unsafe { + std::ptr::copy_nonoverlapping(self.buffer.as_ptr().add(offset), dst, size); + } + } + + /// Drops a value in place at absolute buffer offset `offset`, without + /// altering the stack's tracked length. + /// + /// # Safety + /// `offset` must point to a live, valid value; `run_drop` must correctly + /// run that value's destructor given a pointer to its start. + pub unsafe fn drop_at(&mut self, offset: usize, run_drop: impl FnOnce(*mut u8)) { + unsafe { run_drop(self.buffer.as_mut_ptr().add(offset).cast::()) }; + } + + /// Truncates the stack back to `new_len`, additionally stripping `padding` + /// bytes that preceded the removed region (scanned the same way + /// [`pop`](Self::pop) does). + /// + /// # Safety + /// No live (undropped) value may exist at or above `new_len`. + pub unsafe fn truncate_to(&mut self, new_len: usize, padding: bool) { + debug_assert!(new_len <= self.buffer.len()); + let padding_count = if padding { + self.buffer[..new_len] + .iter() + .rev() + .take_while(|&x| unsafe { x.assume_init() == 0 }) + .count() + + 1 + } else { + 0 + }; + self.buffer.truncate(new_len - padding_count); + } + + /// Drops a value of `size` bytes at the top of the stack in place, then + /// removes it (and any padding that preceded it). + /// + /// # Safety + /// The top `size` bytes (plus padding if `padding` is true) must be a + /// live, valid value; `run_drop` must correctly run its destructor given a + /// pointer to its start. + pub unsafe fn drop_sized( + &mut self, + size: usize, + padding: bool, + run_drop: impl FnOnce(*mut u8), + ) { + debug_assert!(size <= self.buffer.len()); + let p = self.buffer.len() - size; + unsafe { + self.drop_at(p, run_drop); + self.truncate_to(p, padding); + } + } + + /// Repacks `sizes.len()` already-pushed values (currently at the absolute + /// byte offsets in `src_offsets`) into one contiguous, self-contained + /// region of `total_size` bytes starting at `dest_base`, placing element + /// `i` at `dest_base + dest_offsets[i]`. Adjusts the tracked length to + /// `dest_base + total_size` and returns whether leading padding was + /// inserted between `ambient_start` and `dest_base`. + /// + /// - Precondition: `src_offsets`, `dest_offsets`, and `sizes` have equal + /// length; `dest_base >= ambient_start`; the source ranges are + /// currently valid, initialized bytes. + /// + /// - Complexity: O(n) in `sizes.len()`. + /// + /// # Safety + /// The offsets and sizes must correctly describe the actual bytes in the + /// buffer; no two destination ranges may overlap. `src_offsets` and + /// `dest_offsets` must be in the same relative element order (element `i`'s + /// source and destination must both be the `i`-th non-overlapping range in + /// their respective layouts) — this method does not reorder elements, only + /// re-pads between them. Given that and `dest_base >= ambient_start`, each + /// element's destination start is guaranteed to be at or after every + /// earlier element's source end, which is what makes processing in reverse + /// index order below safe against clobbering not-yet-read source bytes. + pub unsafe fn repack( + &mut self, + ambient_start: usize, + dest_base: usize, + total_size: usize, + src_offsets: &[usize], + dest_offsets: &[usize], + sizes: &[usize], + ) -> bool { + debug_assert!(dest_base >= ambient_start); + debug_assert_eq!(src_offsets.len(), sizes.len()); + debug_assert_eq!(dest_offsets.len(), sizes.len()); + + let target_len = dest_base + total_size; + let current_len = self.buffer.len(); + let grown_len = current_len.max(target_len); + unsafe { + if grown_len > current_len { + self.buffer.reserve(grown_len - current_len); + self.buffer.set_len(grown_len); + } + let base_ptr = self.buffer.as_mut_ptr(); + // Process highest index first: each element's destination is + // provably at or after every earlier element's source end (see + // `# Safety` above), so this order never overwrites source bytes + // an earlier iteration still needs to read. + for i in (0..sizes.len()).rev() { + std::ptr::copy( + base_ptr.add(src_offsets[i]), + base_ptr.add(dest_base + dest_offsets[i]), + sizes[i], + ); + } + if dest_base > ambient_start { + self.buffer[ambient_start].write(1); + self.buffer[ambient_start + 1..dest_base].fill(MaybeUninit::new(0)); + } + self.buffer.set_len(target_len); + } + dest_base > ambient_start + } + + /// Pops a value of type `T` from the stack. Does not change the stack capacity. + /// + /// # Safety + /// + /// The type `T` must be the same type as the value on the top of the stack. + /// Incorrect usage can lead to undefined behavior. + /// + /// # Type Parameters + /// + /// * `T`: The type of the value to pop. + /// + /// # Examples + /// + /// ``` + /// use cel_runtime::raw_stack::RawStack; + /// let mut stack = RawStack::with_base_alignment(align_of::()); + /// let padding = stack.push(100u32); + /// let value: u32 = unsafe { stack.pop(padding) }; + /// ``` + pub unsafe fn pop(&mut self, padding: bool) -> T { + let p: usize = self.buffer.len() - size_of::(); + let result = unsafe { std::ptr::read(self.buffer.as_ptr().add(p).cast::()) }; + // count the number of trailing 0s in the buffer before the result + let padding_count = if padding { + self.buffer[..p] + .iter() + .rev() + .take_while(|&x| unsafe { x.assume_init() == 0 }) + .count() + + 1 + } else { + 0 + }; + self.buffer.truncate(p - padding_count); + result + } + + /// Pops a value of type `T` from the stack and drops it. + /// + /// # Safety + /// + /// The type `T` must be the same type as the value on the top of the stack. + /// Incorrect usage can lead to undefined behavior. + /// + /// # Note + /// + /// This cannot use `drop_in_place` because the type may not be aligned. + pub unsafe fn drop(&mut self, padding: bool) { + unsafe { self.pop::(padding) }; + } +} + +/* Test module */ +#[cfg(test)] +mod tests { + use super::*; + use std::cmp::max; + + #[test] + fn push_pop_u32() { + let mut stack = RawStack::with_base_alignment(align_of::()); + let padding = stack.push(10u32); + let result: u32 = unsafe { stack.pop(padding) }; + assert_eq!(result, 10); + } + + #[test] + fn multiple_push_pop() { + let mut stack = RawStack::with_base_alignment(align_of::()); + let padding1 = stack.push(1u32); + let padding2 = stack.push(2u32); + let padding3 = stack.push(3u32); + let v3: u32 = unsafe { stack.pop(padding3) }; + let v2: u32 = unsafe { stack.pop(padding2) }; + let v1: u32 = unsafe { stack.pop(padding1) }; + assert_eq!(v1, 1); + assert_eq!(v2, 2); + assert_eq!(v3, 3); + } + + #[test] + fn push_pop_different_types() { + let mut stack = RawStack::with_base_alignment(max(align_of::(), align_of::())); + let padding1 = stack.push(42u32); + let padding2 = stack.push(3.14f64); + let value_f: f64 = unsafe { stack.pop(padding2) }; + let value_u: u32 = unsafe { stack.pop(padding1) }; + assert_eq!(value_f, 3.14); + assert_eq!(value_u, 42); + } + + #[test] + fn len_reflects_pushed_bytes() { + let mut stack = RawStack::with_base_alignment(align_of::()); + assert_eq!(stack.len(), 0); + let _ = stack.push(7u32); + assert_eq!(stack.len(), size_of::()); + } + + #[test] + fn push_raw_round_trips_like_push() { + let mut stack = RawStack::with_base_alignment(align_of::()); + let padding1 = stack.push(1u8); + let value = 3.14f64; + let padding2 = unsafe { + stack.push_raw( + align_of::(), + size_of::(), + (&value as *const f64).cast::>(), + ) + }; + let popped: f64 = unsafe { stack.pop(padding2) }; + assert_eq!(popped, 3.14); + let popped_u8: u8 = unsafe { stack.pop(padding1) }; + assert_eq!(popped_u8, 1); + } + + #[test] + fn push_raw_padding_matches_typed_push() { + let mut stack_a = RawStack::with_base_alignment(align_of::()); + let _ = stack_a.push(1u8); + let padding_typed = stack_a.push(2.5f64); + + let mut stack_b = RawStack::with_base_alignment(align_of::()); + let _ = stack_b.push(1u8); + let value = 2.5f64; + let padding_raw = unsafe { + stack_b.push_raw( + align_of::(), + size_of::(), + (&value as *const f64).cast::>(), + ) + }; + assert_eq!(padding_typed, padding_raw); + } + + #[test] + fn copy_from_reads_bytes_at_offset() { + let mut stack = RawStack::with_base_alignment(align_of::()); + let _ = stack.push(10u32); + let _ = stack.push(20u32); + let mut buf = [0u8; 4]; + unsafe { stack.copy_from(0, 4, buf.as_mut_ptr().cast::>()) }; + assert_eq!(u32::from_ne_bytes(buf), 10); + } + + #[test] + fn drop_at_runs_destructor_without_changing_length() { + use std::sync::Arc; + use std::sync::atomic::{AtomicUsize, Ordering}; + + struct DropCounter(Arc); + impl Drop for DropCounter { + fn drop(&mut self) { + self.0.fetch_add(1, Ordering::SeqCst); + } + } + + let count = Arc::new(AtomicUsize::new(0)); + let mut stack = RawStack::with_base_alignment(align_of::()); + let _ = stack.push(DropCounter(count.clone())); + let len_before = stack.len(); + unsafe { + stack.drop_at(0, |ptr| std::ptr::drop_in_place(ptr.cast::())); + } + assert_eq!(count.load(Ordering::SeqCst), 1); + assert_eq!(stack.len(), len_before); + } + + #[test] + fn truncate_to_strips_recorded_padding() { + let mut stack = RawStack::with_base_alignment(align_of::()); + let _ = stack.push(1u8); + let padding = stack.push(2.5f64); // padding == true: 7 bytes inserted before the f64 + let len_with_value = stack.len(); + unsafe { stack.truncate_to(len_with_value - size_of::(), padding) }; + assert_eq!(stack.len(), 1); // back to just the u8 + } + + #[test] + fn drop_sized_combines_drop_at_and_truncate_to() { + use std::sync::Arc; + use std::sync::atomic::{AtomicUsize, Ordering}; + + struct DropCounter(Arc); + impl Drop for DropCounter { + fn drop(&mut self) { + self.0.fetch_add(1, Ordering::SeqCst); + } + } + + let count = Arc::new(AtomicUsize::new(0)); + let mut stack = RawStack::with_base_alignment(align_of::()); + let _ = stack.push(1u8); + let padding = stack.push(DropCounter(count.clone())); + unsafe { + stack.drop_sized(size_of::(), padding, |ptr| { + std::ptr::drop_in_place(ptr.cast::()) + }); + } + assert_eq!(count.load(Ordering::SeqCst), 1); + assert_eq!(stack.len(), 1); + } + + #[test] + fn repack_moves_elements_to_ideal_offsets_and_reports_padding() { + // Ambient layout: [u8 @0][pad][u32 @4][u8 @8] — u8 then u32 then u8, each + // pushed with ordinary alignment relative to a 1-byte ambient start. + let mut stack = RawStack::with_base_alignment(align_of::()); + let ambient_start = stack.len(); // 0 + let _ = stack.push(0xAAu8); // element 0: ambient offset 0 + let _p1 = stack.push(0xBBBB_BBBBu32); // element 1: ambient offset 4 (1 byte padded to 4) + let _p2 = stack.push(0xCCu8); // element 2: ambient offset 8 + + // Ideal (self-contained) layout for (u8, u32, u8) from a zero base: + // offset 0 (u8), offset 4 (u32, aligned up from 1), offset 8 (u8) -> total 9, + // rounded to the tuple's own max align (4) -> total_size 12. + let src_offsets = [0usize, 4, 8]; + let dest_offsets = [0usize, 4, 8]; + let sizes = [1usize, 4, 1]; + let total_size = 12usize; + let dest_base = 0usize; // ambient_start (0) is already 4-aligned + + let padding = unsafe { + stack.repack( + ambient_start, + dest_base, + total_size, + &src_offsets, + &dest_offsets, + &sizes, + ) + }; + assert!( + !padding, + "ambient_start was already aligned; no leading pad expected" + ); + assert_eq!(stack.len(), dest_base + total_size); + + let mut a = [0u8; 1]; + let mut b = [0u8; 4]; + let mut c = [0u8; 1]; + unsafe { + stack.copy_from(dest_base, 1, a.as_mut_ptr().cast::>()); + stack.copy_from(dest_base + 4, 4, b.as_mut_ptr().cast::>()); + stack.copy_from(dest_base + 8, 1, c.as_mut_ptr().cast::>()); + } + assert_eq!(a[0], 0xAA); + assert_eq!(u32::from_ne_bytes(b), 0xBBBB_BBBB); + assert_eq!(c[0], 0xCC); + } + + #[test] + fn repack_shifts_right_without_corrupting_unread_source_bytes() { + // A misaligned ambient_start forces dest_base > ambient_start, which + // means the destination of the *first* tuple element can land inside + // the *source* range of a *later*, not-yet-copied element. Processing + // elements low-index-first would silently corrupt that later element's + // bytes before they're read; this test fails under that ordering and + // passes under the correct (reverse) ordering. + let mut stack = RawStack::with_base_alignment(align_of::()); + let _ = stack.push(0xFFu8); // sentinel, ambient offset 0, before the tuple + let ambient_start = stack.len(); // 1: misaligned relative to u32's 4-byte align + let _ = stack.push(0xDDu8); // element 0: ambient offset 1 + let _ = stack.push(0x1122_3344u32); // element 1: ambient offset 4 (3 bytes padded) + + // Ideal (u8, u32) layout from zero: offset 0 (u8), offset 4 (u32) -> total 8. + let src_offsets = [1usize, 4]; + let dest_offsets = [0usize, 4]; + let sizes = [1usize, 4]; + let total_size = 8usize; + let dest_base = 4usize; // align_index(4, ambient_start=1) == 4, so this shifts right + + let padding = unsafe { + stack.repack( + ambient_start, + dest_base, + total_size, + &src_offsets, + &dest_offsets, + &sizes, + ) + }; + assert!( + padding, + "ambient_start (1) is not 4-aligned; a leading pad is expected" + ); + assert_eq!(stack.len(), dest_base + total_size); + + let mut sentinel = [0u8; 1]; + let mut a = [0u8; 1]; + let mut b = [0u8; 4]; + unsafe { + stack.copy_from(0, 1, sentinel.as_mut_ptr().cast::>()); + stack.copy_from(dest_base, 1, a.as_mut_ptr().cast::>()); + stack.copy_from(dest_base + 4, 4, b.as_mut_ptr().cast::>()); + } + assert_eq!( + sentinel[0], 0xFF, + "bytes before the tuple must be untouched" + ); + assert_eq!(a[0], 0xDD); + assert_eq!( + u32::from_ne_bytes(b), + 0x1122_3344, + "the u32's bytes must survive the repack uncorrupted" + ); + } +} From 8ced449e0a26aeb07b511318cf8524cbecc72872 Mon Sep 17 00:00:00 2001 From: Sean Parent Date: Wed, 8 Jul 2026 14:18:42 -0700 Subject: [PATCH 25/29] fix(cel-parser): improve tuple or group parsing logic and error handling --- cel-parser/src/lib.rs | 154 ++++++++++++++++++++++-------------------- 1 file changed, 82 insertions(+), 72 deletions(-) diff --git a/cel-parser/src/lib.rs b/cel-parser/src/lib.rs index bf26f59..55a1c8a 100644 --- a/cel-parser/src/lib.rs +++ b/cel-parser/src/lib.rs @@ -929,19 +929,16 @@ impl CELParser { } /// `primary_expression = literal | identifier | tuple_or_group | if_expression.` - /// `tuple_or_group = "(" [ or_expression ["," [ or_expression { "," or_expression } ]] ] ")".` /// - /// Dispatches to [`is_if_expression`](Self::is_if_expression) when the `if` keyword is seen. - /// `()` parses as unit, `(expr)` as grouping, `(expr,)` as a 1-tuple, and - /// `(expr, expr, ...)` as an n-tuple. + /// Dispatches to [`is_if_expression`](Self::is_if_expression) when the `if` keyword is seen, + /// and to [`is_tuple_or_group`](Self::is_tuple_or_group) when `(` is seen. /// /// # Errors /// /// Returns an error if: /// - A literal value cannot be parsed (e.g., integer out of range). /// - An identifier is not found in the op lookup table. - /// - A parenthesized expression or tuple literal is malformed, has a missing or - /// misplaced comma, or is missing its closing `)`. + /// - A tuple-or-group expression fails to parse. /// - An `if` expression fails to parse. fn is_primary_expression(&mut self) -> Result { match self.peek_token() { @@ -968,75 +965,88 @@ impl CELParser { Some(Token::OpenDelim { delimiter: Delimiter::Parenthesis, .. - }) => { + }) => self.is_tuple_or_group(), + _ => Ok(false), + } + } + + /// `tuple_or_group = "(" [ or_expression ["," [ or_expression { "," or_expression } ]] ] ")".` + /// + /// `()` parses as unit, `(expr)` as grouping, `(expr,)` as a 1-tuple, and + /// `(expr, expr, ...)` as an n-tuple. + /// + /// - Precondition: The next token is `Token::OpenDelim` with `Delimiter::Parenthesis`. + /// + /// # Errors + /// + /// Returns an error if the parenthesized expression or tuple literal is malformed, has a + /// missing or misplaced comma, or is missing its closing `)`. + fn is_tuple_or_group(&mut self) -> Result { + self.advance(); + // Unit expression: () + if matches!( + self.peek_token(), + Some(Token::CloseDelim { + delimiter: Delimiter::Parenthesis, + .. + }) + ) { + self.advance(); + self.context.just(()); + return Ok(true); + } + let ambient_start = self.context.current_stack_offset(); + if !self.is_or_expression()? { + return Err(self.error_at("expected expression")); + } + if matches!( + self.peek_token(), + Some(Token::CloseDelim { + delimiter: Delimiter::Parenthesis, + .. + }) + ) { + // Grouping: exactly one expression, no comma. + self.advance(); + return Ok(true); + } + if !self.is_punctuation(",") { + return Err(self.error_at("expected ',' or closing parenthesis")); + } + let mut count = 1; + if matches!( + self.peek_token(), + Some(Token::CloseDelim { + delimiter: Delimiter::Parenthesis, + .. + }) + ) { + // Single element + trailing comma: 1-tuple. + self.advance(); + self.context.make_tuple(count, ambient_start); + return Ok(true); + } + loop { + if !self.is_or_expression()? { + return Err(self.error_at("expected expression after ','")); + } + count += 1; + if matches!( + self.peek_token(), + Some(Token::CloseDelim { + delimiter: Delimiter::Parenthesis, + .. + }) + ) { self.advance(); - // Unit expression: () - if matches!( - self.peek_token(), - Some(Token::CloseDelim { - delimiter: Delimiter::Parenthesis, - .. - }) - ) { - self.advance(); - self.context.just(()); - return Ok(true); - } - let ambient_start = self.context.current_stack_offset(); - if !self.is_or_expression()? { - return Err(self.error_at("expected expression")); - } - if matches!( - self.peek_token(), - Some(Token::CloseDelim { - delimiter: Delimiter::Parenthesis, - .. - }) - ) { - // Grouping: exactly one expression, no comma. - self.advance(); - return Ok(true); - } - if !self.is_punctuation(",") { - return Err(self.error_at("expected ',' or closing parenthesis")); - } - let mut count = 1; - if matches!( - self.peek_token(), - Some(Token::CloseDelim { - delimiter: Delimiter::Parenthesis, - .. - }) - ) { - // Single element + trailing comma: 1-tuple. - self.advance(); - self.context.make_tuple(count, ambient_start); - return Ok(true); - } - loop { - if !self.is_or_expression()? { - return Err(self.error_at("expected expression after ','")); - } - count += 1; - if matches!( - self.peek_token(), - Some(Token::CloseDelim { - delimiter: Delimiter::Parenthesis, - .. - }) - ) { - self.advance(); - break; - } - if !self.is_punctuation(",") { - return Err(self.error_at("expected ',' or closing parenthesis")); - } - } - self.context.make_tuple(count, ambient_start); - Ok(true) + break; + } + if !self.is_punctuation(",") { + return Err(self.error_at("expected ',' or closing parenthesis")); } - _ => Ok(false), } + self.context.make_tuple(count, ambient_start); + Ok(true) } /// `if_expression = "if" or_expression "{" or_expression "}" [ "else" ( "{" or_expression "}" | if_expression ) ].` From 8e5258f9d18da840bf96f1c4bd9158bd898108b8 Mon Sep 17 00:00:00 2001 From: Sean Parent Date: Thu, 9 Jul 2026 12:34:06 -0700 Subject: [PATCH 26/29] fix(cel-runtime): strip a tuple's own leading pad when extracting an element extract_tuple_element left the tuple's leading alignment padding as dead, untracked bytes on the stack after tuple_index, so a later tuple literal built from the extracted result computed its element offsets against the wrong ambient start and could read garbage. Truncate past the tuple's own pad using its recorded padding flag, and recompute the extracted element's own padding flag fresh (relative to the recovered ambient offset) instead of inheriting the tuple's. Co-Authored-By: Claude Sonnet 5 --- cel-parser/src/lib.rs | 14 +++++++++ cel-runtime/src/dyn_segment.rs | 53 +++++++++++++++++++++++----------- 2 files changed, 50 insertions(+), 17 deletions(-) diff --git a/cel-parser/src/lib.rs b/cel-parser/src/lib.rs index 55a1c8a..1d46c1d 100644 --- a/cel-parser/src/lib.rs +++ b/cel-parser/src/lib.rs @@ -1373,6 +1373,20 @@ mod tests { assert!(seg.call0::().unwrap()); } + #[test] + fn tuple_containing_indexed_nested_tuple_result() { + // Regression test: extracting an element from a misaligned nested + // tuple must not leave the tuple's own leading padding as dead space + // on the stack — otherwise a later tuple literal built from this + // result computes its element offsets against the wrong ambient + // start and reads garbage. (7, not 1: the dead gap's own marker + // byte is hardcoded to value 1, so a value of 1 here would pass + // even when reading the wrong offset.) + let mut parser = CELParser::new(OpLookup::new()); + let mut seg = parser.parse_str("(0u8, (7u8, 2u64).0).1").unwrap(); + assert_eq!(seg.call0::().unwrap(), 7); + } + #[test] fn index_second_element_of_tuple() { let mut parser = CELParser::new(OpLookup::new()); diff --git a/cel-runtime/src/dyn_segment.rs b/cel-runtime/src/dyn_segment.rs index 0e81205..9573797 100644 --- a/cel-runtime/src/dyn_segment.rs +++ b/cel-runtime/src/dyn_segment.rs @@ -87,22 +87,27 @@ unsafe fn drop_tuple(ptr: *mut u8, associated: &[AssociatedType]) { /// Extracts element `index` from the tuple currently on top of `stack`, /// dropping every other element, leaving just the extracted value on top. /// -/// Never strips the tuple's own leading padding (if any): the extracted -/// element inherits that same leading-padding relationship unchanged, since -/// `tuple_base` is by construction already aligned for every element inside -/// the tuple (the tuple's own alignment is the max of all its elements'), so -/// re-pushing the target from `tuple_base` introduces no padding of its own. +/// Also strips the tuple's own leading padding (if any): once only one +/// element survives, that space is dead — no `StackInfo` entry accounts for +/// it — so it must not linger, or later offset computations (e.g. for a +/// tuple literal built from this result) would disagree with the real +/// stack. The extracted element is re-pushed at its own natural alignment +/// relative to the ambient offset the tuple originally started at, which +/// `expected_padding` (computed the same way at parse time) predicts. /// /// - Complexity: O(n) in the tuple's arity. /// /// # Safety /// The top `tuple_size` bytes of `stack` must be a live tuple value whose -/// layout matches `associated`. +/// layout matches `associated`, and `tuple_padding` must be that tuple's own +/// recorded leading-padding flag. unsafe fn extract_tuple_element( stack: &mut RawStack, tuple_size: usize, + tuple_padding: bool, associated: &[AssociatedType], index: usize, + expected_padding: bool, ) { let tuple_base = stack.len() - tuple_size; let target = &associated[index]; @@ -135,13 +140,14 @@ unsafe fn extract_tuple_element( } unsafe { - // padding=false: this truncates only down to tuple_base, never past - // the tuple's own leading pad (see doc comment above). - stack.truncate_to(tuple_base, false); + // tuple_padding: strip the tuple's own leading pad too, all the way + // back to the true ambient offset it was built from — not just down + // to tuple_base (see doc comment above). + stack.truncate_to(tuple_base, tuple_padding); let repushed_padding = stack.push_raw(target.align, target.size, scratch.as_ptr()); - debug_assert!( - !repushed_padding, - "tuple_base is already aligned for every element inside the tuple" + debug_assert_eq!( + repushed_padding, expected_padding, + "extracted element's padding must match the parse-time prediction" ); } } @@ -474,17 +480,30 @@ impl DynSegment { ); debug_assert!(index < info.associated.len(), "tuple_index out of range"); - // The extracted element inherits the tuple's own leading-padding - // relationship unchanged — see extract_tuple_element's doc comment - // for why no realignment is needed or performed. let target = info.associated[index].clone(); let associated = info.associated.clone(); let tuple_padding = info.padding; let tuple_size = info.size; + // The tuple's own leading pad becomes dead space once torn down to + // one element, so the extracted element gets its own padding flag: + // recomputed relative to the ambient offset the tuple was originally + // built from (with the tuple's own entry already popped above, this + // replay gives exactly that offset), not inherited from the tuple. + let ambient_before_tuple = self.stack_offset_after(self.stack_ids.len()); + let new_offset = align_index(target.align, ambient_before_tuple); + let new_padding = new_offset != ambient_before_tuple; + self.segment.raw0_(move |stack| { unsafe { - extract_tuple_element(stack, tuple_size, &associated, index); + extract_tuple_element( + stack, + tuple_size, + tuple_padding, + &associated, + index, + new_padding, + ); } Ok(()) }); @@ -492,7 +511,7 @@ impl DynSegment { self.stack_ids.push(StackInfo { type_id: target.type_id, type_name: target.type_name, - padding: tuple_padding, + padding: new_padding, size: target.size, align: target.align, raw_dropper: target.dropper, From ab3e10ef6b5ae3e30920c418fc0b02b0ce9142eb Mon Sep 17 00:00:00 2001 From: Sean Parent Date: Thu, 9 Jul 2026 13:35:31 -0700 Subject: [PATCH 27/29] fix(cel-runtime): compare tuple shapes, not just the erased marker type, in join2 Every tuple shares the same erased DynTuple TypeId, so join2's plain type_id comparison accepted if/else (and short-circuit &&/||) branches whose tuple results had different arity or element types, silently merging incompatible layouts. Add a recursive shape comparison that inspects each tuple's associated element types (and nested tuples within them) instead of relying on the shared marker type. Co-Authored-By: Claude Sonnet 5 --- cel-parser/src/lib.rs | 24 ++++++++++++++++++++++++ cel-runtime/src/dyn_segment.rs | 26 +++++++++++++++++++++++++- 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/cel-parser/src/lib.rs b/cel-parser/src/lib.rs index 1d46c1d..e89574b 100644 --- a/cel-parser/src/lib.rs +++ b/cel-parser/src/lib.rs @@ -2162,6 +2162,30 @@ mod tests { assert!(parser.parse_str("if true { 1i32 } else { true }").is_err()); } + #[test] + fn if_branch_tuple_arity_mismatch_is_error() { + // Regression test: every tuple shares the same erased `DynTuple` + // marker type, so a naive type_id comparison would accept branches + // with genuinely different tuple shapes — join2 must compare shapes, + // not just the marker type, and reject this. + let mut parser = CELParser::new(OpLookup::new()); + let result = parser.parse_str("if false { (1i32, 2i32) } else { (3i64, 4i64, 5i64) }.0"); + assert!( + result.is_err(), + "branches with different tuple shapes must not be accepted" + ); + } + + #[test] + fn if_branch_tuple_element_type_mismatch_is_error() { + let mut parser = CELParser::new(OpLookup::new()); + let result = parser.parse_str("if false { (1i32, 2i32) } else { (3i64, 4i64) }.0"); + assert!( + result.is_err(), + "branches with the same arity but different element types must not be accepted" + ); + } + #[test] fn if_missing_open_brace_is_error() { let mut parser = CELParser::new(OpLookup::new()); diff --git a/cel-runtime/src/dyn_segment.rs b/cel-runtime/src/dyn_segment.rs index 9573797..16a7922 100644 --- a/cel-runtime/src/dyn_segment.rs +++ b/cel-runtime/src/dyn_segment.rs @@ -84,6 +84,30 @@ unsafe fn drop_tuple(ptr: *mut u8, associated: &[AssociatedType]) { } } +/// Returns whether every element `TypeId` in `a` and `b` matches, in order — +/// recursing into nested tuple elements' own `associated` shapes rather than +/// stopping at their shared [`DynTuple`] marker `TypeId`. +/// +/// - Complexity: O(n) in the total number of (nested) elements. +fn tuple_shapes_match(a: &[AssociatedType], b: &[AssociatedType]) -> bool { + a.len() == b.len() + && a.iter().zip(b).all(|(x, y)| { + x.type_id == y.type_id + && (x.type_id != TypeId::of::() + || tuple_shapes_match(&x.associated, &y.associated)) + }) +} + +/// Returns whether `a` and `b` describe the same type — for tuples, this +/// means the same element shape (recursively), not just the shared +/// [`DynTuple`] marker `TypeId`, since every tuple shares that one `TypeId` +/// regardless of arity or element types. +fn stack_info_shapes_match(a: &StackInfo, b: &StackInfo) -> bool { + a.type_id == b.type_id + && (a.type_id != TypeId::of::() + || tuple_shapes_match(&a.associated, &b.associated)) +} + /// Extracts element `index` from the tuple currently on top of `stack`, /// dropping every other element, leaving just the extracted value on top. /// @@ -853,7 +877,7 @@ impl DynSegment { fragment_1.stack_ids.len() ); ensure!( - fragment_0.stack_ids[0].type_id == fragment_1.stack_ids[0].type_id, + stack_info_shapes_match(&fragment_0.stack_ids[0], &fragment_1.stack_ids[0]), "fragment result types must match" ); From f9a612cba55370bb2d0d76607a2d1e7b1c6b71ee Mon Sep 17 00:00:00 2001 From: Sean Parent Date: Thu, 9 Jul 2026 13:43:11 -0700 Subject: [PATCH 28/29] feat(tests): add playground tests for macro token output --- cel-rs-macros/tests/playground.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 cel-rs-macros/tests/playground.rs diff --git a/cel-rs-macros/tests/playground.rs b/cel-rs-macros/tests/playground.rs new file mode 100644 index 0000000..ff98625 --- /dev/null +++ b/cel-rs-macros/tests/playground.rs @@ -0,0 +1,12 @@ +//! Playground tests for macro token output. + +#[cfg(all(test, feature = "playground"))] + +mod playground { + #[test] + fn tuple_index() { + use cel_rs_macros::print_tokens; + + print_tokens! {(0,(0,1)).1.0.2.5} + } +} From cac47b2b5c21e987ffb30772c3f9054ddd5a5296 Mon Sep 17 00:00:00 2001 From: Sean Parent Date: Thu, 9 Jul 2026 13:56:14 -0700 Subject: [PATCH 29/29] test(cel-runtime): verify repack's move-without-drop is sound; document nested-shape gap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a regression test confirming repack's ptr::copy-based relocation (without dropping the vacated source) never leaks or double-drops a non-Copy element, even when the element is actually moved rather than left in place — the same memmove pattern std itself uses (e.g. Vec::remove), sound because make_tuple's bookkeeping only ever tracks the new location afterward. Document TupleOpSignature's flat `shape` field: it has no way to express a nested tuple's inner arity/element types, so a nested-tuple operand position can only ever be recorded as "any tuple." No current caller relies on nested-shape precision; a real fix would need a recursive shape type. Co-Authored-By: Claude Sonnet 5 --- cel-parser/src/op_table.rs | 7 +++++ cel-runtime/src/dyn_segment.rs | 52 ++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/cel-parser/src/op_table.rs b/cel-parser/src/op_table.rs index f67653a..717ded5 100644 --- a/cel-parser/src/op_table.rs +++ b/cel-parser/src/op_table.rs @@ -69,10 +69,17 @@ pub type OpFn = fn(&mut DynSegment, SourceSpan) -> Result<()>; /// vector when there are non-tuple operands) simply never matches, rather /// than panicking — it is only safe to omit the whole vector when /// `tuple_operand_index` is the *only* operand position. +/// +/// `shape` is flat: an element position that is itself a nested tuple can +/// only be recorded as `DynTuple`'s `TypeId`, which matches *any* nested +/// tuple at that position regardless of its inner arity or element types. +/// Two registrations that would only differ by that inner shape are not +/// distinguishable — do not rely on nested-tuple precision at this level. pub struct TupleOpSignature { /// Operator/function name this signature is registered under. pub name: String, /// Expected element `TypeId`s, in order, for the tuple-shaped operand. + /// See the struct-level note on nested tuples. pub shape: Vec, /// Which peeked operand position must be the tuple. pub tuple_operand_index: usize, diff --git a/cel-runtime/src/dyn_segment.rs b/cel-runtime/src/dyn_segment.rs index 16a7922..4d8f44b 100644 --- a/cel-runtime/src/dyn_segment.rs +++ b/cel-runtime/src/dyn_segment.rs @@ -1436,6 +1436,58 @@ mod tests { assert_eq!(drop_count.load(Ordering::SeqCst), 1); } + #[test] + fn repack_relocated_element_drops_exactly_once() { + // Regression test for the concern that repack's ptr::copy-based move + // never runs destructors at an element's old (pre-relocation) offset: + // that's the same memmove-without-drop pattern std itself uses (e.g. + // Vec::remove) and is sound as long as nothing treats the vacated + // slot as live afterward, which make_tuple's bookkeeping guarantees. + use std::sync::Arc; + use std::sync::atomic::{AtomicUsize, Ordering}; + + #[derive(Clone)] + struct DropCounter(Arc); + impl Drop for DropCounter { + fn drop(&mut self) { + self.0.fetch_add(1, Ordering::SeqCst); + } + } + + #[repr(align(16))] + #[allow(dead_code)] + struct Over16(u8); + + let drop_count = Arc::new(AtomicUsize::new(0)); + let tracker = DropCounter(drop_count.clone()); + + let mut seg = DynSegment::new::<()>(); + seg.op0(|| 0xEEu8); // sentinel, forces a misaligned ambient_start + let ambient_start = seg.current_stack_offset(); // 1 + seg.op0(move || tracker.clone()); // element 0: DropCounter, align 8 + seg.op0(|| Over16(0)); // element 1: align 16, forces tuple_align=16 + // tuple_align (16) > DropCounter's own align (8), so dest_base + // (align_index(16, 1) == 16) differs from DropCounter's natural + // ambient position (align_index(8, 1) == 8): repack must physically + // relocate it, leaving its old bytes behind. + seg.make_tuple(2, ambient_start); + seg.tuple_index(0); // keep the DropCounter, drop Over16 in place + seg.op2(|_sentinel: u8, val: DropCounter| val).unwrap(); + + let extracted = seg.call0::().unwrap(); + assert_eq!( + drop_count.load(Ordering::SeqCst), + 0, + "extracting must not have already dropped the relocated element" + ); + drop(extracted); + assert_eq!( + drop_count.load(Ordering::SeqCst), + 1, + "relocated element must drop exactly once (no leak, no double-drop)" + ); + } + #[test] fn tuple_index_combined_with_another_op() { // Mirrors the spec's `5 + (0, 1).1` case: indexing must leave the stack in