diff --git a/ndc_analyser/src/analyser.rs b/ndc_analyser/src/analyser.rs index 3f41ac16..c6387fa8 100644 --- a/ndc_analyser/src/analyser.rs +++ b/ndc_analyser/src/analyser.rs @@ -6,8 +6,8 @@ use itertools::{Itertools, izip}; use ndc_core::{StaticType, TypeSignature}; use ndc_lexer::Span; use ndc_parser::{ - Binding, Candidate, Expression, ExpressionLocation, ForBody, ForIteration, FunctionParameter, - Lvalue, NodeId, + AugmentedAssignmentPlan, Binding, Candidate, Expression, ExpressionLocation, ForBody, + ForIteration, FunctionParameter, Lvalue, NodeId, SourceLocalCounts, }; /// Side table holding semantic information keyed by AST node identity. @@ -19,6 +19,9 @@ pub struct AnalysisResult { /// Inferred return types for functions without explicit annotations. /// Keyed by the FunctionDeclaration's `NodeId`. pub inferred_return_types: HashMap, + /// Analyser-assigned source-local high-water marks consumed by the compiler + /// before it allocates hidden temporaries. + pub source_local_counts: SourceLocalCounts, /// Errors accumulated during analysis. Non-empty when the analyser /// encountered problems but was able to continue with fallback types. pub errors: Vec, @@ -86,6 +89,13 @@ impl Analyser { ) -> Result { let typ = self.analyse_inner(expr_loc)?; self.result.expr_types.insert(expr_loc.id, typ.clone()); + if self.scope_tree.current_function_is_top_level() { + self.result.source_local_counts.top_level = self + .result + .source_local_counts + .top_level + .max(self.scope_tree.current_function_local_count()); + } Ok(typ) } @@ -169,25 +179,7 @@ impl Analyser { Expression::Assignment { l_value, r_value } => { let old_type = self.resolve_lvalue_or_any(l_value, *span); let new_type = self.analyse_or_any(r_value); - - if let Lvalue::Identifier { - resolved: Some(target), - .. - } = l_value - { - let widened = old_type.lub(&new_type); - if widened != old_type - && let Err(annotated_type) = - self.scope_tree.update_binding_type(*target, widened) - && !new_type.is_subtype(&annotated_type) - { - self.emit(AnalysisError::mismatched_types( - &new_type, - &annotated_type, - *span, - )); - } - } + self.validate_lvalue_write(l_value, &old_type, &new_type, *span); Ok(StaticType::unit()) } @@ -195,18 +187,15 @@ impl Analyser { l_value, r_value, operation, - resolved_assign_operation, - resolved_operation, + plan, } => { let left_type = self.resolve_single_lvalue(l_value, *span)?; let right_type = self.analyse_or_any(r_value); - let arg_types = vec![left_type, right_type]; + let arg_types = vec![left_type.clone(), right_type.clone()]; - // Resolve both `op=` and `op` so we can widen the lvalue - // by the result type of whichever one actually fires. let ResolvedCall { binding: assign_binding, - .. + return_type: assign_return, } = self.scope_tree.resolve_call( &format!("{operation}="), &arg_types, @@ -218,63 +207,68 @@ impl Analyser { } = self .scope_tree .resolve_call(operation, &arg_types, CallKind::Operator); + let has_op_binding = !matches!(op_binding, Binding::None); + let assign_is_eligible = Self::augmented_rhs_is_compatible(&left_type, &right_type); - *resolved_assign_operation = assign_binding; - *resolved_operation = op_binding; - - // Either form satisfies the call: `op=` mutates in place; - // `op` falls back through `a = a op b`. Only error when both - // are missing — e.g. `Map -= Map` is fine via `-=` even when - // `-` itself has no Map overload. - if matches!(resolved_assign_operation, Binding::None) - && matches!(resolved_operation, Binding::None) - { - self.emit(AnalysisError::function_not_found( - operation, &arg_types, *span, - )); - } - - if !matches!(resolved_operation, Binding::None) { - let result_type = op_return; - match l_value { - Lvalue::Identifier { - resolved: Some(target), - .. - } => { - let widened = arg_types[0].lub(&result_type); - if widened != arg_types[0] - && let Err(annotated_type) = - self.scope_tree.update_binding_type(*target, widened) - && !result_type.is_subtype(&annotated_type) - { - self.emit(AnalysisError::mismatched_types( - &result_type, - &annotated_type, - *span, - )); + let writeback_type = match assign_binding { + Binding::Resolved(candidate) if assign_is_eligible => { + *plan = AugmentedAssignmentPlan::Resolved(Binding::Resolved(candidate)); + None + } + Binding::Dynamic(mut assign_candidates) if assign_is_eligible => { + match op_binding.clone() { + Binding::Resolved(candidate) => { + if !assign_candidates.contains(&candidate) { + assign_candidates.push(candidate); + } } - } - Lvalue::Index { value, .. } => { - if let Expression::Identifier { - resolved: Binding::Resolved(Candidate::Scalar(target)), - .. - } = &value.expression - { - let container_type = self.scope_tree.get_type(*target).clone(); - if let Some(elem_type) = container_type.index_element_type() { - let widened_elem = elem_type.lub(&result_type); - if widened_elem != elem_type { - let new_container = - container_type.with_element_type(widened_elem); - let _ = self - .scope_tree - .update_binding_type(*target, new_container); + Binding::Dynamic(op_candidates) => { + for candidate in op_candidates { + if !assign_candidates.contains(&candidate) { + assign_candidates.push(candidate); } } } + Binding::None => {} } - _ => {} + + let result_type = if has_op_binding { + assign_return.lub(&op_return) + } else { + assign_return + }; + *plan = + AugmentedAssignmentPlan::Resolved(Binding::Dynamic(assign_candidates)); + Some(result_type) + } + Binding::Resolved(_) | Binding::Dynamic(_) => { + // A specialized mutation exists, but using it would + // change the concrete left type. Reject it here rather + // than falling through to an ordinary operation whose + // erased return type could widen the same target. + self.emit(AnalysisError::mismatched_types( + &right_type, + &left_type, + *span, + )); + *plan = AugmentedAssignmentPlan::Unresolved; + None + } + _ if has_op_binding => { + *plan = AugmentedAssignmentPlan::Resolved(op_binding); + Some(op_return) + } + _ => { + self.emit(AnalysisError::function_not_found( + operation, &arg_types, *span, + )); + *plan = AugmentedAssignmentPlan::Unresolved; + None } + }; + + if let Some(result_type) = writeback_type { + self.validate_lvalue_write(l_value, &left_type, &result_type, *span); } Ok(StaticType::unit()) @@ -329,6 +323,10 @@ impl Analyser { let implicit_return = self.analyse_or_any(body); let explicit_return = self.return_type_stack.pop().unwrap(); *captures = self.scope_tree.current_scope_captures(); + self.result + .source_local_counts + .functions + .insert(*id, self.scope_tree.current_function_local_count()); self.scope_tree.destroy_scope(); // Combine explicit `return` types with the block's implicit return type. @@ -554,25 +552,12 @@ impl Analyser { self.analyse_or_any(block); StaticType::unit() } - ForBody::List { - expr, - accumulator_slot, - .. - } => { - // Reserve the accumulator slot BEFORE analysing the body so - // that nested for-comprehensions receive strictly higher slot - // numbers and cannot collide with this accumulator. - *accumulator_slot = Some(self.scope_tree.reserve_anonymous_slot()); - StaticType::List(Box::new(self.analyse_or_any(expr))) - } + ForBody::List { expr } => StaticType::List(Box::new(self.analyse_or_any(expr))), ForBody::Map { key, value, default, - accumulator_slot, - .. } => { - *accumulator_slot = Some(self.scope_tree.reserve_anonymous_slot()); let key_type = self.analyse_or_any(key); let value_type = if let Some(value) = value { self.analyse_or_any(value) @@ -640,7 +625,11 @@ impl Analyser { let type_of_index_target = self.analyse_or_any(value); let get_args = [type_of_index_target.clone(), index_type.clone()]; - let set_args = [type_of_index_target.clone(), index_type, StaticType::Any]; + let set_args = [ + type_of_index_target.clone(), + index_type.clone(), + StaticType::Any, + ]; // Indexing isn't operator-form for vec purposes: there's no // natural broadcast story for `(list_a, list_b)[i]`. @@ -655,6 +644,13 @@ impl Analyser { .binding, ); + let range_type = StaticType::Iterator(Box::new(StaticType::Int)); + if index_type.is_subtype(&range_type) + && let StaticType::List(element) = &type_of_index_target + { + return Ok(StaticType::List(element.clone())); + } + if let Some(t) = type_of_index_target.index_element_type() { Ok(t) } else { @@ -684,6 +680,111 @@ impl Analyser { } } + /// Specialized `op=` implementations preserve the concrete left type. + /// A tuple left-hand side represents vector dispatch, so a scalar right + /// operand must be compatible with every concrete tuple element. + fn augmented_rhs_is_compatible(left_type: &StaticType, right_type: &StaticType) -> bool { + match (left_type, right_type) { + (StaticType::Tuple(left), StaticType::Tuple(right)) => { + left.len() == right.len() + && right + .iter() + .zip(left) + .all(|(right, left)| right.is_subtype(left)) + } + (StaticType::Tuple(left), right) => left.iter().all(|left| right.is_subtype(left)), + (left, right) => right.is_subtype(left), + } + } + + /// Validate a value that will be stored through an lvalue, widening an + /// inferred binding when the location has a stable variable to update. + fn validate_lvalue_write( + &mut self, + lvalue: &Lvalue, + stored_type: &StaticType, + value_type: &StaticType, + span: Span, + ) { + match lvalue { + Lvalue::Identifier { + resolved: Some(target), + .. + } => { + let widened = stored_type.lub(value_type); + if widened != *stored_type + && let Err(annotated_type) = + self.scope_tree.update_binding_type(*target, widened) + && !value_type.is_subtype(&annotated_type) + { + self.emit(AnalysisError::mismatched_types( + value_type, + &annotated_type, + span, + )); + } + } + Lvalue::Index { value, index, .. } => { + if value_type.is_subtype(stored_type) { + return; + } + + let range_type = StaticType::Iterator(Box::new(StaticType::Int)); + let is_slice = self + .result + .expr_types + .get(&index.id) + .is_some_and(|index_type| index_type.is_subtype(&range_type)); + let (stored_element_type, value_element_type) = if is_slice { + let Some(stored_element_type) = stored_type.index_element_type() else { + self.emit(AnalysisError::mismatched_types( + value_type, + stored_type, + span, + )); + return; + }; + let Some(value_element_type) = value_type.sequence_element_type() else { + self.emit(AnalysisError::mismatched_types( + value_type, + stored_type, + span, + )); + return; + }; + (stored_element_type, value_element_type) + } else { + (stored_type.clone(), value_type.clone()) + }; + + if let Expression::Identifier { + resolved: Binding::Resolved(Candidate::Scalar(target)), + .. + } = &value.expression + { + let container_type = self.scope_tree.get_type(*target).clone(); + let widened_container = container_type + .with_element_type(stored_element_type.lub(&value_element_type)); + if widened_container != container_type + && self + .scope_tree + .update_binding_type(*target, widened_container) + .is_ok() + { + return; + } + } + + self.emit(AnalysisError::mismatched_types( + value_type, + stored_type, + span, + )); + } + Lvalue::Identifier { resolved: None, .. } | Lvalue::Sequence(_) => {} + } + } + /// Resolve expressions as arguments to a function and return the function arity fn resolve_parameters_declarative( &mut self, @@ -928,3 +1029,201 @@ impl AnalysisError { } } } + +#[cfg(test)] +mod tests { + use super::*; + use ndc_lexer::{Lexer, SourceId}; + use ndc_parser::Parser; + + fn analyse_with_globals( + source: &str, + globals: Vec<(String, StaticType)>, + ) -> (StaticType, AnalysisResult) { + let tokens = Lexer::new(source, SourceId::SYNTHETIC) + .collect::, _>>() + .expect("lex failed"); + let mut expressions = Parser::from_tokens(tokens).parse().expect("parse failed"); + let mut analyser = Analyser::from_scope_tree(ScopeTree::from_global_scope(globals)); + let mut last_type = StaticType::unit(); + for expression in &mut expressions { + last_type = analyser.analyse(expression).expect("analysis failed"); + } + (last_type, analyser.take_result()) + } + + fn analyse_last_type_with_globals( + source: &str, + globals: Vec<(String, StaticType)>, + ) -> StaticType { + let (last_type, result) = analyse_with_globals(source, globals); + assert!( + result.errors.is_empty(), + "analysis errors: {:?}", + result.errors + ); + last_type + } + + fn analyse_last_type(source: &str) -> StaticType { + analyse_last_type_with_globals(source, vec![]) + } + + fn assert_analysis_error(source: &str, globals: Vec<(String, StaticType)>, expected: &str) { + let (_, result) = analyse_with_globals(source, globals); + assert!( + result + .errors + .iter() + .any(|error| error.to_string().contains(expected)), + "expected an error containing {expected:?}, got {:?}", + result.errors, + ); + } + + #[test] + fn inferred_list_index_assignment_widens_element_type() { + assert_eq!( + analyse_last_type("let values = [1]; values[0] = \"two\"; values"), + StaticType::List(Box::new(StaticType::Any)), + ); + } + + #[test] + fn source_local_counts_are_recorded_per_function_frame() { + let (_, result) = analyse_with_globals( + r#" + let top = 0; + { let scoped = 1; scoped }; + fn outer(arg) { + let local = arg; + fn inner(inner_arg) { + let nested = inner_arg; + nested + }; + local + }; + "#, + vec![], + ); + + assert!( + result.errors.is_empty(), + "analysis errors: {:?}", + result.errors + ); + assert_eq!(result.source_local_counts.top_level, 2); + let mut function_counts: Vec<_> = result + .source_local_counts + .functions + .values() + .copied() + .collect(); + function_counts.sort_unstable(); + assert_eq!(function_counts, [2, 3]); + } + + #[test] + fn top_level_source_local_count_is_monotonic_across_batches() { + let mut analyser = Analyser::from_scope_tree(ScopeTree::from_global_scope(vec![])); + + for (source, expected) in [ + ("{ let first = 1; let second = 2; second };", 2), + ("let root = 0;", 2), + ("let next = 1; let last = 2;", 3), + ] { + let tokens = Lexer::new(source, SourceId::SYNTHETIC) + .collect::, _>>() + .expect("lex failed"); + let mut expressions = Parser::from_tokens(tokens).parse().expect("parse failed"); + for expression in &mut expressions { + analyser.analyse(expression).expect("analysis failed"); + } + + let result = analyser.take_result(); + assert!( + result.errors.is_empty(), + "analysis errors: {:?}", + result.errors + ); + assert_eq!(result.source_local_counts.top_level, expected); + } + } + + #[test] + fn inferred_list_slice_assignment_widens_element_type() { + assert_eq!( + analyse_last_type("let values = [1]; values[0..1] = [\"two\"]; values"), + StaticType::List(Box::new(StaticType::Any)), + ); + } + + #[test] + fn inferred_map_index_assignment_widens_value_and_preserves_key_type() { + assert_eq!( + analyse_last_type("let values = %{\"one\": 1}; values[\"two\"] = \"two\"; values"), + StaticType::Map { + key: Box::new(StaticType::String), + value: Box::new(StaticType::Any), + }, + ); + } + + #[test] + fn inferred_index_augmented_assignment_widens_element_type() { + let add = StaticType::Function { + parameters: Some(vec![StaticType::Int, StaticType::Float]), + return_type: Box::new(StaticType::Number), + }; + assert_eq!( + analyse_last_type_with_globals( + "let values = [1]; values[0] += 0.5; values", + vec![("+".to_string(), add)], + ), + StaticType::List(Box::new(StaticType::Number)), + ); + } + + #[test] + fn compatible_specialized_assignment_preserves_left_type() { + let list_any = StaticType::List(Box::new(StaticType::Any)); + let append = StaticType::Function { + parameters: Some(vec![list_any.clone(), list_any.clone()]), + return_type: Box::new(list_any), + }; + + assert_eq!( + analyse_last_type_with_globals( + "let values = [1]; values ++= [2]; values", + vec![("++=".to_string(), append)], + ), + StaticType::List(Box::new(StaticType::Int)), + ); + } + + #[test] + fn incompatible_specialized_assignment_is_rejected() { + let list_any = StaticType::List(Box::new(StaticType::Any)); + let concat = StaticType::Function { + parameters: Some(vec![list_any.clone(), list_any.clone()]), + return_type: Box::new(list_any), + }; + + assert_analysis_error( + "let values = [1]; values ++= [\"two\"];", + vec![ + ("++=".to_string(), concat.clone()), + ("++".to_string(), concat.clone()), + ], + "mismatched types: found List but expected List", + ); + assert_analysis_error( + "let values: List = [1]; values ++= [\"two\"];", + vec![ + ("++=".to_string(), concat.clone()), + ("++".to_string(), concat), + ], + "mismatched types: found List but expected List", + ); + } +} diff --git a/ndc_analyser/src/scope.rs b/ndc_analyser/src/scope.rs index 288fd585..8c09e47a 100644 --- a/ndc_analyser/src/scope.rs +++ b/ndc_analyser/src/scope.rs @@ -130,6 +130,9 @@ pub(crate) struct Scope { creates_environment: bool, // Only true for function scopes and for-loop iterations base_offset: usize, function_scope_idx: usize, + /// Source-local high-water mark for this function frame. Only meaningful + /// on the function scope itself; child scopes update their owning frame. + local_count: usize, identifiers: Vec, upvalues: Vec<(String, CaptureSource)>, } @@ -145,6 +148,7 @@ impl Scope { creates_environment: true, base_offset: 0, function_scope_idx, + local_count: 0, identifiers: Vec::default(), upvalues: Vec::default(), } @@ -160,6 +164,7 @@ impl Scope { creates_environment: false, base_offset, function_scope_idx, + local_count: 0, identifiers: Vec::default(), upvalues: Vec::default(), } @@ -479,6 +484,18 @@ impl ScopeTree { .collect() } + /// Highest source-local slot used by the current function frame, plus one. + /// Child scopes update this value as bindings are allocated, so destroyed + /// block and iteration scopes remain represented in the high-water mark. + pub(crate) fn current_function_local_count(&self) -> usize { + let function_scope_idx = self.scopes[self.current_scope_idx].function_scope_idx; + self.scopes[function_scope_idx].local_count + } + + pub(crate) fn current_function_is_top_level(&self) -> bool { + self.scopes[self.current_scope_idx].function_scope_idx == 0 + } + // When the Analyser encounters an identifier as the rhs of an expression during resolution it // will use this method to lookup if that identifier has already been seen. pub(crate) fn get_binding_any(&mut self, ident: &str) -> Option { @@ -857,9 +874,11 @@ impl ScopeTree { ident: String, binding: TypeBinding, ) -> ResolvedVar { - ResolvedVar::Local { - slot: self.scopes[self.current_scope_idx].allocate(ident, binding), - } + let function_scope_idx = self.scopes[self.current_scope_idx].function_scope_idx; + let slot = self.scopes[self.current_scope_idx].allocate(ident, binding); + self.scopes[function_scope_idx].local_count = + self.scopes[function_scope_idx].local_count.max(slot + 1); + ResolvedVar::Local { slot } } /// Check whether the current scope already has a `fn` declaration with @@ -868,18 +887,6 @@ impl ScopeTree { self.scopes[self.current_scope_idx].has_function_with_arity(name, arity) } - /// Reserve a slot in the current scope without creating a named binding. - /// Used to allocate the list/map accumulator before analysing the body of a - /// for-comprehension, so that any nested comprehensions receive strictly - /// higher slot numbers and cannot collide with this accumulator. - /// - /// Uses `"\x00"` as a sentinel name that can never collide with user identifiers - /// since the lexer never produces null bytes. - pub(crate) fn reserve_anonymous_slot(&mut self) -> usize { - self.scopes[self.current_scope_idx] - .allocate("\x00".to_string(), TypeBinding::Inferred(StaticType::Any)) - } - /// Try to update a binding's type. Returns `Err` with the annotated type /// if the binding has an explicit type annotation and cannot be widened. pub(crate) fn update_binding_type( diff --git a/ndc_core/src/static_type.rs b/ndc_core/src/static_type.rs index 8fc82ab2..b9308062 100644 --- a/ndc_core/src/static_type.rs +++ b/ndc_core/src/static_type.rs @@ -584,6 +584,10 @@ impl StaticType { Self::MinHeap(_) => Self::MinHeap(Box::new(new_elem)), Self::MaxHeap(_) => Self::MaxHeap(Box::new(new_elem)), Self::Deque(_) => Self::Deque(Box::new(new_elem)), + Self::Map { key, .. } => Self::Map { + key: key.clone(), + value: Box::new(new_elem), + }, _ => self.clone(), } } diff --git a/ndc_interpreter/src/lib.rs b/ndc_interpreter/src/lib.rs index cc7dfda1..06078345 100644 --- a/ndc_interpreter/src/lib.rs +++ b/ndc_interpreter/src/lib.rs @@ -1,7 +1,7 @@ use ndc_analyser::{Analyser, ScopeTree}; use ndc_core::FunctionRegistry; use ndc_lexer::{Lexer, SourceDb, SourceId, TokenLocation}; -use ndc_parser::ExpressionLocation; +use ndc_parser::{ExpressionLocation, SourceLocalCounts}; use ndc_vm::compiler::Compiler; use ndc_vm::value::CompiledFunction; use ndc_vm::{OutputSink, Vm}; @@ -121,8 +121,11 @@ impl Interpreter { pub fn compile_str(&mut self, input: &str) -> Result { let source_id = self.source_db.add("", input); - let (expressions, _) = self.parse_and_analyse(input, source_id)?; - Ok(Compiler::compile(expressions.into_iter())?) + let (expressions, analysis, _) = self.parse_and_analyse(input, source_id)?; + Ok(Compiler::compile( + expressions.into_iter(), + analysis.source_local_counts, + )?) } /// Like [`Self::compile_str`] but skips the peephole optimizer. @@ -132,8 +135,11 @@ impl Interpreter { input: &str, ) -> Result { let source_id = self.source_db.add("", input); - let (expressions, _) = self.parse_and_analyse(input, source_id)?; - Ok(Compiler::compile_unoptimized(expressions.into_iter())?) + let (expressions, analysis, _) = self.parse_and_analyse(input, source_id)?; + Ok(Compiler::compile_unoptimized( + expressions.into_iter(), + analysis.source_local_counts, + )?) } pub fn disassemble_str(&mut self, input: &str) -> Result { @@ -171,8 +177,9 @@ impl Interpreter { // fails — otherwise the analyser would remember declarations from a line // that never actually ran. let analyser_checkpoint = self.analyser.checkpoint(); - let (expressions, mut timings) = self.parse_and_analyse(input, source_id)?; - let vm_result = self.interpret_vm(input, expressions.into_iter()); + let (expressions, analysis, mut timings) = self.parse_and_analyse(input, source_id)?; + let vm_result = + self.interpret_vm(input, expressions.into_iter(), analysis.source_local_counts); if vm_result.is_err() { self.analyser.restore(analyser_checkpoint); } @@ -186,7 +193,7 @@ impl Interpreter { &mut self, input: &str, source_id: SourceId, - ) -> Result<(Vec, ExecutionTimings), InterpreterError> { + ) -> Result<(Vec, AnalysisResult, ExecutionTimings), InterpreterError> { let mut timings = ExecutionTimings::default(); let tokens = measure(&mut timings, Phase::Lexing, || { @@ -203,6 +210,7 @@ impl Interpreter { // Hard errors (structural issues) still abort immediately. if let Err(e) = self.analyser.analyse(e) { self.analyser.restore(checkpoint.clone()); + self.analyser.take_result(); return Err(InterpreterError::Resolver { causes: vec![e] }); } } @@ -218,7 +226,8 @@ impl Interpreter { }); } - Ok((expressions, timings)) + let analysis = self.analyser.take_result(); + Ok((expressions, analysis, timings)) } fn interpret_vm( @@ -226,6 +235,7 @@ impl Interpreter { #[cfg(feature = "trace")] input: &str, #[cfg(not(feature = "trace"))] _input: &str, expressions: impl Iterator, + source_local_counts: SourceLocalCounts, ) -> Result<(Value, ExecutionTimings), InterpreterError> { use ndc_vm::{Function as VmFunction, Object as VmObject, Value as VmValue}; @@ -248,7 +258,7 @@ impl Interpreter { OutputSink::Stdout }; let (code, checkpoint) = measure(&mut timings, Phase::Compiling, || { - Compiler::compile_resumable(expressions) + Compiler::compile_resumable(expressions, source_local_counts) })?; let mut vm = Vm::new(code, globals).with_output(output); #[cfg(feature = "trace")] @@ -271,7 +281,7 @@ impl Interpreter { // find their locals on the stack. let old_checkpoint = checkpoint.clone(); let (code, new_checkpoint) = measure(&mut timings, Phase::Compiling, || { - checkpoint.resume(expressions) + checkpoint.resume(expressions, source_local_counts) })?; vm.resume_from_halt(code, globals, resume_ip, prev_num_locals); #[cfg(feature = "trace")] diff --git a/ndc_parser/src/expression.rs b/ndc_parser/src/expression.rs index 912bd76e..c46f0b2a 100644 --- a/ndc_parser/src/expression.rs +++ b/ndc_parser/src/expression.rs @@ -4,6 +4,7 @@ use ndc_core::{StaticType, TypeSignature}; use ndc_lexer::Span; use num::BigInt; use num::complex::Complex64; +use std::collections::HashMap; use std::sync::atomic::{AtomicU32, Ordering}; /// Unique identity for an AST node. Used as a key in side tables (e.g. the @@ -19,6 +20,23 @@ impl NodeId { } } +/// Analyser-assigned source-local high-water marks for each compilation frame. +/// +/// The top-level count remains monotonic across REPL analysis batches. Nested +/// functions use independent slot namespaces and are keyed by their declaration +/// node so the compiler can reserve their source slots before hidden temporaries. +/// +/// This is transitional metadata while the compiler consumes the resolved AST +/// directly. When HIR is introduced, lowering should represent source locals +/// and generated temporaries as logical local IDs. A frame-layout pass can then +/// assign slots and store `num_locals` directly on each HIR module or function, +/// replacing this top-level count and `NodeId` side table. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct SourceLocalCounts { + pub top_level: usize, + pub functions: HashMap, +} + #[derive(Debug, Eq, PartialEq, Clone)] pub enum Binding { None, @@ -26,6 +44,17 @@ pub enum Binding { Dynamic(Vec), // figure it out at runtime } +/// The operation selected by the analyser for an augmented assignment. +/// +/// Every selected operation returns the updated left value internally. The +/// compiler writes that value back through the assignment target, while the +/// augmented-assignment expression itself evaluates to unit. +#[derive(Debug, Eq, PartialEq, Clone)] +pub enum AugmentedAssignmentPlan { + Unresolved, + Resolved(Binding), +} + #[derive(Debug, Eq, PartialEq, Clone, Copy)] pub enum ResolvedVar { Local { slot: usize }, @@ -110,8 +139,7 @@ pub enum Expression { l_value: Lvalue, r_value: Box, operation: String, - resolved_assign_operation: Binding, - resolved_operation: Binding, + plan: AugmentedAssignmentPlan, }, FunctionDeclaration { name: Option, @@ -192,13 +220,11 @@ pub enum ForBody { Block(ExpressionLocation), List { expr: ExpressionLocation, - accumulator_slot: Option, }, Map { key: ExpressionLocation, value: Option, default: Option>, - accumulator_slot: Option, }, } diff --git a/ndc_parser/src/lib.rs b/ndc_parser/src/lib.rs index 198b87f4..31dd04ed 100644 --- a/ndc_parser/src/lib.rs +++ b/ndc_parser/src/lib.rs @@ -3,8 +3,8 @@ mod operator; mod parser; pub use expression::{ - Binding, Candidate, CaptureSource, Expression, ExpressionLocation, ForBody, ForIteration, - FunctionParameter, Lvalue, NodeId, ResolvedVar, + AugmentedAssignmentPlan, Binding, Candidate, CaptureSource, Expression, ExpressionLocation, + ForBody, ForIteration, FunctionParameter, Lvalue, NodeId, ResolvedVar, SourceLocalCounts, }; pub use operator::{BinaryOperator, LogicalOperator, UnaryOperator}; pub use parser::Error; diff --git a/ndc_parser/src/parser.rs b/ndc_parser/src/parser.rs index 8a298f80..13d3d492 100644 --- a/ndc_parser/src/parser.rs +++ b/ndc_parser/src/parser.rs @@ -2,7 +2,8 @@ use std::fmt::Write; use crate::expression::Expression; use crate::expression::{ - Binding, ExpressionLocation, ForBody, ForIteration, FunctionParameter, Lvalue, NodeId, + AugmentedAssignmentPlan, Binding, ExpressionLocation, ForBody, ForIteration, FunctionParameter, + Lvalue, NodeId, }; use crate::operator::{BinaryOperator, LogicalOperator, UnaryOperator}; use ndc_core::{Parameter, StaticType, TypeSignature}; @@ -376,8 +377,7 @@ impl Parser { .expect("guaranteed to produce an lvalue"), r_value: Box::new(expression), operation: operation_identifier, - resolved_assign_operation: Binding::None, - resolved_operation: Binding::None, + plan: AugmentedAssignmentPlan::Unresolved, }; Ok(op_assign.to_location(start.merge(end))) @@ -850,7 +850,6 @@ impl Parser { Some(Token::For) => { let result = ForBody::List { expr: expr.simplify(), - accumulator_slot: None, }; self.for_comprehension(left_square_bracket_span, result, &Token::RightSquareBracket) } @@ -1321,7 +1320,6 @@ impl Parser { key: key_expr, value: value_expr, default, - accumulator_slot: None, }, &Token::RightCurlyBracket, ); diff --git a/ndc_stdlib/src/list.rs b/ndc_stdlib/src/list.rs index 0c45b0e2..f4be2b66 100644 --- a/ndc_stdlib/src/list.rs +++ b/ndc_stdlib/src/list.rs @@ -325,7 +325,7 @@ pub mod ops { StaticType::List(Box::new(StaticType::Any)), StaticType::List(Box::new(StaticType::Any)), ]), - return_type: Box::new(StaticType::Tuple(vec![])), + return_type: Box::new(StaticType::List(Box::new(StaticType::Any))), }, func: NativeFunc::Simple(Box::new(|args| { let [left, right] = args else { diff --git a/ndc_stdlib/src/string.rs b/ndc_stdlib/src/string.rs index 6e2c061c..dcf28f4c 100644 --- a/ndc_stdlib/src/string.rs +++ b/ndc_stdlib/src/string.rs @@ -36,7 +36,7 @@ mod inner { } /// Appends the right string to the left string in place. - #[function(name = "++=")] + #[function(name = "++=", return_type = String)] pub fn op_list_concat(left: &mut StringRepr, right: &mut StringRepr) -> Value { if Rc::ptr_eq(left, right) { let new = right.borrow().repeat(2).clone(); diff --git a/ndc_vm/src/compiler.rs b/ndc_vm/src/compiler.rs index dd85a17f..ad05c27e 100644 --- a/ndc_vm/src/compiler.rs +++ b/ndc_vm/src/compiler.rs @@ -4,8 +4,9 @@ use crate::{Object, Value}; use ndc_core::{StaticType, TypeSignature}; use ndc_lexer::Span; use ndc_parser::{ - Binding, Candidate, CaptureSource, Expression, ExpressionLocation, ForBody, ForIteration, - FunctionParameter, LogicalOperator, Lvalue, ResolvedVar, + AugmentedAssignmentPlan, Binding, Candidate, CaptureSource, Expression, ExpressionLocation, + ForBody, ForIteration, FunctionParameter, LogicalOperator, Lvalue, NodeId, ResolvedVar, + SourceLocalCounts, }; use std::rc::Rc; @@ -16,6 +17,7 @@ pub struct Compiler { loop_stack: Vec, allow_return: bool, optimize: bool, + source_local_counts: Rc, } impl Default for Compiler { @@ -26,6 +28,7 @@ impl Default for Compiler { loop_stack: Vec::new(), allow_return: false, optimize: true, + source_local_counts: Rc::new(SourceLocalCounts::default()), } } } @@ -33,11 +36,10 @@ impl Default for Compiler { impl Compiler { pub fn compile( expressions: impl Iterator, + source_local_counts: SourceLocalCounts, ) -> Result { - let mut compiler = Self::default(); - for expr_loc in expressions { - compiler.compile_expr(expr_loc)?; - } + let mut compiler = Self::with_source_local_counts(true, source_local_counts); + compiler.compile_batch(expressions.collect())?; Ok(compiler.finish()?.0) } @@ -46,14 +48,10 @@ impl Compiler { /// debugging tools (e.g. a future `--no-optimize` disassembler flag). pub fn compile_unoptimized( expressions: impl Iterator, + source_local_counts: SourceLocalCounts, ) -> Result { - let mut compiler = Self { - optimize: false, - ..Default::default() - }; - for expr_loc in expressions { - compiler.compile_expr(expr_loc)?; - } + let mut compiler = Self::with_source_local_counts(false, source_local_counts); + compiler.compile_batch(expressions.collect())?; Ok(compiler.finish()?.0) } @@ -67,14 +65,10 @@ impl Compiler { /// the emitted chunk, and shifting instructions invalidates that. pub fn compile_resumable( expressions: impl Iterator, + source_local_counts: SourceLocalCounts, ) -> Result<(CompiledFunction, Self), CompileError> { - let mut compiler = Self { - optimize: false, - ..Default::default() - }; - for expr_loc in expressions { - compiler.compile_expr(expr_loc)?; - } + let mut compiler = Self::with_source_local_counts(false, source_local_counts); + compiler.compile_batch(expressions.collect())?; compiler.finish() } @@ -88,14 +82,37 @@ impl Compiler { pub fn resume( self, new_expressions: impl Iterator, + source_local_counts: SourceLocalCounts, ) -> Result<(CompiledFunction, Self), CompileError> { let mut compiler = self; // checkpoint has no trailing Halt - for expr_loc in new_expressions { - compiler.compile_expr(expr_loc)?; - } + compiler.num_locals = compiler.num_locals.max(source_local_counts.top_level); + compiler.source_local_counts = Rc::new(source_local_counts); + compiler.compile_batch(new_expressions.collect())?; compiler.finish() } + fn with_source_local_counts(optimize: bool, source_local_counts: SourceLocalCounts) -> Self { + Self { + num_locals: source_local_counts.top_level, + optimize, + source_local_counts: Rc::new(source_local_counts), + ..Default::default() + } + } + + fn compile_batch(&mut self, expressions: Vec) -> Result<(), CompileError> { + for expression in expressions { + self.compile_expr(expression)?; + } + Ok(()) + } + + fn allocate_temp(&mut self) -> usize { + let slot = self.num_locals; + self.num_locals += 1; + slot + } + /// The instruction index where the trailing `Halt` was written. /// When resuming, this is the `ip` to start from in the new function. pub fn halt_ip(&self) -> usize { @@ -132,7 +149,9 @@ impl Compiler { expression_location: ExpressionLocation, ) -> Result<(), CompileError> { let ExpressionLocation { - expression, span, .. + expression, + span, + id, } = expression_location; match expression { Expression::BoolLiteral(b) => { @@ -240,124 +259,24 @@ impl Compiler { Expression::OpAssignment { l_value, r_value, - resolved_assign_operation, - resolved_operation, + plan, .. } => { - match l_value { - Lvalue::Identifier { - resolved, - span: lv_span, - .. - } => { - let var = resolved.expect("lvalue must be resolved"); - match Self::op_assign_strategy(&resolved_assign_operation) { - OpAssignStrategy::InPlaceScalar => { - // `|=`, `&=`, `++=` over a List/Map/String: - // the in-place op mutates the value's Rc and - // returns unit (or the lhs). The slot already - // points at the shared Rc, so just discard. - self.compile_binding(resolved_assign_operation, span)?; - self.emit_get_var(var, lv_span); - self.compile_expr(*r_value)?; - self.ir.write(OpCode::Call(2), span); - self.ir.write(OpCode::Pop, span); - } - OpAssignStrategy::DynamicMerge => { - // `op=` exists but either dispatches to - // multiple candidates at runtime, or resolved - // to a single vec candidate that produces a - // fresh tuple. Either way the result must be - // stored back via SetVar — `op=` returns lhs - // when it mutates in place, or a fresh value - // when vec'd. - let (opcode, callee_binding): (OpCode, Binding) = - match resolved_assign_operation { - Binding::Resolved(Candidate::Vec(_)) => { - (OpCode::CallVec(2), resolved_assign_operation) - } - Binding::Dynamic(assign_candidates) => { - // Merge with `op` candidates so the runtime - // dispatcher can fall back to `a op b` shape - // for arg types `op=` doesn't accept. - let mut merged = assign_candidates; - match resolved_operation { - Binding::Dynamic(c) => merged.extend(c), - Binding::Resolved(c) => merged.push(c), - Binding::None => {} - } - (OpCode::Call(2), Binding::Dynamic(merged)) - } - _ => unreachable!( - "DynamicMerge fires only for Resolved(Vec) or Dynamic op=" - ), - }; - self.compile_binding(callee_binding, span)?; - self.emit_get_var(var, lv_span); - self.compile_expr(*r_value)?; - self.ir.write(opcode, span); - self.emit_set_var(var, lv_span); - } - OpAssignStrategy::FallbackToOp => { - // No `op=` overload: lower to `lhs = lhs op rhs`. - // Vec-resolved `op` (e.g. `a += (3, 4)` on - // `Tuple`) goes through `CallVec` - // for the speed-up; everything else is `Call`. - let opcode = Self::call_opcode_for(&resolved_operation, 2); - self.compile_binding(resolved_operation, span)?; - self.emit_get_var(var, lv_span); - self.compile_expr(*r_value)?; - self.ir.write(opcode, span); - self.emit_set_var(var, lv_span); - } - } - } - Lvalue::Index { - value, - index, - resolved_get, - resolved_set, - } => { - let container_span = value.span; - let index_span = index.span; - - let tmp_container = self.num_locals; - let tmp_index = self.num_locals + 1; - self.num_locals += 2; - - self.compile_expr(*value)?; - self.ir - .write(OpCode::SetLocal(tmp_container), container_span); - self.compile_expr(*index)?; - self.ir.write(OpCode::SetLocal(tmp_index), index_span); - - self.compile_binding( - resolved_set.expect("[]= must be resolved"), - container_span.merge(index_span), - )?; - self.ir - .write(OpCode::GetLocal(tmp_container), container_span); - self.ir.write(OpCode::GetLocal(tmp_index), index_span); - - let op_opcode = Self::call_opcode_for(&resolved_operation, 2); - self.compile_binding(resolved_operation, span)?; - self.compile_binding( - resolved_get.expect("[] must be resolved"), - index_span, - )?; - self.ir - .write(OpCode::GetLocal(tmp_container), container_span); - self.ir.write(OpCode::GetLocal(tmp_index), index_span); - self.ir.write(OpCode::Call(2), span); // [](container, index) → current_value - self.compile_expr(*r_value)?; - self.ir.write(op_opcode, span); // op(current_value, r_value) → new_value - self.ir.write(OpCode::Call(3), span); // []=(container, index, new_value) - self.ir.write(OpCode::Pop, span); // discard []= result; common code below pushes unit + let target = PreparedAssignmentTarget::prepare(self, l_value, span)?; + let binding = match plan { + AugmentedAssignmentPlan::Resolved(binding) => binding, + AugmentedAssignmentPlan::Unresolved => { + return Err(CompileError::unresolved_binding(span)); } - Lvalue::Sequence(_) => { - return Err(CompileError::lvalue_required_to_be_single_identifier(span)); - } - } + }; + + let opcode = Self::call_opcode_for(&binding, 2); + self.compile_binding(binding, span)?; + target.emit_read(self)?; + self.compile_expr(*r_value)?; + self.ir.write(opcode, span); + target.emit_store(self)?; + let idx = self.ir.add_constant(Value::unit()); self.ir.write(OpCode::Constant(idx), span); } @@ -373,6 +292,7 @@ impl Compiler { } => { let type_signature = FunctionParameter::from_params(¶meters); self.compile_function_decl( + id, name, resolved_name, *body, @@ -524,8 +444,7 @@ impl Compiler { // 4. Call []= function // 5. Pop the return value - let tmp_value = self.num_locals; - self.num_locals += 1; + let tmp_value = self.allocate_temp(); self.ir.write(OpCode::SetLocal(tmp_value), span); self.compile_binding(resolved_set.expect("[]= must be resolved"), span)?; @@ -736,6 +655,7 @@ impl Compiler { #[allow(clippy::too_many_arguments)] fn compile_function_decl( &mut self, + node_id: NodeId, name: Option, resolved_name: Option, body: ExpressionLocation, @@ -759,10 +679,17 @@ impl Compiler { }, return_type: Box::new(return_type.clone()), }; + let function_source_locals = self + .source_local_counts + .functions + .get(&node_id) + .copied() + .ok_or_else(|| CompileError::missing_source_local_count(node_id, span))?; let mut fn_compiler = Self { - num_locals: num_params, + num_locals: num_params.max(function_source_locals), allow_return: true, optimize: self.optimize, + source_local_counts: Rc::clone(&self.source_local_counts), ..Default::default() }; fn_compiler.compile_expr(body)?; @@ -832,13 +759,8 @@ impl Compiler { })?; Ok(()) } - ForBody::List { - expr, - accumulator_slot, - } => { - let tmp_list = accumulator_slot - .ok_or_else(|| CompileError::unresolved_accumulator_slot(span))?; - self.num_locals = self.num_locals.max(tmp_list + 1); + ForBody::List { expr } => { + let tmp_list = self.allocate_temp(); self.ir.write(OpCode::MakeList(0), span); self.ir.write(OpCode::SetLocal(tmp_list), span); self.compile_for_iterations(iterations, span, &mut |this| { @@ -853,11 +775,8 @@ impl Compiler { key, value, default, - accumulator_slot, } => { - let tmp_map = accumulator_slot - .ok_or_else(|| CompileError::unresolved_accumulator_slot(span))?; - self.num_locals = self.num_locals.max(tmp_map + 1); + let tmp_map = self.allocate_temp(); let has_default = default.is_some(); if let Some(default) = default { self.compile_expr(*default)?; @@ -981,32 +900,123 @@ struct LoopContext { break_instructions: Vec, } -/// Which lowering shape an `op=` site takes. -enum OpAssignStrategy { - /// A scalar `op=` overload exists and was resolved exactly. The op - /// mutates the value's Rc in place; the result is discarded. - InPlaceScalar, - /// `op=` resolved to multiple candidates. Merge with `op` candidates and - /// dispatch at runtime; store the result back. - DynamicMerge, - /// No `op=` overload — lower to `lhs = lhs op rhs`. - FallbackToOp, +/// An assignment location whose side-effecting components have already been +/// evaluated and cached. This lets augmented assignment use one lowering for +/// every target without evaluating an index expression more than once. +enum PreparedAssignmentTarget { + Variable { + variable: ResolvedVar, + span: Span, + }, + Index { + cached_container: usize, + cached_index: usize, + container_span: Span, + index_span: Span, + getter: Binding, + setter: Binding, + }, } -impl Compiler { - fn op_assign_strategy(op_assign: &Binding) -> OpAssignStrategy { - match op_assign { - Binding::Resolved(Candidate::Scalar(_)) => OpAssignStrategy::InPlaceScalar, - // A vec-resolved op= would produce a fresh tuple result that must - // be stored back; the merge path handles that correctly via - // SetVar after the call. (In practice the stdlib has no such - // overload, but this keeps the contract uniform.) - Binding::Resolved(Candidate::Vec(_)) | Binding::Dynamic(_) => { - OpAssignStrategy::DynamicMerge +impl PreparedAssignmentTarget { + fn prepare(compiler: &mut Compiler, l_value: Lvalue, span: Span) -> Result { + match l_value { + Lvalue::Identifier { resolved, span, .. } => Ok(Self::Variable { + variable: resolved.expect("lvalue must be resolved"), + span, + }), + Lvalue::Index { + value, + index, + resolved_get, + resolved_set, + } => { + let container_span = value.span; + let index_span = index.span; + let cached_container = compiler.allocate_temp(); + let cached_index = compiler.allocate_temp(); + + compiler.compile_expr(*value)?; + compiler + .ir + .write(OpCode::SetLocal(cached_container), container_span); + compiler.compile_expr(*index)?; + compiler + .ir + .write(OpCode::SetLocal(cached_index), index_span); + + Ok(Self::Index { + cached_container, + cached_index, + container_span, + index_span, + getter: resolved_get.expect("[] must be resolved"), + setter: resolved_set.expect("[]= must be resolved"), + }) } - Binding::None => OpAssignStrategy::FallbackToOp, + Lvalue::Sequence(_) => Err(CompileError::lvalue_required_to_be_single_identifier(span)), } } + + fn emit_read(&self, compiler: &mut Compiler) -> Result<(), CompileError> { + match self { + Self::Variable { variable, span } => compiler.emit_get_var(*variable, *span), + Self::Index { + cached_container, + cached_index, + container_span, + index_span, + getter, + .. + } => { + compiler.compile_binding(getter.clone(), *index_span)?; + compiler + .ir + .write(OpCode::GetLocal(*cached_container), *container_span); + compiler + .ir + .write(OpCode::GetLocal(*cached_index), *index_span); + compiler + .ir + .write(OpCode::Call(2), container_span.merge(*index_span)); + } + } + Ok(()) + } + + fn emit_store(&self, compiler: &mut Compiler) -> Result<(), CompileError> { + match self { + Self::Variable { variable, span } => compiler.emit_set_var(*variable, *span), + Self::Index { + cached_container, + cached_index, + container_span, + index_span, + setter, + .. + } => { + let cached_value = compiler.allocate_temp(); + let target_span = container_span.merge(*index_span); + + compiler + .ir + .write(OpCode::SetLocal(cached_value), target_span); + compiler.compile_binding(setter.clone(), target_span)?; + compiler + .ir + .write(OpCode::GetLocal(*cached_container), *container_span); + compiler + .ir + .write(OpCode::GetLocal(*cached_index), *index_span); + compiler + .ir + .write(OpCode::GetLocal(cached_value), target_span); + compiler.ir.write(OpCode::Call(3), target_span); + compiler.ir.write(OpCode::Pop, target_span); + } + } + Ok(()) + } } /// Returns the minimum local slot referenced by an lvalue, used to determine @@ -1049,6 +1059,15 @@ pub struct CompileError { } impl CompileError { + fn missing_source_local_count(node_id: NodeId, span: Span) -> Self { + Self { + text: format!( + "missing source-local count for function node {node_id:?}; analysed AST and metadata must be compiled together" + ), + span, + } + } + fn unresolved_binding(span: Span) -> Self { Self { text: "encountered unresolved binding during compilation, this is probably an internal error".to_string(), @@ -1083,14 +1102,6 @@ impl CompileError { } } - fn unresolved_accumulator_slot(span: Span) -> Self { - Self { - text: "accumulator slot was not assigned by the analyser; this is an internal error" - .to_string(), - span, - } - } - pub fn span(&self) -> Span { self.span } diff --git a/tests/compiler/tests/compiler.rs b/tests/compiler/tests/compiler.rs index 3426d22d..2fd86acc 100644 --- a/tests/compiler/tests/compiler.rs +++ b/tests/compiler/tests/compiler.rs @@ -14,7 +14,7 @@ fn compile(input: &str) -> Vec { .collect::, _>>() .expect("lex failed"); let expressions = Parser::from_tokens(tokens).parse().expect("parse failed"); - Compiler::compile_unoptimized(expressions.into_iter()) + Compiler::compile_unoptimized(expressions.into_iter(), Default::default()) .expect("compile failed") .opcodes() .to_vec() @@ -42,6 +42,16 @@ fn compile_with_stdlib(input: &str) -> Vec { .to_vec() } +fn compile_with_stdlib_unoptimized(input: &str) -> Vec { + let mut interp = ndc_interpreter::Interpreter::capturing(); + interp.configure(ndc_stdlib::register); + interp + .compile_str_unoptimized(input) + .expect("compile failed") + .opcodes() + .to_vec() +} + // if true { 1 } // // 0: Constant(0) push `true` @@ -325,6 +335,125 @@ fn test_assignment() { ); } +// let value = [1]; +// value ++= [2]; +// +// A specialized `++=` mutates and returns the left value. Even though the +// mutation is visible through aliases, SetLocal must write the returned value +// back before the compiler pushes unit for the assignment expression: +// +// Call(2), SetLocal(0), Constant(_), Pop +// +// The indexed form prepares the container and index in temporary locals, +// calls `++=`, then passes its result to `[]=`. The setter's unit result is +// discarded before the assignment's own unit is pushed: +// +// Call(3), Pop, Constant(_) +#[test] +fn test_augmented_assignment_always_writes_back() { + let variable = compile_with_stdlib_unoptimized("let value = [1]; value ++= [2];"); + assert!( + variable + .windows(4) + .any(|ops| matches!(ops, [Call(2), SetLocal(0), Constant(_), Pop])), + "specialized variable augmentation must write back its result and push unit: {variable:?}", + ); + + let index = compile_with_stdlib_unoptimized( + "let value = [1]; let values = [value]; values[0] ++= [2];", + ); + assert!( + index + .windows(3) + .any(|ops| matches!(ops, [Call(3), Pop, Constant(_)])), + "specialized indexed augmentation must write back through []= and push unit: {index:?}", + ); +} + +// let value = 1; +// value += 2; +// +// Ordinary `+` produces a replacement value, so variable augmentation stores +// the result and then produces unit: +// +// Call(2), SetLocal(0), Constant(_), Pop +// +// let values = [1]; +// values[0] += 2; +// +// Indexed augmentation instead sends the replacement through `[]=`. Its unit +// result is popped before the assignment expression's unit is produced: +// +// Call(3), Pop, Constant(_) +#[test] +fn test_augmented_assignment_writeback_uses_target_store_shape() { + let variable = compile_with_stdlib_unoptimized("let value = 1; value += 2;"); + assert!( + variable + .windows(4) + .any(|ops| matches!(ops, [Call(2), SetLocal(0), Constant(_), Pop])), + "writeback variable augmentation must store the operation result and push unit: {variable:?}", + ); + + let index = compile_with_stdlib_unoptimized("let values = [1]; values[0] += 2;"); + assert!( + index + .windows(3) + .any(|ops| matches!(ops, [Call(3), Pop, Constant(_)])), + "writeback index augmentation must discard the setter result before pushing unit: {index:?}", + ); +} + +// let values = [1]; +// values[0] += { let delta = 2; delta }; +// +// Source locals are assigned first: +// +// slot 0: values +// slot 1: delta +// +// Preparing an indexed assignment then reserves non-overlapping compiler +// temporaries after the source-local high-water mark: +// +// slot 2: cached container +// slot 3: cached index +// slot 4: operation result passed to `[]=` +#[test] +fn test_augmented_assignment_temporaries_follow_source_locals() { + let ops = + compile_with_stdlib_unoptimized("let values = [1]; values[0] += { let delta = 2; delta };"); + + assert!( + ops.iter().any(|op| matches!(op, SetLocal(1))), + "the rhs block local should retain analyser-assigned slot 1: {ops:?}", + ); + assert!( + ops.iter().any(|op| matches!(op, SetLocal(2))) + && ops.iter().any(|op| matches!(op, SetLocal(3))) + && ops.iter().any(|op| matches!(op, SetLocal(4))), + "prepared-target temporaries must be allocated after both source locals: {ops:?}", + ); +} + +// Resolved function AST and its analyser metadata form one compilation unit. +// Omitting the per-function frame size must fail instead of silently reserving +// only parameter slots and allowing hidden temporaries to overlap body locals. +#[test] +fn test_missing_function_source_local_count_is_rejected() { + let mut interp = ndc_interpreter::Interpreter::capturing(); + let (expressions, _) = interp + .analyse_str("fn f() { let local = 1; local }") + .expect("analysis failed"); + + let Err(error) = Compiler::compile(expressions.into_iter(), Default::default()) else { + panic!("compilation should reject missing source-local metadata"); + }; + assert!( + error.to_string().contains("missing source-local count"), + "unexpected compile error: {error}", + ); +} + // { let a = 3; a } // // Declaration stores 3 into pre-allocated slot 0. diff --git a/tests/compiler/tests/optimizer.rs b/tests/compiler/tests/optimizer.rs index 0470a90a..27144cc7 100644 --- a/tests/compiler/tests/optimizer.rs +++ b/tests/compiler/tests/optimizer.rs @@ -13,14 +13,14 @@ fn parse(input: &str) -> Vec { } fn unoptimized(input: &str) -> Vec { - Compiler::compile_unoptimized(parse(input).into_iter()) + Compiler::compile_unoptimized(parse(input).into_iter(), Default::default()) .expect("compile failed") .opcodes() .to_vec() } fn optimized(input: &str) -> Vec { - Compiler::compile(parse(input).into_iter()) + Compiler::compile(parse(input).into_iter(), Default::default()) .expect("compile failed") .opcodes() .to_vec() diff --git a/tests/functional/programs/004_basic/055_unified_op_assign.ndc b/tests/functional/programs/004_basic/055_unified_op_assign.ndc new file mode 100644 index 00000000..2933e52c --- /dev/null +++ b/tests/functional/programs/004_basic/055_unified_op_assign.ndc @@ -0,0 +1,90 @@ +// Exact indexed op= must mutate the value in place and preserve aliases. +let aliased = [1]; +let nested = [aliased]; +nested[0] ++= [2, 3]; +assert_eq(aliased, [1, 2, 3]); +assert_eq(nested[0], [1, 2, 3]); + +// Map -= Map exists without an ordinary Map - Map overload. Indexed +// augmentation must therefore use -= directly. +let set = %{1, 2, 3}; +let sets = [set]; +sets[0] -= %{2}; +assert_eq(set, %{1, 3}); +assert_eq(sets[0], %{1, 3}); + +// An Any-typed indexed value resolves dynamically. op= candidates have +// priority over ordinary op candidates, and their returned lhs is written back. +let dynamic_alias = [10]; +let dynamic_values: List = [dynamic_alias]; +dynamic_values[0] ++= [20]; +assert_eq(dynamic_alias, [10, 20]); +assert_eq(dynamic_values[0], [10, 20]); + +// Location components, the current-value read, and the rhs execute once in +// source order. The source expression evaluates to unit. +let events = []; +let ordered_values = [1]; +fn target() { + events ++= ["target"]; + ordered_values +} +fn location() { + events ++= ["index"]; + 0 +} +fn replacement() { + events ++= ["rhs"]; + 2 +} +assert_eq((target()[location()] += replacement()), ()); +assert_eq(ordered_values, [3]); +assert_eq(events, ["target", "index", "rhs"]); + +// Getters may return detached values. The updated result must still be stored +// through the target even when the specialized operator mutates that result. +let text = "ab"; +text[0] ++= "x"; +assert_eq(text, "axb"); + +let sliced = [1, 2]; +sliced[0..1] ++= [3]; +assert_eq(sliced, [1, 3, 2]); + +// Compatible specialized mutation preserves the fixed container type. Any on +// the target remains the explicit escape hatch for heterogeneous contents. +let typed_values: List = [1]; +typed_values ++= [2]; +assert_eq(typed_values, [1, 2]); + +let mixed_values: List = [1]; +mixed_values ++= ["two"]; +assert_eq(mixed_values, [1, "two"]); + +// Compiler temporaries must live beyond every analyser-assigned local whose +// lifetime overlaps them. Block-local slots are deliberately reused across +// the target, index, and rhs expressions here. +let block_values = [1]; +({ + let target_alias = block_values; + target_alias +})[{ + let block_index = 0; + block_index +}] += { + let delta = 2; + delta +}; +assert_eq(block_values, [3]); + +// Nested functions have an independent local-slot namespace and need their +// own source-local high-water mark before compiler temporaries are allocated. +fn update_in_function() { + let values = [1]; + values[0] += { + let delta = 2; + delta + }; + values +} +assert_eq(update_in_function(), [3]); diff --git a/tests/functional/programs/004_basic/056_annotated_list_index_assignment.ndc b/tests/functional/programs/004_basic/056_annotated_list_index_assignment.ndc new file mode 100644 index 00000000..915fcd7a --- /dev/null +++ b/tests/functional/programs/004_basic/056_annotated_list_index_assignment.ndc @@ -0,0 +1,3 @@ +// expect-error: mismatched types: found String but expected Int +let values: List = [1]; +values[0] = "two"; diff --git a/tests/functional/programs/004_basic/057_annotated_map_index_assignment.ndc b/tests/functional/programs/004_basic/057_annotated_map_index_assignment.ndc new file mode 100644 index 00000000..8051cbf4 --- /dev/null +++ b/tests/functional/programs/004_basic/057_annotated_map_index_assignment.ndc @@ -0,0 +1,3 @@ +// expect-error: mismatched types: found String but expected Int +let values: Map = %{"one": 1}; +values["one"] = "two"; diff --git a/tests/functional/programs/004_basic/058_annotated_list_index_op_assignment.ndc b/tests/functional/programs/004_basic/058_annotated_list_index_op_assignment.ndc new file mode 100644 index 00000000..c0bf7d58 --- /dev/null +++ b/tests/functional/programs/004_basic/058_annotated_list_index_op_assignment.ndc @@ -0,0 +1,3 @@ +// expect-error: mismatched types: found Number but expected Int +let values: List = [1]; +values[0] += 0.5; diff --git a/tests/functional/programs/004_basic/059_annotated_map_index_op_assignment.ndc b/tests/functional/programs/004_basic/059_annotated_map_index_op_assignment.ndc new file mode 100644 index 00000000..26554cd8 --- /dev/null +++ b/tests/functional/programs/004_basic/059_annotated_map_index_op_assignment.ndc @@ -0,0 +1,3 @@ +// expect-error: mismatched types: found Number but expected Int +let values: Map = %{"one": 1}; +values["one"] += 0.5; diff --git a/tests/functional/programs/004_basic/060_inferred_index_assignment_widens.ndc b/tests/functional/programs/004_basic/060_inferred_index_assignment_widens.ndc new file mode 100644 index 00000000..121916e2 --- /dev/null +++ b/tests/functional/programs/004_basic/060_inferred_index_assignment_widens.ndc @@ -0,0 +1,11 @@ +let values = [1]; +values[0] = "two"; +// The concatenation requires values[0] to have widened from Int to Any so +// that operator dispatch is deferred until runtime. +assert_eq(values[0] ++ "!", "two!"); + +let mapped = %{"one": 1}; +mapped["two"] = "two"; +// Map value inference must widen for the same reason while preserving the +// String key type. +assert_eq(mapped["two"] ++ "!", "two!"); diff --git a/tests/functional/programs/004_basic/061_non_widenable_index_assignment.ndc b/tests/functional/programs/004_basic/061_non_widenable_index_assignment.ndc new file mode 100644 index 00000000..595a6b76 --- /dev/null +++ b/tests/functional/programs/004_basic/061_non_widenable_index_assignment.ndc @@ -0,0 +1,3 @@ +// expect-error: mismatched types: found String but expected Int +fn values() -> List => [1]; +values()[0] = "two"; diff --git a/tests/functional/programs/004_basic/062_non_widenable_index_op_assignment.ndc b/tests/functional/programs/004_basic/062_non_widenable_index_op_assignment.ndc new file mode 100644 index 00000000..df42dd15 --- /dev/null +++ b/tests/functional/programs/004_basic/062_non_widenable_index_op_assignment.ndc @@ -0,0 +1,3 @@ +// expect-error: mismatched types: found Number but expected Int +fn values() -> List => [1]; +values()[0] += 0.5; diff --git a/tests/functional/programs/004_basic/065_incompatible_specialized_op_assignment.ndc b/tests/functional/programs/004_basic/065_incompatible_specialized_op_assignment.ndc new file mode 100644 index 00000000..cd551592 --- /dev/null +++ b/tests/functional/programs/004_basic/065_incompatible_specialized_op_assignment.ndc @@ -0,0 +1,3 @@ +// expect-error: mismatched types: found List but expected List +let values = [1]; +values ++= ["two"]; diff --git a/tests/functional/tests/repl.rs b/tests/functional/tests/repl.rs index 1d5c0c89..32ac1320 100644 --- a/tests/functional/tests/repl.rs +++ b/tests/functional/tests/repl.rs @@ -62,6 +62,18 @@ fn multiple_variables_persist() { assert_eq!(out.trim(), "12"); } +#[test] +fn resumed_compilation_reserves_new_source_locals_before_temporaries() { + // The second batch assigns `delta` slot 1. Indexed augmentation must move + // its hidden temporaries beyond the analyser's updated high-water mark. + let out = repl_output(&[ + "let values = [1];", + "values[0] += { let delta = 2; delta };", + "print(values)", + ]); + assert_eq!(out.trim(), "[3]"); +} + #[test] fn function_defined_on_earlier_line_is_callable() { let out = repl_output(&["fn double(x) => x * 2", "print(double(7))"]); diff --git a/tests/proptest/tests/panic.rs b/tests/proptest/tests/panic.rs index 6f0f9725..6db9cffc 100644 --- a/tests/proptest/tests/panic.rs +++ b/tests/proptest/tests/panic.rs @@ -247,7 +247,7 @@ fn run_pipeline(tokens: Vec) { return; } - let compiled = match Compiler::compile(expressions.into_iter()) { + let compiled = match Compiler::compile(expressions.into_iter(), Default::default()) { Ok(c) => c, Err(_) => return, };