diff --git a/Cargo.lock b/Cargo.lock index 74bf3c0..f1e0fac 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -749,6 +749,18 @@ dependencies = [ "crossbeam-utils", ] +[[package]] +name = "console" +version = "0.15.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "054ccb5b10f9f2cbf51eb355ca1d05c2d279ce1804688d0db74b4733a5aeafd8" +dependencies = [ + "encode_unicode", + "libc", + "once_cell", + "windows-sys 0.59.0", +] + [[package]] name = "core-foundation" version = "0.9.4" @@ -1025,6 +1037,12 @@ dependencies = [ "bytemuck", ] +[[package]] +name = "encode_unicode" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "34aa73646ffb006b8f5147f3dc182bd4bcb190227ce861fc4a4844bf8e3cb2c0" + [[package]] name = "endi" version = "1.1.1" @@ -1660,6 +1678,19 @@ dependencies = [ "serde_core", ] +[[package]] +name = "insta" +version = "1.46.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e82db8c87c7f1ccecb34ce0c24399b8a73081427f3c7c50a5d597925356115e4" +dependencies = [ + "console", + "once_cell", + "serde", + "similar", + "tempfile", +] + [[package]] name = "is_terminal_polyfill" version = "1.70.2" @@ -1884,6 +1915,7 @@ dependencies = [ "chrono", "clap", "eframe", + "insta", "regex", ] @@ -2760,6 +2792,12 @@ version = "0.3.8" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e320a6c5ad31d271ad523dcf3ad13e2767ad8b1cb8f047f75a8aeaf8da139da2" +[[package]] +name = "similar" +version = "2.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bbbb5d9659141646ae647b42fe094daf6c6192d1620870b449d9557f748b2daa" + [[package]] name = "slab" version = "0.4.12" diff --git a/Cargo.toml b/Cargo.toml index 7ef18b7..b2208c1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,3 +9,6 @@ eframe = "0.33.3" clap = { version = "4.5", features = ["derive"] } chrono = "0.4" regex = "1.10" + +[dev-dependencies] +insta = { version = "1.39", features = ["yaml"] } diff --git a/src/ast/compiler/optimizer.rs b/src/ast/compiler/optimizer.rs index 3b87b5f..68c8123 100644 --- a/src/ast/compiler/optimizer.rs +++ b/src/ast/compiler/optimizer.rs @@ -3,9 +3,9 @@ use crate::ast::compiler::bound_nodes::{BoundKind, TypedNode, Address}; use crate::ast::types::{Value, StaticType}; use crate::ast::vm::Closure; use crate::ast::nodes::Node; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; -/// The Optimizer performs Phase 2 (Cracking) and Phase 2.5 (Aggressive Collapsing). +/// The Optimizer performs Phase 2 (Cracking) and Phase 2.5/2.6 (Aggressive Collapsing & DCE). pub struct Optimizer { pub level: u32, max_passes: usize, @@ -45,6 +45,7 @@ impl Optimizer { kind: BoundKind::Constant(val.clone()), }; } + sub.used_slots.insert(slot); } Address::Upvalue(idx) => { if let Some(val) = sub.upvalues.get(&idx) { @@ -148,6 +149,33 @@ impl Optimizer { } if self.level >= 2 { + // Phase 2.6: Dead Code Elimination (DCE) + // Remove side-effect-free expressions from the block that are not the result. + if !new_exprs.is_empty() { + let last_idx = new_exprs.len() - 1; + let mut filtered = Vec::with_capacity(new_exprs.len()); + for (i, e) in new_exprs.into_iter().enumerate() { + if i == last_idx { + filtered.push(e); + continue; + } + + // A statement is removable if it's pure (no side effects) + // or if it's a local assignment to a variable that is never used or captured. + let removable = match &e.kind { + BoundKind::DefLocal { slot, .. } | BoundKind::Set { addr: Address::Local(slot), .. } => { + !sub.used_slots.contains(slot) && !sub.captured_slots.contains(slot) && self.is_side_effect_free(&e) + } + _ => self.is_side_effect_free(&e), + }; + + if !removable { + filtered.push(e); + } + } + new_exprs = filtered; + } + if new_exprs.is_empty() { return Node { identity: node.identity, kind: BoundKind::Nop, ty: StaticType::Void }; } else if new_exprs.len() == 1 { @@ -169,6 +197,8 @@ impl Optimizer { match capture_addr { Address::Local(slot) => { if let Some(val) = sub.locals.get(slot) { inlined_val = Some(val.clone()); } + // Mark slot as captured in parent scope + sub.captured_slots.insert(*slot); } Address::Upvalue(idx) => { if let Some(val) = sub.upvalues.get(idx) { inlined_val = Some(val.clone()); } @@ -203,6 +233,9 @@ impl Optimizer { }, BoundKind::DefLocal { name, slot, value, captured_by } => { + if !captured_by.is_empty() { + sub.captured_slots.insert(slot); + } let value = Box::new(self.visit_node(*value, sub)); if let BoundKind::Constant(val) = &value.kind { sub.add_local(slot, val.clone()); @@ -244,6 +277,21 @@ impl Optimizer { Node { identity: node.identity, kind: new_kind, ty: new_ty } } + fn is_side_effect_free(&self, node: &TypedNode) -> bool { + match &node.kind { + BoundKind::Constant(_) | BoundKind::Get { .. } | BoundKind::Parameter { .. } | BoundKind::Nop | BoundKind::Lambda { .. } => true, + BoundKind::Tuple { elements } => elements.iter().all(|e| self.is_side_effect_free(e)), + BoundKind::Record { fields } => fields.iter().all(|(k, v)| self.is_side_effect_free(k) && self.is_side_effect_free(v)), + BoundKind::If { cond, then_br, else_br } => { + self.is_side_effect_free(cond) && self.is_side_effect_free(then_br) && else_br.as_ref().is_none_or(|e| self.is_side_effect_free(e)) + } + BoundKind::Block { exprs } => exprs.iter().all(|e| self.is_side_effect_free(e)), + BoundKind::Expansion { bound_expanded, .. } => self.is_side_effect_free(bound_expanded), + BoundKind::DefLocal { value, .. } | BoundKind::Set { value, .. } => self.is_side_effect_free(value), + _ => false, // Call and DefGlobal are considered impure + } + } + fn try_beta_reduce(&self, params: &TypedNode, args: &TypedNode, body: TypedNode) -> Option { if self.contains_def_local(&body) { return None; @@ -354,11 +402,20 @@ struct SubstitutionMap { locals: HashMap, /// Mapping Address::Upvalue(idx) -> Value upvalues: HashMap, + /// Tracking which local slots are actually used (not inlined) + used_slots: HashSet, + /// Tracking which local slots are captured by lambdas + captured_slots: HashSet, } impl SubstitutionMap { fn new() -> Self { - Self { locals: HashMap::new(), upvalues: HashMap::new() } + Self { + locals: HashMap::new(), + upvalues: HashMap::new(), + used_slots: HashSet::new(), + captured_slots: HashSet::new(), + } } fn add_local(&mut self, slot: u32, val: Value) { @@ -376,7 +433,7 @@ impl SubstitutionMap { if let Some(res) = mapping.get(idx as usize) { match res { Some(new_idx) => (BoundKind::Get { addr: Address::Upvalue(*new_idx), name }, node.ty), - None => (BoundKind::Get { addr: Address::Upvalue(idx), name }, node.ty), // Should have been inlined + None => (BoundKind::Get { addr: Address::Upvalue(idx), name }, node.ty), } } else { (BoundKind::Get { addr: Address::Upvalue(idx), name }, node.ty) @@ -384,8 +441,6 @@ impl SubstitutionMap { }, BoundKind::Lambda { params, upvalues, body, positional_count } => { - // IMPORTANT: If this nested lambda captures an upvalue from our current scope, - // we MUST re-index it in its own capture list! let mut next_upvalues = Vec::new(); for addr in upvalues { if let Address::Upvalue(idx) = addr @@ -393,18 +448,14 @@ impl SubstitutionMap { if let Some(new_idx) = res { next_upvalues.push(Address::Upvalue(*new_idx)); } - continue; // Inlined or re-indexed + continue; } next_upvalues.push(addr); } - - // Note: We don't recurse into the body with the SAME mapping, - // because nested Get(Upvalue) nodes refer to THIS lambda's capture list. (BoundKind::Lambda { params, upvalues: next_upvalues, body, positional_count }, node.ty) }, BoundKind::If { cond, then_br, else_br } => { - let cond = Box::new(self.reindex_upvalues(*cond, mapping)); let then_br = Box::new(self.reindex_upvalues(*then_br, mapping)); let else_br = else_br.map(|e| Box::new(self.reindex_upvalues(*e, mapping))); @@ -441,3 +492,50 @@ impl SubstitutionMap { Node { identity: node.identity, kind: new_kind, ty: new_ty } } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::ast::environment::Environment; + + fn get_optimized_dump(source: &str, level: u32) -> String { + let mut env = Environment::new(); + env.optimization_level = level; + env.dump_ast(source).expect("Compilation failed during test") + } + + #[test] + fn test_opt_folding() { + let dump = get_optimized_dump("(+ 10 20)", 2); + insta::assert_snapshot!(dump); + } + + #[test] + fn test_opt_local_inlining() { + // Wrap in a do-block within a lambda to force DefLocal + let source = "(fn [] (do (def x 10) (+ x 5)))"; + let dump = get_optimized_dump(source, 2); + insta::assert_snapshot!(dump); + } + + #[test] + fn test_opt_dce_unused() { + let source = "(fn [] (do (def x 10) 42))"; + let dump = get_optimized_dump(source, 2); + insta::assert_snapshot!(dump); + } + + #[test] + fn test_opt_assignment_safety() { + let source = "(fn [] (do (def x 10) (assign x 20) x))"; + let dump = get_optimized_dump(source, 2); + insta::assert_snapshot!(dump); + } + + #[test] + fn test_opt_lambda_cracking() { + let source = "(fn [] (do (def x 10) (fn [] x)))"; + let dump = get_optimized_dump(source, 2); + insta::assert_snapshot!(dump); + } +}