diff --git a/src/ast/compiler/optimizer/engine.rs b/src/ast/compiler/optimizer/engine.rs index 96c760c..0574930 100644 --- a/src/ast/compiler/optimizer/engine.rs +++ b/src/ast/compiler/optimizer/engine.rs @@ -10,7 +10,7 @@ use std::rc::Rc; use super::folder::Folder; use super::inliner::Inliner; use super::substitution_map::SubstitutionMap; -use super::utils::{PathTracker, UsageInfo}; +use super::utils::{collect_pattern_addrs, PathTracker, UsageInfo}; pub struct Optimizer { pub enabled: bool, @@ -500,7 +500,7 @@ impl Optimizer { let is_last = i == last_idx; if self.enabled && !is_last { let removable = match &e.kind { - NodeKind::Def { pattern, value, .. } => { + NodeKind::Def { pattern, value, info: def_info } => { let addr = Self::extract_def_addr(pattern); if let Some(addr) = addr { !info.is_used(&addr) @@ -511,8 +511,23 @@ impl Optimizer { }) && (value.ty.purity >= Purity::SideEffectFree || matches!(value.kind, NodeKind::Lambda { .. })) + } else if def_info.captured_by.is_empty() { + // Destructuring def: safe to remove only when no closure + // captures any binding and every bound slot is unused. + let mut addrs = Vec::new(); + collect_pattern_addrs(pattern, &mut addrs); + !addrs.is_empty() + && addrs.iter().all(|a| { + !info.is_used(a) + && if let Address::Local(slot) = a { + !sub.captured_slots.contains(slot) + } else { + true + } + }) + && (value.ty.purity >= Purity::SideEffectFree + || matches!(value.kind, NodeKind::Lambda { .. })) } else { - // Destructuring def without single addr - not removable false } } diff --git a/src/ast/compiler/optimizer/utils.rs b/src/ast/compiler/optimizer/utils.rs index 474ed4c..0d45efd 100644 --- a/src/ast/compiler/optimizer/utils.rs +++ b/src/ast/compiler/optimizer/utils.rs @@ -199,7 +199,7 @@ impl UsageInfo { } } - fn collect_assigned_from_target(&mut self, node: &AnalyzedNode) { + pub fn collect_assigned_from_target(&mut self, node: &AnalyzedNode) { match &node.kind { NodeKind::Identifier { binding, .. } => { let addr = match binding { @@ -217,3 +217,28 @@ impl UsageInfo { } } } + +/// Collects all addresses declared in a binding pattern (Identifier Declaration or nested Tuple). +/// +/// Used by the optimizer to enumerate all slots bound by a destructuring `def` pattern, +/// enabling dead-def elimination for patterns that `extract_def_addr` cannot handle +/// (i.e. anything other than a plain `Identifier`). +pub fn collect_pattern_addrs(pattern: &AnalyzedNode, out: &mut Vec>) { + match &pattern.kind { + NodeKind::Identifier { + binding: IdentifierBinding::Declaration { addr, .. }, + .. + } => { + out.push(*addr); + } + NodeKind::Def { pattern: inner, .. } => { + collect_pattern_addrs(inner, out); + } + NodeKind::Tuple { elements } => { + for el in elements { + collect_pattern_addrs(el, out); + } + } + _ => {} + } +} diff --git a/src/integration_test.rs b/src/integration_test.rs index 41c6fdc..b023421 100644 --- a/src/integration_test.rs +++ b/src/integration_test.rs @@ -814,4 +814,104 @@ mod tests { let res = env.run_script(source); assert!(res.is_ok(), "Expansion failed to strip placeholders: {:?}", res.err()); } + + // --- Optimizer robustness: destructuring correctness (regression tests) --- + + #[test] + fn test_dead_def_destructuring_correctness() { + // A dead destructuring def must not change the result — with or without optimization. + let source = "(do (def [x y] [1 2]) 42)"; + let mut env_opt = Environment::new(); + env_opt.optimization = true; + let mut env_no = Environment::new(); + env_no.optimization = false; + assert_eq!( + format!("{}", env_opt.run_script(source).unwrap()), + format!("{}", env_no.run_script(source).unwrap()) + ); + } + + #[test] + fn test_captured_slot_destructuring() { + let env = Environment::new(); + // A closure capturing both slots from a destructuring def must work correctly. + let source = r#" + (do + (def [x y] [10 20]) + (def get-sum (fn [] (+ x y))) + (get-sum)) + "#; + assert_eq!(format!("{}", env.run_script(source).unwrap()), "30"); + } + + #[test] + fn test_destructuring_capture_mutation() { + let env = Environment::new(); + // A closure mutating a slot bound via destructuring must see the updated value. + let source = r#" + (do + (def [a b] [1 2]) + (def inc! (fn [] (assign a (+ a b)))) + (inc!) + (inc!) + a) + "#; + // 1+2=3, 3+2=5 + assert_eq!(format!("{}", env.run_script(source).unwrap()), "5"); + } + + #[test] + fn test_inline_two_lambdas_same_slots() { + let env = Environment::new(); + // Two lambdas each binding a local at slot 0 get inlined into the same scope. + // Slot remapping must prevent collision between them. + let source = r#" + (do + (def add1 (fn [x] (+ x 1))) + (def add2 (fn [x] (+ x 2))) + (+ (add1 10) (add2 20))) + "#; + assert_eq!(format!("{}", env.run_script(source).unwrap()), "33"); + let dump = env.dump_ast(source).unwrap(); + assert!( + dump.contains("Constant: 33"), + "Should be fully folded to Constant 33. Dump:\n{}", + dump + ); + } + + #[test] + fn test_multipass_const_propagation() { + let env = Environment::new(); + // A constant chain where each step is only resolvable after the previous has been folded. + // Requires multiple optimizer passes. + let source = r#" + (do + (def a 1) + (def b (+ a 1)) + (def c (+ b 1)) + (+ c 1)) + "#; + assert_eq!(format!("{}", env.run_script(source).unwrap()), "4"); + let dump = env.dump_ast(source).unwrap(); + assert!( + dump.contains("Constant: 4"), + "Multi-pass chain should fold to Constant 4. Dump:\n{}", + dump + ); + } + + #[test] + fn test_dead_destructuring_def_eliminated() { + let env = Environment::new(); + // A dead destructuring def with a pure RHS should be eliminated by the optimizer, + // just like a dead simple def is. Without the fix, the Def node stays in the AST + // and wastes stack slots. + let dump = env.dump_ast("(do (def [x y] [1 2]) 42)").unwrap(); + assert!( + !dump.contains("Def"), + "Dead destructuring def should be eliminated from AST. Dump:\n{}", + dump + ); + } }