From d08fab4f7352eba366c5ad61a1e757e4c0d24ad0 Mon Sep 17 00:00:00 2001 From: Michael Schimmel Date: Sat, 7 Mar 2026 21:10:48 +0100 Subject: [PATCH] Refactor inlining logic into try_inline function This commit extracts the logic for inlining function calls into a new private method, `try_inline`. This improves code organization and reduces duplication in the `Optimizer`'s `visit_node` method. The extracted method handles the preparation of substitutions and the recursive visiting of the inlined node. It also ensures that the substitution map state is correctly synchronized back to the calling context. --- examples/sma.myc | 6 ++- src/ast/compiler/optimizer/engine.rs | 68 +++++++++++++++------------- src/integration_test.rs | 31 +++++++++++++ 3 files changed, 72 insertions(+), 33 deletions(-) diff --git a/examples/sma.myc b/examples/sma.myc index ca3acff..7a30853 100644 --- a/examples/sma.myc +++ b/examples/sma.myc @@ -5,6 +5,8 @@ (macro printx [s] `(fn [x] (print ~s x))) - (pipe [(.close src)] (printx "close=")) - (pipe [(pipe [(.close src)] (SMA 20))] (printx "sma=")) + (pipe src print) + (pipe [(pipe [(.close src)] (SMA 20))] (printx "sma20=")) + (pipe [(pipe [(.close src)] (SMA 5))] (printx "sma5=")) + (pipe [(pipe [(.close src)] (SMA 100))] (printx "sma100=")) ) diff --git a/src/ast/compiler/optimizer/engine.rs b/src/ast/compiler/optimizer/engine.rs index 6d6c95c..e6c8192 100644 --- a/src/ast/compiler/optimizer/engine.rs +++ b/src/ast/compiler/optimizer/engine.rs @@ -70,6 +70,33 @@ impl Optimizer { (*current).clone() } + fn try_inline( + &self, + params: &AnalyzedNode, + arg_nodes: &[Rc], + body: &AnalyzedNode, + sub: &mut SubstitutionMap, + path: &mut PathTracker, + base_sub: Option, + ) -> Option> { + let inliner = Inliner::new(&self.globals, &self.global_purity); + let mut inner_sub = base_sub.unwrap_or_else(|| sub.new_for_inlining()); + + if inliner.prepare_beta_reduction(params, arg_nodes, body, &mut inner_sub).is_some() { + let res = self.visit_node(Rc::new(body.clone()), &mut inner_sub, path); + + // Sync back state to parent substitution map + sub.next_slot = inner_sub.next_slot; + sub.used.extend(inner_sub.used.iter().cloned()); + sub.assigned.extend(inner_sub.assigned.iter().cloned()); + sub.captured_slots.extend(inner_sub.captured_slots.iter().cloned()); + + Some(res) + } else { + None + } + } + fn visit_node( &self, node_rc: Rc, @@ -249,15 +276,7 @@ impl Optimizer { && path.enter_lambda(&callee_opt.identity) { path.inlining_depth += 1; - let mut inner_sub = sub.new_for_inlining(); - let collapsed = if inliner - .prepare_beta_reduction(params, &arg_nodes, body, &mut inner_sub) - .is_some() - { - Some(self.visit_node(body.clone(), &mut inner_sub, path)) - } else { - None - }; + let collapsed = self.try_inline(params, &arg_nodes, body, sub, path, None); path.inlining_depth -= 1; path.exit_lambda(&callee_opt.identity); @@ -286,17 +305,9 @@ impl Optimizer { && positional_count.is_some() && !lambda_node.ty.is_recursive { - let mut inner_sub = sub.new_for_inlining(); path.inlining_stack.insert(*idx); path.inlining_depth += 1; - let collapsed = if inliner - .prepare_beta_reduction(params, &arg_nodes, body, &mut inner_sub) - .is_some() - { - Some(self.visit_node(body.clone(), &mut inner_sub, path)) - } else { - None - }; + let collapsed = self.try_inline(params, &arg_nodes, body, sub, path, None); path.inlining_depth -= 1; path.inlining_stack.remove(idx); @@ -323,19 +334,14 @@ impl Optimizer { } path.inlining_depth += 1; - let collapsed = if inliner - .prepare_beta_reduction( - &closure.parameter_node, - &arg_nodes, - &closure.function_node, - &mut closure_sub, - ) - .is_some() - { - Some(self.visit_node(closure.function_node.clone(), &mut closure_sub, path)) - } else { - None - }; + let collapsed = self.try_inline( + &closure.parameter_node, + &arg_nodes, + &closure.function_node, + sub, + path, + Some(closure_sub), + ); path.inlining_depth -= 1; path.exit_lambda(&closure.function_node.identity); diff --git a/src/integration_test.rs b/src/integration_test.rs index 9fe22b2..804b5dd 100644 --- a/src/integration_test.rs +++ b/src/integration_test.rs @@ -783,4 +783,35 @@ mod tests { let res = env.run_script(source).unwrap(); assert_eq!(format!("{}", res), "20"); } + + #[test] + fn test_reproduce_inlining_slot_clash_crash() { + let mut env = Environment::new(); + env.optimization = true; + + let source = r#" +(do + (def MY_SMA + (fn [length] + (do + (def history (series :float)) + (fn [val] + (do + (push history val) + (len history)))))) + + (def src (create-random-ohlc 42 100)) + (def s (.close src)) + + [(pipe [s] (MY_SMA 5))] + [(pipe [s] (SMA 20))] +) +"#; + // Note: Using SMA from RTL and MY_SMA locally to mix it up. + // Actually, for full independence, let's use MY_SMA twice. + let source_fixed = source.replace("(SMA 20)", "(MY_SMA 20)"); + + let res = env.run_script(&source_fixed); + assert!(res.is_ok(), "Inlining slot clash triggered: {:?}", res.err()); + } }