From bc287d27dd70291bfb4e6e1f70aa07ce9dd0b57d Mon Sep 17 00:00:00 2001 From: Brummel Date: Sat, 21 Mar 2026 15:35:28 +0100 Subject: [PATCH] Add destructuring support to dead-def elimination Introduce `collect_pattern_addrs` to gather all bound addresses from a destructuring pattern. This enables the optimizer to remove unused destructuring definitions, similar to how simple unused definitions are handled. Added integration tests to verify this behavior and other optimizer robustness scenarios. --- src/ast/compiler/optimizer/engine.rs | 21 +++++- src/ast/compiler/optimizer/utils.rs | 27 +++++++- src/integration_test.rs | 100 +++++++++++++++++++++++++++ 3 files changed, 144 insertions(+), 4 deletions(-) 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 + ); + } }