From 260669cba1206fa6434e2eb6582c731bb15b994b Mon Sep 17 00:00:00 2001 From: Michael Schimmel Date: Tue, 10 Mar 2026 13:06:56 +0100 Subject: [PATCH] Perf: Restore core optimizations and stabilize VM execution This commit restores and enhances several high-performance features that were previously lost, while fixing critical bugs in scope management. Performance Optimizations: - Zero-Copy TCO: Refactored TailCallRequest to use (Rc, usize), moving arguments directly on the VM stack via drain/resize instead of heap-allocating Vecs. - Slice-based Calls: Native functions and field accessors now operate directly on stack slices, significantly reducing memory churn. - SoA Push Fast-Path: Restored specialized 'push' intrinsic for RecordSeries. Matches layouts via Arc::ptr_eq to distribute values directly into columns without keyword lookups. Architectural Improvements: - Static Stack Frames (max_slots): The Binder now calculates the maximum required slots per lambda. The VM pre-resizes the stack frame, preventing collisions between local variables and temporary call arguments. - Macro Hygiene: Fixed a bug where macro-internal variables could overwrite outer call arguments due to stack overlap. - Trait Refactoring: Unified series operations via a polymorphic 'push_value' on the Series trait, with a generic implementation for ScalarSeries. Code Quality: - Resolved all 'cargo clippy' warnings (collapsible ifs, redundant borrows, etc). - Restored 100% test pass rate (64/64 tests). - Verified stability of all examples and benchmarks. --- src/ast/compiler/analyzer.rs | 3 +- src/ast/compiler/binder.rs | 114 +++------------ src/ast/compiler/bound_nodes.rs | 8 +- src/ast/compiler/captures.rs | 2 + src/ast/compiler/optimizer/engine.rs | 12 +- .../compiler/optimizer/substitution_map.rs | 2 + src/ast/compiler/specializer.rs | 2 + src/ast/compiler/tco.rs | 2 + src/ast/compiler/type_checker.rs | 4 + src/ast/environment.rs | 2 + src/ast/parser.rs | 33 +++-- src/ast/rtl/intrinsics.rs | 39 +++++ src/ast/rtl/series.rs | 109 +++++++++----- src/ast/rtl/streams.rs | 2 +- src/ast/types.rs | 30 ++-- src/ast/vm.rs | 133 ++++++------------ 16 files changed, 237 insertions(+), 260 deletions(-) diff --git a/src/ast/compiler/analyzer.rs b/src/ast/compiler/analyzer.rs index 9e96617..d34b158 100644 --- a/src/ast/compiler/analyzer.rs +++ b/src/ast/compiler/analyzer.rs @@ -160,6 +160,7 @@ impl<'a> Analyzer<'a> { upvalues, body, positional_count, + max_slots, } => { self.lambda_stack.push(node.identity.clone()); let params_m = self.visit(params.clone()); @@ -173,6 +174,7 @@ impl<'a> Analyzer<'a> { upvalues: upvalues.clone(), body: Rc::new(body_m), positional_count: *positional_count, + max_slots: *max_slots, }, Purity::Pure, ) @@ -328,7 +330,6 @@ impl<'a> Analyzer<'a> { BoundKind::Extension(_) => (BoundKind::Nop, Purity::Impure), BoundKind::Error => (BoundKind::Error, Purity::Impure), - _ => (BoundKind::Error, Purity::Impure), }; crate::ast::nodes::Node { diff --git a/src/ast/compiler/binder.rs b/src/ast/compiler/binder.rs index 6a2f19a..aa862c7 100644 --- a/src/ast/compiler/binder.rs +++ b/src/ast/compiler/binder.rs @@ -18,14 +18,16 @@ struct LocalInfo { #[derive(Debug, Clone)] struct CompilerScope { levels: Vec>, - slot_count: u32, + current_slots: u32, + pub max_slots: u32, } impl CompilerScope { fn new() -> Self { Self { levels: vec![HashMap::new()], - slot_count: 0, + current_slots: 0, + max_slots: 0, } } @@ -34,7 +36,9 @@ impl CompilerScope { } fn pop_level(&mut self) { - self.levels.pop(); + if let Some(level) = self.levels.pop() { + self.current_slots -= level.len() as u32; + } if self.levels.is_empty() { // Safety fallback: always have at least one level self.levels.push(HashMap::new()); @@ -49,7 +53,7 @@ impl CompilerScope { sym.name )); } - let slot = LocalSlot(self.slot_count); + let slot = LocalSlot(self.current_slots); current.insert( sym.clone(), LocalInfo { @@ -58,7 +62,8 @@ impl CompilerScope { _ty: StaticType::Any, }, ); - self.slot_count += 1; + self.current_slots += 1; + self.max_slots = self.max_slots.max(self.current_slots); Ok(slot) } @@ -112,7 +117,7 @@ impl Binder { // Final result: A lambda that represents the whole script let root_scope = binder.functions.pop().unwrap(); - let positional_count = root_scope.scope.slot_count; + let positional_count = 0; // Root lambda has no parameters let root_lambda = Node { identity: node.identity.clone(), @@ -125,6 +130,7 @@ impl Binder { upvalues: root_scope.upvalues, body: Rc::new(bound), positional_count, + max_slots: root_scope.scope.max_slots, }, ty: (), }; @@ -256,6 +262,10 @@ impl Binder { }); let bound_params = self.bind_parameter(params, diag); + + // The number of parameters is exactly the number of slots allocated so far. + let positional_count = self.functions.last().unwrap().scope.current_slots; + let bound_body = self.bind(body, diag); let func = self.functions.pop().unwrap(); @@ -265,7 +275,8 @@ impl Binder { params: Rc::new(bound_params), upvalues: func.upvalues, body: Rc::new(bound_body), - positional_count: func.scope.slot_count, + positional_count, + max_slots: func.scope.max_slots, }, ) } @@ -401,95 +412,8 @@ impl Binder { } } - fn bind_definition( - &mut self, - target: &Node, - value: Rc, - diag: &mut Diagnostics, - identity: Identity, - ) -> BoundNode { - match &target.kind { - UntypedKind::Identifier(sym) => { - let current_fn = self.functions.last_mut().unwrap(); - match current_fn.scope.define(sym, target.identity.clone()) { - Ok(slot) => self.make_node( - identity, - BoundKind::Define { - name: sym.clone(), - addr: Address::Local(slot), - kind: DeclarationKind::Variable, - value, - identity: target.identity.clone(), - captured_by: Rc::new(RefCell::new(Vec::new())), - }, - ), - Err(msg) => { - diag.push_error(msg, Some(target.identity.clone())); - self.make_node(identity, BoundKind::Error) - } - } - } - UntypedKind::Tuple { elements: _ } => { - // Destructuring: (def [a b] [1 2]) - let bound_pattern = self.bind_parameter(target, diag); - self.make_node( - identity, - BoundKind::Destructure { - pattern: Rc::new(bound_pattern), - value, - }, - ) - } - _ => { - diag.push_error("Invalid definition target", Some(target.identity.clone())); - self.make_node(identity, BoundKind::Error) - } - } - } + fn bind_assignment_pattern(&mut self, node: &Node, diag: &mut Diagnostics) -> BoundNode { - fn bind_assignment( - &mut self, - target: &Node, - value: Rc, - diag: &mut Diagnostics, - identity: Identity, - ) -> BoundNode { - match &target.kind { - UntypedKind::Identifier(sym) => { - let addr = self.resolve(sym, target.identity.clone(), diag); - if let Address::Global(_) = addr { - diag.push_error( - format!("Cannot assign to immutable global '{}'", sym.name), - Some(target.identity.clone()), - ); - } - self.make_node( - identity, - BoundKind::Set { - addr, - value, - }, - ) - } - UntypedKind::Tuple { elements: _ } => { - // Destructuring assignment: (assign [a b] [1 2]) - let bound_pattern = self.bind_assignment_pattern(target, diag); - self.make_node( - identity, - BoundKind::Destructure { - pattern: Rc::new(bound_pattern), - value, - }, - ) - } - _ => { - diag.push_error("Invalid assignment target", Some(target.identity.clone())); - self.make_node(identity, BoundKind::Error) - } - } - } - - fn bind_assignment_pattern(&mut self, node: &Node, diag: &mut Diagnostics) -> BoundNode { match &node.kind { UntypedKind::Identifier(sym) => { let addr = self.resolve(sym, node.identity.clone(), diag); diff --git a/src/ast/compiler/bound_nodes.rs b/src/ast/compiler/bound_nodes.rs index f4acc3f..ecac74c 100644 --- a/src/ast/compiler/bound_nodes.rs +++ b/src/ast/compiler/bound_nodes.rs @@ -138,8 +138,10 @@ pub enum BoundKind { // The list of variables captured from enclosing scopes upvalues: Vec, body: Rc>, - /// Static optimization: number of slots used by this lambda. + /// Static optimization: number of positional parameters. positional_count: u32, + /// Static optimization: maximum number of local slots needed by this lambda. + max_slots: u32, }, Call { @@ -261,14 +263,16 @@ where upvalues: ua, body: ba, positional_count: pca, + max_slots: msa, }, BoundKind::Lambda { params: pb, upvalues: ub, body: bb, positional_count: pcb, + max_slots: msb, }, - ) => Rc::ptr_eq(pa, pb) && ua == ub && Rc::ptr_eq(ba, bb) && pca == pcb, + ) => pa == pb && ua == ub && ba == bb && pca == pcb && msa == msb, ( BoundKind::Call { callee: ca, diff --git a/src/ast/compiler/captures.rs b/src/ast/compiler/captures.rs index c1f201c..7f25e76 100644 --- a/src/ast/compiler/captures.rs +++ b/src/ast/compiler/captures.rs @@ -72,12 +72,14 @@ impl CapturePass { upvalues, body, positional_count, + max_slots, } => { node.kind = BoundKind::Lambda { params: Rc::new(Self::transform(params.as_ref().clone(), capture_map)), upvalues, body: Rc::new(Self::transform(body.as_ref().clone(), capture_map)), positional_count, + max_slots, }; } diff --git a/src/ast/compiler/optimizer/engine.rs b/src/ast/compiler/optimizer/engine.rs index 0cd0ff4..34412fa 100644 --- a/src/ast/compiler/optimizer/engine.rs +++ b/src/ast/compiler/optimizer/engine.rs @@ -505,24 +505,23 @@ impl Optimizer { block_changed = true; } - if !block_changed { + if !block_changed && Rc::ptr_eq(&new_result, result) { return node_rc; } - if self.enabled { - if new_statements.is_empty() { - return new_result; - } + if self.enabled && new_statements.is_empty() { + return new_result; } (BoundKind::Block { statements: new_statements, result: new_result }, node.ty.clone()) - } + } BoundKind::Lambda { params, upvalues, body, positional_count, + max_slots, } => { let mut info = UsageInfo::default(); info.collect(node); @@ -590,6 +589,7 @@ impl Optimizer { upvalues: new_upvalues, body: reindexed_body, positional_count: *positional_count, + max_slots: *max_slots, }, node.ty.clone(), ) diff --git a/src/ast/compiler/optimizer/substitution_map.rs b/src/ast/compiler/optimizer/substitution_map.rs index 6b87cdb..295aa51 100644 --- a/src/ast/compiler/optimizer/substitution_map.rs +++ b/src/ast/compiler/optimizer/substitution_map.rs @@ -118,6 +118,7 @@ impl SubstitutionMap { upvalues, body, positional_count, + max_slots, } => { let mut next_upvalues = Vec::new(); for source in upvalues { @@ -129,6 +130,7 @@ impl SubstitutionMap { upvalues: next_upvalues, body: body.clone(), positional_count: *positional_count, + max_slots: *max_slots, }, node.ty.clone(), ) diff --git a/src/ast/compiler/specializer.rs b/src/ast/compiler/specializer.rs index fcbfece..f50efdc 100644 --- a/src/ast/compiler/specializer.rs +++ b/src/ast/compiler/specializer.rs @@ -92,6 +92,7 @@ impl Specializer { upvalues, body, positional_count, + max_slots, } => { let params = Rc::new(self.visit_node(params.as_ref().clone())); let body = Rc::new(self.visit_node(body.as_ref().clone())); @@ -101,6 +102,7 @@ impl Specializer { upvalues, body, positional_count, + max_slots, }, node.ty.clone(), ) diff --git a/src/ast/compiler/tco.rs b/src/ast/compiler/tco.rs index df1e71c..4845ad8 100644 --- a/src/ast/compiler/tco.rs +++ b/src/ast/compiler/tco.rs @@ -96,11 +96,13 @@ impl TCO { upvalues, body, positional_count, + max_slots, } => BoundKind::Lambda { params: Rc::new(Self::transform(params.clone(), false)), upvalues: upvalues.clone(), body: Rc::new(Self::transform(body.clone(), true)), positional_count: *positional_count, + max_slots: *max_slots, }, BoundKind::Set { addr, value } => BoundKind::Set { diff --git a/src/ast/compiler/type_checker.rs b/src/ast/compiler/type_checker.rs index 68d494b..6c7dce8 100644 --- a/src/ast/compiler/type_checker.rs +++ b/src/ast/compiler/type_checker.rs @@ -91,6 +91,7 @@ impl TypeChecker { upvalues, body, positional_count, + max_slots, } => { let mut upvalue_types = Vec::with_capacity(upvalues.len()); for _ in upvalues { @@ -131,6 +132,7 @@ impl TypeChecker { upvalues: upvalues.clone(), body: Rc::new(body_typed), positional_count: *positional_count, + max_slots: *max_slots, }, ty: fn_ty, } @@ -470,6 +472,7 @@ impl TypeChecker { upvalues, body, positional_count, + max_slots, } => { let mut upvalue_types = Vec::with_capacity(upvalues.len()); for source in upvalues { @@ -506,6 +509,7 @@ impl TypeChecker { upvalues: upvalues.clone(), body: Rc::new(body_typed), positional_count: *positional_count, + max_slots: *max_slots, }, fn_ty, ) diff --git a/src/ast/environment.rs b/src/ast/environment.rs index 3e0ec32..17f6dcc 100644 --- a/src/ast/environment.rs +++ b/src/ast/environment.rs @@ -632,6 +632,7 @@ impl Environment { upvalues, body, positional_count, + max_slots, } = &node.kind && upvalues.is_empty() { @@ -641,6 +642,7 @@ impl Environment { body.clone(), vec![], *positional_count, + *max_slots, )); let closure_obj: Rc = closure; diff --git a/src/ast/parser.rs b/src/ast/parser.rs index 3c8dfee..c7fe5fb 100644 --- a/src/ast/parser.rs +++ b/src/ast/parser.rs @@ -213,23 +213,22 @@ impl<'a> Parser<'a> { // Peek ahead to see if it's (def ...) or (assign ...) let mut lexer_clone = self.lexer.clone(); // We expect the next token after '(' - if let Ok(token) = lexer_clone.next_token() { - if let TokenKind::Identifier(ref id) = token.kind { - if id.as_ref() == "def" || id.as_ref() == "assign" { - // It is a statement! - let start_loc = self.advance().location; // consume '(' - let identity = NodeIdentity::new(start_loc); - let _head = self.advance(); // consume 'def' or 'assign' - - let node = if id.as_ref() == "def" { - self.parse_def(identity) - } else { - self.parse_assign(identity) - }; - self.expect(TokenKind::RightParen); - return node; - } - } + if let Ok(token) = lexer_clone.next_token() + && let TokenKind::Identifier(ref id) = token.kind + && (id.as_ref() == "def" || id.as_ref() == "assign") + { + // It is a statement! + let start_loc = self.advance().location; // consume '(' + let identity = NodeIdentity::new(start_loc); + let _head = self.advance(); // consume 'def' or 'assign' + + let node = if id.as_ref() == "def" { + self.parse_def(identity) + } else { + self.parse_assign(identity) + }; + self.expect(TokenKind::RightParen); + return node; } } self.parse_expression() diff --git a/src/ast/rtl/intrinsics.rs b/src/ast/rtl/intrinsics.rs index 01e0891..8897b46 100644 --- a/src/ast/rtl/intrinsics.rs +++ b/src/ast/rtl/intrinsics.rs @@ -109,6 +109,45 @@ pub fn lookup(name: &str, args: &[StaticType]) -> Option<(Value, StaticType)> { )), // --- Constant Unary for -1 (decrement optimization) --- + // --- Series & Streams --- + ("push", [StaticType::Any, StaticType::Any]) => Some(( + Value::make_function(Purity::Impure, |args| { + if args.len() < 2 { return Value::Void; } + let series_val = &args[0]; + let item = &args[1]; + + if let Value::Object(obj) = series_val { + if let Some(rs) = obj.as_any().downcast_ref::() { + // SoA Fast-Path: Push record values directly if they match the layout. + if let Value::Record(layout, values) = item + && std::sync::Arc::ptr_eq(rs.layout(), layout) + { + rs.push_record_values(values); + return Value::Void; + } + } + + // Fallback to dynamic push (works for ScalarSeries and RecordSeries) + if let Some(series) = obj.as_series() { + series.push_value(item.clone()); + } + } + Value::Void + }), + StaticType::Void, + )), + ("len", [StaticType::Any]) => Some(( + Value::make_function(Purity::Pure, |args| { + if let Some(Value::Object(obj)) = args.first() + && let Some(series) = obj.as_series() + { + return Value::Int(series.len() as i64); + } + Value::Int(0) + }), + StaticType::Int, + )), + // Special case: tak uses (- x 1). The specializer sees Call("-", [Int, Int]). _ => None, } diff --git a/src/ast/rtl/series.rs b/src/ast/rtl/series.rs index cb2b382..e1956af 100644 --- a/src/ast/rtl/series.rs +++ b/src/ast/rtl/series.rs @@ -77,22 +77,32 @@ impl RingBuffer { /// (No pointers/heaps like String or Record). pub trait ScalarValue: Copy + Debug + 'static { fn to_value(self) -> Value; + fn from_value(v: Value) -> Option; } impl ScalarValue for f64 { fn to_value(self) -> Value { Value::Float(self) } + fn from_value(v: Value) -> Option { + v.as_float() + } } impl ScalarValue for i64 { fn to_value(self) -> Value { Value::Int(self) } + fn from_value(v: Value) -> Option { + v.as_int() + } } impl ScalarValue for bool { fn to_value(self) -> Value { Value::Bool(self) } + fn from_value(v: Value) -> Option { + if let Value::Bool(b) = v { Some(b) } else { None } + } } /// A highly optimized, type-safe series for primitive values (Float, Int, Bool). @@ -139,6 +149,9 @@ impl Series for ScalarSeries { fn get_item(&self, index: usize) -> Option { self.data.borrow().get(index).map(|&v| v.to_value()) } + fn push_value(&self, value: Value) { + SeriesMember::push_into_member(self, value); + } fn len(&self) -> usize { self.data.borrow().len() } @@ -187,6 +200,9 @@ impl Series for ValueSeries { fn get_item(&self, index: usize) -> Option { self.data.borrow().get(index).cloned() } + fn push_value(&self, value: Value) { + SeriesMember::push_into_member(self, value); + } fn len(&self) -> usize { self.data.borrow().len() } @@ -203,35 +219,19 @@ impl Series for ValueSeries { /// of the exact type (Type Erasure for member arrays). pub trait SeriesMember: Object + Series { /// Pushes a dynamic Value into the series, performing the necessary downcast. - fn push_value(&self, value: Value); + fn push_into_member(&self, value: Value); } -impl SeriesMember for ScalarSeries { - fn push_value(&self, value: Value) { - if let Some(v) = value.as_float() { - self.data.borrow_mut().push(v); - } - } -} - -impl SeriesMember for ScalarSeries { - fn push_value(&self, value: Value) { - if let Some(v) = value.as_int() { - self.data.borrow_mut().push(v); - } - } -} - -impl SeriesMember for ScalarSeries { - fn push_value(&self, value: Value) { - if let Value::Bool(v) = value { +impl SeriesMember for ScalarSeries { + fn push_into_member(&self, value: Value) { + if let Some(v) = T::from_value(value) { self.data.borrow_mut().push(v); } } } impl SeriesMember for ValueSeries { - fn push_value(&self, value: Value) { + fn push_into_member(&self, value: Value) { self.data.borrow_mut().push(value); } } @@ -284,6 +284,22 @@ impl RecordSeries { .map(|idx| self.fields[idx].clone()) } + pub fn layout(&self) -> &Arc { + &self.layout + } + + /// The high-performance SoA push! + /// Instead of pushing a single `Value::Record` (which requires keyword lookups), + /// we push pre-extracted fields in order. + pub fn push_record_values(&self, values: &[Value]) { + if values.len() != self.fields.len() { + return; + } + for (i, field) in self.fields.iter().enumerate() { + field.borrow_mut().push_into_member(values[i].clone()); + } + } + /// Dynamically reconstructs the full Record at the given lookback index. pub fn get_record(&self, index: usize) -> Option { if self.fields.is_empty() { @@ -329,6 +345,23 @@ impl Series for RecordSeries { fn get_item(&self, index: usize) -> Option { self.get_record(index) } + + fn push_value(&self, value: Value) { + if let Value::Record(layout, values) = &value + && Arc::ptr_eq(&self.layout, layout) + { + self.push_record_values(values); + return; + } + + // Fallback: extract fields by name + let mut row = Vec::with_capacity(self.fields.len()); + for (kw, _) in &self.layout.fields { + row.push(value.get_field(*kw).unwrap_or(Value::Void)); + } + self.push_record_values(&row); + } + fn len(&self) -> usize { self.fields.first().map(|f| f.borrow().len()).unwrap_or(0) } @@ -383,6 +416,9 @@ impl Series for SeriesView { fn get_item(&self, index: usize) -> Option { self.inner.borrow().get_item(index) } + fn push_value(&self, value: Value) { + self.inner.borrow_mut().push_value(value); + } fn len(&self) -> usize { self.inner.borrow().len() } @@ -432,6 +468,9 @@ impl Series for SharedSeries { .get(index) .map(|&v| (self.converter)(v)) } + fn push_value(&self, _value: Value) { + // Shared series are read-only from scripts + } fn len(&self) -> usize { self.buffer.borrow().len() } @@ -467,6 +506,9 @@ impl Series for SharedValueSeries { fn get_item(&self, index: usize) -> Option { self.buffer.borrow().get(index).cloned() } + fn push_value(&self, _value: Value) { + // Shared series are read-only from scripts + } fn len(&self) -> usize { self.buffer.borrow().len() } @@ -520,6 +562,9 @@ impl Series for SharedRecordSeries { Some(Value::Record(self.layout.clone(), Rc::new(vals))) } + fn push_value(&self, _value: Value) { + // Shared series are read-only from scripts + } fn len(&self) -> usize { self.field_buffers .first() @@ -602,25 +647,11 @@ pub fn register(env: &Environment) { // Polymorphic push logic if let Some(rs) = any.downcast_ref::() { - if let Value::Record(_, values) = val { - for (i, v) in values.iter().enumerate() { - if let Some(f) = rs.fields.get(i) { - f.borrow().push_value(v.clone()); - } - } - } else { - panic!("push to RecordSeries expects a record"); - } - } else if let Some(s) = any.downcast_ref::>() { - s.push_value(val.clone()); - } else if let Some(s) = any.downcast_ref::>() { - s.push_value(val.clone()); - } else if let Some(s) = any.downcast_ref::>() { - s.push_value(val.clone()); - } else if let Some(s) = any.downcast_ref::() { - s.push_value(val.clone()); + rs.push_value(val.clone()); + } else if let Some(series) = obj.as_series() { + series.push_value(val.clone()); } else { - panic!("Object is not a mutable series"); + panic!("Object is not a mutable series: {}", obj.type_name()); } return Value::Void; } diff --git a/src/ast/rtl/streams.rs b/src/ast/rtl/streams.rs index 9063abd..eca55a4 100644 --- a/src/ast/rtl/streams.rs +++ b/src/ast/rtl/streams.rs @@ -251,7 +251,7 @@ impl Observer for RecordPusher { if let Value::Record(_, values) = value { for (i, v) in values.iter().enumerate() { if let Some(buf) = self.field_buffers.get(i) { - buf.borrow_mut().push_value(v.clone()); + buf.borrow_mut().push_into_member(v.clone()); } } } diff --git a/src/ast/types.rs b/src/ast/types.rs index bbdb1c3..3848b81 100644 --- a/src/ast/types.rs +++ b/src/ast/types.rs @@ -202,6 +202,9 @@ pub trait Series { /// Gets the value at the specified lookback index (0 = newest). fn get_item(&self, index: usize) -> Option; + /// Pushes a new value into the series. + fn push_value(&self, value: Value); + /// Returns the current number of elements in the buffer. fn len(&self) -> usize; @@ -259,7 +262,7 @@ pub enum Value { Function(Rc), Object(Rc), // For compiled Closures and other opaque types Cell(Rc>), // Boxed value for captures - TailCallRequest(Box<(Rc, Vec)>), // Internal: For TCO (Boxed to keep Value small) + TailCallRequest(Rc, usize), // Internal: For TCO (target object + argc on stack) } impl PartialEq for Value { @@ -280,8 +283,8 @@ impl PartialEq for Value { (Value::Function(a), Value::Function(b)) => Rc::ptr_eq(a, b), (Value::Object(a), Value::Object(b)) => Rc::ptr_eq(a, b), (Value::Cell(a), Value::Cell(b)) => Rc::ptr_eq(a, b), - (Value::TailCallRequest(a), Value::TailCallRequest(b)) => { - Rc::ptr_eq(&a.0, &b.0) && a.1 == b.1 + (Value::TailCallRequest(oa, ca), Value::TailCallRequest(ob, cb)) => { + Rc::ptr_eq(oa, ob) && ca == cb } _ => false, } @@ -527,11 +530,12 @@ impl StaticType { } impl Value { - pub fn as_float(&self) -> Option { + pub fn get_field(&self, key: Keyword) -> Option { match self { - Value::Float(f) => Some(*f), - Value::Int(i) => Some(*i as f64), - _ => None, + Value::Record(layout, values) => { + layout.index_of(key).map(|idx| values[idx].clone()) + } + _ => None } } @@ -544,6 +548,14 @@ impl Value { } } + pub fn as_float(&self) -> Option { + match self { + Value::Float(f) => Some(*f), + Value::Int(i) => Some(*i as f64), + _ => None, + } + } + pub fn is_truthy(&self) -> bool { match self { Value::Void => false, @@ -623,7 +635,7 @@ impl Value { Value::Function(_) => StaticType::Any, // Dynamic function Value::Object(o) => StaticType::Object(o.type_name()), Value::Cell(c) => c.borrow().static_type(), - Value::TailCallRequest(_) => StaticType::Any, // Internal state, but typable as Any + Value::TailCallRequest(_, _) => StaticType::Any, // Internal state, but typable as Any } } } @@ -680,7 +692,7 @@ impl fmt::Display for Value { } } Value::Cell(c) => write!(f, "{}", c.borrow()), - Value::TailCallRequest(_) => write!(f, ""), + Value::TailCallRequest(_, _) => write!(f, ""), } } } diff --git a/src/ast/vm.rs b/src/ast/vm.rs index 1143918..004b9d0 100644 --- a/src/ast/vm.rs +++ b/src/ast/vm.rs @@ -16,6 +16,7 @@ pub struct Closure { pub exec_node: Rc, pub upvalues: Vec>>, pub positional_count: u32, + pub max_slots: u32, } impl Closure { @@ -26,6 +27,7 @@ impl Closure { exec: Rc, upvalues: Vec>>, positional_count: u32, + max_slots: u32, ) -> Self { Self { parameter_node: params, @@ -33,6 +35,7 @@ impl Closure { exec_node: exec, upvalues, positional_count, + max_slots, } } } @@ -156,30 +159,39 @@ impl VM { ) -> Result { loop { match result { - Ok(Value::TailCallRequest(payload)) => { - let (next_obj, next_args) = *payload; + Ok(Value::TailCallRequest(next_obj, argc)) => { + let top = self.stack.len(); + // Shift the arguments to the base of the stack + self.stack.drain(0..top - argc); + if let Some(closure) = next_obj.as_any().downcast_ref::() { - self.stack.clear(); self.frames.push(CallFrame { stack_base: 0, closure: Some(next_obj.clone()), }); - if next_args.len() == closure.positional_count as usize { - self.stack.extend(next_args); - } else { - self.unpack(&closure.parameter_node, Value::make_tuple(next_args))?; + + // Ensure stack has enough room for all local slots + if (closure.max_slots as usize) > self.stack.len() { + self.stack.resize(closure.max_slots as usize, Value::Void); + } + + if argc != closure.positional_count as usize { + let tuple = Value::make_tuple(self.stack[0..argc].to_vec()); + // Keep the stack area for slots, just clear the arguments + for i in 0..argc { self.stack[i] = Value::Void; } + self.unpack(&closure.parameter_node, tuple)?; } result = self.eval_observed(observer, &closure.exec_node); self.frames.pop(); } else if let Some(series) = next_obj.as_series() { - if next_args.len() != 1 { + if argc != 1 { return Err(format!( "{} indexer expects exactly 1 argument (the lookback index), got {}", next_obj.type_name(), - next_args.len() + argc )); } - if let Value::Int(idx) = &next_args[0] { + if let Value::Int(idx) = &self.stack[0] { if *idx < 0 { return Err(format!( "{} lookback index cannot be negative: {}", @@ -192,7 +204,7 @@ impl VM { return Err(format!( "{} index must be an integer, got {}", next_obj.type_name(), - next_args[0] + self.stack[0] )); } } else { @@ -462,14 +474,23 @@ impl VM { self.eval_internal(obs, s)?; } let res = self.eval_internal(obs, result)?; - self.stack.truncate(base); - Ok(res) + + if let Value::TailCallRequest(obj, argc) = res { + let top = self.stack.len(); + // Keep the argc elements, but remove everything else added by the block + self.stack.drain(base..top - argc); + Ok(Value::TailCallRequest(obj, argc)) + } else { + self.stack.truncate(base); + Ok(res) + } } BoundKind::Lambda { params, upvalues, body, positional_count, + max_slots, } => { let mut captured = Vec::with_capacity(upvalues.len()); for source in upvalues { @@ -485,6 +506,7 @@ impl VM { body.clone(), captured, *positional_count, + *max_slots, ); Ok(Value::Object(Rc::new(closure))) } @@ -498,73 +520,9 @@ impl VM { } if node.ty.is_tail { - let arg_vals = self.stack[base..].to_vec(); - self.stack.truncate(base); - - match func_val { - Value::Object(obj) => { - return Ok(Value::TailCallRequest(Box::new((obj, arg_vals)))); - } - Value::Function(f) => return Ok((f.func)(&arg_vals)), - Value::FieldAccessor(k) => { - if arg_vals.len() != 1 { - return Err(format!( - "Field accessor .{} expects exactly 1 argument, got {}", - k.name(), - arg_vals.len() - )); - } - let rec = &arg_vals[0]; - if let Value::Record(layout, values) = rec { - if let Some(idx) = layout.index_of(k) { - return Ok(values[idx].clone()); - } else { - return Err(format!( - "Record does not have field :{}", - k.name() - )); - } - } else if let Value::Object(obj) = rec { - if let Some(rs) = obj - .as_any() - .downcast_ref::() - { - if let Some(field_series) = rs.field(k) { - let view = crate::ast::rtl::series::SeriesView::new( - field_series, - k, - ); - return Ok(Value::Object(std::rc::Rc::new(view))); - } else { - return Err(format!( - "RecordSeries does not have field :{}", - k.name() - )); - } - } else if let Some(sn) = obj.as_any().downcast_ref::() { - let mapped = crate::ast::rtl::streams::build_map_stream(sn.inner.clone(), k); - return Ok(Value::Object(Rc::new(mapped))); - } - return Err(format!( - "Field accessor .{} expects a record, RecordSeries or Stream, got {}", - k.name(), - obj.type_name() - )); - } else { - return Err(format!( - "Field accessor .{} expects a record, RecordSeries or Stream, got {}", - k.name(), - rec - )); - } - } - - _ => { - return Err(format!( - "Tail call target is not a function: {}", - func_val - )); - } + let argc = self.stack.len() - base; + if let Value::Object(obj) = &func_val { + return Ok(Value::TailCallRequest(obj.clone(), argc)); } } @@ -690,11 +648,10 @@ impl VM { }; match result { - Ok(Value::TailCallRequest(payload)) => { - let (next_obj, next_args) = *payload; + Ok(Value::TailCallRequest(next_obj, argc)) => { current_func = Value::Object(next_obj); - self.stack.truncate(base); - self.stack.extend(next_args); + let top = self.stack.len(); + self.stack.drain(base..top - argc); continue; } res => { @@ -710,15 +667,11 @@ impl VM { self.stack.truncate(base); return Err(e); } - let arg_vals = self.stack[base..].to_vec(); - self.stack.truncate(base); + let argc = self.stack.len() - base; let frame = self.frames.last().ok_or("No call frame for 'again'")?; if let Some(closure_obj) = &frame.closure { - Ok(Value::TailCallRequest(Box::new(( - closure_obj.clone(), - arg_vals, - )))) + Ok(Value::TailCallRequest(closure_obj.clone(), argc)) } else { Err("'again' called outside of a closure".to_string()) }