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.
This commit is contained in:
+4
-2
@@ -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="))
|
||||
)
|
||||
|
||||
@@ -70,6 +70,33 @@ impl Optimizer {
|
||||
(*current).clone()
|
||||
}
|
||||
|
||||
fn try_inline(
|
||||
&self,
|
||||
params: &AnalyzedNode,
|
||||
arg_nodes: &[Rc<AnalyzedNode>],
|
||||
body: &AnalyzedNode,
|
||||
sub: &mut SubstitutionMap,
|
||||
path: &mut PathTracker,
|
||||
base_sub: Option<SubstitutionMap>,
|
||||
) -> Option<Rc<AnalyzedNode>> {
|
||||
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<AnalyzedNode>,
|
||||
@@ -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);
|
||||
|
||||
|
||||
@@ -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());
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user