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 a3f94b7..e89574b 100644 --- a/cel-parser/src/lib.rs +++ b/cel-parser/src/lib.rs @@ -25,8 +25,9 @@ //! additive_expression = multiplicative_expression { ("+" | "-") multiplicative_expression }. //! 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. +//! 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 ) ]. //! parameter_list = [ or_expression { "," or_expression } ]. //! ``` @@ -799,42 +800,117 @@ impl CELParser { } } - /// `postfix_expression = primary_expression { "(" parameter_list ")" }.` + /// `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()? { 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")), } - _ => 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(".") { + 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(); + 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")); + } + // base10_digits() returns the decimal digits with underscores + // 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 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}")) + })?; + 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 '.'")), + } + } 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) } + /// 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. @@ -852,16 +928,17 @@ impl CELParser { Ok(count) } - /// `primary_expression = literal | identifier | "(" or_expression ")" | if_expression.` + /// `primary_expression = literal | identifier | tuple_or_group | if_expression.` /// - /// Dispatches to [`is_if_expression`](Self::is_if_expression) when the `if` keyword is seen. + /// 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 is malformed or 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() { @@ -888,36 +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); - } - if !self.is_or_expression()? { - return Err(self.error_at("expected expression")); - } - match self.peek_token() { - Some(Token::CloseDelim { - delimiter: Delimiter::Parenthesis, - .. - }) => { - self.advance(); - Ok(true) - } - _ => Err(self.error_at("expected closing parenthesis")), - } + 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 ) ].` @@ -1128,6 +1257,215 @@ 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 seg: DynSegment = 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 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 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 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()); + 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 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 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()); @@ -1824,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-parser/src/op_table.rs b/cel-parser/src/op_table.rs index bf0a0b7..717ded5 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; @@ -56,6 +56,40 @@ 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 never read). +/// +/// `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. +/// +/// `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, + /// 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 +1002,7 @@ impl BuiltinScope { pub struct OpLookup { scopes: Vec, builtin_scope: BuiltinScope, + tuple_signatures: Vec, } impl OpLookup { @@ -984,9 +1019,56 @@ 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.type_id == TypeId::of::() + && 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 || sig.operand_type_ids.get(i) == Some(&info.type_id) + }); + if others_match { + (sig.op_fn)(segment, span)?; + return Ok(true); + } + } + Ok(false) + } + /// Pushes a new scope onto the stack. /// /// Accepts a closure directly; it is boxed internally. The scope should return @@ -1051,6 +1133,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 +1617,98 @@ 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)" + ); + } + + #[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-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/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} + } +} diff --git a/cel-runtime/src/dyn_segment.rs b/cel-runtime/src/dyn_segment.rs index 1de6108..4d8f44b 100644 --- a/cel-runtime/src/dyn_segment.rs +++ b/cel-runtime/src/dyn_segment.rs @@ -5,11 +5,13 @@ 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; 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. @@ -33,24 +35,152 @@ 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; + +/// `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) }; + } +} + +/// 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. +/// +/// 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`, 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]; + debug_assert!(tuple_base.is_multiple_of(target.align)); + + // 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, + 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 { + // 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_eq!( + repushed_padding, expected_padding, + "extracted element's padding must match the parse-time prediction" + ); + } +} + /// 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 +188,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 +225,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 @@ -136,7 +264,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 { @@ -152,20 +286,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)), } } @@ -193,26 +339,208 @@ 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::(), 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::(); + } + + /// 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_offset_after(self.stack_ids.len()) + } + + /// 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 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 + /// 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 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); + 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); + + associated.push(AssociatedType { + type_id: elem.type_id, + type_name: elem.type_name.clone(), + offset, + size: elem.size, + align: elem.align, + dropper: elem.raw_dropper, + associated: elem.associated.clone(), + }); + + offset += elem.size; + offset = align_index(tuple_align, 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(); + 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, + }); + } + + /// 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 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, + tuple_padding, + &associated, + index, + new_padding, + ); + } + Ok(()) + }); + + self.stack_ids.push(StackInfo { + type_id: target.type_id, + type_name: target.type_name, + padding: new_padding, + size: target.size, + align: target.align, + raw_dropper: target.dropper, + associated: target.associated, + }); } /// Returns the padding flags for the top N entries of the type stack. @@ -230,17 +558,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) } @@ -533,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" ); @@ -635,6 +979,92 @@ 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 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(), + offset, + size: elem_info.size, + align: elem_info.align, + dropper: elem_info.raw_dropper, + associated: elem_info.associated.clone(), + }; + offset += elem_info.size; + offset = align_index(align_so_far, offset); + 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)] @@ -899,4 +1329,341 @@ 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); + } + + #[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); + // 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] + 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 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 + // 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 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); + // 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] + 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); + } + + #[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)"); + } + + /// `(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(()) + } } diff --git a/cel-runtime/src/raw_stack.rs b/cel-runtime/src/raw_stack.rs index f21b5a8..0ceb4e1 100644 --- a/cel-runtime/src/raw_stack.rs +++ b/cel-runtime/src/raw_stack.rs @@ -1,373 +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. - 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| 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). + /// + /// `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" + ); + } +} 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. 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..9bd03bd --- /dev/null +++ b/docs/superpowers/specs/2026-07-07-cel-tuples-design.md @@ -0,0 +1,243 @@ +# 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. 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 = []