From 8d97ebd303102ddc0fb607cd47a26a96a5cfefa6 Mon Sep 17 00:00:00 2001 From: Eugene Talagrand Date: Tue, 18 Nov 2025 22:13:37 -0800 Subject: [PATCH 1/6] Introduce the ability to register type-safe custom builtin extension functions --- README.md | 65 ++++ src/ast.rs | 45 ++- src/evaluator.rs | 334 +++++++++++++++++- src/intooperation.rs | 784 +++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 1 + 5 files changed, 1221 insertions(+), 8 deletions(-) create mode 100644 src/intooperation.rs diff --git a/README.md b/README.md index 0bd2daa..5579d56 100644 --- a/README.md +++ b/README.md @@ -87,6 +87,71 @@ fn main() -> Result<(), Box> { } ``` +### Registering Custom Builtins + +You can extend the evaluator with your own builtins. There are two +APIs, both of which end up using the same underlying machinery: + +1. **Raw slice API** (closest to the core engine): + +```rust +use rulesxp::ast::Value; +use rulesxp::evaluator::{create_global_env, Environment}; +use rulesxp::Error; + +fn my_custom_function(args: &[Value]) -> Result { + println!("called with {} args", args.len()); + Ok(Value::Unspecified) +} + +let mut env: Environment = create_global_env(); +env.register_builtin_function("my-func", my_custom_function); +// Now (my-func) / {"my-func": [...]} can be used from Scheme / JSONLogic +``` + +2. **Typed API** (ergonomic Rust signatures, automatic conversion): + +```rust +use rulesxp::ast::Value; +use rulesxp::evaluator::{create_global_env, Environment}; + +// Fixed arity: arguments are converted from `Value` automatically +fn add2(a: i64, b: i64) -> i64 { + a + b +} + +// Zero-argument builtin +fn forty_two() -> i64 { 42 } + +// List argument converted to a slice of `Value` +fn first_and_list_len(args: &[Value]) -> Value { + Value::List(vec![ + args.first().cloned().unwrap_or(Value::Unspecified), + Value::Number(args.len() as i64), + ]) +} + +let mut env: Environment = create_global_env(); +env.register_builtin_operation("add2", add2); +env.register_builtin_operation("forty-two", forty_two); +env.register_builtin_operation("first-and-len", first_and_list_len); + +// These builtins are then available from evaluated expressions +``` + +The typed API currently supports: + +- **Parameter types**: `i64`, `bool`, `&str`, borrowed values `&Value`, + and list arguments via slices such as `&[i64]`, `&[bool]`, `&[&str]`, + and `&[Value]` (when the call site passes a list value). +- **Return types**: any type implementing the internal `IntoValue` trait + (currently `Value`, `i64`, `bool`, `String`, and `&str`), or + `Result` where `R` is one of those and `E: Display`. + +Arity is enforced automatically; conversion errors become `TypeError`, +and any user error from a `Result<_, E>` is wrapped into +`Error::EvalError`. + ### Command Line Tools #### Interactive REPL (a demo is also available) diff --git a/src/ast.rs b/src/ast.rs index eb7726f..ad70644 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -7,8 +7,8 @@ /// types, making it easy to build Values from Rust literals, arrays, slices, and /// vectors. Equality and display logic are customized to match Scheme semantics, including /// round-trip compatibility for precompiled operations. -use crate::Error; use crate::builtinops::BuiltinOp; +use crate::intooperation::OperationFn; /// Type alias for number values in interpreter pub(crate) type NumberType = i64; @@ -55,7 +55,7 @@ pub(crate) fn is_valid_symbol(name: &str) -> bool { /// - `val(42)` for values, `sym("name")` for symbols, `nil()` for empty lists /// - `val([1, 2, 3])` for homogeneous lists /// - `val(vec![sym("op"), val(42)])` for mixed lists -#[derive(Debug, Clone)] +#[derive(Clone)] pub enum Value { /// Numbers (integers only) Number(NumberType), @@ -77,7 +77,10 @@ pub enum Value { /// Uses id string for equality comparison instead of function pointer BuiltinFunction { id: String, - func: fn(&[Value]) -> Result, + // Stored as an Arc to allow dynamic wrapping of typed Rust functions/closures. + // Trait object enables registering strongly typed functions (e.g. fn(i64, i64)->i64) + // that are automatically converted to the canonical evaluator signature. + func: std::sync::Arc, }, /// User-defined functions (params, body, closure env) Function { @@ -90,6 +93,42 @@ pub enum Value { Unspecified, } +impl std::fmt::Debug for Value { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Value::Number(n) => write!(f, "Number({n})"), + Value::Symbol(s) => write!(f, "Symbol({s})"), + Value::String(s) => write!(f, "String(\"{s}\")"), + Value::Bool(b) => write!(f, "Bool({b})"), + Value::List(list) => { + write!(f, "List(")?; + for (i, v) in list.iter().enumerate() { + if i > 0 { + write!(f, ", ")?; + } + write!(f, "{:?}", v)?; + } + write!(f, ")") + } + Value::PrecompiledOp { op_id, args, .. } => { + write!(f, "PrecompiledOp({op_id}, args=[")?; + for (i, a) in args.iter().enumerate() { + if i > 0 { + write!(f, ", ")?; + } + write!(f, "{:?}", a)?; + } + write!(f, "])") + } + Value::BuiltinFunction { id, .. } => write!(f, "BuiltinFunction({id})"), + Value::Function { params, body, .. } => { + write!(f, "Function(params={params:?}, body={:?})", body) + } + Value::Unspecified => write!(f, "Unspecified"), + } + } +} + // From trait implementations for Value - enables .into() conversion impl From<&str> for Value { fn from(s: &str) -> Self { diff --git a/src/evaluator.rs b/src/evaluator.rs index a48a649..c01db02 100644 --- a/src/evaluator.rs +++ b/src/evaluator.rs @@ -1,8 +1,10 @@ use crate::Error; use crate::MAX_EVAL_DEPTH; use crate::ast::Value; -use crate::builtinops::get_builtin_ops; +use crate::builtinops::{Arity, get_builtin_ops}; +use crate::intooperation::{IntoFixedOperation, IntoVariadicOperation, OperationFn}; use std::collections::HashMap; +use std::sync::Arc; /// Environment for variable bindings #[derive(Debug, Clone, PartialEq, Default)] @@ -38,9 +40,15 @@ impl Environment { /// Register a custom builtin function in the environment for use by Scheme/JSONLogic. /// + /// This is the low-level API: it accepts a function that already + /// works on `&[Value]` and returns `Result`. Internally + /// it is wired through the same machinery as + /// [`Environment::register_builtin_operation`]. For most new code, + /// prefer the typed API instead of manipulating `Value` directly. + /// /// # Arguments /// * `name` - The name by which the function can be called - /// * `func` - A function pointer that takes a slice of Values and returns a Result + /// * `func` - A function pointer that takes a slice of `Value` and returns a `Result` /// /// # Example /// ``` @@ -62,11 +70,116 @@ impl Environment { name: &str, func: fn(&[Value]) -> Result, ) { + // Wrap the raw slice-based function into the canonical + // `OperationFn` so it can be stored directly as a + // `BuiltinFunction`. + let f = func; + let wrapped: Arc = Arc::new(move |args: Vec| f(&args)); + + self.bindings.insert( + name.to_string(), + Value::BuiltinFunction { + id: name.to_string(), + func: wrapped, + }, + ); + } + + /// Register a strongly-typed Rust function as a builtin operation using + /// automatic argument extraction and result conversion. + /// + /// This allows writing natural Rust functions like: + /// + /// ```rust,ignore + /// fn add(a: i64, b: i64) -> i64 { a + b } + /// let mut env = rulesxp::evaluator::create_global_env(); + /// env.register_builtin_operation("add", add); + /// // Now (+ 2 3) and (add 2 3) both work if + also registered + /// ``` + /// + /// Builtins can also return `Result` where `E: Display`: + /// + /// ```rust,ignore + /// use std::fmt::Display; + /// + /// fn safe_div(a: i64, b: i64) -> Result { + /// if b == 0 { + /// Err("division by zero") + /// } else { + /// Ok(a / b) + /// } + /// } + /// + /// let mut env = rulesxp::evaluator::create_global_env(); + /// env.register_builtin_operation("safe-div", safe_div); + /// ``` + /// + /// Supported parameter types (initial set): + /// - `i64` (number) + /// - `bool` (boolean) + /// - `&str` (borrowed string slices) + /// - `&Value` (borrowed access to the raw AST value) + /// - list arguments via slices such as `&[i64]`, `&[bool]`, + /// `&[&str]`, and `&[Value]` (when the call site passes a list) + /// + /// Supported return types: + /// - any type implementing the internal `IntoValue` trait + /// (currently `Value`, `i64`, `bool`, `String`, and `&str`) + /// - `Result` where `R` is one of the above and `E: Display` + /// + /// If you need true rest-parameter / variadic behavior (functions + /// that see all arguments as `&[Value]` or `&[i64]`), use + /// [`Environment::register_variadic_builtin_operation`] instead. + /// + /// Arity is enforced automatically. Conversion errors yield `TypeError`, + /// and any `E` from a `Result` is converted to `Error::EvalError`. + pub fn register_builtin_operation(&mut self, name: &str, func: F) + where + F: IntoFixedOperation + 'static, + { + let wrapped = func.into_operation(); + self.bindings.insert( + name.to_string(), + Value::BuiltinFunction { + id: name.to_string(), + func: wrapped, + }, + ); + } + + /// Register a variadic builtin operation with explicit arity metadata. + /// + /// This is intended for functions whose Rust signature includes a + /// "rest" parameter, either as a raw slice of values, + /// `fn(&[Value]) -> Result`, or as a typed slice such + /// as `fn(&[i64])` or `fn(i64, &[i64])`. + /// Fixed-arity functions should use + /// [`Environment::register_builtin_operation`] instead. + /// + /// The provided [`Arity`] is used to validate the total number of + /// arguments at call time, since minimum/maximum argument counts for + /// variadic operations are not always derivable from the Rust type + /// signature alone. + pub fn register_variadic_builtin_operation( + &mut self, + name: &str, + arity: Arity, + func: F, + ) where + F: IntoVariadicOperation + 'static, + { + let inner = func.into_variadic_operation(); + let arity_for_closure = arity; + let wrapped = std::sync::Arc::new(move |args: Vec| { + arity_for_closure.validate(args.len())?; + inner(args) + }); + self.bindings.insert( name.to_string(), Value::BuiltinFunction { id: name.to_string(), - func, + func: wrapped, }, ); } @@ -203,7 +316,7 @@ fn eval_list(elements: &[Value], env: &mut Environment, depth: usize) -> Result< // Apply the function match &func { // Dynamic function calls - Value::BuiltinFunction { func: f, .. } => f(&args), + Value::BuiltinFunction { func, .. } => func(args), Value::Function { params, body, @@ -409,11 +522,12 @@ pub fn create_global_env() -> Environment { for builtin_op in get_builtin_ops() { if let crate::builtinops::OpKind::Function(func) = &builtin_op.op_kind { // Use BuiltinFunction for environment bindings (dynamic calls through symbols) + let f = *func; env.define( builtin_op.scheme_id.to_owned(), Value::BuiltinFunction { id: builtin_op.scheme_id.to_owned(), - func: *func, + func: Arc::new(move |args: Vec| f(&args)), }, ); } @@ -426,9 +540,219 @@ pub fn create_global_env() -> Environment { #[expect(clippy::unwrap_used)] // test code OK mod tests { use super::*; + use crate::Error; use crate::ast::{nil, sym, val}; + use crate::intooperation::Rest; use crate::scheme::parse_scheme; + #[test] + fn test_register_builtin_operation_add() { + fn add(a: i64, b: i64) -> i64 { + a + b + } + let mut env = create_global_env(); + env.register_builtin_operation::<_, (i64, i64), i64>("add2", add); + let expr = parse_scheme("(add2 7 5)").unwrap(); + let result = eval(&expr, &mut env).unwrap(); + assert_eq!(result, Value::Number(12)); + } + + #[test] + fn test_register_builtin_operation_zero_arg() { + fn forty_two() -> i64 { + 42 + } + + let mut env = create_global_env(); + env.register_builtin_operation("forty-two", forty_two); + + let expr = parse_scheme("(forty-two)").unwrap(); + let result = eval(&expr, &mut env).unwrap(); + assert_eq!(result, Value::Number(42)); + } + + #[test] + fn test_register_builtin_operation_result_builtin() { + fn safe_div(a: i64, b: i64) -> Result { + if b == 0 { + Err("division by zero") + } else { + Ok(a / b) + } + } + + let mut env = create_global_env(); + // For functions returning Result<_, E>, Rust currently requires + // specifying the argument tuple `Args` and the output type `R` + // when using the typed API. + env.register_builtin_operation::<_, (i64, i64), i64>("safe-div", safe_div); + + // Success case + let expr_ok = parse_scheme("(safe-div 6 3)").unwrap(); + let result_ok = eval(&expr_ok, &mut env).unwrap(); + assert_eq!(result_ok, Value::Number(2)); + + // Error case: division by zero surfaces as EvalError containing the message + let expr_err = parse_scheme("(safe-div 1 0)").unwrap(); + let err = eval(&expr_err, &mut env).unwrap_err(); + let msg = format!("{err}"); + assert!(msg.contains("division by zero")); + } + + #[test] + fn test_register_builtin_operation_vec_i64_list_param() { + fn sum_list(nums: &[i64]) -> i64 { + nums.iter().copied().sum() + } + + let mut env = create_global_env(); + env.register_builtin_operation::<_, (&[i64],), i64>("sum-list", sum_list); + + let expr = parse_scheme("(sum-list '(1 2 3 4))").unwrap(); + let result = eval(&expr, &mut env).unwrap(); + assert_eq!(result, Value::Number(10)); + } + + #[test] + fn test_register_builtin_operation_with_rest_values() { + fn first_and_rest_count(args: Rest) -> Result { + let args_slice = args.as_slice(); + + if args_slice.is_empty() { + return Err(Error::arity_error(1, 0)); + } + + let first_number = match args_slice.first().expect("arity checked above") { + Value::Number(n) => *n, + _ => return Err(Error::TypeError("first argument must be a number".into())), + }; + + Ok(Value::List(vec![ + Value::Number(first_number), + Value::Number((args_slice.len() - 1) as i64), + ])) + } + + let mut env = create_global_env(); + env.register_variadic_builtin_operation::<_, (Rest,), Value>( + "first-rest-count", + Arity::AtLeast(1), + first_and_rest_count, + ); + + let expr = parse_scheme("(first-rest-count 42 \"x\" #t 7)").unwrap(); + let result = eval(&expr, &mut env).unwrap(); + assert_eq!( + result, + Value::List(vec![Value::Number(42), Value::Number(3)]) + ); + } + + #[test] + fn test_register_builtin_operation_varargs_borrowed_values_all() { + fn count_numbers(args: Rest) -> Result { + let args_slice = args.as_slice(); + + let count = args_slice + .iter() + .filter(|v| matches!(v, Value::Number(_))) + .count() as i64; + + Ok(Value::Number(count)) + } + + let mut env = create_global_env(); + env.register_variadic_builtin_operation::<_, (Rest,), Value>( + "count-numbers", + Arity::AtLeast(0), + count_numbers, + ); + + let expr = parse_scheme("(count-numbers 1 \"x\" 2 #t 3)").unwrap(); + let result = eval(&expr, &mut env).unwrap(); + assert_eq!(result, Value::Number(3)); + } + + #[test] + fn test_register_builtin_operation_varargs_all_i64() { + fn sum_all(nums: Rest) -> i64 { + nums.iter().copied().sum() + } + + let mut env = create_global_env(); + env.register_variadic_builtin_operation::<_, (Rest,), i64>( + "sum-varargs-all", + Arity::AtLeast(0), + sum_all, + ); + + let expr = parse_scheme("(sum-varargs-all 1 2 3 4)").unwrap(); + let result = eval(&expr, &mut env).unwrap(); + assert_eq!(result, Value::Number(10)); + } + + #[test] + fn test_register_builtin_operation_varargs_fixed_plus_rest() { + fn weighted_sum(weight: i64, nums: Rest) -> i64 { + weight * nums.iter().copied().sum::() + } + + let mut env = create_global_env(); + env.register_variadic_builtin_operation::<_, (i64, Rest), i64>( + "weighted-sum", + Arity::AtLeast(1), + weighted_sum, + ); + + let expr = parse_scheme("(weighted-sum 2 1 2 3)").unwrap(); + let result = eval(&expr, &mut env).unwrap(); + assert_eq!(result, Value::Number(12)); + } + + #[test] + fn test_register_variadic_builtin_operation_with_explicit_arity() { + fn sum_all(nums: Rest) -> i64 { + nums.iter().copied().sum() + } + + let mut env = create_global_env(); + env.register_variadic_builtin_operation::<_, (Rest,), i64>( + "sum-all-min1", + Arity::AtLeast(1), + sum_all, + ); + + // Valid call: three numeric arguments. + let expr_ok = parse_scheme("(sum-all-min1 1 2 3)").unwrap(); + let result_ok = eval(&expr_ok, &mut env).unwrap(); + assert_eq!(result_ok, Value::Number(6)); + + // Invalid call: zero arguments should fail Arity::AtLeast(1). + let expr_err = parse_scheme("(sum-all-min1)").unwrap(); + let err = eval(&expr_err, &mut env).unwrap_err(); + match err { + crate::Error::ArityError { .. } => {} + other => panic!("expected ArityError, got {other:?}"), + } + } + + #[test] + fn test_builtin_comparison_dynamic_uses_typed_mechanism() { + let mut env = create_global_env(); + + // Dynamic higher-order use of a builtin comparison: the `>` + // operator is passed as a value and called with three + // arguments. This exercises dynamic builtin calls through + // the environment using the shared builtin registry. + let expr = parse_scheme("((lambda (op a b c) (op a b c)) > 9 6 2)").unwrap(); + let result = eval(&expr, &mut env).unwrap(); + assert_eq!(result, Value::Bool(true)); + + let expr_false = parse_scheme("((lambda (op a b c) (op a b c)) > 9 6 7)").unwrap(); + let result_false = eval(&expr_false, &mut env).unwrap(); + assert_eq!(result_false, Value::Bool(false)); + } + /// Test result variants for comprehensive testing #[derive(Debug)] enum TestResult { diff --git a/src/intooperation.rs b/src/intooperation.rs new file mode 100644 index 0000000..afb03a0 --- /dev/null +++ b/src/intooperation.rs @@ -0,0 +1,784 @@ +use crate::Error; +use crate::ast::Value; +use std::fmt::Display; +use std::sync::Arc; + +/// Canonical erased builtin function type used by the evaluator. +/// +/// Builtins receive ownership of their argument vector, enabling +/// implementations that consume or rearrange arguments if desired. +pub type OperationFn = dyn Fn(Vec) -> Result + Send + Sync; + +// ===================================================================== +// Internal machinery for fixed-arity argument conversion +// +// All `unsafe` in this crate is deliberately confined to this module. +// The only primitive is `widen_slice_lifetime`, which is used by the +// `FromParam` implementations for slice parameters and by the +// variadic "rest" machinery for slice-like parameters. See the docs +// on that function and on `FromParam` for the full safety argument +// and the limitations in Rust's current type system that force this +// design. +// ===================================================================== + +mod sealed { + //! Types that may be used as builtin parameters via `FromParam`. + //! + //! This `Sealed` trait prevents code outside this module from + //! implementing `FromParam`, which keeps the safety invariants + //! around `widen_slice_lifetime` local and auditable. + //! + //! To add a new builtin parameter type, you must: + //! - add it here, and + //! - add a matching `FromParam` implementation in this module. + //! + //! Slice-like types must be carefully reviewed, as they typically + //! rely on `widen_slice_lifetime` and its safety invariants. + + pub trait Sealed {} + + impl Sealed for &super::Value {} + impl Sealed for i64 {} + impl Sealed for bool {} + impl Sealed for &str {} + impl Sealed for &[i64] {} + impl Sealed for &[bool] {} + impl Sealed for &[&str] {} + impl Sealed for &[super::Value] {} +} + +/// Core trait used by the fixed-arity adapters to turn `Value` nodes +/// into strongly-typed parameters. +/// +/// This trait is **internal** to this module and sealed; the set of +/// supported parameter types is fixed to those we explicitly impl it +/// for above in `sealed`. +/// +/// For example, a builtin like +/// `fn sum(first: i64, rest: &[i64]) -> Result` +/// will be invoked via `FromParam` implementations for `i64` and +/// `&[i64]`. +/// +/// The associated `Storage<'a>` type gives each parameter kind a place +/// to put temporary data (typically a `Vec` for slice parameters). +/// The `Param<'a>` family then describes the parameter type as seen by +/// the builtin function for a given lifetime `'a` of the underlying +/// AST `Value`s. +/// +/// ### Why is there `unsafe` here? +/// +/// For scalar parameters (`i64`, `bool`, `&str`, `&Value`) the +/// implementation is entirely safe. For slice parameters like +/// `&[i64]`, `&[bool]`, and `&[&str]` we must populate a `Vec` and +/// then hand the builtin a slice `&[T]` that appears to live for +/// `Param<'a>`'s lifetime. In reality the slice is only valid for the +/// duration of the adapter's stack frame that owns the `Vec`. +/// +/// The fixed-arity adapters call `FromParam::from_arg` inside a +/// function that immediately invokes the builtin and then drops all +/// `Storage` values before returning. Expressing "this reference is +/// valid only for the body of this call" is not something Rust's +/// current type system can do in a reusable trait, so we use a small +/// `unsafe` helper to widen the slice lifetime while **relying on the +/// calling pattern** to keep things sound. +/// +/// Concretely, Rust today lacks: +/// - a way to express an "intersection" lifetime like +/// `min('vec, 'a)` for data borrowed from both a local `Vec` and +/// the input `&'a Value`; +/// - the ability to state, in the trait, that `Param<'a>` values +/// cannot outlive the specific adapter call that created them, +/// even though the adapter is used under a higher-ranked +/// `for<'a>` bound; +/// - a safe standard-library abstraction that captures this +/// "stack-bounded borrow from scratch space" pattern for slices. +/// +/// Instead, we: +/// - keep `FromParam` sealed and internal to this module; +/// - centralise the `unsafe` in `widen_slice_lifetime` with a detailed +/// `# Safety` explanation; and +/// - structure the fixed-arity adapters so that `Param<'a>` is never +/// stored or allowed to escape the call to the builtin. +pub(crate) trait FromParam: sealed::Sealed { + /// Per-call scratch space, scoped to the lifetime of the argument + /// values. For scalar parameters this is typically `()`; for slices + /// it is usually a `Vec` that we borrow from. + type Storage<'a>: Default; + + /// The parameter type as seen by the builtin for a given lifetime + /// of the underlying AST values. + type Param<'a>; + + /// Convert a single AST argument into this parameter type. + fn from_arg<'a>( + value: &'a Value, + storage: &mut Self::Storage<'a>, + ) -> Result, Error>; +} + +impl FromParam for &Value { + type Storage<'a> = (); + type Param<'a> = &'a Value; + + fn from_arg<'a>( + value: &'a Value, + _storage: &mut Self::Storage<'a>, + ) -> Result, Error> { + Ok(value) + } +} + +impl FromParam for i64 { + type Storage<'a> = (); + type Param<'a> = i64; + + fn from_arg<'a>( + value: &'a Value, + _storage: &mut Self::Storage<'a>, + ) -> Result, Error> { + if let Value::Number(n) = value { + Ok(*n) + } else { + Err(Error::TypeError("expected number".into())) + } + } +} + +impl FromParam for bool { + type Storage<'a> = (); + type Param<'a> = bool; + + fn from_arg<'a>( + value: &'a Value, + _storage: &mut Self::Storage<'a>, + ) -> Result, Error> { + if let Value::Bool(b) = value { + Ok(*b) + } else { + Err(Error::TypeError("expected boolean".into())) + } + } +} + +impl FromParam for &str { + type Storage<'a> = (); + type Param<'a> = &'a str; + + fn from_arg<'a>( + value: &'a Value, + _storage: &mut Self::Storage<'a>, + ) -> Result, Error> { + if let Value::String(s) = value { + Ok(s.as_str()) + } else { + Err(Error::TypeError("expected string".into())) + } + } +} + +/// Widen the lifetime of a slice to match the higher-ranked lifetime +/// used by the fixed-arity adapters. +/// +/// This function is the **only** place `unsafe` is used in the +/// argument-conversion machinery. It is private to this module and is +/// only called from `FromParam` and variadic rest-parameter +/// implementations for slice-like parameters. +/// +/// # Safety +/// +/// - The caller must ensure that the returned reference does not +/// outlive the stack frame that owns the backing storage of +/// `slice` (typically a local `Vec`). +/// - In this module, that guarantee is provided by the fixed-arity +/// `IntoOperation` implementations: they allocate all `Storage` +/// values as locals, call `FromParam::from_arg` to create +/// `Param<'a>` values, immediately invoke the builtin `F`, and then +/// drop all `Storage` before returning. +/// - `Param<'a>` values are never stored in the returned +/// `OperationFn` closure or in any other longer-lived structure; +/// they are passed directly to the builtin and then discarded. +/// +/// Rust's type system cannot currently express this "stack-bounded +/// borrow through a trait" pattern in a fully safe way, because the +/// trait uses a higher-ranked lifetime `for<'a>` while the actual +/// borrow from the local `Vec` is tied to a particular call frame. +/// The combination of sealing, centralising this helper, and the +/// structure of the fixed-arity adapters is what makes this sound. +fn widen_slice_lifetime<'short, 'long, T>(slice: &'short [T]) -> &'long [T] { + // SAFETY: Callers in this module uphold the safety contract above + // by never letting the returned reference escape the adapter call + // that owns the backing storage. + unsafe { &*(slice as *const [T]) } +} + +/// Typed list parameters for fixed-arity functions: a single +/// `Value::List` argument converted to a slice of elements. +impl FromParam for &[i64] { + type Storage<'a> = Vec; + type Param<'a> = &'a [i64]; + + fn from_arg<'a>( + value: &'a Value, + storage: &mut Self::Storage<'a>, + ) -> Result, Error> { + storage.clear(); + let items = match value { + Value::List(items) => items, + _ => return Err(Error::TypeError("expected list".into())), + }; + + for item in items { + match item { + Value::Number(n) => storage.push(*n), + _ => return Err(Error::TypeError("expected number in list".into())), + } + } + + let slice: &[i64] = storage.as_slice(); + let slice: &'a [i64] = widen_slice_lifetime(slice); + Ok(slice) + } +} + +impl FromParam for &[bool] { + type Storage<'a> = Vec; + type Param<'a> = &'a [bool]; + + fn from_arg<'a>( + value: &'a Value, + storage: &mut Self::Storage<'a>, + ) -> Result, Error> { + storage.clear(); + let items = match value { + Value::List(items) => items, + _ => return Err(Error::TypeError("expected list".into())), + }; + + for item in items { + match item { + Value::Bool(b) => storage.push(*b), + _ => return Err(Error::TypeError("expected boolean in list".into())), + } + } + + let slice: &[bool] = storage.as_slice(); + let slice: &'a [bool] = widen_slice_lifetime(slice); + Ok(slice) + } +} + +impl FromParam for &[&str] { + type Storage<'a> = Vec<&'a str>; + type Param<'a> = &'a [&'a str]; + + fn from_arg<'a>( + value: &'a Value, + storage: &mut Self::Storage<'a>, + ) -> Result, Error> { + storage.clear(); + let items = match value { + Value::List(items) => items, + _ => return Err(Error::TypeError("expected list".into())), + }; + + for item in items { + match item { + Value::String(s) => storage.push(s.as_str()), + _ => return Err(Error::TypeError("expected string in list".into())), + } + } + + let slice: &[&str] = storage.as_slice(); + let slice: &'a [&'a str] = widen_slice_lifetime(slice); + Ok(slice) + } +} + +impl FromParam for &[Value] { + type Storage<'a> = (); + type Param<'a> = &'a [Value]; + + fn from_arg<'a>( + value: &'a Value, + _storage: &mut Self::Storage<'a>, + ) -> Result, Error> { + if let Value::List(items) = value { + Ok(items.as_slice()) + } else { + Err(Error::TypeError("expected list".into())) + } + } +} + +/// Convert strongly-typed Rust results into AST values. +pub trait IntoValue { + fn into_value(self) -> Value; +} + +impl IntoValue for Value { + fn into_value(self) -> Value { + self + } +} + +impl IntoValue for i64 { + fn into_value(self) -> Value { + Value::Number(self) + } +} + +impl IntoValue for bool { + fn into_value(self) -> Value { + Value::Bool(self) + } +} + +impl IntoValue for String { + fn into_value(self) -> Value { + Value::String(self) + } +} + +impl IntoValue for &str { + fn into_value(self) -> Value { + Value::String(self.to_owned()) + } +} + +/// Normalize both plain values and `Result`-returning functions into `Result`. +pub trait IntoResult { + fn into_result(self) -> Result; +} + +impl IntoResult for T +where + T: IntoValue, +{ + fn into_result(self) -> Result { + Ok(self) + } +} + +impl IntoResult for Result +where + E: Display, +{ + fn into_result(self) -> Result { + self.map_err(|e| Error::EvalError(e.to_string())) + } +} + +/// Marker type for variadic "rest" parameters. +/// +/// `Rest` is a thin newtype over `Vec` that behaves like a +/// slice via `Deref`/`AsRef`. Variadic adapters simply build a fresh +/// `Vec` for the rest arguments and pass it to the user function; +/// no unsafe code or lifetime juggling is required. +#[derive(Debug, Clone)] +pub struct Rest { + items: Vec, +} + +impl Rest { + pub(crate) fn new(items: Vec) -> Self { + Rest { items } + } + + /// Borrow the rest arguments as a slice. + pub fn as_slice(&self) -> &[T] { + &self.items + } + + /// Consume the wrapper and return the owned vector of rest + /// arguments. + pub fn into_vec(self) -> Vec { + self.items + } +} + +impl std::ops::Deref for Rest { + type Target = [T]; + + fn deref(&self) -> &Self::Target { + &self.items + } +} + +impl AsRef<[T]> for Rest { + fn as_ref(&self) -> &[T] { + &self.items + } +} + +/// Internal trait used by the variadic adapters to convert the tail +/// of the argument list into a typed rest parameter. +/// +/// The associated `Param<'a>` type is generic over the lifetime of the +/// underlying argument slice. Implementations for owned element types +/// (`Rest`, `Rest`) allocate a fresh `Vec`; borrowed +/// variants like `Rest<&str>` borrow directly from the `Value`s. +trait FromRestParam { + type Param<'a>; + + fn from_rest<'a>(values: &'a [Value]) -> Result, Error>; +} + +/// Convert a strongly-typed Rust function or closure into the erased +/// [`OperationFn`], parameterized by an argument tuple type. +pub trait IntoOperation { + fn into_operation(self) -> Arc; +} + +/// Marker trait for fixed-arity operations (no rest parameters). +/// +/// Implemented for Rust functions and closures whose arguments are +/// drawn from types implementing [`FromParam`]. This is a thin wrapper +/// over [`IntoOperation`] so that the evaluator API can distinguish +/// fixed-arity from variadic registrations. +pub trait IntoFixedOperation: IntoOperation {} + +impl IntoFixedOperation for F where F: IntoOperation {} + +/// Trait for operations registered via the variadic API. +/// +/// Implemented for functions whose Rust signature includes a variadic +/// "rest" parameter, expressed using the [`Rest`] marker type in the +/// last position of the argument tuple. +pub trait IntoVariadicOperation { + fn into_variadic_operation(self) -> Arc; +} + +// ===================================================================== +// Variadic helpers used by Rest-based adapters +// ===================================================================== + +// --- `FromRestParam` implementations for supported rest element types --- + +// Numeric rest parameter: collects all numeric arguments into an owned +// `Vec` for convenient processing. +impl FromRestParam for Rest { + type Param<'a> = Rest; + + fn from_rest<'a>(values: &'a [Value]) -> Result, Error> { + let mut nums: Vec = Vec::new(); + + for v in values { + match v { + Value::Number(n) => nums.push(*n), + _ => return Err(Error::TypeError("expected number".into())), + } + } + + Ok(Rest::new(nums)) + } +} + +// Boolean rest parameter: collects all boolean arguments into an owned +// `Vec`. +impl FromRestParam for Rest { + type Param<'a> = Rest; + + fn from_rest<'a>(values: &'a [Value]) -> Result, Error> { + let mut bools: Vec = Vec::new(); + + for v in values { + match v { + Value::Bool(b) => bools.push(*b), + _ => return Err(Error::TypeError("expected boolean".into())), + } + } + + Ok(Rest::new(bools)) + } +} +// Borrowed string rest parameter: builds a `Rest<&str>` borrowing string +// slices directly from the argument values. +impl FromRestParam for Rest<&str> { + type Param<'a> = Rest<&'a str>; + + fn from_rest<'a>(values: &'a [Value]) -> Result, Error> { + let mut items: Vec<&'a str> = Vec::new(); + + for v in values { + match v { + Value::String(s) => items.push(s.as_str()), + _ => return Err(Error::TypeError("expected string".into())), + } + } + + Ok(Rest::new(items)) + } +} + +// --- 0-arg prefix: functions that see all args as the rest parameter --- + +// Specialised implementation for `Rest` that **consumes** the +// incoming `Vec` without cloning. This is the most direct and +// allocation-free way to expose variadic arguments when the builtin is +// happy to own the `Value`s. +impl IntoVariadicOperation<(Rest,), R> for F +where + F: Fn(Rest) -> FR + Send + Sync + 'static, + FR: IntoResult + 'static, + R: IntoValue + 'static, +{ + fn into_variadic_operation(self) -> Arc { + Arc::new(move |args: Vec| { + (|| { + let rest_param = Rest::new(args); + let result: FR = (self)(rest_param); + let value: R = result.into_result()?; + Ok(value.into_value()) + })() + }) + } +} + +// Generic implementation for rest-only functions using element types +// such as `Rest`, `Rest`, or `Rest<&str>`, built from a +// borrowed slice of the argument `Value`s via `FromRestParam`. +impl IntoVariadicOperation<(RestT,), R> for F +where + RestT: FromRestParam, + F: for<'a> Fn(::Param<'a>) -> FR + Send + Sync + 'static, + FR: IntoResult + 'static, + R: IntoValue + 'static, +{ + fn into_variadic_operation(self) -> Arc { + Arc::new(move |args: Vec| { + (|| { + let rest_param = ::from_rest(&args[..])?; + let result: FR = (self)(rest_param); + let value: R = result.into_result()?; + Ok(value.into_value()) + })() + }) + } +} + +/// Helper macro to implement `IntoVariadicOperation` for functions with a fixed +/// prefix followed by a `Rest<_>` parameter in the last position. +/// +/// The fixed prefix is handled via `FromParam` in exactly the same way +/// as the non-variadic adapters. The rest parameter is constructed via +/// `FromRestParam` from the remaining arguments. +macro_rules! impl_into_variadic_operation_for_prefix_and_rest { + ($prefix:expr, $( $idx:tt => $v:ident, $p:ident : $A:ident ),+ ) => { + impl IntoVariadicOperation<($( $A, )+ RestT,), R> for F + where + RestT: FromRestParam, + $( $A: FromParam, )+ + F: for<'a> Fn( + $( <$A as FromParam>::Param<'a> ),+, + ::Param<'a>, + ) -> FR + + Send + + Sync + + 'static, + FR: IntoResult + 'static, + R: IntoValue + 'static, + { + fn into_variadic_operation(self) -> Arc { + Arc::new(move |args: Vec| { + match &args[..] { + [ $( $v ),+, rest @ .. ] => { + (|| { + let mut prefix_storage: ( + $( <$A as FromParam>::Storage<'_>, )+ + ) = Default::default(); + + $( + let $p: <$A as FromParam>::Param<'_> = + <$A as FromParam>::from_arg( + $v, + &mut prefix_storage.$idx, + )?; + )+ + + let rest_param = ::from_rest(rest)?; + + let result: FR = (self)( $( $p ),+, rest_param ); + let value: R = result.into_result()?; + Ok(value.into_value()) + })() + } + _ => Err(Error::arity_error($prefix, args.len())), + } + }) + } + } + }; +} + +impl_into_variadic_operation_for_prefix_and_rest!(1, 0 => v0, p0: A1); +impl_into_variadic_operation_for_prefix_and_rest!(2, 0 => v0, p0: A1, 1 => v1, p1: A2); +impl_into_variadic_operation_for_prefix_and_rest!(3, 0 => v0, p0: A1, 1 => v1, p1: A2, 2 => v2, p2: A3); +impl_into_variadic_operation_for_prefix_and_rest!(4, 0 => v0, p0: A1, 1 => v1, p1: A2, 2 => v2, p2: A3, 3 => v3, p3: A4); +impl_into_variadic_operation_for_prefix_and_rest!(5, 0 => v0, p0: A1, 1 => v1, p1: A2, 2 => v2, p2: A3, 3 => v3, p3: A4, 4 => v4, p4: A5); +impl_into_variadic_operation_for_prefix_and_rest!(6, 0 => v0, p0: A1, 1 => v1, p1: A2, 2 => v2, p2: A3, 3 => v3, p3: A4, 4 => v4, p4: A5, 5 => v5, p5: A6); +impl_into_variadic_operation_for_prefix_and_rest!(7, 0 => v0, p0: A1, 1 => v1, p1: A2, 2 => v2, p2: A3, 3 => v3, p3: A4, 4 => v4, p4: A5, 5 => v5, p5: A6, 6 => v6, p6: A7); + +/// Specialised prefix+rest implementations for `Rest` that +/// **consume** the incoming `Vec` tail without cloning. These +/// mirror the generic `impl_into_variadic_operation_for_prefix_and_rest` +/// macros but use `Vec::split_off` to move the rest arguments into a +/// fresh `Vec` that backs the `Rest` parameter. +macro_rules! impl_into_variadic_operation_for_prefix_and_rest_value { + ($prefix:expr, $( $idx:tt => $v:ident, $p:ident : $A:ident ),+ ) => { + impl IntoVariadicOperation<($( $A, )+ Rest,), R> for F + where + $( $A: FromParam, )+ + F: for<'a> Fn( + $( <$A as FromParam>::Param<'a> ),+, + Rest, + ) -> FR + + Send + + Sync + + 'static, + FR: IntoResult + 'static, + R: IntoValue + 'static, + { + fn into_variadic_operation(self) -> Arc { + Arc::new(move |mut args: Vec| { + if args.len() < $prefix { + return Err(Error::arity_error($prefix, args.len())); + } + + // Move the tail values into a fresh Vec backing + // the `Rest` parameter, leaving the prefix + // values in-place for `FromParam` to borrow from. + let rest_values = args.split_off($prefix); + + match &args[..] { + [ $( $v ),+ ] => { + (|| { + let mut prefix_storage: ( + $( <$A as FromParam>::Storage<'_>, )+ + ) = Default::default(); + + $( + let $p: <$A as FromParam>::Param<'_> = + <$A as FromParam>::from_arg( + $v, + &mut prefix_storage.$idx, + )?; + )+ + + let rest_param = Rest::new(rest_values); + let result: FR = (self)( $( $p ),+, rest_param ); + let value: R = result.into_result()?; + Ok(value.into_value()) + })() + } + _ => Err(Error::arity_error($prefix, args.len())), + } + }) + } + } + }; +} + +impl_into_variadic_operation_for_prefix_and_rest_value!(1, 0 => v0, p0: A1); +impl_into_variadic_operation_for_prefix_and_rest_value!(2, 0 => v0, p0: A1, 1 => v1, p1: A2); +impl_into_variadic_operation_for_prefix_and_rest_value!(3, 0 => v0, p0: A1, 1 => v1, p1: A2, 2 => v2, p2: A3); +impl_into_variadic_operation_for_prefix_and_rest_value!(4, 0 => v0, p0: A1, 1 => v1, p1: A2, 2 => v2, p2: A3, 3 => v3, p3: A4); +impl_into_variadic_operation_for_prefix_and_rest_value!(5, 0 => v0, p0: A1, 1 => v1, p1: A2, 2 => v2, p2: A3, 3 => v3, p3: A4, 4 => v4, p4: A5); +impl_into_variadic_operation_for_prefix_and_rest_value!(6, 0 => v0, p0: A1, 1 => v1, p1: A2, 2 => v2, p2: A3, 3 => v3, p3: A4, 4 => v4, p4: A5, 5 => v5, p5: A6); +impl_into_variadic_operation_for_prefix_and_rest_value!(7, 0 => v0, p0: A1, 1 => v1, p1: A2, 2 => v2, p2: A3, 3 => v3, p3: A4, 4 => v4, p4: A5, 5 => v5, p5: A6, 6 => v6, p6: A7); + +// ===================================================================== +// Fixed-arity adapters +// ===================================================================== + +/// Helper macro to implement `IntoOperation` for functions of various +/// arities using pattern matching on the argument slice. +/// +/// This macro preserves the safety invariants relied upon by +/// `FromParam` and `widen_slice_lifetime` by: +/// - allocating all `Storage` values as locals inside the adapter; +/// - calling `FromParam::from_arg` to build `Param<'_>` for each +/// argument; +/// - immediately invoking the builtin function `F` with those +/// parameters; and +/// - dropping all storage before returning. +/// +/// It also avoids the cascade of `get/expect` indexing by matching on +/// `&args[..]` to perform arity checking and destructuring in one +/// place. +/// +/// This macro is the only place `FromParam::from_arg` is used for +/// fixed-arity builtins, so its structure (local storage tuple plus +/// immediate call) is part of the safety contract described on +/// `widen_slice_lifetime`. +macro_rules! impl_into_operation_for_arity { + ($arity:expr, $( $idx:tt => $v:ident, $p:ident : $A:ident ),+ ) => { + impl IntoOperation<($( $A, )+), R> for F + where + F: for<'a> Fn( $( <$A as FromParam>::Param<'a> ),+ ) -> FR + + Send + + Sync + + 'static, + FR: IntoResult + 'static, + R: IntoValue + 'static, + $( $A: FromParam, )+ + { + fn into_operation(self) -> Arc { + Arc::new(move |args: Vec| { + match &args[..] { + [ $( $v ),+ ] => { + (|| { + // Tuple of per-argument storage; for arity 1 this is a 1-tuple + // `(::Storage<'_>,)`. + let mut storage: ( + $( <$A as FromParam>::Storage<'_>, )+ + ) = Default::default(); + + $( + let $p: <$A as FromParam>::Param<'_> = + <$A as FromParam>::from_arg( + $v, + &mut storage.$idx, + )?; + )+ + + let result: FR = (self)( $( $p ),+ ); + let value: R = result.into_result()?; + Ok(value.into_value()) + })() + } + _ => Err(Error::arity_error($arity, args.len())), + } + }) + } + } + }; +} + +// 0-arg functions / closures +impl IntoOperation<(), R> for F +where + F: Fn() -> FR + Send + Sync + 'static, + FR: IntoResult + 'static, + R: IntoValue + 'static, +{ + fn into_operation(self) -> Arc { + Arc::new(move |args: Vec| { + if !args.is_empty() { + return Err(Error::arity_error(0, args.len())); + } + + let result: FR = (self)(); + let value: R = result.into_result()?; + Ok(value.into_value()) + }) + } +} + +impl_into_operation_for_arity!(1, 0 => v0, p0: A1); +impl_into_operation_for_arity!(2, 0 => v0, p0: A1, 1 => v1, p1: A2); +impl_into_operation_for_arity!(3, 0 => v0, p0: A1, 1 => v1, p1: A2, 2 => v2, p2: A3); +impl_into_operation_for_arity!(4, 0 => v0, p0: A1, 1 => v1, p1: A2, 2 => v2, p2: A3, 3 => v3, p3: A4); +impl_into_operation_for_arity!(5, 0 => v0, p0: A1, 1 => v1, p1: A2, 2 => v2, p2: A3, 3 => v3, p3: A4, 4 => v4, p4: A5); +impl_into_operation_for_arity!(6, 0 => v0, p0: A1, 1 => v1, p1: A2, 2 => v2, p2: A3, 3 => v3, p3: A4, 4 => v4, p4: A5, 5 => v5, p5: A6); +impl_into_operation_for_arity!(7, 0 => v0, p0: A1, 1 => v1, p1: A2, 2 => v2, p2: A3, 3 => v3, p3: A4, 4 => v4, p4: A5, 5 => v5, p5: A6, 6 => v6, p6: A7); +impl_into_operation_for_arity!(8, 0 => v0, p0: A1, 1 => v1, p1: A2, 2 => v2, p2: A3, 3 => v3, p3: A4, 4 => v4, p4: A5, 5 => v5, p5: A6, 6 => v6, p6: A7, 7 => v7, p7: A8); diff --git a/src/lib.rs b/src/lib.rs index d800b06..5454490 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -113,6 +113,7 @@ impl fmt::Display for Error { pub mod ast; pub mod builtinops; pub mod evaluator; +pub mod intooperation; #[cfg(feature = "jsonlogic")] pub mod jsonlogic; From 019d1e88afc8e15abd1986a496b5382bd3fe39b8 Mon Sep 17 00:00:00 2001 From: Eugene Talagrand Date: Thu, 20 Nov 2025 12:25:07 -0800 Subject: [PATCH 2/6] Switch to use iterator-based signatures for lists and "rest" --- src/ast.rs | 52 ++- src/evaluator.rs | 114 +++--- src/intooperation.rs | 929 +++++++++++++++++-------------------------- 3 files changed, 455 insertions(+), 640 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index ad70644..c5fa6a6 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -1,12 +1,14 @@ -/// This module defines the core Abstract Syntax Tree (AST) types and helper functions -/// for representing values in the interpreter. The main enum, [`Value`], covers -/// all Scheme data types, including numbers, symbols, strings, booleans, lists, built-in -/// and user-defined functions, and precompiled operations. Ergonomic helper functions -/// such as [`val`], [`sym`], and [`nil`] are provided for convenient AST construction -/// in both code and tests. The module also implements conversion traits for common Rust -/// types, making it easy to build Values from Rust literals, arrays, slices, and -/// vectors. Equality and display logic are customized to match Scheme semantics, including -/// round-trip compatibility for precompiled operations. +//! This module defines the core Abstract Syntax Tree (AST) types and helper functions +//! for representing values in the interpreter. The main enum, [`Value`], covers +//! all Scheme data types, including numbers, symbols, strings, booleans, lists, built-in +//! and user-defined functions, and precompiled operations. Ergonomic helper functions +//! such as [`val`], [`sym`], and [`nil`] are provided for convenient AST construction +//! in both code and tests. The module also implements conversion traits for common Rust +//! types, making it easy to build Values from Rust literals, arrays, slices, and +//! vectors. Equality and display logic are customized to match Scheme semantics, including +//! round-trip compatibility for precompiled operations. + +use crate::Error; use crate::builtinops::BuiltinOp; use crate::intooperation::OperationFn; @@ -106,7 +108,7 @@ impl std::fmt::Debug for Value { if i > 0 { write!(f, ", ")?; } - write!(f, "{:?}", v)?; + write!(f, "{v:?}")?; } write!(f, ")") } @@ -116,13 +118,13 @@ impl std::fmt::Debug for Value { if i > 0 { write!(f, ", ")?; } - write!(f, "{:?}", a)?; + write!(f, "{a:?}")?; } write!(f, "])") } Value::BuiltinFunction { id, .. } => write!(f, "BuiltinFunction({id})"), Value::Function { params, body, .. } => { - write!(f, "Function(params={params:?}, body={:?})", body) + write!(f, "Function(params={params:?}, body={body:?})") } Value::Unspecified => write!(f, "Unspecified"), } @@ -185,6 +187,32 @@ impl + Clone> From<&[T]> for Value { } } +// Fallible conversions from `Value` back into primitive Rust types. + +impl std::convert::TryInto for Value { + type Error = Error; + + fn try_into(self) -> Result { + if let Value::Number(n) = self { + Ok(n) + } else { + Err(Error::TypeError("expected number".into())) + } + } +} + +impl std::convert::TryInto for Value { + type Error = Error; + + fn try_into(self) -> Result { + if let Value::Bool(b) = self { + Ok(b) + } else { + Err(Error::TypeError("expected boolean".into())) + } + } +} + /// Helper function for creating symbols - works great in mixed lists! /// Accepts both &str and String via Into<&str> #[cfg_attr(not(test), expect(dead_code))] diff --git a/src/evaluator.rs b/src/evaluator.rs index c01db02..03ac4d5 100644 --- a/src/evaluator.rs +++ b/src/evaluator.rs @@ -2,7 +2,7 @@ use crate::Error; use crate::MAX_EVAL_DEPTH; use crate::ast::Value; use crate::builtinops::{Arity, get_builtin_ops}; -use crate::intooperation::{IntoFixedOperation, IntoVariadicOperation, OperationFn}; +use crate::intooperation::{IntoOperation, IntoVariadicOperation, OperationFn}; use std::collections::HashMap; use std::sync::Arc; @@ -118,24 +118,35 @@ impl Environment { /// - `i64` (number) /// - `bool` (boolean) /// - `&str` (borrowed string slices) - /// - `&Value` (borrowed access to the raw AST value) - /// - list arguments via slices such as `&[i64]`, `&[bool]`, - /// `&[&str]`, and `&[Value]` (when the call site passes a list) + /// - `Value` (owned access to the raw AST value) + /// - `ValueListIterator<'_>` (iterates over elements of a list argument as `&Value`) + /// - `NumIterator<'_>` (iterates over numeric elements of a list argument) + /// - `BoolIterator<'_>` (iterates over boolean elements of a list argument) + /// - `StringIterator<'_>` (iterates over string elements of a list argument) + /// + /// Additional scalar parameter types can be supported by adding + /// `impl TryInto for Value` in `ast.rs`; those + /// automatically participate via the blanket `FromParam` impl. + /// + /// More advanced list-style and variadic behaviour (e.g. rest + /// parameters spanning multiple arguments) can be expressed using + /// the iterator-based APIs described on + /// [`Environment::register_variadic_builtin_operation`]. /// /// Supported return types: - /// - any type implementing the internal `IntoValue` trait - /// (currently `Value`, `i64`, `bool`, `String`, and `&str`) - /// - `Result` where `R` is one of the above and `E: Display` + /// - any type `R` where `R: Into` (for example `Value`, + /// `i64`, `bool`, `String`, and `&str`) + /// - `Result` where `R: Into` and `E: Display` /// /// If you need true rest-parameter / variadic behavior (functions - /// that see all arguments as `&[Value]` or `&[i64]`), use - /// [`Environment::register_variadic_builtin_operation`] instead. + /// that see all arguments via iterators over the argument tail), + /// use [`Environment::register_variadic_builtin_operation`] instead. /// /// Arity is enforced automatically. Conversion errors yield `TypeError`, /// and any `E` from a `Result` is converted to `Error::EvalError`. pub fn register_builtin_operation(&mut self, name: &str, func: F) where - F: IntoFixedOperation + 'static, + F: IntoOperation + 'static, { let wrapped = func.into_operation(); self.bindings.insert( @@ -150,9 +161,14 @@ impl Environment { /// Register a variadic builtin operation with explicit arity metadata. /// /// This is intended for functions whose Rust signature includes a - /// "rest" parameter, either as a raw slice of values, - /// `fn(&[Value]) -> Result`, or as a typed slice such - /// as `fn(&[i64])` or `fn(i64, &[i64])`. + /// "rest" parameter, expressed using iterator types from the + /// [`crate::intooperation`] module. + /// + /// Examples: + /// - rest of all arguments as values: `fn(ValueListIterator<'_>) -> R` + /// - numeric tail: `fn(NumIterator<'_>) -> R` + /// - fixed prefix plus numeric tail: `fn(i64, NumIterator<'_>) -> R` + /// /// Fixed-arity functions should use /// [`Environment::register_builtin_operation`] instead. /// @@ -542,7 +558,7 @@ mod tests { use super::*; use crate::Error; use crate::ast::{nil, sym, val}; - use crate::intooperation::Rest; + use crate::intooperation::{NumIterator, NumRest, ValueListIterator, ValuesRest}; use crate::scheme::parse_scheme; #[test] @@ -564,7 +580,7 @@ mod tests { } let mut env = create_global_env(); - env.register_builtin_operation("forty-two", forty_two); + env.register_builtin_operation::<_, (), i64>("forty-two", forty_two); let expr = parse_scheme("(forty-two)").unwrap(); let result = eval(&expr, &mut env).unwrap(); @@ -582,9 +598,9 @@ mod tests { } let mut env = create_global_env(); - // For functions returning Result<_, E>, Rust currently requires - // specifying the argument tuple `Args` and the output type `R` - // when using the typed API. + // Functions returning Result<_, E> are supported via IntoResult. + // Explicit type annotations on the registration help type inference + // in builds that enable all targets. env.register_builtin_operation::<_, (i64, i64), i64>("safe-div", safe_div); // Success case @@ -600,41 +616,38 @@ mod tests { } #[test] - fn test_register_builtin_operation_vec_i64_list_param() { - fn sum_list(nums: &[i64]) -> i64 { - nums.iter().copied().sum() + fn test_register_builtin_operation_list_numeric_iterator_param() { + fn sum_list(nums: NumIterator<'_>) -> i64 { + nums.sum() } let mut env = create_global_env(); - env.register_builtin_operation::<_, (&[i64],), i64>("sum-list", sum_list); + env.register_builtin_operation::<_, (NumIterator<'static>,), i64>("sum-list", sum_list); - let expr = parse_scheme("(sum-list '(1 2 3 4))").unwrap(); + let expr = parse_scheme("(sum-list (list 1 2 3 4))").unwrap(); let result = eval(&expr, &mut env).unwrap(); assert_eq!(result, Value::Number(10)); } #[test] fn test_register_builtin_operation_with_rest_values() { - fn first_and_rest_count(args: Rest) -> Result { - let args_slice = args.as_slice(); - - if args_slice.is_empty() { - return Err(Error::arity_error(1, 0)); - } - - let first_number = match args_slice.first().expect("arity checked above") { - Value::Number(n) => *n, - _ => return Err(Error::TypeError("first argument must be a number".into())), + fn first_and_rest_count(mut args: ValueListIterator<'_>) -> Result { + let first = match args.next() { + Some(Value::Number(n)) => *n, + Some(_) => return Err(Error::TypeError("first argument must be a number".into())), + None => return Err(Error::arity_error(1, 0)), }; + let rest_count = args.count() as i64; + Ok(Value::List(vec![ - Value::Number(first_number), - Value::Number((args_slice.len() - 1) as i64), + Value::Number(first), + Value::Number(rest_count), ])) } let mut env = create_global_env(); - env.register_variadic_builtin_operation::<_, (Rest,), Value>( + env.register_variadic_builtin_operation::<_, (ValuesRest,), Value>( "first-rest-count", Arity::AtLeast(1), first_and_rest_count, @@ -650,19 +663,14 @@ mod tests { #[test] fn test_register_builtin_operation_varargs_borrowed_values_all() { - fn count_numbers(args: Rest) -> Result { - let args_slice = args.as_slice(); - - let count = args_slice - .iter() - .filter(|v| matches!(v, Value::Number(_))) - .count() as i64; + fn count_numbers(args: ValueListIterator<'_>) -> Result { + let count = args.filter(|v| matches!(v, Value::Number(_))).count() as i64; Ok(Value::Number(count)) } let mut env = create_global_env(); - env.register_variadic_builtin_operation::<_, (Rest,), Value>( + env.register_variadic_builtin_operation::<_, (ValuesRest,), Value>( "count-numbers", Arity::AtLeast(0), count_numbers, @@ -675,12 +683,12 @@ mod tests { #[test] fn test_register_builtin_operation_varargs_all_i64() { - fn sum_all(nums: Rest) -> i64 { - nums.iter().copied().sum() + fn sum_all(nums: NumIterator<'_>) -> i64 { + nums.sum::() } let mut env = create_global_env(); - env.register_variadic_builtin_operation::<_, (Rest,), i64>( + env.register_variadic_builtin_operation::<_, (NumRest,), i64>( "sum-varargs-all", Arity::AtLeast(0), sum_all, @@ -693,12 +701,12 @@ mod tests { #[test] fn test_register_builtin_operation_varargs_fixed_plus_rest() { - fn weighted_sum(weight: i64, nums: Rest) -> i64 { - weight * nums.iter().copied().sum::() + fn weighted_sum(weight: i64, nums: NumIterator<'_>) -> i64 { + weight * nums.sum::() } let mut env = create_global_env(); - env.register_variadic_builtin_operation::<_, (i64, Rest), i64>( + env.register_variadic_builtin_operation::<_, (i64, NumRest), i64>( "weighted-sum", Arity::AtLeast(1), weighted_sum, @@ -711,12 +719,12 @@ mod tests { #[test] fn test_register_variadic_builtin_operation_with_explicit_arity() { - fn sum_all(nums: Rest) -> i64 { - nums.iter().copied().sum() + fn sum_all(nums: NumIterator<'_>) -> i64 { + nums.sum::() } let mut env = create_global_env(); - env.register_variadic_builtin_operation::<_, (Rest,), i64>( + env.register_variadic_builtin_operation::<_, (NumRest,), i64>( "sum-all-min1", Arity::AtLeast(1), sum_all, diff --git a/src/intooperation.rs b/src/intooperation.rs index afb03a0..40a1d64 100644 --- a/src/intooperation.rs +++ b/src/intooperation.rs @@ -1,6 +1,7 @@ use crate::Error; use crate::ast::Value; use std::fmt::Display; +use std::iter::FusedIterator; use std::sync::Arc; /// Canonical erased builtin function type used by the evaluator. @@ -12,162 +13,61 @@ pub type OperationFn = dyn Fn(Vec) -> Result + Send + Sync; // ===================================================================== // Internal machinery for fixed-arity argument conversion // -// All `unsafe` in this crate is deliberately confined to this module. -// The only primitive is `widen_slice_lifetime`, which is used by the -// `FromParam` implementations for slice parameters and by the -// variadic "rest" machinery for slice-like parameters. See the docs -// on that function and on `FromParam` for the full safety argument -// and the limitations in Rust's current type system that force this -// design. +// This module defines `FromParam`, which is used by the fixed-arity +// adapters to turn AST `Value` nodes into strongly-typed Rust +// parameters. All conversions are implemented here so that the +// supported parameter types are easy to audit. // ===================================================================== -mod sealed { - //! Types that may be used as builtin parameters via `FromParam`. - //! - //! This `Sealed` trait prevents code outside this module from - //! implementing `FromParam`, which keeps the safety invariants - //! around `widen_slice_lifetime` local and auditable. - //! - //! To add a new builtin parameter type, you must: - //! - add it here, and - //! - add a matching `FromParam` implementation in this module. - //! - //! Slice-like types must be carefully reviewed, as they typically - //! rely on `widen_slice_lifetime` and its safety invariants. - - pub trait Sealed {} - - impl Sealed for &super::Value {} - impl Sealed for i64 {} - impl Sealed for bool {} - impl Sealed for &str {} - impl Sealed for &[i64] {} - impl Sealed for &[bool] {} - impl Sealed for &[&str] {} - impl Sealed for &[super::Value] {} -} - /// Core trait used by the fixed-arity adapters to turn `Value` nodes /// into strongly-typed parameters. /// -/// This trait is **internal** to this module and sealed; the set of -/// supported parameter types is fixed to those we explicitly impl it -/// for above in `sealed`. -/// -/// For example, a builtin like -/// `fn sum(first: i64, rest: &[i64]) -> Result` -/// will be invoked via `FromParam` implementations for `i64` and -/// `&[i64]`. -/// -/// The associated `Storage<'a>` type gives each parameter kind a place -/// to put temporary data (typically a `Vec` for slice parameters). -/// The `Param<'a>` family then describes the parameter type as seen by -/// the builtin function for a given lifetime `'a` of the underlying -/// AST `Value`s. -/// -/// ### Why is there `unsafe` here? -/// -/// For scalar parameters (`i64`, `bool`, `&str`, `&Value`) the -/// implementation is entirely safe. For slice parameters like -/// `&[i64]`, `&[bool]`, and `&[&str]` we must populate a `Vec` and -/// then hand the builtin a slice `&[T]` that appears to live for -/// `Param<'a>`'s lifetime. In reality the slice is only valid for the -/// duration of the adapter's stack frame that owns the `Vec`. -/// -/// The fixed-arity adapters call `FromParam::from_arg` inside a -/// function that immediately invokes the builtin and then drops all -/// `Storage` values before returning. Expressing "this reference is -/// valid only for the body of this call" is not something Rust's -/// current type system can do in a reusable trait, so we use a small -/// `unsafe` helper to widen the slice lifetime while **relying on the -/// calling pattern** to keep things sound. -/// -/// Concretely, Rust today lacks: -/// - a way to express an "intersection" lifetime like -/// `min('vec, 'a)` for data borrowed from both a local `Vec` and -/// the input `&'a Value`; -/// - the ability to state, in the trait, that `Param<'a>` values -/// cannot outlive the specific adapter call that created them, -/// even though the adapter is used under a higher-ranked -/// `for<'a>` bound; -/// - a safe standard-library abstraction that captures this -/// "stack-bounded borrow from scratch space" pattern for slices. -/// -/// Instead, we: -/// - keep `FromParam` sealed and internal to this module; -/// - centralise the `unsafe` in `widen_slice_lifetime` with a detailed -/// `# Safety` explanation; and -/// - structure the fixed-arity adapters so that `Param<'a>` is never -/// stored or allowed to escape the call to the builtin. -pub(crate) trait FromParam: sealed::Sealed { - /// Per-call scratch space, scoped to the lifetime of the argument - /// values. For scalar parameters this is typically `()`; for slices - /// it is usually a `Vec` that we borrow from. - type Storage<'a>: Default; - +/// The associated `Param<'a>` type is the parameter type as seen by +/// the builtin for a given lifetime of the local `Value` slots used +/// during argument conversion. +pub(crate) trait FromParam { /// The parameter type as seen by the builtin for a given lifetime /// of the underlying AST values. type Param<'a>; /// Convert a single AST argument into this parameter type. - fn from_arg<'a>( - value: &'a Value, - storage: &mut Self::Storage<'a>, - ) -> Result, Error>; + /// + /// Implementations may either borrow from the provided `Value` + /// (for types such as `&str` and the borrowed iterators), or + /// consume it by value (for `Value` itself). + fn from_arg<'a>(value: &'a mut Value) -> Result, Error>; } -impl FromParam for &Value { - type Storage<'a> = (); - type Param<'a> = &'a Value; +impl FromParam for Value { + type Param<'a> = Value; - fn from_arg<'a>( - value: &'a Value, - _storage: &mut Self::Storage<'a>, - ) -> Result, Error> { - Ok(value) + fn from_arg<'a>(value: &'a mut Value) -> Result, Error> { + // Move the `Value` out so that builtin functions can consume + // owned payloads (such as strings or lists) without cloning. + Ok(std::mem::replace(value, Value::Unspecified)) } } -impl FromParam for i64 { - type Storage<'a> = (); - type Param<'a> = i64; - - fn from_arg<'a>( - value: &'a Value, - _storage: &mut Self::Storage<'a>, - ) -> Result, Error> { - if let Value::Number(n) = value { - Ok(*n) - } else { - Err(Error::TypeError("expected number".into())) - } - } -} - -impl FromParam for bool { - type Storage<'a> = (); - type Param<'a> = bool; +// Blanket implementation for by-value primitive parameters that can be +// obtained from a `Value` via the standard `TryInto` trait. This covers +// types such as `i64` and `bool` for which we provide +// `impl TryInto for Value` in `ast.rs`. +impl FromParam for T +where + Value: std::convert::TryInto, +{ + type Param<'a> = T; - fn from_arg<'a>( - value: &'a Value, - _storage: &mut Self::Storage<'a>, - ) -> Result, Error> { - if let Value::Bool(b) = value { - Ok(*b) - } else { - Err(Error::TypeError("expected boolean".into())) - } + fn from_arg<'a>(value: &'a mut Value) -> Result, Error> { + let owned = std::mem::replace(value, Value::Unspecified); + >::try_into(owned) } } impl FromParam for &str { - type Storage<'a> = (); type Param<'a> = &'a str; - fn from_arg<'a>( - value: &'a Value, - _storage: &mut Self::Storage<'a>, - ) -> Result, Error> { + fn from_arg<'a>(value: &'a mut Value) -> Result, Error> { if let Value::String(s) = value { Ok(s.as_str()) } else { @@ -176,172 +76,60 @@ impl FromParam for &str { } } -/// Widen the lifetime of a slice to match the higher-ranked lifetime -/// used by the fixed-arity adapters. -/// -/// This function is the **only** place `unsafe` is used in the -/// argument-conversion machinery. It is private to this module and is -/// only called from `FromParam` and variadic rest-parameter -/// implementations for slice-like parameters. -/// -/// # Safety -/// -/// - The caller must ensure that the returned reference does not -/// outlive the stack frame that owns the backing storage of -/// `slice` (typically a local `Vec`). -/// - In this module, that guarantee is provided by the fixed-arity -/// `IntoOperation` implementations: they allocate all `Storage` -/// values as locals, call `FromParam::from_arg` to create -/// `Param<'a>` values, immediately invoke the builtin `F`, and then -/// drop all `Storage` before returning. -/// - `Param<'a>` values are never stored in the returned -/// `OperationFn` closure or in any other longer-lived structure; -/// they are passed directly to the builtin and then discarded. -/// -/// Rust's type system cannot currently express this "stack-bounded -/// borrow through a trait" pattern in a fully safe way, because the -/// trait uses a higher-ranked lifetime `for<'a>` while the actual -/// borrow from the local `Vec` is tied to a particular call frame. -/// The combination of sealing, centralising this helper, and the -/// structure of the fixed-arity adapters is what makes this sound. -fn widen_slice_lifetime<'short, 'long, T>(slice: &'short [T]) -> &'long [T] { - // SAFETY: Callers in this module uphold the safety contract above - // by never letting the returned reference escape the adapter call - // that owns the backing storage. - unsafe { &*(slice as *const [T]) } -} - -/// Typed list parameters for fixed-arity functions: a single -/// `Value::List` argument converted to a slice of elements. -impl FromParam for &[i64] { - type Storage<'a> = Vec; - type Param<'a> = &'a [i64]; - - fn from_arg<'a>( - value: &'a Value, - storage: &mut Self::Storage<'a>, - ) -> Result, Error> { - storage.clear(); - let items = match value { - Value::List(items) => items, - _ => return Err(Error::TypeError("expected list".into())), - }; - - for item in items { - match item { - Value::Number(n) => storage.push(*n), - _ => return Err(Error::TypeError("expected number in list".into())), - } - } - - let slice: &[i64] = storage.as_slice(); - let slice: &'a [i64] = widen_slice_lifetime(slice); - Ok(slice) - } -} +// No slice-based `FromParam` implementations are provided in the +// iterator-based design. If a builtin needs to work with lists or +// rest-style arguments it should use the iterator-based parameters +// defined below instead. -impl FromParam for &[bool] { - type Storage<'a> = Vec; - type Param<'a> = &'a [bool]; - - fn from_arg<'a>( - value: &'a Value, - storage: &mut Self::Storage<'a>, - ) -> Result, Error> { - storage.clear(); - let items = match value { - Value::List(items) => items, - _ => return Err(Error::TypeError("expected list".into())), - }; - - for item in items { - match item { - Value::Bool(b) => storage.push(*b), - _ => return Err(Error::TypeError("expected boolean in list".into())), - } - } +// ===================================================================== +// FromParam support for iterator parameters (list arguments) +// ===================================================================== - let slice: &[bool] = storage.as_slice(); - let slice: &'a [bool] = widen_slice_lifetime(slice); - Ok(slice) - } -} +impl<'b> FromParam for ValueListIterator<'b> { + type Param<'a> = ValueListIterator<'a>; -impl FromParam for &[&str] { - type Storage<'a> = Vec<&'a str>; - type Param<'a> = &'a [&'a str]; - - fn from_arg<'a>( - value: &'a Value, - storage: &mut Self::Storage<'a>, - ) -> Result, Error> { - storage.clear(); - let items = match value { - Value::List(items) => items, - _ => return Err(Error::TypeError("expected list".into())), - }; - - for item in items { - match item { - Value::String(s) => storage.push(s.as_str()), - _ => return Err(Error::TypeError("expected string in list".into())), - } + fn from_arg<'a>(value: &'a mut Value) -> Result, Error> { + if let Value::List(items) = value { + Ok(ValueListIterator::new(items.as_slice())) + } else { + Err(Error::TypeError("expected list".into())) } - - let slice: &[&str] = storage.as_slice(); - let slice: &'a [&'a str] = widen_slice_lifetime(slice); - Ok(slice) } } -impl FromParam for &[Value] { - type Storage<'a> = (); - type Param<'a> = &'a [Value]; +impl<'b> FromParam for NumIterator<'b> { + type Param<'a> = NumIterator<'a>; - fn from_arg<'a>( - value: &'a Value, - _storage: &mut Self::Storage<'a>, - ) -> Result, Error> { + fn from_arg<'a>(value: &'a mut Value) -> Result, Error> { if let Value::List(items) = value { - Ok(items.as_slice()) + NumIterator::new(items.as_slice()) } else { Err(Error::TypeError("expected list".into())) } } } -/// Convert strongly-typed Rust results into AST values. -pub trait IntoValue { - fn into_value(self) -> Value; -} - -impl IntoValue for Value { - fn into_value(self) -> Value { - self - } -} - -impl IntoValue for i64 { - fn into_value(self) -> Value { - Value::Number(self) - } -} +impl<'b> FromParam for BoolIterator<'b> { + type Param<'a> = BoolIterator<'a>; -impl IntoValue for bool { - fn into_value(self) -> Value { - Value::Bool(self) + fn from_arg<'a>(value: &'a mut Value) -> Result, Error> { + if let Value::List(items) = value { + BoolIterator::new(items.as_slice()) + } else { + Err(Error::TypeError("expected list".into())) + } } } -impl IntoValue for String { - fn into_value(self) -> Value { - Value::String(self) - } -} +impl<'b> FromParam for StringIterator<'b> { + type Param<'a> = StringIterator<'a>; -impl IntoValue for &str { - fn into_value(self) -> Value { - Value::String(self.to_owned()) + fn from_arg<'a>(value: &'a mut Value) -> Result, Error> { + if let Value::List(items) = value { + StringIterator::new(items.as_slice()) + } else { + Err(Error::TypeError("expected list".into())) + } } } @@ -350,10 +138,7 @@ pub trait IntoResult { fn into_result(self) -> Result; } -impl IntoResult for T -where - T: IntoValue, -{ +impl IntoResult for T { fn into_result(self) -> Result { Ok(self) } @@ -368,309 +153,326 @@ where } } -/// Marker type for variadic "rest" parameters. +// ===================================================================== +// Iterator-based parameter types +// ===================================================================== + +/// Borrowed iterator over a sequence of AST `Value` references. /// -/// `Rest` is a thin newtype over `Vec` that behaves like a -/// slice via `Deref`/`AsRef`. Variadic adapters simply build a fresh -/// `Vec` for the rest arguments and pass it to the user function; -/// no unsafe code or lifetime juggling is required. -#[derive(Debug, Clone)] -pub struct Rest { - items: Vec, -} - -impl Rest { - pub(crate) fn new(items: Vec) -> Self { - Rest { items } +/// This is the shared base type for all list/sequence-parameter +/// iterators. Typed iterators such as [`NumIterator`], [`BoolIterator`] +/// and [`StringIterator`] wrap this to provide element-level typing. +#[derive(Debug, Clone, Copy)] +pub struct ValueListIterator<'a> { + values: &'a [Value], + index: usize, +} + +impl<'a> ValueListIterator<'a> { + pub(crate) fn new(values: &'a [Value]) -> Self { + ValueListIterator { values, index: 0 } } +} - /// Borrow the rest arguments as a slice. - pub fn as_slice(&self) -> &[T] { - &self.items +impl<'a> Iterator for ValueListIterator<'a> { + type Item = &'a Value; + + fn next(&mut self) -> Option { + let v = self.values.get(self.index)?; + self.index += 1; + Some(v) } - /// Consume the wrapper and return the owned vector of rest - /// arguments. - pub fn into_vec(self) -> Vec { - self.items + fn size_hint(&self) -> (usize, Option) { + let remaining = self.values.len().saturating_sub(self.index); + (remaining, Some(remaining)) } } -impl std::ops::Deref for Rest { - type Target = [T]; +impl<'a> ExactSizeIterator for ValueListIterator<'a> {} +impl<'a> FusedIterator for ValueListIterator<'a> {} - fn deref(&self) -> &Self::Target { - &self.items - } +/// Borrowed iterator over numeric arguments, performing type checking +/// as elements are pulled. Internally this wraps a [`ValueListIterator`] +/// and narrows each element to `i64`. +#[derive(Debug, Clone, Copy)] +pub struct NumIterator<'a> { + inner: ValueListIterator<'a>, } -impl AsRef<[T]> for Rest { - fn as_ref(&self) -> &[T] { - &self.items +impl<'a> NumIterator<'a> { + /// Build a numeric iterator over the provided values, performing a + /// single upfront type check that all elements are numbers. + pub(crate) fn new(values: &'a [Value]) -> Result { + for v in values { + if !matches!(v, Value::Number(_)) { + return Err(Error::TypeError("expected number".into())); + } + } + + Ok(NumIterator { + inner: ValueListIterator::new(values), + }) } } -/// Internal trait used by the variadic adapters to convert the tail -/// of the argument list into a typed rest parameter. -/// -/// The associated `Param<'a>` type is generic over the lifetime of the -/// underlying argument slice. Implementations for owned element types -/// (`Rest`, `Rest`) allocate a fresh `Vec`; borrowed -/// variants like `Rest<&str>` borrow directly from the `Value`s. -trait FromRestParam { - type Param<'a>; +impl<'a> Iterator for NumIterator<'a> { + type Item = i64; - fn from_rest<'a>(values: &'a [Value]) -> Result, Error>; -} + fn next(&mut self) -> Option { + let v = self.inner.next()?; -/// Convert a strongly-typed Rust function or closure into the erased -/// [`OperationFn`], parameterized by an argument tuple type. -pub trait IntoOperation { - fn into_operation(self) -> Arc; -} + if let Value::Number(n) = v { + Some(*n) + } else { + // `new` guarantees all elements are numbers. + debug_assert!(false, "NumIterator saw non-number after construction"); + None + } + } -/// Marker trait for fixed-arity operations (no rest parameters). -/// -/// Implemented for Rust functions and closures whose arguments are -/// drawn from types implementing [`FromParam`]. This is a thin wrapper -/// over [`IntoOperation`] so that the evaluator API can distinguish -/// fixed-arity from variadic registrations. -pub trait IntoFixedOperation: IntoOperation {} + fn size_hint(&self) -> (usize, Option) { + self.inner.size_hint() + } +} -impl IntoFixedOperation for F where F: IntoOperation {} +impl<'a> ExactSizeIterator for NumIterator<'a> {} +impl<'a> FusedIterator for NumIterator<'a> {} -/// Trait for operations registered via the variadic API. -/// -/// Implemented for functions whose Rust signature includes a variadic -/// "rest" parameter, expressed using the [`Rest`] marker type in the -/// last position of the argument tuple. -pub trait IntoVariadicOperation { - fn into_variadic_operation(self) -> Arc; +/// Borrowed iterator over boolean arguments, performing type checking +/// as elements are pulled. Internally this wraps a +/// [`ValueListIterator`] and narrows each element to `bool`. +#[derive(Debug, Clone, Copy)] +pub struct BoolIterator<'a> { + inner: ValueListIterator<'a>, } -// ===================================================================== -// Variadic helpers used by Rest-based adapters -// ===================================================================== +impl<'a> BoolIterator<'a> { + /// Build a boolean iterator over the provided values, performing a + /// single upfront type check that all elements are booleans. + pub(crate) fn new(values: &'a [Value]) -> Result { + for v in values { + if !matches!(v, Value::Bool(_)) { + return Err(Error::TypeError("expected boolean".into())); + } + } -// --- `FromRestParam` implementations for supported rest element types --- + Ok(BoolIterator { + inner: ValueListIterator::new(values), + }) + } +} -// Numeric rest parameter: collects all numeric arguments into an owned -// `Vec` for convenient processing. -impl FromRestParam for Rest { - type Param<'a> = Rest; +impl<'a> Iterator for BoolIterator<'a> { + type Item = bool; - fn from_rest<'a>(values: &'a [Value]) -> Result, Error> { - let mut nums: Vec = Vec::new(); + fn next(&mut self) -> Option { + let v = self.inner.next()?; - for v in values { - match v { - Value::Number(n) => nums.push(*n), - _ => return Err(Error::TypeError("expected number".into())), - } + if let Value::Bool(b) = v { + Some(*b) + } else { + // `new` guarantees all elements are booleans. + debug_assert!(false, "BoolIterator saw non-boolean after construction"); + None } + } - Ok(Rest::new(nums)) + fn size_hint(&self) -> (usize, Option) { + self.inner.size_hint() } } -// Boolean rest parameter: collects all boolean arguments into an owned -// `Vec`. -impl FromRestParam for Rest { - type Param<'a> = Rest; +impl<'a> ExactSizeIterator for BoolIterator<'a> {} +impl<'a> FusedIterator for BoolIterator<'a> {} - fn from_rest<'a>(values: &'a [Value]) -> Result, Error> { - let mut bools: Vec = Vec::new(); +/// Borrowed iterator over string arguments, performing type checking +/// as elements are pulled. Internally this wraps a +/// [`ValueListIterator`] and narrows each element to `&str`. +#[derive(Debug, Clone, Copy)] +pub struct StringIterator<'a> { + inner: ValueListIterator<'a>, +} +impl<'a> StringIterator<'a> { + /// Build a string iterator over the provided values, performing a + /// single upfront type check that all elements are strings. + pub(crate) fn new(values: &'a [Value]) -> Result { for v in values { - match v { - Value::Bool(b) => bools.push(*b), - _ => return Err(Error::TypeError("expected boolean".into())), + if !matches!(v, Value::String(_)) { + return Err(Error::TypeError("expected string".into())); } } - Ok(Rest::new(bools)) + Ok(StringIterator { + inner: ValueListIterator::new(values), + }) } } -// Borrowed string rest parameter: builds a `Rest<&str>` borrowing string -// slices directly from the argument values. -impl FromRestParam for Rest<&str> { - type Param<'a> = Rest<&'a str>; - fn from_rest<'a>(values: &'a [Value]) -> Result, Error> { - let mut items: Vec<&'a str> = Vec::new(); +impl<'a> Iterator for StringIterator<'a> { + type Item = &'a str; - for v in values { - match v { - Value::String(s) => items.push(s.as_str()), - _ => return Err(Error::TypeError("expected string".into())), - } + fn next(&mut self) -> Option { + let v = self.inner.next()?; + + if let Value::String(s) = v { + Some(s.as_str()) + } else { + // `new` guarantees all elements are strings. + debug_assert!(false, "StringIterator saw non-string after construction"); + None } + } - Ok(Rest::new(items)) + fn size_hint(&self) -> (usize, Option) { + self.inner.size_hint() } } -// --- 0-arg prefix: functions that see all args as the rest parameter --- +impl<'a> ExactSizeIterator for StringIterator<'a> {} +impl<'a> FusedIterator for StringIterator<'a> {} -// Specialised implementation for `Rest` that **consumes** the -// incoming `Vec` without cloning. This is the most direct and -// allocation-free way to expose variadic arguments when the builtin is -// happy to own the `Value`s. -impl IntoVariadicOperation<(Rest,), R> for F -where - F: Fn(Rest) -> FR + Send + Sync + 'static, - FR: IntoResult + 'static, - R: IntoValue + 'static, -{ - fn into_variadic_operation(self) -> Arc { - Arc::new(move |args: Vec| { - (|| { - let rest_param = Rest::new(args); - let result: FR = (self)(rest_param); - let value: R = result.into_result()?; - Ok(value.into_value()) - })() - }) +// ===================================================================== +// Rest-parameter support for variadic operations +// ===================================================================== + +/// Core trait used to construct rest-parameter values from a slice of +/// AST arguments. +/// +/// The associated `Param<'a>` is the type actually seen by the builtin +/// function for a given lifetime of the underlying value slice. +pub(crate) trait FromRest { + type Param<'a>; + + fn from_rest<'a>(slice: &'a [Value]) -> Result, Error>; +} + +impl FromRest for ValueListIterator<'static> { + type Param<'a> = ValueListIterator<'a>; + + fn from_rest<'a>(slice: &'a [Value]) -> Result, Error> { + Ok(ValueListIterator::new(slice)) + } +} + +impl FromRest for NumIterator<'static> { + type Param<'a> = NumIterator<'a>; + + fn from_rest<'a>(slice: &'a [Value]) -> Result, Error> { + NumIterator::new(slice) + } +} + +impl FromRest for BoolIterator<'static> { + type Param<'a> = BoolIterator<'a>; + + fn from_rest<'a>(slice: &'a [Value]) -> Result, Error> { + BoolIterator::new(slice) + } +} + +impl FromRest for StringIterator<'static> { + type Param<'a> = StringIterator<'a>; + + fn from_rest<'a>(slice: &'a [Value]) -> Result, Error> { + StringIterator::new(slice) } } -// Generic implementation for rest-only functions using element types -// such as `Rest`, `Rest`, or `Rest<&str>`, built from a -// borrowed slice of the argument `Value`s via `FromRestParam`. -impl IntoVariadicOperation<(RestT,), R> for F +/// Marker type used in `Args` tuples to indicate that a parameter +/// position is populated from the variadic "rest" arguments using +/// [`FromRest`]. +#[derive(Debug, Clone, Copy)] +pub struct Rest(std::marker::PhantomData); + +// Convenience aliases for common rest-parameter iterator types used in +// tests and documentation. These are only used at the type level; the +// actual parameters seen by builtin functions are the lifetime- +// parameterised iterator types above. +pub type ValuesRest = Rest>; +pub type NumRest = Rest>; +pub type BoolRest = Rest>; +pub type StringRest = Rest>; + +/// Convert a strongly-typed Rust function or closure into the erased +/// [`OperationFn`], parameterized by an argument tuple type. +pub trait IntoOperation { + fn into_operation(self) -> Arc; +} + +/// Trait for operations registered via the variadic API. +/// +/// Implemented for functions whose Rust signature includes a variadic +/// "rest" parameter, expressed using the iterator types defined in +/// this module (`ValueListIterator<'a>`, `NumIterator<'a>`, +/// `BoolIterator<'a>`, or `StringIterator<'a>`), optionally after a +/// fixed prefix of `FromParam` parameters. +pub trait IntoVariadicOperation { + fn into_variadic_operation(self) -> Arc; +} + +// ===================================================================== +// Variadic adapters using iterator-based rest parameters +// ===================================================================== + +/// Adapter for functions whose Rust signature consists only of a rest +/// parameter expressed via one of the iterator types in this module +/// (e.g. `ValueListIterator<'a>`, `NumIterator<'a>`, etc.). +impl IntoVariadicOperation<(Rest,), R> for F where - RestT: FromRestParam, - F: for<'a> Fn(::Param<'a>) -> FR + Send + Sync + 'static, + I: FromRest, + F: for<'a> Fn(::Param<'a>) -> FR + Send + Sync + 'static, FR: IntoResult + 'static, - R: IntoValue + 'static, + R: Into + 'static, { fn into_variadic_operation(self) -> Arc { Arc::new(move |args: Vec| { - (|| { - let rest_param = ::from_rest(&args[..])?; - let result: FR = (self)(rest_param); - let value: R = result.into_result()?; - Ok(value.into_value()) - })() + let rest_param: ::Param<'_> = ::from_rest(&args[..])?; + let result: FR = (self)(rest_param); + let value: R = result.into_result()?; + Ok(value.into()) }) } } -/// Helper macro to implement `IntoVariadicOperation` for functions with a fixed -/// prefix followed by a `Rest<_>` parameter in the last position. -/// -/// The fixed prefix is handled via `FromParam` in exactly the same way -/// as the non-variadic adapters. The rest parameter is constructed via -/// `FromRestParam` from the remaining arguments. +/// Helper macro to implement `IntoVariadicOperation` for functions with +/// a fixed prefix of `FromParam` parameters followed by a single rest +/// parameter expressed using one of the iterator types in this module. macro_rules! impl_into_variadic_operation_for_prefix_and_rest { - ($prefix:expr, $( $idx:tt => $v:ident, $p:ident : $A:ident ),+ ) => { - impl IntoVariadicOperation<($( $A, )+ RestT,), R> for F - where - RestT: FromRestParam, - $( $A: FromParam, )+ - F: for<'a> Fn( - $( <$A as FromParam>::Param<'a> ),+, - ::Param<'a>, - ) -> FR - + Send - + Sync - + 'static, - FR: IntoResult + 'static, - R: IntoValue + 'static, - { - fn into_variadic_operation(self) -> Arc { - Arc::new(move |args: Vec| { - match &args[..] { - [ $( $v ),+, rest @ .. ] => { - (|| { - let mut prefix_storage: ( - $( <$A as FromParam>::Storage<'_>, )+ - ) = Default::default(); - - $( - let $p: <$A as FromParam>::Param<'_> = - <$A as FromParam>::from_arg( - $v, - &mut prefix_storage.$idx, - )?; - )+ - - let rest_param = ::from_rest(rest)?; - - let result: FR = (self)( $( $p ),+, rest_param ); - let value: R = result.into_result()?; - Ok(value.into_value()) - })() - } - _ => Err(Error::arity_error($prefix, args.len())), - } - }) - } - } - }; -} - -impl_into_variadic_operation_for_prefix_and_rest!(1, 0 => v0, p0: A1); -impl_into_variadic_operation_for_prefix_and_rest!(2, 0 => v0, p0: A1, 1 => v1, p1: A2); -impl_into_variadic_operation_for_prefix_and_rest!(3, 0 => v0, p0: A1, 1 => v1, p1: A2, 2 => v2, p2: A3); -impl_into_variadic_operation_for_prefix_and_rest!(4, 0 => v0, p0: A1, 1 => v1, p1: A2, 2 => v2, p2: A3, 3 => v3, p3: A4); -impl_into_variadic_operation_for_prefix_and_rest!(5, 0 => v0, p0: A1, 1 => v1, p1: A2, 2 => v2, p2: A3, 3 => v3, p3: A4, 4 => v4, p4: A5); -impl_into_variadic_operation_for_prefix_and_rest!(6, 0 => v0, p0: A1, 1 => v1, p1: A2, 2 => v2, p2: A3, 3 => v3, p3: A4, 4 => v4, p4: A5, 5 => v5, p5: A6); -impl_into_variadic_operation_for_prefix_and_rest!(7, 0 => v0, p0: A1, 1 => v1, p1: A2, 2 => v2, p2: A3, 3 => v3, p3: A4, 4 => v4, p4: A5, 5 => v5, p5: A6, 6 => v6, p6: A7); - -/// Specialised prefix+rest implementations for `Rest` that -/// **consume** the incoming `Vec` tail without cloning. These -/// mirror the generic `impl_into_variadic_operation_for_prefix_and_rest` -/// macros but use `Vec::split_off` to move the rest arguments into a -/// fresh `Vec` that backs the `Rest` parameter. -macro_rules! impl_into_variadic_operation_for_prefix_and_rest_value { - ($prefix:expr, $( $idx:tt => $v:ident, $p:ident : $A:ident ),+ ) => { - impl IntoVariadicOperation<($( $A, )+ Rest,), R> for F + ($prefix:expr, $( $v:ident, $p:ident : $A:ident ),+ ) => { + impl IntoVariadicOperation<( $( $A, )+ Rest, ), R> for F where + I: FromRest, $( $A: FromParam, )+ F: for<'a> Fn( $( <$A as FromParam>::Param<'a> ),+, - Rest, + ::Param<'a>, ) -> FR + Send + Sync + 'static, FR: IntoResult + 'static, - R: IntoValue + 'static, + R: Into + 'static, { fn into_variadic_operation(self) -> Arc { Arc::new(move |mut args: Vec| { - if args.len() < $prefix { - return Err(Error::arity_error($prefix, args.len())); - } - - // Move the tail values into a fresh Vec backing - // the `Rest` parameter, leaving the prefix - // values in-place for `FromParam` to borrow from. - let rest_values = args.split_off($prefix); - - match &args[..] { - [ $( $v ),+ ] => { - (|| { - let mut prefix_storage: ( - $( <$A as FromParam>::Storage<'_>, )+ - ) = Default::default(); - - $( - let $p: <$A as FromParam>::Param<'_> = - <$A as FromParam>::from_arg( - $v, - &mut prefix_storage.$idx, - )?; - )+ - - let rest_param = Rest::new(rest_values); - let result: FR = (self)( $( $p ),+, rest_param ); - let value: R = result.into_result()?; - Ok(value.into_value()) - })() + let len = args.len(); + match args.as_mut_slice() { + &mut [ $( ref mut $v ),+, ref mut rest @ .. ] => { + $( + let $p: <$A as FromParam>::Param<'_> = + <$A as FromParam>::from_arg($v)?; + )+ + + let rest_param: ::Param<'_> = + ::from_rest(&*rest)?; + + let result: FR = (self)( $( $p ),+, rest_param ); + let value: R = result.into_result()?; + Ok(value.into()) } - _ => Err(Error::arity_error($prefix, args.len())), + _ => Err(Error::arity_error($prefix, len)), } }) } @@ -678,75 +480,52 @@ macro_rules! impl_into_variadic_operation_for_prefix_and_rest_value { }; } -impl_into_variadic_operation_for_prefix_and_rest_value!(1, 0 => v0, p0: A1); -impl_into_variadic_operation_for_prefix_and_rest_value!(2, 0 => v0, p0: A1, 1 => v1, p1: A2); -impl_into_variadic_operation_for_prefix_and_rest_value!(3, 0 => v0, p0: A1, 1 => v1, p1: A2, 2 => v2, p2: A3); -impl_into_variadic_operation_for_prefix_and_rest_value!(4, 0 => v0, p0: A1, 1 => v1, p1: A2, 2 => v2, p2: A3, 3 => v3, p3: A4); -impl_into_variadic_operation_for_prefix_and_rest_value!(5, 0 => v0, p0: A1, 1 => v1, p1: A2, 2 => v2, p2: A3, 3 => v3, p3: A4, 4 => v4, p4: A5); -impl_into_variadic_operation_for_prefix_and_rest_value!(6, 0 => v0, p0: A1, 1 => v1, p1: A2, 2 => v2, p2: A3, 3 => v3, p3: A4, 4 => v4, p4: A5, 5 => v5, p5: A6); -impl_into_variadic_operation_for_prefix_and_rest_value!(7, 0 => v0, p0: A1, 1 => v1, p1: A2, 2 => v2, p2: A3, 3 => v3, p3: A4, 4 => v4, p4: A5, 5 => v5, p5: A6, 6 => v6, p6: A7); +impl_into_variadic_operation_for_prefix_and_rest!(1, v0, p0: A1); +impl_into_variadic_operation_for_prefix_and_rest!(2, v0, p0: A1, v1, p1: A2); +impl_into_variadic_operation_for_prefix_and_rest!(3, v0, p0: A1, v1, p1: A2, v2, p2: A3); +impl_into_variadic_operation_for_prefix_and_rest!(4, v0, p0: A1, v1, p1: A2, v2, p2: A3, v3, p3: A4); +impl_into_variadic_operation_for_prefix_and_rest!(5, v0, p0: A1, v1, p1: A2, v2, p2: A3, v3, p3: A4, v4, p4: A5); +impl_into_variadic_operation_for_prefix_and_rest!(6, v0, p0: A1, v1, p1: A2, v2, p2: A3, v3, p3: A4, v4, p4: A5, v5, p5: A6); +impl_into_variadic_operation_for_prefix_and_rest!(7, v0, p0: A1, v1, p1: A2, v2, p2: A3, v3, p3: A4, v4, p4: A5, v5, p5: A6, v6, p6: A7); // ===================================================================== // Fixed-arity adapters // ===================================================================== /// Helper macro to implement `IntoOperation` for functions of various -/// arities using pattern matching on the argument slice. -/// -/// This macro preserves the safety invariants relied upon by -/// `FromParam` and `widen_slice_lifetime` by: -/// - allocating all `Storage` values as locals inside the adapter; -/// - calling `FromParam::from_arg` to build `Param<'_>` for each -/// argument; -/// - immediately invoking the builtin function `F` with those -/// parameters; and -/// - dropping all storage before returning. +/// arities. /// -/// It also avoids the cascade of `get/expect` indexing by matching on -/// `&args[..]` to perform arity checking and destructuring in one -/// place. -/// -/// This macro is the only place `FromParam::from_arg` is used for -/// fixed-arity builtins, so its structure (local storage tuple plus -/// immediate call) is part of the safety contract described on -/// `widen_slice_lifetime`. +/// It performs arity checking up front, then destructures the owned +/// `Vec` into local `Value` slots so that `FromParam` can either +/// borrow from or consume each argument as needed before invoking the +/// builtin function. macro_rules! impl_into_operation_for_arity { - ($arity:expr, $( $idx:tt => $v:ident, $p:ident : $A:ident ),+ ) => { - impl IntoOperation<($( $A, )+), R> for F + ($arity:expr, $( $v:ident, $p:ident : $A:ident ),+ ) => { + impl IntoOperation<( $( $A, )+ ), R> for F where F: for<'a> Fn( $( <$A as FromParam>::Param<'a> ),+ ) -> FR + Send + Sync + 'static, FR: IntoResult + 'static, - R: IntoValue + 'static, + R: Into + 'static, $( $A: FromParam, )+ { fn into_operation(self) -> Arc { - Arc::new(move |args: Vec| { - match &args[..] { - [ $( $v ),+ ] => { - (|| { - // Tuple of per-argument storage; for arity 1 this is a 1-tuple - // `(::Storage<'_>,)`. - let mut storage: ( - $( <$A as FromParam>::Storage<'_>, )+ - ) = Default::default(); - - $( - let $p: <$A as FromParam>::Param<'_> = - <$A as FromParam>::from_arg( - $v, - &mut storage.$idx, - )?; - )+ - - let result: FR = (self)( $( $p ),+ ); - let value: R = result.into_result()?; - Ok(value.into_value()) - })() + Arc::new(move |mut args: Vec| { + let len = args.len(); + match args.as_mut_slice() { + &mut [ $( ref mut $v ),+ ] => { + $( + let $p: <$A as FromParam>::Param<'_> = + <$A as FromParam>::from_arg($v)?; + )+ + + let result: FR = (self)( $( $p ),+ ); + let value: R = result.into_result()?; + Ok(value.into()) } - _ => Err(Error::arity_error($arity, args.len())), + _ => Err(Error::arity_error($arity, len)), } }) } @@ -759,7 +538,7 @@ impl IntoOperation<(), R> for F where F: Fn() -> FR + Send + Sync + 'static, FR: IntoResult + 'static, - R: IntoValue + 'static, + R: Into + 'static, { fn into_operation(self) -> Arc { Arc::new(move |args: Vec| { @@ -769,16 +548,16 @@ where let result: FR = (self)(); let value: R = result.into_result()?; - Ok(value.into_value()) + Ok(value.into()) }) } } -impl_into_operation_for_arity!(1, 0 => v0, p0: A1); -impl_into_operation_for_arity!(2, 0 => v0, p0: A1, 1 => v1, p1: A2); -impl_into_operation_for_arity!(3, 0 => v0, p0: A1, 1 => v1, p1: A2, 2 => v2, p2: A3); -impl_into_operation_for_arity!(4, 0 => v0, p0: A1, 1 => v1, p1: A2, 2 => v2, p2: A3, 3 => v3, p3: A4); -impl_into_operation_for_arity!(5, 0 => v0, p0: A1, 1 => v1, p1: A2, 2 => v2, p2: A3, 3 => v3, p3: A4, 4 => v4, p4: A5); -impl_into_operation_for_arity!(6, 0 => v0, p0: A1, 1 => v1, p1: A2, 2 => v2, p2: A3, 3 => v3, p3: A4, 4 => v4, p4: A5, 5 => v5, p5: A6); -impl_into_operation_for_arity!(7, 0 => v0, p0: A1, 1 => v1, p1: A2, 2 => v2, p2: A3, 3 => v3, p3: A4, 4 => v4, p4: A5, 5 => v5, p5: A6, 6 => v6, p6: A7); -impl_into_operation_for_arity!(8, 0 => v0, p0: A1, 1 => v1, p1: A2, 2 => v2, p2: A3, 3 => v3, p3: A4, 4 => v4, p4: A5, 5 => v5, p5: A6, 6 => v6, p6: A7, 7 => v7, p7: A8); +impl_into_operation_for_arity!(1, v0, p0: A1); +impl_into_operation_for_arity!(2, v0, p0: A1, v1, p1: A2); +impl_into_operation_for_arity!(3, v0, p0: A1, v1, p1: A2, v2, p2: A3); +impl_into_operation_for_arity!(4, v0, p0: A1, v1, p1: A2, v2, p2: A3, v3, p3: A4); +impl_into_operation_for_arity!(5, v0, p0: A1, v1, p1: A2, v2, p2: A3, v3, p3: A4, v4, p4: A5); +impl_into_operation_for_arity!(6, v0, p0: A1, v1, p1: A2, v2, p2: A3, v3, p3: A4, v4, p4: A5, v5, p5: A6); +impl_into_operation_for_arity!(7, v0, p0: A1, v1, p1: A2, v2, p2: A3, v3, p3: A4, v4, p4: A5, v5, p5: A6, v6, p6: A7); +impl_into_operation_for_arity!(8, v0, p0: A1, v1, p1: A2, v2, p2: A3, v3, p3: A4, v4, p4: A5, v5, p5: A6, v6, p6: A7, v7, p7: A8); From 0e15b96b25087b11fe927f0cc06c237005186515 Mon Sep 17 00:00:00 2001 From: Eugene Talagrand Date: Fri, 21 Nov 2025 11:40:30 -0800 Subject: [PATCH 3/6] Minimize turbofish requirements & release readiness --- README.md | 152 +++++---- examples/repl.rs | 10 +- src/ast.rs | 2 +- src/evaluator.rs | 472 +++++++++++---------------- src/{ => evaluator}/intooperation.rs | 433 +++++++++++------------- src/lib.rs | 1 - 6 files changed, 466 insertions(+), 604 deletions(-) rename src/{ => evaluator}/intooperation.rs (55%) diff --git a/README.md b/README.md index 5579d56..3bb9c36 100644 --- a/README.md +++ b/README.md @@ -87,71 +87,6 @@ fn main() -> Result<(), Box> { } ``` -### Registering Custom Builtins - -You can extend the evaluator with your own builtins. There are two -APIs, both of which end up using the same underlying machinery: - -1. **Raw slice API** (closest to the core engine): - -```rust -use rulesxp::ast::Value; -use rulesxp::evaluator::{create_global_env, Environment}; -use rulesxp::Error; - -fn my_custom_function(args: &[Value]) -> Result { - println!("called with {} args", args.len()); - Ok(Value::Unspecified) -} - -let mut env: Environment = create_global_env(); -env.register_builtin_function("my-func", my_custom_function); -// Now (my-func) / {"my-func": [...]} can be used from Scheme / JSONLogic -``` - -2. **Typed API** (ergonomic Rust signatures, automatic conversion): - -```rust -use rulesxp::ast::Value; -use rulesxp::evaluator::{create_global_env, Environment}; - -// Fixed arity: arguments are converted from `Value` automatically -fn add2(a: i64, b: i64) -> i64 { - a + b -} - -// Zero-argument builtin -fn forty_two() -> i64 { 42 } - -// List argument converted to a slice of `Value` -fn first_and_list_len(args: &[Value]) -> Value { - Value::List(vec![ - args.first().cloned().unwrap_or(Value::Unspecified), - Value::Number(args.len() as i64), - ]) -} - -let mut env: Environment = create_global_env(); -env.register_builtin_operation("add2", add2); -env.register_builtin_operation("forty-two", forty_two); -env.register_builtin_operation("first-and-len", first_and_list_len); - -// These builtins are then available from evaluated expressions -``` - -The typed API currently supports: - -- **Parameter types**: `i64`, `bool`, `&str`, borrowed values `&Value`, - and list arguments via slices such as `&[i64]`, `&[bool]`, `&[&str]`, - and `&[Value]` (when the call site passes a list value). -- **Return types**: any type implementing the internal `IntoValue` trait - (currently `Value`, `i64`, `bool`, `String`, and `&str`), or - `Result` where `R` is one of those and `E: Display`. - -Arity is enforced automatically; conversion errors become `TypeError`, -and any user error from a `Result<_, E>` is wrapped into -`Error::EvalError`. - ### Command Line Tools #### Interactive REPL (a demo is also available) @@ -213,6 +148,93 @@ let result = evaluator::eval(&expr, &mut env).unwrap(); println!("{}", result); // 6 ``` +### Registering Custom Builtins + +You can extend the evaluator with your own builtins using strongly-typed +Rust functions. These are registered as **builtin operations** on the +`Environment`. + +#### Fixed-arity builtins + +```rust +use rulesxp::{Error, ast::Value, evaluator}; + +// Infallible builtin: returns a bare i64 +fn add2(a: i64, b: i64) -> i64 { + a + b +} + +// Fallible builtin: returns Result +fn safe_div(a: i64, b: i64) -> Result { + if b == 0 { + Err(Error::EvalError("division by zero".into())) + } else { + Ok(a / b) + } +} + +let mut env = evaluator::create_global_env(); +env.register_builtin_operation::<(i64, i64)>("add2", add2); +env.register_builtin_operation::<(i64, i64)>("safe-div", safe_div); + +// Now you can call (add2 7 5) or (safe-div 6 3) from Scheme +``` + +#### List and variadic builtins + +For list-style and variadic behavior, use the iterator-based +parameter types re-exported from `rulesxp::evaluator`. + +```rust +use rulesxp::{Error, ast::Value, evaluator}; +use rulesxp::builtinops::Arity; +use rulesxp::evaluator::{NumIter, ValueIter}; + +// Single list argument: (sum-list (list 1 2 3 4)) => 10 +fn sum_list(nums: NumIter<'_>) -> i64 { + nums.sum() +} + +// Variadic over all arguments: (count-numbers 1 "x" 2 #t 3) => 3 +fn count_numbers(args: ValueIter<'_>) -> i64 { + args.filter(|v| matches!(v, Value::Number(_))).count() as i64 +} + +let mut env = evaluator::create_global_env(); + +// List parameter from a single list argument +env.register_builtin_operation::<(NumIter<'static>,)>("sum-list", sum_list); + +// Variadic builtin with explicit arity metadata +env.register_variadic_builtin_operation::<(ValueIter<'static>,)>( + "count-numbers", + Arity::AtLeast(0), + count_numbers, +); +``` + +The typed registration APIs currently support: + +- **Parameter types** (as elements of the `Args` tuple): + - `i64` (number) + - `bool` (boolean) + - `&str` (borrowed string slices) + - `Value` (owned access to the raw AST value) + - `ValueIter<'_>` (iterate over `&Value` from a list/rest argument) + - `NumIter<'_>` (iterate over numeric elements as `i64`) + - `BoolIter<'_>` (iterate over boolean elements as `bool`) + - `StringIter<'_>` (iterate over string elements as `&str`) + +- **Return types**: + - `Result` + - `Result` where `T: Into` (for example `i64`, + `bool`, `&str`, arrays/vectors of those types, or `Vec`) + - bare `T` where `T: Into` (for infallible helpers, which are + automatically wrapped as `Ok(T)`) + +Arity is enforced automatically. Conversion errors yield `TypeError`, +and builtin errors are surfaced directly as `Error` values. + ## Current Status ### Implemented diff --git a/examples/repl.rs b/examples/repl.rs index c42a609..abeeeeb 100644 --- a/examples/repl.rs +++ b/examples/repl.rs @@ -40,7 +40,7 @@ fn run_repl() { let mut env = evaluator::create_global_env(); // Register custom function that can be called from user code for demonstration purposes - env.register_builtin_function("help", print_help); + env.register_builtin_operation::<()>("help", print_help); let mut jsonlogic_mode = false; @@ -58,7 +58,7 @@ fn run_repl() { // Handle special commands match line { ":help" => { - _ = print_help(&[]).is_ok(); + _ = print_help().is_ok(); continue; } ":env" => { @@ -144,11 +144,7 @@ fn run_repl() { } } -fn print_help(args: &[Value]) -> Result { - if !args.is_empty() { - return Err(Error::arity_error(0, args.len())); - } - +fn print_help() -> Result { println!("Mini Scheme Interpreter with JSONLogic Support:"); println!(" :help - Show this help message"); println!(" :env - Show current environment bindings"); diff --git a/src/ast.rs b/src/ast.rs index c5fa6a6..3184e50 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -10,7 +10,7 @@ use crate::Error; use crate::builtinops::BuiltinOp; -use crate::intooperation::OperationFn; +use crate::evaluator::intooperation::OperationFn; /// Type alias for number values in interpreter pub(crate) type NumberType = i64; diff --git a/src/evaluator.rs b/src/evaluator.rs index 03ac4d5..236d906 100644 --- a/src/evaluator.rs +++ b/src/evaluator.rs @@ -1,8 +1,11 @@ +use self::intooperation::{IntoOperation, IntoVariadicOperation}; use crate::Error; use crate::MAX_EVAL_DEPTH; use crate::ast::Value; use crate::builtinops::{Arity, get_builtin_ops}; -use crate::intooperation::{IntoOperation, IntoVariadicOperation, OperationFn}; + +pub(crate) mod intooperation; +pub use self::intooperation::{BoolIter, NumIter, StringIter, ValueIter}; use std::collections::HashMap; use std::sync::Arc; @@ -38,80 +41,40 @@ impl Environment { .or_else(|| self.parent.as_ref().and_then(|parent| parent.get(name))) } - /// Register a custom builtin function in the environment for use by Scheme/JSONLogic. - /// - /// This is the low-level API: it accepts a function that already - /// works on `&[Value]` and returns `Result`. Internally - /// it is wired through the same machinery as - /// [`Environment::register_builtin_operation`]. For most new code, - /// prefer the typed API instead of manipulating `Value` directly. - /// - /// # Arguments - /// * `name` - The name by which the function can be called - /// * `func` - A function pointer that takes a slice of `Value` and returns a `Result` - /// - /// # Example - /// ``` - /// use rulesxp::evaluator::{Environment, create_global_env}; - /// use rulesxp::ast::Value; - /// use rulesxp::Error; - /// - /// fn my_custom_function(args: &[Value]) -> Result { - /// println!("Custom function called with {} args", args.len()); - /// Ok(Value::Unspecified) - /// } - /// - /// let mut env = create_global_env(); - /// env.register_builtin_function("my-func", my_custom_function); - /// // Now (my-func) can be called from evaluated expressions - /// ``` - pub fn register_builtin_function( - &mut self, - name: &str, - func: fn(&[Value]) -> Result, - ) { - // Wrap the raw slice-based function into the canonical - // `OperationFn` so it can be stored directly as a - // `BuiltinFunction`. - let f = func; - let wrapped: Arc = Arc::new(move |args: Vec| f(&args)); - - self.bindings.insert( - name.to_string(), - Value::BuiltinFunction { - id: name.to_string(), - func: wrapped, - }, - ); - } - /// Register a strongly-typed Rust function as a builtin operation using /// automatic argument extraction and result conversion. /// /// This allows writing natural Rust functions like: /// /// ```rust,ignore - /// fn add(a: i64, b: i64) -> i64 { a + b } - /// let mut env = rulesxp::evaluator::create_global_env(); - /// env.register_builtin_operation("add", add); + /// use rulesxp::{Error, evaluator}; + /// + /// // Infallible builtin: returns a bare i64 + /// fn add(a: i64, b: i64) -> i64 { + /// a + b + /// } + /// let mut env = evaluator::create_global_env(); + /// env.register_builtin_operation::<(i64, i64)>("add", add); /// // Now (+ 2 3) and (add 2 3) both work if + also registered /// ``` /// - /// Builtins can also return `Result` where `E: Display`: + /// Builtins that may fail return `Result` where `T` is either + /// a Value or a type convertible into Value, and encode + /// their own error messages: /// /// ```rust,ignore - /// use std::fmt::Display; + /// use rulesxp::{Error, evaluator}; /// - /// fn safe_div(a: i64, b: i64) -> Result { + /// fn safe_div(a: i64, b: i64) -> Result { /// if b == 0 { - /// Err("division by zero") + /// Err(Error::EvalError("division by zero".into())) /// } else { /// Ok(a / b) /// } /// } /// - /// let mut env = rulesxp::evaluator::create_global_env(); - /// env.register_builtin_operation("safe-div", safe_div); + /// let mut env = evaluator::create_global_env(); + /// env.register_builtin_operation::<(i64, i64)>("safe-div", safe_div); /// ``` /// /// Supported parameter types (initial set): @@ -119,35 +82,39 @@ impl Environment { /// - `bool` (boolean) /// - `&str` (borrowed string slices) /// - `Value` (owned access to the raw AST value) - /// - `ValueListIterator<'_>` (iterates over elements of a list argument as `&Value`) - /// - `NumIterator<'_>` (iterates over numeric elements of a list argument) - /// - `BoolIterator<'_>` (iterates over boolean elements of a list argument) - /// - `StringIterator<'_>` (iterates over string elements of a list argument) + /// - `ValueIter<'_>` (iterates over elements of a list argument as `&Value`) + /// - `NumIter<'_>` (iterates over numeric elements of a list argument) + /// - `BoolIter<'_>` (iterates over boolean elements of a list argument) + /// - `StringIter<'_>` (iterates over string elements of a list argument) /// /// Additional scalar parameter types can be supported by adding - /// `impl TryInto for Value` in `ast.rs`; those - /// automatically participate via the blanket `FromParam` impl. + /// `impl TryInto for Value`. /// - /// More advanced list-style and variadic behaviour (e.g. rest + /// More advanced list-style and variadic behavior (e.g. rest /// parameters spanning multiple arguments) can be expressed using /// the iterator-based APIs described on /// [`Environment::register_variadic_builtin_operation`]. /// /// Supported return types: - /// - any type `R` where `R: Into` (for example `Value`, - /// `i64`, `bool`, `String`, and `&str`) - /// - `Result` where `R: Into` and `E: Display` + /// - `Result` + /// - `Result` where `T: Into` (for example + /// `i64`, `bool`, `&str`, or arrays/vectors of these types) + /// - bare `T` where `T: Into` (for infallible helpers, + /// automatically wrapped as `Ok(T)`) /// /// If you need true rest-parameter / variadic behavior (functions /// that see all arguments via iterators over the argument tail), /// use [`Environment::register_variadic_builtin_operation`] instead. /// - /// Arity is enforced automatically. Conversion errors yield `TypeError`, - /// and any `E` from a `Result` is converted to `Error::EvalError`. - pub fn register_builtin_operation(&mut self, name: &str, func: F) - where - F: IntoOperation + 'static, - { + /// Arity is enforced automatically. Conversion errors yield + /// `TypeError`, and builtin errors are surfaced directly as + /// `Error` values. + #[expect(private_bounds)] // IntoOperation is an internal adapter trait modeling an input function + pub fn register_builtin_operation( + &mut self, + name: &str, + func: impl IntoOperation + 'static, + ) { let wrapped = func.into_operation(); self.bindings.insert( name.to_string(), @@ -161,13 +128,19 @@ impl Environment { /// Register a variadic builtin operation with explicit arity metadata. /// /// This is intended for functions whose Rust signature includes a - /// "rest" parameter, expressed using iterator types from the - /// [`crate::intooperation`] module. + /// "rest" parameter, expressed using iterator types such as + /// [`ValueIter`] and [`NumIter`]. /// /// Examples: - /// - rest of all arguments as values: `fn(ValueListIterator<'_>) -> R` - /// - numeric tail: `fn(NumIterator<'_>) -> R` - /// - fixed prefix plus numeric tail: `fn(i64, NumIterator<'_>) -> R` + /// - rest of all arguments as values: + /// `fn(ValueIter<'_>) -> Result` with + /// `Args = (ValueIter<'static>,)` + /// - numeric tail: + /// `fn(NumIter<'_>) -> Result` with + /// `Args = (NumIter<'static>,)` + /// - fixed prefix plus numeric tail: + /// `fn(i64, NumIter<'_>) -> Result` with + /// `Args = (i64, NumIter<'static>)` /// /// Fixed-arity functions should use /// [`Environment::register_builtin_operation`] instead. @@ -176,14 +149,13 @@ impl Environment { /// arguments at call time, since minimum/maximum argument counts for /// variadic operations are not always derivable from the Rust type /// signature alone. - pub fn register_variadic_builtin_operation( + #[expect(private_bounds)] // IntoVariadicOperation is an internal trait modeling an input function + pub fn register_variadic_builtin_operation( &mut self, name: &str, arity: Arity, - func: F, - ) where - F: IntoVariadicOperation + 'static, - { + func: impl IntoVariadicOperation + 'static, + ) { let inner = func.into_variadic_operation(); let arity_for_closure = arity; let wrapped = std::sync::Arc::new(move |args: Vec| { @@ -558,215 +530,16 @@ mod tests { use super::*; use crate::Error; use crate::ast::{nil, sym, val}; - use crate::intooperation::{NumIterator, NumRest, ValueListIterator, ValuesRest}; + use crate::evaluator::{NumIter, ValueIter}; use crate::scheme::parse_scheme; - #[test] - fn test_register_builtin_operation_add() { - fn add(a: i64, b: i64) -> i64 { - a + b - } - let mut env = create_global_env(); - env.register_builtin_operation::<_, (i64, i64), i64>("add2", add); - let expr = parse_scheme("(add2 7 5)").unwrap(); - let result = eval(&expr, &mut env).unwrap(); - assert_eq!(result, Value::Number(12)); - } - - #[test] - fn test_register_builtin_operation_zero_arg() { - fn forty_two() -> i64 { - 42 - } - - let mut env = create_global_env(); - env.register_builtin_operation::<_, (), i64>("forty-two", forty_two); - - let expr = parse_scheme("(forty-two)").unwrap(); - let result = eval(&expr, &mut env).unwrap(); - assert_eq!(result, Value::Number(42)); - } - - #[test] - fn test_register_builtin_operation_result_builtin() { - fn safe_div(a: i64, b: i64) -> Result { - if b == 0 { - Err("division by zero") - } else { - Ok(a / b) - } - } - - let mut env = create_global_env(); - // Functions returning Result<_, E> are supported via IntoResult. - // Explicit type annotations on the registration help type inference - // in builds that enable all targets. - env.register_builtin_operation::<_, (i64, i64), i64>("safe-div", safe_div); - - // Success case - let expr_ok = parse_scheme("(safe-div 6 3)").unwrap(); - let result_ok = eval(&expr_ok, &mut env).unwrap(); - assert_eq!(result_ok, Value::Number(2)); - - // Error case: division by zero surfaces as EvalError containing the message - let expr_err = parse_scheme("(safe-div 1 0)").unwrap(); - let err = eval(&expr_err, &mut env).unwrap_err(); - let msg = format!("{err}"); - assert!(msg.contains("division by zero")); - } - - #[test] - fn test_register_builtin_operation_list_numeric_iterator_param() { - fn sum_list(nums: NumIterator<'_>) -> i64 { - nums.sum() - } - - let mut env = create_global_env(); - env.register_builtin_operation::<_, (NumIterator<'static>,), i64>("sum-list", sum_list); - - let expr = parse_scheme("(sum-list (list 1 2 3 4))").unwrap(); - let result = eval(&expr, &mut env).unwrap(); - assert_eq!(result, Value::Number(10)); - } - - #[test] - fn test_register_builtin_operation_with_rest_values() { - fn first_and_rest_count(mut args: ValueListIterator<'_>) -> Result { - let first = match args.next() { - Some(Value::Number(n)) => *n, - Some(_) => return Err(Error::TypeError("first argument must be a number".into())), - None => return Err(Error::arity_error(1, 0)), - }; - - let rest_count = args.count() as i64; - - Ok(Value::List(vec![ - Value::Number(first), - Value::Number(rest_count), - ])) - } - - let mut env = create_global_env(); - env.register_variadic_builtin_operation::<_, (ValuesRest,), Value>( - "first-rest-count", - Arity::AtLeast(1), - first_and_rest_count, - ); - - let expr = parse_scheme("(first-rest-count 42 \"x\" #t 7)").unwrap(); - let result = eval(&expr, &mut env).unwrap(); - assert_eq!( - result, - Value::List(vec![Value::Number(42), Value::Number(3)]) - ); - } - - #[test] - fn test_register_builtin_operation_varargs_borrowed_values_all() { - fn count_numbers(args: ValueListIterator<'_>) -> Result { - let count = args.filter(|v| matches!(v, Value::Number(_))).count() as i64; - - Ok(Value::Number(count)) - } - - let mut env = create_global_env(); - env.register_variadic_builtin_operation::<_, (ValuesRest,), Value>( - "count-numbers", - Arity::AtLeast(0), - count_numbers, - ); - - let expr = parse_scheme("(count-numbers 1 \"x\" 2 #t 3)").unwrap(); - let result = eval(&expr, &mut env).unwrap(); - assert_eq!(result, Value::Number(3)); - } - - #[test] - fn test_register_builtin_operation_varargs_all_i64() { - fn sum_all(nums: NumIterator<'_>) -> i64 { - nums.sum::() - } - - let mut env = create_global_env(); - env.register_variadic_builtin_operation::<_, (NumRest,), i64>( - "sum-varargs-all", - Arity::AtLeast(0), - sum_all, - ); - - let expr = parse_scheme("(sum-varargs-all 1 2 3 4)").unwrap(); - let result = eval(&expr, &mut env).unwrap(); - assert_eq!(result, Value::Number(10)); - } - - #[test] - fn test_register_builtin_operation_varargs_fixed_plus_rest() { - fn weighted_sum(weight: i64, nums: NumIterator<'_>) -> i64 { - weight * nums.sum::() - } - - let mut env = create_global_env(); - env.register_variadic_builtin_operation::<_, (i64, NumRest), i64>( - "weighted-sum", - Arity::AtLeast(1), - weighted_sum, - ); - - let expr = parse_scheme("(weighted-sum 2 1 2 3)").unwrap(); - let result = eval(&expr, &mut env).unwrap(); - assert_eq!(result, Value::Number(12)); - } - - #[test] - fn test_register_variadic_builtin_operation_with_explicit_arity() { - fn sum_all(nums: NumIterator<'_>) -> i64 { - nums.sum::() - } - - let mut env = create_global_env(); - env.register_variadic_builtin_operation::<_, (NumRest,), i64>( - "sum-all-min1", - Arity::AtLeast(1), - sum_all, - ); - - // Valid call: three numeric arguments. - let expr_ok = parse_scheme("(sum-all-min1 1 2 3)").unwrap(); - let result_ok = eval(&expr_ok, &mut env).unwrap(); - assert_eq!(result_ok, Value::Number(6)); - - // Invalid call: zero arguments should fail Arity::AtLeast(1). - let expr_err = parse_scheme("(sum-all-min1)").unwrap(); - let err = eval(&expr_err, &mut env).unwrap_err(); - match err { - crate::Error::ArityError { .. } => {} - other => panic!("expected ArityError, got {other:?}"), - } - } - - #[test] - fn test_builtin_comparison_dynamic_uses_typed_mechanism() { - let mut env = create_global_env(); - - // Dynamic higher-order use of a builtin comparison: the `>` - // operator is passed as a value and called with three - // arguments. This exercises dynamic builtin calls through - // the environment using the shared builtin registry. - let expr = parse_scheme("((lambda (op a b c) (op a b c)) > 9 6 2)").unwrap(); - let result = eval(&expr, &mut env).unwrap(); - assert_eq!(result, Value::Bool(true)); - - let expr_false = parse_scheme("((lambda (op a b c) (op a b c)) > 9 6 7)").unwrap(); - let result_false = eval(&expr_false, &mut env).unwrap(); - assert_eq!(result_false, Value::Bool(false)); - } - /// Test result variants for comprehensive testing #[derive(Debug)] enum TestResult { EvalResult(Value), // Evaluation should succeed with this value SpecificError(&'static str), // Evaluation should fail with error containing this string Error, // Evaluation should fail (any error) + ArityError, // Evaluation should fail specifically with an ArityError } use TestResult::*; @@ -821,6 +594,14 @@ mod tests { } } + (Err(crate::Error::ArityError { .. }), ArityError) => {} // Expected specific arity error + (Ok(actual), ArityError) => { + panic!("{test_id}: expected ArityError, got successful result {actual:?}"); + } + (Err(err), ArityError) => { + panic!("{test_id}: expected ArityError, got different error {err:?}"); + } + (Err(_), Error) => {} // Expected generic error (Err(e), SpecificError(expected_text)) => { let error_msg = format!("{e}"); @@ -850,6 +631,129 @@ mod tests { } } + /// Run tests in a caller-provided environment (e.g., with custom builtins registered). + fn run_tests_in_specific_environment( + env: &mut Environment, + test_cases: Vec<(&str, TestResult)>, + ) { + for (i, (input, expected)) in test_cases.iter().enumerate() { + let test_id = format!("#custom-{}", i + 1); + execute_test_case(input, expected, env, &test_id); + } + } + + #[test] + fn test_custom_builtin_operations() { + // Custom builtins exercise the adapter layer: fixed arity, zero-arg, + // list iterators, explicit arity metadata, and variadic rest-parameters. + fn add(a: i64, b: i64) -> i64 { + a + b + } + + fn forty_two() -> i64 { + 42 + } + + fn safe_div(a: i64, b: i64) -> Result { + if b == 0 { + Err(Error::EvalError("division by zero".into())) + } else { + Ok(a / b) + } + } + + fn sum_list(nums: NumIter<'_>) -> Result { + Ok(nums.sum()) + } + + fn first_and_rest_count(mut args: ValueIter<'_>) -> Result, Error> { + let first = match args.next() { + Some(Value::Number(n)) => *n, + Some(_) => return Err(Error::TypeError("first argument must be a number".into())), + None => return Err(Error::arity_error(1, 0)), + }; + + let rest_count = args.count() as i64; + + Ok(vec![first, rest_count]) + } + + fn count_numbers(args: ValueIter<'_>) -> Result { + let count = args.filter(|v| matches!(v, Value::Number(_))).count() as i64; + Ok(count) + } + + fn sum_all(nums: NumIter<'_>) -> Result { + Ok(nums.sum::()) + } + + fn weighted_sum(weight: i64, nums: NumIter<'_>) -> Result { + Ok(weight * nums.sum::()) + } + + fn sum_all_min1(nums: NumIter<'_>) -> Result { + Ok(nums.sum::()) + } + + let mut env = create_global_env(); + + // Fixed-arity and zero-arg builtins. + env.register_builtin_operation::<(i64, i64)>("add2", add); + env.register_builtin_operation::<()>("forty-two", forty_two); + env.register_builtin_operation::<(i64, i64)>("safe-div", safe_div); + + // Iterator-based list and variadic builtins. + env.register_builtin_operation::<(NumIter<'static>,)>("sum-list", sum_list); + env.register_variadic_builtin_operation::<(ValueIter<'static>,)>( + "first-rest-count", + Arity::AtLeast(1), + first_and_rest_count, + ); + env.register_variadic_builtin_operation::<(ValueIter<'static>,)>( + "count-numbers", + Arity::AtLeast(0), + count_numbers, + ); + env.register_variadic_builtin_operation::<(NumIter<'static>,)>( + "sum-varargs-all", + Arity::AtLeast(0), + sum_all, + ); + env.register_variadic_builtin_operation::<(i64, NumIter<'static>)>( + "weighted-sum", + Arity::AtLeast(1), + weighted_sum, + ); + env.register_variadic_builtin_operation::<(NumIter<'static>,)>( + "sum-all-min1", + Arity::AtLeast(1), + sum_all_min1, + ); + + let test_cases = vec![ + // Fixed-arity and zero-arg builtins. + ("(add2 7 5)", success(12)), + ("(forty-two)", success(42)), + ("(safe-div 6 3)", success(2)), + // Error case: division by zero surfaces as EvalError containing the message. + ("(safe-div 1 0)", SpecificError("division by zero")), + // Iterator-based list and variadic builtins. + ("(sum-list (list 1 2 3 4))", success(10)), + ("(first-rest-count 42 \"x\" #t 7)", success([42, 3])), + ("(count-numbers 1 \"x\" 2 #t 3)", success(3)), + ("(sum-varargs-all 1 2 3 4)", success(10)), + ("(weighted-sum 2 1 2 3)", success(12)), + // Explicit arity checking for variadic builtin. + ("(sum-all-min1 1 2 3)", success(6)), + ("(sum-all-min1)", ArityError), + // Dynamic higher-order use of a builtin comparison: pass `>` as a value. + ("((lambda (op a b c) (op a b c)) > 9 6 2)", success(true)), + ("((lambda (op a b c) (op a b c)) > 9 6 7)", success(false)), + ]; + + run_tests_in_specific_environment(&mut env, test_cases); + } + #[test] #[expect(clippy::too_many_lines)] // Comprehensive test coverage is intentionally thorough fn test_comprehensive_operations_data_driven() { diff --git a/src/intooperation.rs b/src/evaluator/intooperation.rs similarity index 55% rename from src/intooperation.rs rename to src/evaluator/intooperation.rs index 40a1d64..13f15d5 100644 --- a/src/intooperation.rs +++ b/src/evaluator/intooperation.rs @@ -1,14 +1,22 @@ use crate::Error; use crate::ast::Value; -use std::fmt::Display; use std::iter::FusedIterator; +use std::marker::PhantomData; use std::sync::Arc; +// NOTE: This module is internal plumbing for the evaluator. +// It defines the adapter layer that turns strongly-typed Rust +// functions into the erased `OperationFn` used at runtime. +// +// External users should interact with `Environment` and the +// registration APIs in `evaluator.rs`; this module is kept +// crate-private but retains rich comments for maintainers. + /// Canonical erased builtin function type used by the evaluator. /// /// Builtins receive ownership of their argument vector, enabling /// implementations that consume or rearrange arguments if desired. -pub type OperationFn = dyn Fn(Vec) -> Result + Send + Sync; +pub(crate) type OperationFn = dyn Fn(Vec) -> Result + Send + Sync; // ===================================================================== // Internal machinery for fixed-arity argument conversion @@ -85,253 +93,193 @@ impl FromParam for &str { // FromParam support for iterator parameters (list arguments) // ===================================================================== -impl<'b> FromParam for ValueListIterator<'b> { - type Param<'a> = ValueListIterator<'a>; - - fn from_arg<'a>(value: &'a mut Value) -> Result, Error> { - if let Value::List(items) = value { - Ok(ValueListIterator::new(items.as_slice())) - } else { - Err(Error::TypeError("expected list".into())) - } - } -} - -impl<'b> FromParam for NumIterator<'b> { - type Param<'a> = NumIterator<'a>; +impl<'b, K> FromParam for TypedValueIter<'b, K> +where + K: ValueElementKind, +{ + type Param<'a> = TypedValueIter<'a, K>; fn from_arg<'a>(value: &'a mut Value) -> Result, Error> { if let Value::List(items) = value { - NumIterator::new(items.as_slice()) + TypedValueIter::::new(items.as_slice()) } else { Err(Error::TypeError("expected list".into())) } } } -impl<'b> FromParam for BoolIterator<'b> { - type Param<'a> = BoolIterator<'a>; - - fn from_arg<'a>(value: &'a mut Value) -> Result, Error> { - if let Value::List(items) = value { - BoolIterator::new(items.as_slice()) - } else { - Err(Error::TypeError("expected list".into())) - } - } -} +// ===================================================================== +// Generic typed iterator built on top of the standard slice iterator +// ===================================================================== -impl<'b> FromParam for StringIterator<'b> { - type Param<'a> = StringIterator<'a>; +/// Marker trait describing how to view a `Value` slice as a typed +/// iterator. Implementations perform any necessary upfront validation +/// and map each `Value` to the element type. +#[doc(hidden)] +pub trait ValueElementKind { + type Item<'a>; - fn from_arg<'a>(value: &'a mut Value) -> Result, Error> { - if let Value::List(items) = value { - StringIterator::new(items.as_slice()) - } else { - Err(Error::TypeError("expected list".into())) - } - } + fn precheck(slice: &[Value]) -> Result<(), Error>; + fn project<'a>(v: &'a Value) -> Self::Item<'a>; } -/// Normalize both plain values and `Result`-returning functions into `Result`. -pub trait IntoResult { - fn into_result(self) -> Result; +/// Generic iterator over a list of `Value`s, parameterized by a +/// [`ValueElementKind`] that determines the element type and +/// validation. +#[doc(hidden)] +pub struct TypedValueIter<'a, K: ValueElementKind> { + inner: std::slice::Iter<'a, Value>, + _marker: PhantomData, } -impl IntoResult for T { - fn into_result(self) -> Result { - Ok(self) +impl<'a, K> TypedValueIter<'a, K> +where + K: ValueElementKind, +{ + pub(crate) fn new(values: &'a [Value]) -> Result { + K::precheck(values)?; + Ok(TypedValueIter { + inner: values.iter(), + _marker: PhantomData, + }) } } -impl IntoResult for Result +impl<'a, K> Iterator for TypedValueIter<'a, K> where - E: Display, + K: ValueElementKind, { - fn into_result(self) -> Result { - self.map_err(|e| Error::EvalError(e.to_string())) + type Item = K::Item<'a>; + + fn next(&mut self) -> Option { + let v = self.inner.next()?; + Some(K::project(v)) + } + + fn size_hint(&self) -> (usize, Option) { + self.inner.size_hint() } } -// ===================================================================== -// Iterator-based parameter types -// ===================================================================== +impl<'a, K> ExactSizeIterator for TypedValueIter<'a, K> where K: ValueElementKind {} +impl<'a, K> FusedIterator for TypedValueIter<'a, K> where K: ValueElementKind {} -/// Borrowed iterator over a sequence of AST `Value` references. -/// -/// This is the shared base type for all list/sequence-parameter -/// iterators. Typed iterators such as [`NumIterator`], [`BoolIterator`] -/// and [`StringIterator`] wrap this to provide element-level typing. -#[derive(Debug, Clone, Copy)] -pub struct ValueListIterator<'a> { - values: &'a [Value], - index: usize, -} +// Concrete element kinds and their iterator aliases -impl<'a> ValueListIterator<'a> { - pub(crate) fn new(values: &'a [Value]) -> Self { - ValueListIterator { values, index: 0 } - } -} +/// Element kind that views each `Value` as a borrowed reference, +/// used for iterating over raw AST values. +#[doc(hidden)] +pub struct ValueKind; -impl<'a> Iterator for ValueListIterator<'a> { - type Item = &'a Value; +impl ValueElementKind for ValueKind { + type Item<'a> = &'a Value; - fn next(&mut self) -> Option { - let v = self.values.get(self.index)?; - self.index += 1; - Some(v) + fn precheck(_slice: &[Value]) -> Result<(), Error> { + Ok(()) } - fn size_hint(&self) -> (usize, Option) { - let remaining = self.values.len().saturating_sub(self.index); - (remaining, Some(remaining)) + fn project<'a>(v: &'a Value) -> Self::Item<'a> { + v } } -impl<'a> ExactSizeIterator for ValueListIterator<'a> {} -impl<'a> FusedIterator for ValueListIterator<'a> {} +#[doc(hidden)] +pub struct NumberKind; -/// Borrowed iterator over numeric arguments, performing type checking -/// as elements are pulled. Internally this wraps a [`ValueListIterator`] -/// and narrows each element to `i64`. -#[derive(Debug, Clone, Copy)] -pub struct NumIterator<'a> { - inner: ValueListIterator<'a>, -} +impl ValueElementKind for NumberKind { + type Item<'a> = i64; -impl<'a> NumIterator<'a> { - /// Build a numeric iterator over the provided values, performing a - /// single upfront type check that all elements are numbers. - pub(crate) fn new(values: &'a [Value]) -> Result { - for v in values { + fn precheck(slice: &[Value]) -> Result<(), Error> { + for v in slice { if !matches!(v, Value::Number(_)) { return Err(Error::TypeError("expected number".into())); } } - - Ok(NumIterator { - inner: ValueListIterator::new(values), - }) + Ok(()) } -} - -impl<'a> Iterator for NumIterator<'a> { - type Item = i64; - - fn next(&mut self) -> Option { - let v = self.inner.next()?; + fn project<'a>(v: &'a Value) -> Self::Item<'a> { if let Value::Number(n) = v { - Some(*n) + *n } else { - // `new` guarantees all elements are numbers. - debug_assert!(false, "NumIterator saw non-number after construction"); - None + debug_assert!(false, "NumberKind::project saw non-number after precheck"); + unreachable!("NumberKind invariant violated") } } - - fn size_hint(&self) -> (usize, Option) { - self.inner.size_hint() - } } -impl<'a> ExactSizeIterator for NumIterator<'a> {} -impl<'a> FusedIterator for NumIterator<'a> {} +#[doc(hidden)] +pub struct BoolKind; -/// Borrowed iterator over boolean arguments, performing type checking -/// as elements are pulled. Internally this wraps a -/// [`ValueListIterator`] and narrows each element to `bool`. -#[derive(Debug, Clone, Copy)] -pub struct BoolIterator<'a> { - inner: ValueListIterator<'a>, -} +impl ValueElementKind for BoolKind { + type Item<'a> = bool; -impl<'a> BoolIterator<'a> { - /// Build a boolean iterator over the provided values, performing a - /// single upfront type check that all elements are booleans. - pub(crate) fn new(values: &'a [Value]) -> Result { - for v in values { + fn precheck(slice: &[Value]) -> Result<(), Error> { + for v in slice { if !matches!(v, Value::Bool(_)) { return Err(Error::TypeError("expected boolean".into())); } } - - Ok(BoolIterator { - inner: ValueListIterator::new(values), - }) + Ok(()) } -} - -impl<'a> Iterator for BoolIterator<'a> { - type Item = bool; - - fn next(&mut self) -> Option { - let v = self.inner.next()?; + fn project<'a>(v: &'a Value) -> Self::Item<'a> { if let Value::Bool(b) = v { - Some(*b) + *b } else { - // `new` guarantees all elements are booleans. - debug_assert!(false, "BoolIterator saw non-boolean after construction"); - None + debug_assert!(false, "BoolKind::project saw non-boolean after precheck"); + unreachable!("BoolKind invariant violated") } } - - fn size_hint(&self) -> (usize, Option) { - self.inner.size_hint() - } } -impl<'a> ExactSizeIterator for BoolIterator<'a> {} -impl<'a> FusedIterator for BoolIterator<'a> {} +#[doc(hidden)] +pub struct StringKind; -/// Borrowed iterator over string arguments, performing type checking -/// as elements are pulled. Internally this wraps a -/// [`ValueListIterator`] and narrows each element to `&str`. -#[derive(Debug, Clone, Copy)] -pub struct StringIterator<'a> { - inner: ValueListIterator<'a>, -} +impl ValueElementKind for StringKind { + type Item<'a> = &'a str; -impl<'a> StringIterator<'a> { - /// Build a string iterator over the provided values, performing a - /// single upfront type check that all elements are strings. - pub(crate) fn new(values: &'a [Value]) -> Result { - for v in values { + fn precheck(slice: &[Value]) -> Result<(), Error> { + for v in slice { if !matches!(v, Value::String(_)) { return Err(Error::TypeError("expected string".into())); } } - - Ok(StringIterator { - inner: ValueListIterator::new(values), - }) + Ok(()) } -} - -impl<'a> Iterator for StringIterator<'a> { - type Item = &'a str; - - fn next(&mut self) -> Option { - let v = self.inner.next()?; + fn project<'a>(v: &'a Value) -> Self::Item<'a> { if let Value::String(s) = v { - Some(s.as_str()) + s.as_str() } else { - // `new` guarantees all elements are strings. - debug_assert!(false, "StringIterator saw non-string after construction"); - None + debug_assert!(false, "StringKind::project saw non-string after precheck"); + unreachable!("StringKind invariant violated") } } - - fn size_hint(&self) -> (usize, Option) { - self.inner.size_hint() - } } -impl<'a> ExactSizeIterator for StringIterator<'a> {} -impl<'a> FusedIterator for StringIterator<'a> {} +/// Borrowed iterator over a sequence of AST `Value` references. +/// +/// This is the shared base type for all list/sequence-parameter +/// iterators. Typed iterators such as [`NumIter`], [`BoolIter`] +/// and [`StringIter`] are specializations that provide +/// element-level typing. +pub type ValueIter<'a> = TypedValueIter<'a, ValueKind>; + +/// Borrowed iterator over numeric arguments, performing type checking +/// as elements are pulled. Internally this is a specialization of +/// [`TypedValueIter`] that narrows each element to `i64`. +pub type NumIter<'a> = TypedValueIter<'a, NumberKind>; + +/// Borrowed iterator over boolean arguments, performing type checking +/// as elements are pulled. Internally this is a specialization of +/// [`TypedValueIter`] that narrows each element to `bool`. +pub type BoolIter<'a> = TypedValueIter<'a, BoolKind>; + +/// Borrowed iterator over string arguments, performing type checking +/// as elements are pulled. Internally this is a specialization of +/// [`TypedValueIter`] that narrows each element to `&str`. +pub type StringIter<'a> = TypedValueIter<'a, StringKind>; // ===================================================================== // Rest-parameter support for variadic operations @@ -348,56 +296,56 @@ pub(crate) trait FromRest { fn from_rest<'a>(slice: &'a [Value]) -> Result, Error>; } -impl FromRest for ValueListIterator<'static> { - type Param<'a> = ValueListIterator<'a>; +impl FromRest for TypedValueIter<'static, K> +where + K: ValueElementKind, +{ + type Param<'a> = TypedValueIter<'a, K>; fn from_rest<'a>(slice: &'a [Value]) -> Result, Error> { - Ok(ValueListIterator::new(slice)) + TypedValueIter::::new(slice) } } -impl FromRest for NumIterator<'static> { - type Param<'a> = NumIterator<'a>; +// ===================================================================== +// Return-type adaptation for builtin functions +// ===================================================================== - fn from_rest<'a>(slice: &'a [Value]) -> Result, Error> { - NumIterator::new(slice) - } +/// Internal trait that normalizes builtin return types to the +/// canonical `Result` expected by the evaluator. +/// +/// Builtins typically return either `Result` directly +/// or `Result` for some `T` that implements `Into`. +pub(crate) trait IntoValueResult { + fn into_value_result(self) -> Result; } -impl FromRest for BoolIterator<'static> { - type Param<'a> = BoolIterator<'a>; - - fn from_rest<'a>(slice: &'a [Value]) -> Result, Error> { - BoolIterator::new(slice) +impl IntoValueResult for Result +where + T: Into, +{ + fn into_value_result(self) -> Result { + self.map(Into::into) } } -impl FromRest for StringIterator<'static> { - type Param<'a> = StringIterator<'a>; - - fn from_rest<'a>(slice: &'a [Value]) -> Result, Error> { - StringIterator::new(slice) +impl IntoValueResult for T +where + T: Into, +{ + fn into_value_result(self) -> Result { + Ok(self.into()) } } -/// Marker type used in `Args` tuples to indicate that a parameter -/// position is populated from the variadic "rest" arguments using -/// [`FromRest`]. -#[derive(Debug, Clone, Copy)] -pub struct Rest(std::marker::PhantomData); - -// Convenience aliases for common rest-parameter iterator types used in -// tests and documentation. These are only used at the type level; the -// actual parameters seen by builtin functions are the lifetime- -// parameterised iterator types above. -pub type ValuesRest = Rest>; -pub type NumRest = Rest>; -pub type BoolRest = Rest>; -pub type StringRest = Rest>; - -/// Convert a strongly-typed Rust function or closure into the erased -/// [`OperationFn`], parameterized by an argument tuple type. -pub trait IntoOperation { +/// Public trait for converting strongly-typed Rust functions or +/// closures into the erased [`OperationFn`], parameterized by an +/// argument tuple type. +/// +/// Builtins implemented via this trait may return any type `R` that +/// implements [`IntoValueResult`], typically `Result` +/// or `Result` where `T: Into`. +pub(crate) trait IntoOperation { fn into_operation(self) -> Arc; } @@ -405,10 +353,10 @@ pub trait IntoOperation { /// /// Implemented for functions whose Rust signature includes a variadic /// "rest" parameter, expressed using the iterator types defined in -/// this module (`ValueListIterator<'a>`, `NumIterator<'a>`, -/// `BoolIterator<'a>`, or `StringIterator<'a>`), optionally after a +/// this module (`ValueIter<'a>`, `NumIter<'a>`, +/// `BoolIter<'a>`, or `StringIter<'a>`), optionally after a /// fixed prefix of `FromParam` parameters. -pub trait IntoVariadicOperation { +pub(crate) trait IntoVariadicOperation { fn into_variadic_operation(self) -> Arc; } @@ -418,42 +366,40 @@ pub trait IntoVariadicOperation { /// Adapter for functions whose Rust signature consists only of a rest /// parameter expressed via one of the iterator types in this module -/// (e.g. `ValueListIterator<'a>`, `NumIterator<'a>`, etc.). -impl IntoVariadicOperation<(Rest,), R> for F +/// (e.g. `ValueIter<'a>`, `NumIter<'a>`, etc.). +impl IntoVariadicOperation<(I,)> for F where I: FromRest, - F: for<'a> Fn(::Param<'a>) -> FR + Send + Sync + 'static, - FR: IntoResult + 'static, - R: Into + 'static, + F: for<'a> Fn(::Param<'a>) -> R + Send + Sync + 'static, + R: IntoValueResult, { fn into_variadic_operation(self) -> Arc { Arc::new(move |args: Vec| { let rest_param: ::Param<'_> = ::from_rest(&args[..])?; - let result: FR = (self)(rest_param); - let value: R = result.into_result()?; - Ok(value.into()) + let result: R = (self)(rest_param); + result.into_value_result() }) } } -/// Helper macro to implement `IntoVariadicOperation` for functions with -/// a fixed prefix of `FromParam` parameters followed by a single rest -/// parameter expressed using one of the iterator types in this module. +/// Helper macro to implement `IntoVariadicOperation` for functions +/// with a fixed prefix of `FromParam` parameters followed by a single +/// rest parameter expressed using one of the iterator types in this +/// module. macro_rules! impl_into_variadic_operation_for_prefix_and_rest { ($prefix:expr, $( $v:ident, $p:ident : $A:ident ),+ ) => { - impl IntoVariadicOperation<( $( $A, )+ Rest, ), R> for F + impl IntoVariadicOperation<( $( $A, )+ I, )> for F where I: FromRest, $( $A: FromParam, )+ F: for<'a> Fn( $( <$A as FromParam>::Param<'a> ),+, ::Param<'a>, - ) -> FR + ) -> R + Send + Sync + 'static, - FR: IntoResult + 'static, - R: Into + 'static, + R: IntoValueResult, { fn into_variadic_operation(self) -> Arc { Arc::new(move |mut args: Vec| { @@ -468,9 +414,8 @@ macro_rules! impl_into_variadic_operation_for_prefix_and_rest { let rest_param: ::Param<'_> = ::from_rest(&*rest)?; - let result: FR = (self)( $( $p ),+, rest_param ); - let value: R = result.into_result()?; - Ok(value.into()) + let result: R = (self)( $( $p ),+, rest_param ); + result.into_value_result() } _ => Err(Error::arity_error($prefix, len)), } @@ -501,15 +446,14 @@ impl_into_variadic_operation_for_prefix_and_rest!(7, v0, p0: A1, v1, p1: A2, v2, /// builtin function. macro_rules! impl_into_operation_for_arity { ($arity:expr, $( $v:ident, $p:ident : $A:ident ),+ ) => { - impl IntoOperation<( $( $A, )+ ), R> for F + impl IntoOperation<( $( $A, )+ )> for F where - F: for<'a> Fn( $( <$A as FromParam>::Param<'a> ),+ ) -> FR + F: for<'a> Fn( $( <$A as FromParam>::Param<'a> ),+ ) -> R + Send + Sync + 'static, - FR: IntoResult + 'static, - R: Into + 'static, $( $A: FromParam, )+ + R: IntoValueResult, { fn into_operation(self) -> Arc { Arc::new(move |mut args: Vec| { @@ -521,9 +465,8 @@ macro_rules! impl_into_operation_for_arity { <$A as FromParam>::from_arg($v)?; )+ - let result: FR = (self)( $( $p ),+ ); - let value: R = result.into_result()?; - Ok(value.into()) + let result: R = (self)( $( $p ),+ ); + result.into_value_result() } _ => Err(Error::arity_error($arity, len)), } @@ -534,11 +477,10 @@ macro_rules! impl_into_operation_for_arity { } // 0-arg functions / closures -impl IntoOperation<(), R> for F +impl IntoOperation<()> for F where - F: Fn() -> FR + Send + Sync + 'static, - FR: IntoResult + 'static, - R: Into + 'static, + F: Fn() -> R + Send + Sync + 'static, + R: IntoValueResult, { fn into_operation(self) -> Arc { Arc::new(move |args: Vec| { @@ -546,9 +488,8 @@ where return Err(Error::arity_error(0, args.len())); } - let result: FR = (self)(); - let value: R = result.into_result()?; - Ok(value.into()) + let result: R = (self)(); + result.into_value_result() }) } } diff --git a/src/lib.rs b/src/lib.rs index 5454490..d800b06 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -113,7 +113,6 @@ impl fmt::Display for Error { pub mod ast; pub mod builtinops; pub mod evaluator; -pub mod intooperation; #[cfg(feature = "jsonlogic")] pub mod jsonlogic; From b86f54e885a675f09cf0559d95640eaffe3e8e4f Mon Sep 17 00:00:00 2001 From: Eugene Talagrand Date: Fri, 21 Nov 2025 14:47:21 -0800 Subject: [PATCH 4/6] Move builtin functions to new typesafe mechanics --- src/builtinops.rs | 1050 ++++++++++++++++++++++----------------------- src/evaluator.rs | 14 +- 2 files changed, 529 insertions(+), 535 deletions(-) diff --git a/src/builtinops.rs b/src/builtinops.rs index 117fcab..79b99e8 100644 --- a/src/builtinops.rs +++ b/src/builtinops.rs @@ -51,11 +51,13 @@ use crate::Error; use crate::ast::{NumberType, Value}; +use crate::evaluator::intooperation::{IntoOperation, IntoVariadicOperation, OperationFn}; use crate::evaluator::{ - Environment, eval_and, eval_define, eval_if, eval_lambda, eval_or, eval_quote, + Environment, NumIter, StringIter, ValueIter, eval_and, eval_define, eval_if, eval_lambda, + eval_or, eval_quote, }; use std::collections::HashMap; -use std::sync::LazyLock; +use std::sync::{Arc, LazyLock}; /// Represents the expected number of arguments for an operation #[derive(Debug, Clone, PartialEq)] @@ -99,8 +101,9 @@ impl Arity { /// Represents the implementation of a built-in expression (function or special form) #[derive(Clone)] pub enum OpKind { - /// Regular function that takes arguments and returns a value - Function(fn(&[Value]) -> Result), + /// Regular function that takes evaluated arguments and returns a value + /// via the canonical erased builtin signature used by the evaluator. + Function(Arc), /// Special form that requires access to the environment, unevaluated arguments and current evaluation stack depth SpecialForm(fn(&[Value], &mut Environment, usize) -> Result), } @@ -117,9 +120,7 @@ impl std::fmt::Debug for OpKind { impl PartialEq for OpKind { fn eq(&self, other: &Self) -> bool { match (self, other) { - (OpKind::Function(f1), OpKind::Function(f2)) => { - std::ptr::eq(f1 as *const _, f2 as *const _) - } + (OpKind::Function(f1), OpKind::Function(f2)) => Arc::ptr_eq(f1, f2), (OpKind::SpecialForm(f1), OpKind::SpecialForm(f2)) => { std::ptr::eq(f1 as *const _, f2 as *const _) } @@ -168,25 +169,26 @@ impl BuiltinOp { // Macro to generate numeric comparison functions macro_rules! numeric_comparison { ($name:ident, $op:tt, $op_str:expr) => { - fn $name(args: &[Value]) -> Result { + fn $name(mut args: NumIter<'_>) -> Result { // SCHEME-JSONLOGIC-STRICT: Require at least 2 arguments (both standards allow < 2 args but with different semantics) - if args.len() < 2 { - return Err(Error::arity_error(2, args.len())); + let len = args.len(); + if len < 2 { + return Err(Error::arity_error(2, len)); } // Chain comparisons: all adjacent pairs must satisfy the comparison - for window in args.windows(2) { - match window { - [Value::Number(a), Value::Number(b)] => { - if !(a $op b) { - return Ok(Value::Bool(false)); - } - } - _ => return Err(Error::TypeError(concat!($op_str, " requires numbers").into())), + let mut prev = args + .next() + .expect("ExactSizeIterator len() contract violated for numeric comparison"); + + for current in args { + if !(prev $op current) { + return Ok(false); } + prev = current; } - Ok(Value::Bool(true)) + Ok(true) } }; } @@ -198,391 +200,366 @@ numeric_comparison!(builtin_gt, >, ">"); numeric_comparison!(builtin_le, <=, "<="); numeric_comparison!(builtin_ge, >=, ">="); -fn builtin_add(args: &[Value]) -> Result { +fn builtin_add(args: NumIter<'_>) -> Result { let mut sum = 0 as NumberType; for arg in args { - if let Value::Number(n) = arg { - sum = sum - .checked_add(*n) - .ok_or_else(|| Error::EvalError("Integer overflow in addition".into()))?; - } else { - return Err(Error::TypeError("+ requires numbers".into())); - } + sum = sum + .checked_add(arg) + .ok_or_else(|| Error::EvalError("Integer overflow in addition".into()))?; } - Ok(Value::Number(sum)) + Ok(sum) } -fn builtin_sub(args: &[Value]) -> Result { - match args { - [] => Err(Error::arity_error(1, 0)), - [Value::Number(first)] => { - // Unary minus: check for overflow when negating - let result = first - .checked_neg() - .ok_or_else(|| Error::EvalError("Integer overflow in negation".into()))?; - Ok(Value::Number(result)) - } - [Value::Number(first), rest @ ..] => { - let mut result = *first; - for arg in rest { - if let Value::Number(n) = arg { - result = result.checked_sub(*n).ok_or_else(|| { - Error::EvalError("Integer overflow in subtraction".into()) - })?; - } else { - return Err(Error::TypeError("- requires numbers".into())); - } - } - Ok(Value::Number(result)) - } - _ => Err(Error::TypeError("- requires numbers".into())), - } -} +fn builtin_sub(first: NumberType, rest: NumIter<'_>) -> Result { + let mut iter = rest.peekable(); -fn builtin_mul(args: &[Value]) -> Result { - // SCHEME-STRICT: Require at least 1 argument (Scheme R7RS allows 0 args, returns 1) - if args.is_empty() { - return Err(Error::arity_error(1, 0)); + if iter.peek().is_none() { + return first + .checked_neg() + .ok_or_else(|| Error::EvalError("Integer overflow in negation".into())); } - let mut product = 1 as NumberType; - for arg in args { - if let Value::Number(n) = arg { - product = product - .checked_mul(*n) - .ok_or_else(|| Error::EvalError("Integer overflow in multiplication".into()))?; - } else { - return Err(Error::TypeError("* requires numbers".into())); - } + let mut result = first; + for n in iter { + result = result + .checked_sub(n) + .ok_or_else(|| Error::EvalError("Integer overflow in subtraction".into()))?; } - Ok(Value::Number(product)) + + Ok(result) } -fn builtin_car(args: &[Value]) -> Result { - match args { - [Value::List(list)] => match list.as_slice() { - [] => Err(Error::EvalError("car of empty list".into())), - [first, ..] => Ok(first.clone()), - }, - [_] => Err(Error::TypeError("car requires a list".into())), - _ => Err(Error::arity_error(1, args.len())), +// SCHEME-STRICT: Require at least 1 argument (Scheme R7RS allows 0 args, returns 1) +fn builtin_mul(first: NumberType, rest: NumIter<'_>) -> Result { + let mut product = first; + for n in rest { + product = product + .checked_mul(n) + .ok_or_else(|| Error::EvalError("Integer overflow in multiplication".into()))?; } + Ok(product) } -fn builtin_cdr(args: &[Value]) -> Result { - match args { - [Value::List(list)] => match list.as_slice() { - [] => Err(Error::EvalError("cdr of empty list".into())), - [_, rest @ ..] => Ok(Value::List(rest.to_vec())), - }, - [_] => Err(Error::TypeError("cdr requires a list".into())), - _ => Err(Error::arity_error(1, args.len())), +fn builtin_car(mut list: ValueIter<'_>) -> Result { + match list.next() { + Some(first) => Ok(first.clone()), + None => Err(Error::EvalError("car of empty list".into())), } } -fn builtin_cons(args: &[Value]) -> Result { - match args { - [first, Value::List(rest)] => { - let mut new_list = vec![first.clone()]; - new_list.extend_from_slice(rest); +fn builtin_cdr(mut list: ValueIter<'_>) -> Result { + let Some(_) = list.next() else { + return Err(Error::EvalError("cdr of empty list".into())); + }; + + let rest: Vec = list.cloned().collect(); + Ok(Value::List(rest)) +} + +fn builtin_cons(first: Value, rest: Value) -> Result { + match rest { + Value::List(tail) => { + let mut new_list = vec![first]; + new_list.extend_from_slice(&tail); Ok(Value::List(new_list)) } - [_, _] => Err(Error::TypeError( + _ => Err(Error::TypeError( // SCHEME-STRICT: Require second argument to be a list (Scheme R7RS allows improper lists) "cons requires a list as second argument".to_owned(), )), - _ => Err(Error::arity_error(2, args.len())), } } -fn builtin_list(args: &[Value]) -> Result { - Ok(Value::List(args.to_vec())) +fn builtin_list(args: ValueIter<'_>) -> Value { + Value::List(args.cloned().collect()) } -fn builtin_null(args: &[Value]) -> Result { - match args { - [value] => Ok(Value::Bool(value.is_nil())), - _ => Err(Error::arity_error(1, args.len())), - } +fn builtin_null(value: Value) -> bool { + value.is_nil() } -fn builtin_not(args: &[Value]) -> Result { - match args { - [Value::Bool(b)] => Ok(Value::Bool(!b)), - [_] => Err(Error::TypeError("not requires a boolean argument".into())), - _ => Err(Error::arity_error(1, args.len())), - } +fn builtin_not(b: bool) -> bool { + !b } -fn builtin_equal(args: &[Value]) -> Result { - match args { - [first, second] => { - // Scheme's equal? is structural equality for all types - // JSONLOGIC-STRICT: Reject type coercion - require same types for equality - match (first, second) { - (Value::Bool(_), Value::Bool(_)) - | (Value::Number(_), Value::Number(_)) - | (Value::String(_), Value::String(_)) - | (Value::Symbol(_), Value::Symbol(_)) - | (Value::List(_), Value::List(_)) => { - // Same types - use structural equality - Ok(Value::Bool(first == second)) - } - _ => { - // Different types or non-comparable types - reject type coercion - Err(Error::TypeError( - "JSONLOGIC-STRICT: Equality comparison requires arguments of the same comparable type (no type coercion)".to_owned(), - )) - } - } +fn builtin_equal(first: Value, second: Value) -> Result { + // Scheme's equal? is structural equality for all types + // JSONLOGIC-STRICT: Reject type coercion - require same types for equality + match (&first, &second) { + (Value::Bool(_), Value::Bool(_)) + | (Value::Number(_), Value::Number(_)) + | (Value::String(_), Value::String(_)) + | (Value::Symbol(_), Value::Symbol(_)) + | (Value::List(_), Value::List(_)) => { + // Same types - use structural equality + Ok(first == second) + } + _ => { + // Different types or non-comparable types - reject type coercion + Err(Error::TypeError( + "JSONLOGIC-STRICT: Equality comparison requires arguments of the same comparable type (no type coercion)".to_owned(), + )) } - _ => Err(Error::arity_error(2, args.len())), } } -fn builtin_string_append(args: &[Value]) -> Result { +fn builtin_string_append(args: StringIter<'_>) -> String { let mut result = String::new(); - for arg in args { - if let Value::String(s) = arg { - result.push_str(s); - } else { - return Err(Error::TypeError( - "string-append requires string arguments".into(), - )); - } + for s in args { + result.push_str(s); } - Ok(Value::String(result)) + result } -macro_rules! min_max_op { - ($name:ident, $op:ident, $initial:expr, $op_name:expr) => { - fn $name(args: &[Value]) -> Result { - if args.is_empty() { - return Err(Error::arity_error(1, 0)); - } - let mut result = $initial; - for arg in args { - if let Value::Number(n) = arg { - result = result.$op(*n); - } else { - return Err(Error::TypeError( - concat!($op_name, " requires number arguments").into(), - )); - } - } - Ok(Value::Number(result)) - } - }; +fn builtin_max(first: NumberType, rest: NumIter<'_>) -> NumberType { + let mut result = first; + for n in rest { + result = result.max(n); + } + result } -min_max_op!(builtin_max, max, NumberType::MIN, "max"); -min_max_op!(builtin_min, min, NumberType::MAX, "min"); - -fn builtin_error(args: &[Value]) -> Result { - // Convert a value to error message string - fn value_to_error_string(value: &Value) -> String { - match value { - Value::String(s) => s.clone(), // Remove quotes for error messages - _ => format!("{value}"), // Use Display trait for everything else - } +fn builtin_min(first: NumberType, rest: NumIter<'_>) -> NumberType { + let mut result = first; + for n in rest { + result = result.min(n); } + result +} - // Build multi-part error message - fn build_error_message(first: &Value, rest: &[Value]) -> String { - let mut message = value_to_error_string(first); - for arg in rest { - message.push(' '); - message.push_str(&value_to_error_string(arg)); - } - message +fn builtin_error(args: ValueIter<'_>) -> Result { + let parts: Vec = args + .map(|value| match value { + Value::String(s) => s.clone(), + _ => format!("{value}"), + }) + .collect(); + + let message = if parts.is_empty() { + "Error".to_string() + } else { + parts.join(" ") + }; + + Err(Error::EvalError(message)) +} + +/// Global registry of all built-in operations. +/// +/// We keep the registry layout as a single contiguous collection of +/// `BuiltinOp` values for ease of auditing, but the underlying +/// builtin implementations are wired through the same adapter layer +/// used for custom builtin registration. This is done once at +/// initialization time via a `LazyLock`. +static BUILTIN_OPS: LazyLock> = LazyLock::new(|| { + fn builtin_fixed(f: F) -> Arc + where + F: IntoOperation, + { + >::into_operation(f) } - match args { - [] => Err(Error::EvalError("Error".into())), - [single] => Err(Error::EvalError(value_to_error_string(single))), - [first, rest @ ..] => Err(Error::EvalError(build_error_message(first, rest))), + fn builtin_variadic(f: F) -> Arc + where + F: IntoVariadicOperation, + { + >::into_variadic_operation(f) } -} -/// Global registry of all built-in operations as a simple array -static BUILTIN_OPS: &[BuiltinOp] = &[ - // Arithmetic operations - BuiltinOp { - scheme_id: "+", - jsonlogic_id: "+", - op_kind: OpKind::Function(builtin_add), - arity: Arity::AtLeast(0), - }, - BuiltinOp { - scheme_id: "-", - jsonlogic_id: "-", - op_kind: OpKind::Function(builtin_sub), - arity: Arity::AtLeast(1), - }, - BuiltinOp { - scheme_id: "*", - jsonlogic_id: "*", - op_kind: OpKind::Function(builtin_mul), - arity: Arity::AtLeast(1), // SCHEME-STRICT: Scheme R7RS allows 0 arguments (returns 1) - }, - // Comparison operations - BuiltinOp { - scheme_id: ">", - jsonlogic_id: ">", - op_kind: OpKind::Function(builtin_gt), - arity: Arity::AtLeast(2), - }, - BuiltinOp { - scheme_id: ">=", - jsonlogic_id: ">=", - op_kind: OpKind::Function(builtin_ge), - arity: Arity::AtLeast(2), - }, - BuiltinOp { - scheme_id: "<", - jsonlogic_id: "<", - op_kind: OpKind::Function(builtin_lt), - arity: Arity::AtLeast(2), - }, - BuiltinOp { - scheme_id: "<=", - jsonlogic_id: "<=", - op_kind: OpKind::Function(builtin_le), - arity: Arity::AtLeast(2), - }, - BuiltinOp { - scheme_id: "=", - jsonlogic_id: "scheme-numeric-equals", - op_kind: OpKind::Function(builtin_eq), - arity: Arity::AtLeast(2), - }, - BuiltinOp { - scheme_id: "equal?", - jsonlogic_id: "===", - op_kind: OpKind::Function(builtin_equal), - arity: Arity::Exact(2), - }, - // Logical operations - BuiltinOp { - scheme_id: "not", - jsonlogic_id: "!", - op_kind: OpKind::Function(builtin_not), - arity: Arity::Exact(1), - }, - BuiltinOp { - scheme_id: "and", - jsonlogic_id: "and", - op_kind: OpKind::SpecialForm(eval_and), - arity: Arity::AtLeast(1), // SCHEME-STRICT: Scheme R7RS allows 0 arguments (returns #t) - }, - BuiltinOp { - scheme_id: "or", - jsonlogic_id: "or", - op_kind: OpKind::SpecialForm(eval_or), - arity: Arity::AtLeast(1), // SCHEME-STRICT: Scheme R7RS allows 0 arguments (returns #f) - }, - // Control flow - BuiltinOp { - scheme_id: "if", - jsonlogic_id: "if", - op_kind: OpKind::SpecialForm(eval_if), - // SCHEME-JSONLOGIC-STRICT: Require exactly 3 arguments - // (Scheme allows 2 args with undefined behavior, JSONLogic allows chaining with >3 args) - arity: Arity::Exact(3), - }, - // Special forms for language constructs - BuiltinOp { - scheme_id: "quote", - jsonlogic_id: "scheme-quote", - op_kind: OpKind::SpecialForm(eval_quote), - arity: Arity::Exact(1), - }, - BuiltinOp { - scheme_id: "define", - jsonlogic_id: "scheme-define", - op_kind: OpKind::SpecialForm(eval_define), - arity: Arity::Exact(2), - }, - BuiltinOp { - scheme_id: "lambda", - jsonlogic_id: "scheme-lambda", - op_kind: OpKind::SpecialForm(eval_lambda), - // SCHEME-STRICT: Only supports fixed-arity lambdas (lambda (a b c) body) - // Does not support variadic forms: (lambda args body) or (lambda (a . rest) body) - // Duplicate parameter names are prohibited per R7RS standard - arity: Arity::Exact(2), - }, - // List operations - BuiltinOp { - scheme_id: "car", - jsonlogic_id: "scheme-car", - op_kind: OpKind::Function(builtin_car), - arity: Arity::Exact(1), - }, - BuiltinOp { - scheme_id: "cdr", - jsonlogic_id: "scheme-cdr", - op_kind: OpKind::Function(builtin_cdr), - arity: Arity::Exact(1), - }, - BuiltinOp { - scheme_id: "cons", - jsonlogic_id: "scheme-cons", - op_kind: OpKind::Function(builtin_cons), - arity: Arity::Exact(2), - }, - BuiltinOp { - scheme_id: "list", - jsonlogic_id: "scheme-list", - op_kind: OpKind::Function(builtin_list), - arity: Arity::Any, - }, - BuiltinOp { - scheme_id: "null?", - jsonlogic_id: "scheme-null?", - op_kind: OpKind::Function(builtin_null), - arity: Arity::Exact(1), - }, - // String operations - BuiltinOp { - scheme_id: "string-append", - jsonlogic_id: "cat", - op_kind: OpKind::Function(builtin_string_append), - arity: Arity::Any, - }, - // Math operations - BuiltinOp { - scheme_id: "max", - jsonlogic_id: "max", - op_kind: OpKind::Function(builtin_max), - arity: Arity::AtLeast(1), - }, - BuiltinOp { - scheme_id: "min", - jsonlogic_id: "min", - op_kind: OpKind::Function(builtin_min), - arity: Arity::AtLeast(1), - }, - // Error handling - BuiltinOp { - scheme_id: "error", - jsonlogic_id: "scheme-error", - op_kind: OpKind::Function(builtin_error), - arity: Arity::Any, - }, -]; + vec![ + // Arithmetic operations + BuiltinOp { + scheme_id: "+", + jsonlogic_id: "+", + op_kind: OpKind::Function(builtin_variadic::<(NumIter<'static>,), _>(builtin_add)), + arity: Arity::AtLeast(0), + }, + BuiltinOp { + scheme_id: "-", + jsonlogic_id: "-", + op_kind: OpKind::Function(builtin_variadic::<(NumberType, NumIter<'static>), _>( + builtin_sub, + )), + arity: Arity::AtLeast(1), + }, + BuiltinOp { + scheme_id: "*", + jsonlogic_id: "*", + op_kind: OpKind::Function(builtin_variadic::<(NumberType, NumIter<'static>), _>( + builtin_mul, + )), + arity: Arity::AtLeast(1), // SCHEME-STRICT: Scheme R7RS allows 0 arguments (returns 1) + }, + // Comparison operations + BuiltinOp { + scheme_id: ">", + jsonlogic_id: ">", + op_kind: OpKind::Function(builtin_variadic::<(NumIter<'static>,), _>(builtin_gt)), + arity: Arity::AtLeast(2), + }, + BuiltinOp { + scheme_id: ">=", + jsonlogic_id: ">=", + op_kind: OpKind::Function(builtin_variadic::<(NumIter<'static>,), _>(builtin_ge)), + arity: Arity::AtLeast(2), + }, + BuiltinOp { + scheme_id: "<", + jsonlogic_id: "<", + op_kind: OpKind::Function(builtin_variadic::<(NumIter<'static>,), _>(builtin_lt)), + arity: Arity::AtLeast(2), + }, + BuiltinOp { + scheme_id: "<=", + jsonlogic_id: "<=", + op_kind: OpKind::Function(builtin_variadic::<(NumIter<'static>,), _>(builtin_le)), + arity: Arity::AtLeast(2), + }, + BuiltinOp { + scheme_id: "=", + jsonlogic_id: "scheme-numeric-equals", + op_kind: OpKind::Function(builtin_variadic::<(NumIter<'static>,), _>(builtin_eq)), + arity: Arity::AtLeast(2), + }, + BuiltinOp { + scheme_id: "equal?", + jsonlogic_id: "===", + op_kind: OpKind::Function(builtin_fixed::<(Value, Value), _>(builtin_equal)), + arity: Arity::Exact(2), + }, + // Logical operations + BuiltinOp { + scheme_id: "not", + jsonlogic_id: "!", + op_kind: OpKind::Function(builtin_fixed::<(bool,), _>(builtin_not)), + arity: Arity::Exact(1), + }, + BuiltinOp { + scheme_id: "and", + jsonlogic_id: "and", + op_kind: OpKind::SpecialForm(eval_and), + arity: Arity::AtLeast(1), // SCHEME-STRICT: Scheme R7RS allows 0 arguments (returns #t) + }, + BuiltinOp { + scheme_id: "or", + jsonlogic_id: "or", + op_kind: OpKind::SpecialForm(eval_or), + arity: Arity::AtLeast(1), // SCHEME-STRICT: Scheme R7RS allows 0 arguments (returns #f) + }, + // Control flow + BuiltinOp { + scheme_id: "if", + jsonlogic_id: "if", + op_kind: OpKind::SpecialForm(eval_if), + // SCHEME-JSONLOGIC-STRICT: Require exactly 3 arguments + // (Scheme allows 2 args with undefined behavior, JSONLogic allows chaining with >3 args) + arity: Arity::Exact(3), + }, + // Special forms for language constructs + BuiltinOp { + scheme_id: "quote", + jsonlogic_id: "scheme-quote", + op_kind: OpKind::SpecialForm(eval_quote), + arity: Arity::Exact(1), + }, + BuiltinOp { + scheme_id: "define", + jsonlogic_id: "scheme-define", + op_kind: OpKind::SpecialForm(eval_define), + arity: Arity::Exact(2), + }, + BuiltinOp { + scheme_id: "lambda", + jsonlogic_id: "scheme-lambda", + op_kind: OpKind::SpecialForm(eval_lambda), + // SCHEME-STRICT: Only supports fixed-arity lambdas (lambda (a b c) body) + // Does not support variadic forms: (lambda args body) or (lambda (a . rest) body) + // Duplicate parameter names are prohibited per R7RS standard + arity: Arity::Exact(2), + }, + // List operations + BuiltinOp { + scheme_id: "car", + jsonlogic_id: "scheme-car", + op_kind: OpKind::Function(builtin_fixed::<(ValueIter<'static>,), _>(builtin_car)), + arity: Arity::Exact(1), + }, + BuiltinOp { + scheme_id: "cdr", + jsonlogic_id: "scheme-cdr", + op_kind: OpKind::Function(builtin_fixed::<(ValueIter<'static>,), _>(builtin_cdr)), + arity: Arity::Exact(1), + }, + BuiltinOp { + scheme_id: "cons", + jsonlogic_id: "scheme-cons", + op_kind: OpKind::Function(builtin_fixed::<(Value, Value), _>(builtin_cons)), + arity: Arity::Exact(2), + }, + BuiltinOp { + scheme_id: "list", + jsonlogic_id: "scheme-list", + op_kind: OpKind::Function(builtin_variadic::<(ValueIter<'static>,), _>(builtin_list)), + arity: Arity::Any, + }, + BuiltinOp { + scheme_id: "null?", + jsonlogic_id: "scheme-null?", + op_kind: OpKind::Function(builtin_fixed::<(Value,), _>(builtin_null)), + arity: Arity::Exact(1), + }, + // String operations + BuiltinOp { + scheme_id: "string-append", + jsonlogic_id: "cat", + op_kind: OpKind::Function(builtin_variadic::<(StringIter<'static>,), _>( + builtin_string_append, + )), + arity: Arity::Any, + }, + // Math operations + BuiltinOp { + scheme_id: "max", + jsonlogic_id: "max", + op_kind: OpKind::Function(builtin_variadic::<(NumberType, NumIter<'static>), _>( + builtin_max, + )), + arity: Arity::AtLeast(1), + }, + BuiltinOp { + scheme_id: "min", + jsonlogic_id: "min", + op_kind: OpKind::Function(builtin_variadic::<(NumberType, NumIter<'static>), _>( + builtin_min, + )), + arity: Arity::AtLeast(1), + }, + // Error handling + BuiltinOp { + scheme_id: "error", + jsonlogic_id: "scheme-error", + op_kind: OpKind::Function(builtin_variadic::<(ValueIter<'static>,), _>(builtin_error)), + arity: Arity::Any, + }, + ] +}); /// Lazy static map from scheme_id to BuiltinOp (private - use find_builtin_op_by_scheme_id) -static BUILTIN_SCHEME: LazyLock> = - LazyLock::new(|| BUILTIN_OPS.iter().map(|op| (op.scheme_id, op)).collect()); +static BUILTIN_SCHEME: LazyLock> = LazyLock::new(|| { + let ops: &'static [BuiltinOp] = BUILTIN_OPS.as_slice(); + ops.iter().map(|op| (op.scheme_id, op)).collect() +}); /// Lazy static map from jsonlogic_id to BuiltinOp (private - use find_builtin_op_by_jsonlogic_id) static BUILTIN_JSONLOGIC: LazyLock> = - LazyLock::new(|| BUILTIN_OPS.iter().map(|op| (op.jsonlogic_id, op)).collect()); + LazyLock::new(|| { + let ops: &'static [BuiltinOp] = BUILTIN_OPS.as_slice(); + ops.iter().map(|op| (op.jsonlogic_id, op)).collect() + }); /// Get all builtin operations (for internal use by evaluator) pub(crate) fn get_builtin_ops() -> &'static [BuiltinOp] { - BUILTIN_OPS + BUILTIN_OPS.as_slice() } /// Find a builtin operation by its Scheme identifier @@ -616,6 +593,21 @@ mod tests { Some(val(value)) } + /// Helper to invoke a builtin through the public registry using + /// the canonical erased signature (Vec -> Result). + /// + /// This keeps tests independent of the internal typed helper + /// function signatures while still exercising the adapter layer. + fn call_builtin(name: &str, args: &[Value]) -> Result { + let op = find_scheme_op(name).expect("builtin not found"); + match &op.op_kind { + OpKind::Function(func) => func(args.to_vec()), + OpKind::SpecialForm(_) => { + panic!("expected function builtin in tests, got special form: {name}") + } + } + } + #[test] fn test_builtin_ops_registry() { // Test lookup by both scheme and jsonlogic ids @@ -633,7 +625,7 @@ mod tests { assert!(!add_op.is_special_form()); if let OpKind::Function(func) = &add_op.op_kind { - let result = func(&[val(1), val(2)]).unwrap(); + let result = func(vec![val(1), val(2)]).unwrap(); assert_eq!(result, val(3)); } else { panic!("Expected Function variant"); @@ -692,10 +684,10 @@ mod tests { ); } - /// Macro to create test cases with identifying information for better error messages + /// Macro to create test cases, invoking builtins via the registry. macro_rules! test { - ($expr:expr, $expected:expr) => { - (stringify!($expr), $expr, $expected) + ($name:expr, $args:expr, $expected:expr) => { + ($name, call_builtin($name, $args), $expected) }; } @@ -735,236 +727,240 @@ mod tests { // ================================================================= // Test arithmetic functions - addition - test!(builtin_add(&[]), success(0)), // Identity - test!(builtin_add(&[val(5)]), success(5)), // Single number - test!(builtin_add(&[val(1), val(2), val(3)]), success(6)), // Multiple numbers - test!(builtin_add(&[val(-5), val(10)]), success(5)), // Negative numbers - test!(builtin_add(&[val(0), val(0), val(0)]), success(0)), // Zeros + test!("+", &[], success(0)), // Identity + test!("+", &[val(5)], success(5)), // Single number + test!("+", &[val(1), val(2), val(3)], success(6)), // Multiple numbers + test!("+", &[val(-5), val(10)], success(5)), // Negative numbers + test!("+", &[val(0), val(0), val(0)], success(0)), // Zeros // Test addition error cases - test!(builtin_add(&[val("not a number")]), None), // Invalid type - test!(builtin_add(&[val(1), val(true)]), None), // Mixed types + test!("+", &[val("not a number")], None), // Invalid type + test!("+", &[val(1), val(true)], None), // Mixed types // Test arithmetic functions - subtraction - test!(builtin_sub(&[val(5)]), success(-5)), // Unary minus - test!(builtin_sub(&[val(-5)]), success(5)), // Unary minus of negative - test!(builtin_sub(&[val(10), val(3), val(2)]), success(5)), // Multiple subtraction - test!(builtin_sub(&[val(0), val(5)]), success(-5)), // Zero minus number - test!(builtin_sub(&[val(10), val(0)]), success(10)), // Number minus zero + test!("-", &[val(5)], success(-5)), // Unary minus + test!("-", &[val(-5)], success(5)), // Unary minus of negative + test!("-", &[val(10), val(3), val(2)], success(5)), // Multiple subtraction + test!("-", &[val(0), val(5)], success(-5)), // Zero minus number + test!("-", &[val(10), val(0)], success(10)), // Number minus zero // Test subtraction error cases - test!(builtin_sub(&[]), None), // No arguments - test!(builtin_sub(&[val("not a number")]), None), - test!(builtin_sub(&[val(5), val(false)]), None), + test!("-", &[], None), // No arguments + test!("-", &[val("not a number")], None), + test!("-", &[val(5), val(false)], None), // Test arithmetic functions - multiplication // SCHEME-STRICT: We require at least 1 argument (Scheme R7RS allows 0 args, returns 1) - test!(builtin_mul(&[]), None), // No arguments should error - test!(builtin_mul(&[val(5)]), success(5)), // Single number - test!(builtin_mul(&[val(2), val(3), val(4)]), success(24)), // Multiple numbers - test!(builtin_mul(&[val(-2), val(3)]), success(-6)), // Negative numbers - test!(builtin_mul(&[val(0), val(100)]), success(0)), // Zero multiplication - test!(builtin_mul(&[val(1), val(1), val(1)]), success(1)), // Ones + test!("*", &[], None), // No arguments should error + test!("*", &[val(5)], success(5)), // Single number + test!("*", &[val(2), val(3), val(4)], success(24)), // Multiple numbers + test!("*", &[val(-2), val(3)], success(-6)), // Negative numbers + test!("*", &[val(0), val(100)], success(0)), // Zero multiplication + test!("*", &[val(1), val(1), val(1)], success(1)), // Ones // Test multiplication error cases - test!(builtin_mul(&[val("not a number")]), None), - test!(builtin_mul(&[val(2), nil()]), None), + test!("*", &[val("not a number")], None), + test!("*", &[val(2), nil()], None), // Test comparison functions - greater than - test!(builtin_gt(&[val(7), val(3)]), success(true)), - test!(builtin_gt(&[val(3), val(8)]), success(false)), - test!(builtin_gt(&[val(4), val(4)]), success(false)), // Equal case - test!(builtin_gt(&[val(-1), val(-2)]), success(true)), // Negative numbers + test!(">", &[val(7), val(3)], success(true)), + test!(">", &[val(3), val(8)], success(false)), + test!(">", &[val(4), val(4)], success(false)), // Equal case + test!(">", &[val(-1), val(-2)], success(true)), // Negative numbers // Test chaining behavior: 9 > 6 > 2 should be true since all adjacent pairs satisfy > - test!(builtin_gt(&[val(9), val(6), val(2)]), success(true)), // Chaining true + test!(">", &[val(9), val(6), val(2)], success(true)), // Chaining true // Test chaining that should fail: 9 > 6 > 7 should be false since 6 > 7 is false - test!(builtin_gt(&[val(9), val(6), val(7)]), success(false)), // Chaining false + test!(">", &[val(9), val(6), val(7)], success(false)), // Chaining false // Test comparison error cases (wrong number of args or wrong types) - test!(builtin_gt(&[val(5)]), None), // Too few args - test!(builtin_gt(&[val("a"), val(3)]), None), // Wrong type + test!(">", &[val(5)], None), // Too few args + test!(">", &[val("a"), val(3)], None), // Wrong type // Test comparison functions - greater than or equal - test!(builtin_ge(&[val(8), val(3)]), success(true)), - test!(builtin_ge(&[val(2), val(6)]), success(false)), - test!(builtin_ge(&[val(7), val(7)]), success(true)), // Equal case + test!(">=", &[val(8), val(3)], success(true)), + test!(">=", &[val(2), val(6)], success(false)), + test!(">=", &[val(7), val(7)], success(true)), // Equal case // Test comparison functions - less than - test!(builtin_lt(&[val(2), val(9)]), success(true)), - test!(builtin_lt(&[val(8), val(4)]), success(false)), - test!(builtin_lt(&[val(6), val(6)]), success(false)), // Equal case + test!("<", &[val(2), val(9)], success(true)), + test!("<", &[val(8), val(4)], success(false)), + test!("<", &[val(6), val(6)], success(false)), // Equal case // Test numeric comparison chaining: 1 < 2 < 3 (all adjacent pairs satisfy <) - test!(builtin_lt(&[val(1), val(2), val(3)]), success(true)), // Chaining true + test!("<", &[val(1), val(2), val(3)], success(true)), // Chaining true // Test chaining that should fail: 1 < 3 but not 3 < 2 - test!(builtin_lt(&[val(1), val(3), val(2)]), success(false)), // Chaining false + test!("<", &[val(1), val(3), val(2)], success(false)), // Chaining false // Test comparison functions - less than or equal - test!(builtin_le(&[val(4), val(9)]), success(true)), - test!(builtin_le(&[val(8), val(2)]), success(false)), - test!(builtin_le(&[val(3), val(3)]), success(true)), // Equal case + test!("<=", &[val(4), val(9)], success(true)), + test!("<=", &[val(8), val(2)], success(false)), + test!("<=", &[val(3), val(3)], success(true)), // Equal case // Test numeric equality - test!(builtin_eq(&[val(12), val(12)]), success(true)), - test!(builtin_eq(&[val(8), val(3)]), success(false)), - test!(builtin_eq(&[val(0), val(0)]), success(true)), - test!(builtin_eq(&[val(-1), val(-1)]), success(true)), - test!(builtin_eq(&[val(7), val(7), val(7)]), success(true)), // 7 = 7 = 7 (all equal) - test!(builtin_eq(&[val(9), val(9), val(4)]), success(false)), // 9 = 9 but not 9 = 4 + test!("=", &[val(12), val(12)], success(true)), + test!("=", &[val(8), val(3)], success(false)), + test!("=", &[val(0), val(0)], success(true)), + test!("=", &[val(-1), val(-1)], success(true)), + test!("=", &[val(7), val(7), val(7)], success(true)), // 7 = 7 = 7 (all equal) + test!("=", &[val(9), val(9), val(4)], success(false)), // 9 = 9 but not 9 = 4 // Test structural equality (equal?) - test!(builtin_equal(&[val(11), val(11)]), success(true)), - test!(builtin_equal(&[val(15), val(3)]), success(false)), - test!(builtin_equal(&[val("hello"), val("hello")]), success(true)), - test!(builtin_equal(&[val("hello"), val("world")]), success(false)), - test!(builtin_equal(&[val(true), val(true)]), success(true)), - test!(builtin_equal(&[val(true), val(false)]), success(false)), - test!(builtin_equal(&[nil(), nil()]), success(true)), - test!(builtin_equal(&[val([1]), val([1])]), success(true)), - test!(builtin_equal(&[val(5), val("5")]), None), // Different types - now rejected + test!("equal?", &[val(11), val(11)], success(true)), + test!("equal?", &[val(15), val(3)], success(false)), + test!("equal?", &[val("hello"), val("hello")], success(true)), + test!("equal?", &[val("hello"), val("world")], success(false)), + test!("equal?", &[val(true), val(true)], success(true)), + test!("equal?", &[val(true), val(false)], success(false)), + test!("equal?", &[nil(), nil()], success(true)), + test!("equal?", &[val([1]), val([1])], success(true)), + test!("equal?", &[val(5), val("5")], None), // Different types - now rejected // Test equal? error cases (structural equality requires exactly 2 args) - test!(builtin_equal(&[val(5)]), None), // Too few args - test!(builtin_equal(&[val(5), val(3), val(1)]), None), // Too many args + test!("equal?", &[val(5)], None), // Too few args + test!("equal?", &[val(5), val(3), val(1)], None), // Too many args // Test logical functions - not - test!(builtin_not(&[val(true)]), success(false)), - test!(builtin_not(&[val(false)]), success(true)), + test!("not", &[val(true)], success(false)), + test!("not", &[val(false)], success(true)), // Test not error cases - test!(builtin_not(&[]), None), // No args - test!(builtin_not(&[val(true), val(false)]), None), // Too many args - test!(builtin_not(&[val(1)]), None), // Wrong type - test!(builtin_not(&[val("true")]), None), // Wrong type + test!("not", &[], None), // No args + test!("not", &[val(true), val(false)], None), // Too many args + test!("not", &[val(1)], None), // Wrong type + test!("not", &[val("true")], None), // Wrong type // Test list functions - car - test!(builtin_car(&[val([1, 2, 3])]), success(1)), // First element - test!(builtin_car(&[val(["only"])]), success("only")), // Single element - test!(builtin_car(&[val([val([1]), val(2)])]), success([1])), // Nested list + test!("car", &[val([1, 2, 3])], success(1)), // First element + test!("car", &[val(["only"])], success("only")), // Single element + test!("car", &[val([val([1]), val(2)])], success([1])), // Nested list // Test car error cases - test!(builtin_car(&[]), None), // No args - test!(builtin_car(&[int_list.clone(), int_list.clone()]), None), // Too many args - test!(builtin_car(&[nil()]), None), // Empty list - test!(builtin_car(&[val(42)]), None), // Not a list - test!(builtin_car(&[val("not a list")]), None), // Not a list + test!("car", &[], None), // No args + test!("car", &[int_list.clone(), int_list.clone()], None), // Too many args + test!("car", &[nil()], None), // Empty list + test!("car", &[val(42)], None), // Not a list + test!("car", &[val("not a list")], None), // Not a list // Test list functions - cdr - test!(builtin_cdr(&[val([1, 2, 3])]), success([2, 3])), // Rest of list - test!(builtin_cdr(&[val(["only"])]), Some(nil())), // Single element -> empty - test!(builtin_cdr(&[val([1, 2])]), success([2])), // Two elements + test!("cdr", &[val([1, 2, 3])], success([2, 3])), // Rest of list + test!("cdr", &[val(["only"])], Some(nil())), // Single element -> empty + test!("cdr", &[val([1, 2])], success([2])), // Two elements // Test cdr error cases - test!(builtin_cdr(&[]), None), // No args - test!(builtin_cdr(&[int_list.clone(), int_list]), None), // Too many args - test!(builtin_cdr(&[nil()]), None), // Empty list - test!(builtin_cdr(&[val(true)]), None), // Not a list + test!("cdr", &[], None), // No args + test!("cdr", &[int_list.clone(), int_list], None), // Too many args + test!("cdr", &[nil()], None), // Empty list + test!("cdr", &[val(true)], None), // Not a list // Test list functions - cons - test!(builtin_cons(&[val(0), val([1, 2])]), success([0, 1, 2])), // Prepend to list - test!(builtin_cons(&[val("first"), nil()]), success(["first"])), // Cons to empty - test!( - builtin_cons(&[val([1]), val([2])]), - success([val([1]), val(2)]) - ), // Nested cons + test!("cons", &[val(0), val([1, 2])], success([0, 1, 2])), // Prepend to list + test!("cons", &[val("first"), nil()], success(["first"])), // Cons to empty + test!("cons", &[val([1]), val([2])], success([val([1]), val(2)])), // Nested cons // Test cons error cases - test!(builtin_cons(&[]), None), // No args - test!(builtin_cons(&[val(1)]), None), // Too few args - test!(builtin_cons(&[val(1), val(2), val(3)]), None), // Too many args - test!(builtin_cons(&[val(1), val(2)]), None), // Second arg not a list - test!(builtin_cons(&[val(1), val("not a list")]), None), // Second arg not a list + test!("cons", &[], None), // No args + test!("cons", &[val(1)], None), // Too few args + test!("cons", &[val(1), val(2), val(3)], None), // Too many args + test!("cons", &[val(1), val(2)], None), // Second arg not a list + test!("cons", &[val(1), val("not a list")], None), // Second arg not a list // Test list functions - list - test!(builtin_list(&[]), Some(nil())), // Empty list - test!(builtin_list(&[val(1)]), success([1])), // Single element + test!("list", &[], Some(nil())), // Empty list + test!("list", &[val(1)], success([1])), // Single element test!( - builtin_list(&[val(1), val("hello"), val(true)]), + "list", + &[val(1), val("hello"), val(true)], success([val(1), val("hello"), val(true)]) ), // Mixed types - test!( - builtin_list(&[val([1]), val(2)]), - success([val([1]), val(2)]) - ), // Nested lists + test!("list", &[val([1]), val(2)], success([val([1]), val(2)])), // Nested lists // Test null? function - test!(builtin_null(&[nil()]), success(true)), // Empty list is nil - test!(builtin_null(&[val(42)]), success(false)), // Number is not nil - test!(builtin_null(&[val("")]), success(false)), // Empty string is not nil - test!(builtin_null(&[val(false)]), success(false)), // False is not nil - test!(builtin_null(&[val([1])]), success(false)), // Non-empty list is not nil + test!("null?", &[nil()], success(true)), // Empty list is nil + test!("null?", &[val(42)], success(false)), // Number is not nil + test!("null?", &[val("")], success(false)), // Empty string is not nil + test!("null?", &[val(false)], success(false)), // False is not nil + test!("null?", &[val([1])], success(false)), // Non-empty list is not nil // Test null? error cases - test!(builtin_null(&[]), None), // No args - test!(builtin_null(&[val(1), val(2)]), None), // Too many args + test!("null?", &[], None), // No args + test!("null?", &[val(1), val(2)], None), // Too many args // Test error function - test!(builtin_error(&[]), None), // No args - should produce generic error - test!(builtin_error(&[val("test error")]), None), // String message - test!(builtin_error(&[val(42)]), None), // Number message - test!(builtin_error(&[val(true)]), None), // Bool message - test!( - builtin_error(&[val("Error:"), val("Something went wrong")]), - None - ), // Multiple args + test!("error", &[], None), // No args - should produce generic error + test!("error", &[val("test error")], None), // String message + test!("error", &[val(42)], None), // Number message + test!("error", &[val(true)], None), // Bool message + test!("error", &[val("Error:"), val("Something went wrong")], None), // Multiple args // ================================================================= // ARITHMETIC EDGE CASES // ================================================================= // Integer overflow cases (should fail) - test!(builtin_add(&[val(NumberType::MAX), val(1)]), None), // Addition overflow - test!(builtin_mul(&[val(NumberType::MAX), val(2)]), None), // Multiplication overflow - test!(builtin_sub(&[val(NumberType::MIN)]), None), // Negation overflow - test!(builtin_sub(&[val(NumberType::MIN), val(1)]), None), // Subtraction overflow + test!("+", &[val(NumberType::MAX), val(1)], None), // Addition overflow + test!("*", &[val(NumberType::MAX), val(2)], None), // Multiplication overflow + test!("-", &[val(NumberType::MIN)], None), // Negation overflow + test!("-", &[val(NumberType::MIN), val(1)], None), // Subtraction overflow // Boundary values (should succeed) test!( - builtin_add(&[val(NumberType::MAX), val(0)]), + "+", + &[val(NumberType::MAX), val(0)], success(NumberType::MAX) ), test!( - builtin_sub(&[val(NumberType::MIN), val(0)]), + "-", + &[val(NumberType::MIN), val(0)], success(NumberType::MIN) ), test!( - builtin_mul(&[val(NumberType::MAX), val(1)]), + "*", + &[val(NumberType::MAX), val(1)], success(NumberType::MAX) ), - test!(builtin_mul(&[val(0), val(NumberType::MAX)]), success(0)), + test!("*", &[val(0), val(NumberType::MAX)], success(0)), // Operations with zero - test!(builtin_add(&[val(0)]), success(0)), - test!(builtin_sub(&[val(0)]), success(0)), - test!(builtin_mul(&[val(0)]), success(0)), + test!("+", &[val(0)], success(0)), + test!("-", &[val(0)], success(0)), + test!("*", &[val(0)], success(0)), // Large chain operations - test!(builtin_add(&many_ones), success(100)), - test!(builtin_mul(&many_ones), success(1)), + test!("+", &many_ones, success(100)), + test!("*", &many_ones, success(1)), // ================================================================= // COMPARISON EDGE CASES // ================================================================= // Boundary comparisons test!( - builtin_gt(&[val(NumberType::MAX), val(NumberType::MIN)]), + ">", + &[val(NumberType::MAX), val(NumberType::MIN)], success(true) ), test!( - builtin_lt(&[val(NumberType::MIN), val(NumberType::MAX)]), + "<", + &[val(NumberType::MIN), val(NumberType::MAX)], success(true) ), test!( - builtin_ge(&[val(NumberType::MAX), val(NumberType::MAX)]), + ">=", + &[val(NumberType::MAX), val(NumberType::MAX)], success(true) ), test!( - builtin_le(&[val(NumberType::MIN), val(NumberType::MIN)]), + "<=", + &[val(NumberType::MIN), val(NumberType::MIN)], success(true) ), // Long chain comparisons test!( - builtin_lt(&[val(-5), val(-2), val(0), val(3), val(10)]), + "<", + &[val(-5), val(-2), val(0), val(3), val(10)], success(true) ), test!( - builtin_gt(&[val(10), val(5), val(0), val(-3), val(-8)]), + ">", + &[val(10), val(5), val(0), val(-3), val(-8)], success(true) ), - test!(builtin_lt(&[val(1), val(2), val(1)]), success(false)), // 2 > 1 fails + test!("<", &[val(1), val(2), val(1)], success(false)), // 2 > 1 fails // Numeric equality with many values - test!(builtin_eq(&all_fives), success(true)), - test!(builtin_eq(&mostly_fives), success(false)), + test!("=", &all_fives, success(true)), + test!("=", &mostly_fives, success(false)), // ================================================================= // LIST OPERATIONS EDGE CASES // ================================================================= // Deeply nested lists - test!(builtin_car(&[nested]), success([val([1])])), + test!("car", &[nested], success([val([1])])), // Mixed type lists operations - test!(builtin_car(std::slice::from_ref(&mixed)), success(1)), + test!("car", std::slice::from_ref(&mixed), success(1)), test!( - builtin_cdr(std::slice::from_ref(&mixed)), + "cdr", + std::slice::from_ref(&mixed), success([val("hello"), val(true), nil()]) ), // Cons with various types test!( - builtin_cons(&[val(true), val([val(1), val(2)])]), + "cons", + &[val(true), val([val(1), val(2)])], success([val(true), val(1), val(2)]) ), // List creation with many elements test!( - builtin_list(&many_elements), + "list", + &many_elements, success((0..50).map(val).collect::>()) ), // ================================================================= @@ -972,64 +968,66 @@ mod tests { // ================================================================= // Basic string concatenation - test!(builtin_string_append(&[]), success("")), - test!(builtin_string_append(&[val("hello")]), success("hello")), + test!("string-append", &[], success("")), + test!("string-append", &[val("hello")], success("hello")), test!( - builtin_string_append(&[val("hello"), val(" "), val("world")]), + "string-append", + &[val("hello"), val(" "), val("world")], success("hello world") ), test!( - builtin_string_append(&[val(""), val("test"), val("")]), + "string-append", + &[val(""), val("test"), val("")], success("test") ), // Error cases - non-string arguments - test!(builtin_string_append(&[val(42)]), None), - test!(builtin_string_append(&[val("hello"), val(123)]), None), - test!(builtin_string_append(&[val(true), val("world")]), None), + test!("string-append", &[val(42)], None), + test!("string-append", &[val("hello"), val(123)], None), + test!("string-append", &[val(true), val("world")], None), // ================================================================= // MATH OPERATIONS - MAX/MIN // ================================================================= // Basic max operations - test!(builtin_max(&[val(5)]), success(5)), - test!(builtin_max(&[val(1), val(2), val(3)]), success(3)), - test!(builtin_max(&[val(3), val(1), val(2)]), success(3)), - test!(builtin_max(&[val(-5), val(-1), val(-10)]), success(-1)), + test!("max", &[val(5)], success(5)), + test!("max", &[val(1), val(2), val(3)], success(3)), + test!("max", &[val(3), val(1), val(2)], success(3)), + test!("max", &[val(-5), val(-1), val(-10)], success(-1)), // Basic min operations - test!(builtin_min(&[val(5)]), success(5)), - test!(builtin_min(&[val(1), val(2), val(3)]), success(1)), - test!(builtin_min(&[val(3), val(1), val(2)]), success(1)), - test!(builtin_min(&[val(-5), val(-1), val(-10)]), success(-10)), + test!("min", &[val(5)], success(5)), + test!("min", &[val(1), val(2), val(3)], success(1)), + test!("min", &[val(3), val(1), val(2)], success(1)), + test!("min", &[val(-5), val(-1), val(-10)], success(-10)), // Error cases - no arguments - test!(builtin_max(&[]), None), - test!(builtin_min(&[]), None), + test!("max", &[], None), + test!("min", &[], None), // Error cases - non-number arguments - test!(builtin_max(&[val("hello")]), None), - test!(builtin_min(&[val(true)]), None), - test!(builtin_max(&[val(1), val("hello")]), None), - test!(builtin_min(&[val(1), val(true)]), None), + test!("max", &[val("hello")], None), + test!("min", &[val(true)], None), + test!("max", &[val(1), val("hello")], None), + test!("min", &[val(1), val(true)], None), // ================================================================= // EQUALITY STRICT TYPING - OVERRIDE BASIC EQUAL TESTS // ================================================================= // Type coercion rejection (these should fail) - test!(builtin_equal(&[val(1), val("1")]), None), - test!(builtin_equal(&[val(0), val(false)]), None), - test!(builtin_equal(&[val(true), val(1)]), None), - test!(builtin_equal(&[val(""), nil()]), None), - test!(builtin_equal(&[val(Vec::::new()), val(false)]), None), + test!("equal?", &[val(1), val("1")], None), + test!("equal?", &[val(0), val(false)], None), + test!("equal?", &[val(true), val(1)], None), + test!("equal?", &[val(""), nil()], None), + test!("equal?", &[val(Vec::::new()), val(false)], None), // Complex same-type structures - test!(builtin_equal(&[complex1.clone(), complex2]), success(true)), - test!(builtin_equal(&[complex1, complex3]), success(false)), + test!("equal?", &[complex1.clone(), complex2], success(true)), + test!("equal?", &[complex1, complex3], success(false)), // ================================================================= // LOGICAL OPERATIONS STRICT - ADDITIONAL ERROR CASES // ================================================================= // Non-boolean inputs should fail - test!(builtin_not(&[val(0)]), None), - test!(builtin_not(&[val("")]), None), - test!(builtin_not(&[nil()]), None), - test!(builtin_not(&[val("false")]), None), + test!("not", &[val(0)], None), + test!("not", &[val("")], None), + test!("not", &[nil()], None), + test!("not", &[val("false")], None), ]; for (test_expr, result, expected) in test_cases { @@ -1064,7 +1062,7 @@ mod tests { ]; for (args, expected_msg) in test_cases { - match builtin_error(&args).unwrap_err() { + match call_builtin("error", &args).unwrap_err() { Error::EvalError(msg) => { assert_eq!(msg, expected_msg, "Failed for args: {args:?}"); } diff --git a/src/evaluator.rs b/src/evaluator.rs index 236d906..a13d531 100644 --- a/src/evaluator.rs +++ b/src/evaluator.rs @@ -3,11 +3,11 @@ use crate::Error; use crate::MAX_EVAL_DEPTH; use crate::ast::Value; use crate::builtinops::{Arity, get_builtin_ops}; +use std::sync::Arc; pub(crate) mod intooperation; pub use self::intooperation::{BoolIter, NumIter, StringIter, ValueIter}; use std::collections::HashMap; -use std::sync::Arc; /// Environment for variable bindings #[derive(Debug, Clone, PartialEq, Default)] @@ -238,7 +238,7 @@ fn eval_with_depth_tracking( // Evaluate all arguments using helper function with depth tracking let evaluated_args = eval_args(args, env, depth)?; // Apply the function (arity already validated at parse time) - f(&evaluated_args) + f(evaluated_args) } OpKind::SpecialForm(special_form) => { // Special forms are syntax structures handled here after being converted @@ -510,12 +510,11 @@ pub fn create_global_env() -> Environment { for builtin_op in get_builtin_ops() { if let crate::builtinops::OpKind::Function(func) = &builtin_op.op_kind { // Use BuiltinFunction for environment bindings (dynamic calls through symbols) - let f = *func; env.define( builtin_op.scheme_id.to_owned(), Value::BuiltinFunction { id: builtin_op.scheme_id.to_owned(), - func: Arc::new(move |args: Vec| f(&args)), + func: Arc::clone(func), }, ); } @@ -1006,7 +1005,7 @@ mod tests { // Test undefined variable errors ("undefined-var", Error), // Test type errors propagate through calls - ("(not 42)", SpecificError("boolean argument")), // Type error with specific message + ("(not 42)", SpecificError("expected boolean")), // Type error with specific message ("(car \"not-a-list\")", Error), // Type error // Test errors in nested expressions ("(+ 1 (car \"not-a-list\"))", Error), @@ -1029,10 +1028,7 @@ mod tests { // throughout the chain. If set! were supported this design would need revisiting ("(set! x 42)", SpecificError("Unbound variable: set!")), // Unsupported special forms appear as unbound variables // Type errors - ( - "(+ 1 \"hello\")", - SpecificError("Type error: + requires numbers"), - ), + ("(+ 1 \"hello\")", SpecificError("expected number")), ]; run_comprehensive_tests(test_cases); From 9c2c104bbba6757f6d9cc017d741313b6275dd8c Mon Sep 17 00:00:00 2001 From: Eugene Talagrand Date: Fri, 21 Nov 2025 16:10:45 -0800 Subject: [PATCH 5/6] Update docs & version number for release --- Cargo.lock | 2 +- Cargo.toml | 2 +- README.md | 3 ++- src/ast.rs | 20 +++++++++++--------- src/evaluator/intooperation.rs | 14 +++----------- 5 files changed, 18 insertions(+), 23 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e47e1fa..308e380 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -161,7 +161,7 @@ dependencies = [ [[package]] name = "rulesxp" -version = "0.1.2" +version = "0.2.0" dependencies = [ "nom", "rustyline", diff --git a/Cargo.toml b/Cargo.toml index 2da9614..16081da 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "rulesxp" -version = "0.1.2" +version = "0.2.0" edition = "2024" rust-version = "1.90" description = "Multi-language rules expression evaluator supporting JSONLogic and Scheme with strict typing" diff --git a/README.md b/README.md index 3bb9c36..1e03b49 100644 --- a/README.md +++ b/README.md @@ -178,12 +178,13 @@ env.register_builtin_operation::<(i64, i64)>("add2", add2); env.register_builtin_operation::<(i64, i64)>("safe-div", safe_div); // Now you can call (add2 7 5) or (safe-div 6 3) from Scheme +// Or you can call {"add2" : [7, 5]} or {"safe-div" : [6, 3]} from JSONLogic ``` #### List and variadic builtins For list-style and variadic behavior, use the iterator-based -parameter types re-exported from `rulesxp::evaluator`. +parameter types from `rulesxp::evaluator`. ```rust use rulesxp::{Error, ast::Value, evaluator}; diff --git a/src/ast.rs b/src/ast.rs index 3184e50..037843a 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -1,12 +1,14 @@ -//! This module defines the core Abstract Syntax Tree (AST) types and helper functions -//! for representing values in the interpreter. The main enum, [`Value`], covers -//! all Scheme data types, including numbers, symbols, strings, booleans, lists, built-in -//! and user-defined functions, and precompiled operations. Ergonomic helper functions -//! such as [`val`], [`sym`], and [`nil`] are provided for convenient AST construction -//! in both code and tests. The module also implements conversion traits for common Rust -//! types, making it easy to build Values from Rust literals, arrays, slices, and -//! vectors. Equality and display logic are customized to match Scheme semantics, including -//! round-trip compatibility for precompiled operations. +/* + This module defines the core Abstract Syntax Tree (AST) types and helper functions + for representing values in the interpreter. The main enum, [`Value`], covers + all Scheme data types, including numbers, symbols, strings, booleans, lists, built-in + and user-defined functions, and precompiled operations. Ergonomic helper functions + such as [`val`], [`sym`], and [`nil`] are provided for convenient AST construction + in both code and tests. The module also implements conversion traits for common Rust + types, making it easy to build Values from Rust literals, arrays, slices, and + vectors. Equality and display logic are customized to match Scheme semantics, including + round-trip compatibility for precompiled operations. +*/ use crate::Error; use crate::builtinops::BuiltinOp; diff --git a/src/evaluator/intooperation.rs b/src/evaluator/intooperation.rs index 13f15d5..6166f71 100644 --- a/src/evaluator/intooperation.rs +++ b/src/evaluator/intooperation.rs @@ -259,26 +259,18 @@ impl ValueElementKind for StringKind { } /// Borrowed iterator over a sequence of AST `Value` references. -/// -/// This is the shared base type for all list/sequence-parameter -/// iterators. Typed iterators such as [`NumIter`], [`BoolIter`] -/// and [`StringIter`] are specializations that provide -/// element-level typing. pub type ValueIter<'a> = TypedValueIter<'a, ValueKind>; /// Borrowed iterator over numeric arguments, performing type checking -/// as elements are pulled. Internally this is a specialization of -/// [`TypedValueIter`] that narrows each element to `i64`. +/// as elements are pulled. pub type NumIter<'a> = TypedValueIter<'a, NumberKind>; /// Borrowed iterator over boolean arguments, performing type checking -/// as elements are pulled. Internally this is a specialization of -/// [`TypedValueIter`] that narrows each element to `bool`. +/// as elements are pulled. pub type BoolIter<'a> = TypedValueIter<'a, BoolKind>; /// Borrowed iterator over string arguments, performing type checking -/// as elements are pulled. Internally this is a specialization of -/// [`TypedValueIter`] that narrows each element to `&str`. +/// as elements are pulled. pub type StringIter<'a> = TypedValueIter<'a, StringKind>; // ===================================================================== From 8dc02d48548781d225d5276a9de5d328480bc96c Mon Sep 17 00:00:00 2001 From: Eugene Talagrand Date: Fri, 21 Nov 2025 18:03:40 -0800 Subject: [PATCH 6/6] Eliminate .expect() by construction --- src/builtinops.rs | 44 ++++++++++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/src/builtinops.rs b/src/builtinops.rs index 79b99e8..75f6054 100644 --- a/src/builtinops.rs +++ b/src/builtinops.rs @@ -169,19 +169,21 @@ impl BuiltinOp { // Macro to generate numeric comparison functions macro_rules! numeric_comparison { ($name:ident, $op:tt, $op_str:expr) => { - fn $name(mut args: NumIter<'_>) -> Result { - // SCHEME-JSONLOGIC-STRICT: Require at least 2 arguments (both standards allow < 2 args but with different semantics) - let len = args.len(); - if len < 2 { - return Err(Error::arity_error(2, len)); + fn $name(first: NumberType, rest: NumIter<'_>) -> Result { + let mut iter = rest.peekable(); + + // SCHEME-JSONLOGIC-STRICT: Require at least 2 arguments (both + // standards allow < 2 args but with different semantics). + // If there is no second operand, we have only one argument. + if iter.peek().is_none() { + return Err(Error::arity_error(2, 1)); } - // Chain comparisons: all adjacent pairs must satisfy the comparison - let mut prev = args - .next() - .expect("ExactSizeIterator len() contract violated for numeric comparison"); - - for current in args { + // Chain comparisons: all adjacent pairs must satisfy the comparison. + // Start with the first argument, then compare each subsequent + // element against the previous one. + let mut prev = first; + for current in iter { if !(prev $op current) { return Ok(false); } @@ -394,31 +396,41 @@ static BUILTIN_OPS: LazyLock> = LazyLock::new(|| { BuiltinOp { scheme_id: ">", jsonlogic_id: ">", - op_kind: OpKind::Function(builtin_variadic::<(NumIter<'static>,), _>(builtin_gt)), + op_kind: OpKind::Function(builtin_variadic::<(NumberType, NumIter<'static>), _>( + builtin_gt, + )), arity: Arity::AtLeast(2), }, BuiltinOp { scheme_id: ">=", jsonlogic_id: ">=", - op_kind: OpKind::Function(builtin_variadic::<(NumIter<'static>,), _>(builtin_ge)), + op_kind: OpKind::Function(builtin_variadic::<(NumberType, NumIter<'static>), _>( + builtin_ge, + )), arity: Arity::AtLeast(2), }, BuiltinOp { scheme_id: "<", jsonlogic_id: "<", - op_kind: OpKind::Function(builtin_variadic::<(NumIter<'static>,), _>(builtin_lt)), + op_kind: OpKind::Function(builtin_variadic::<(NumberType, NumIter<'static>), _>( + builtin_lt, + )), arity: Arity::AtLeast(2), }, BuiltinOp { scheme_id: "<=", jsonlogic_id: "<=", - op_kind: OpKind::Function(builtin_variadic::<(NumIter<'static>,), _>(builtin_le)), + op_kind: OpKind::Function(builtin_variadic::<(NumberType, NumIter<'static>), _>( + builtin_le, + )), arity: Arity::AtLeast(2), }, BuiltinOp { scheme_id: "=", jsonlogic_id: "scheme-numeric-equals", - op_kind: OpKind::Function(builtin_variadic::<(NumIter<'static>,), _>(builtin_eq)), + op_kind: OpKind::Function(builtin_variadic::<(NumberType, NumIter<'static>), _>( + builtin_eq, + )), arity: Arity::AtLeast(2), }, BuiltinOp {