Refactor: Use new_inner for substitution maps
The `SubstitutionMap::new_inner()` method provides a more robust way to create nested substitution maps, ensuring that only global substitutions are inherited. This prevents potential issues with overlapping local and upvalue addresses in nested scopes, such as during lambda inlining. This change also includes: - A new `try_fold_record` method in `folder.rs` to optimize record literals into constant values. - Updates to `inliner.rs` to recognize `Value::Record` for inlining. - Various test cases to verify record optimization and constant folding.
This commit is contained in:
@@ -229,7 +229,7 @@ impl Optimizer {
|
|||||||
&& path.enter_lambda(&callee.identity)
|
&& path.enter_lambda(&callee.identity)
|
||||||
{
|
{
|
||||||
path.inlining_depth += 1;
|
path.inlining_depth += 1;
|
||||||
let mut inner_sub = SubstitutionMap::new();
|
let mut inner_sub = sub.new_inner();
|
||||||
let collapsed = if inliner
|
let collapsed = if inliner
|
||||||
.prepare_beta_reduction(params, &arg_nodes, body, &mut inner_sub)
|
.prepare_beta_reduction(params, &arg_nodes, body, &mut inner_sub)
|
||||||
.is_some()
|
.is_some()
|
||||||
@@ -266,7 +266,7 @@ impl Optimizer {
|
|||||||
&& positional_count.is_some()
|
&& positional_count.is_some()
|
||||||
&& !lambda_node.ty.is_recursive
|
&& !lambda_node.ty.is_recursive
|
||||||
{
|
{
|
||||||
let mut inner_sub = SubstitutionMap::new();
|
let mut inner_sub = sub.new_inner();
|
||||||
path.inlining_stack.insert(*idx);
|
path.inlining_stack.insert(*idx);
|
||||||
path.inlining_depth += 1;
|
path.inlining_depth += 1;
|
||||||
let collapsed = if inliner
|
let collapsed = if inliner
|
||||||
@@ -467,7 +467,7 @@ impl Optimizer {
|
|||||||
|
|
||||||
let mut new_upvalues = Vec::new();
|
let mut new_upvalues = Vec::new();
|
||||||
let mut mapping = Vec::new();
|
let mut mapping = Vec::new();
|
||||||
let mut next_inner_subs = SubstitutionMap::new();
|
let mut next_inner_subs = sub.new_inner();
|
||||||
next_inner_subs.assigned = info.assigned;
|
next_inner_subs.assigned = info.assigned;
|
||||||
|
|
||||||
for (old_idx, capture_addr) in original_upvalues.iter().enumerate() {
|
for (old_idx, capture_addr) in original_upvalues.iter().enumerate() {
|
||||||
@@ -543,12 +543,17 @@ impl Optimizer {
|
|||||||
.collect();
|
.collect();
|
||||||
(BoundKind::Tuple { elements }, node.ty.clone())
|
(BoundKind::Tuple { elements }, node.ty.clone())
|
||||||
}
|
}
|
||||||
BoundKind::Record { fields } => {
|
BoundKind::Record { ref fields } => {
|
||||||
let fields = fields
|
let mapped_fields: Vec<_> = fields
|
||||||
.into_iter()
|
.iter()
|
||||||
.map(|(k, v)| (self.visit_node(k, sub, path), self.visit_node(v, sub, path)))
|
.map(|(k, v)| (self.visit_node(k.clone(), sub, path), self.visit_node(v.clone(), sub, path)))
|
||||||
.collect();
|
.collect();
|
||||||
(BoundKind::Record { fields }, node.ty.clone())
|
|
||||||
|
if self.enabled && let Some(folded) = folder.try_fold_record(&mapped_fields, &node) {
|
||||||
|
return folded;
|
||||||
|
}
|
||||||
|
|
||||||
|
(BoundKind::Record { fields: mapped_fields }, node.ty.clone())
|
||||||
}
|
}
|
||||||
BoundKind::Expansion {
|
BoundKind::Expansion {
|
||||||
ref original_call,
|
ref original_call,
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
use crate::ast::compiler::bound_nodes::{Address, AnalyzedNode, BoundKind, NodeMetrics};
|
use crate::ast::compiler::bound_nodes::{Address, AnalyzedNode, BoundKind, NodeMetrics};
|
||||||
use crate::ast::nodes::Node;
|
use crate::ast::nodes::Node;
|
||||||
use crate::ast::types::{Purity, StaticType, Value};
|
use crate::ast::types::{Purity, StaticType, Value, RecordLayout};
|
||||||
use std::cell::RefCell;
|
use std::cell::RefCell;
|
||||||
use std::rc::Rc;
|
use std::rc::Rc;
|
||||||
|
|
||||||
@@ -48,6 +48,32 @@ impl<'a> Folder<'a> {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pub fn try_fold_record(
|
||||||
|
&self,
|
||||||
|
fields: &[(AnalyzedNode, AnalyzedNode)],
|
||||||
|
template: &AnalyzedNode,
|
||||||
|
) -> Option<AnalyzedNode> {
|
||||||
|
let mut layout_fields = Vec::with_capacity(fields.len());
|
||||||
|
let mut values = Vec::with_capacity(fields.len());
|
||||||
|
|
||||||
|
for (k_node, v_node) in fields {
|
||||||
|
if let BoundKind::Constant(Value::Keyword(kw)) = &k_node.kind {
|
||||||
|
if let BoundKind::Constant(val) = &v_node.kind {
|
||||||
|
layout_fields.push((*kw, val.static_type()));
|
||||||
|
values.push(val.clone());
|
||||||
|
} else {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
let layout = RecordLayout::get_or_create(layout_fields);
|
||||||
|
let record_val = Value::Record(layout, Rc::new(values));
|
||||||
|
Some(self.make_constant_node(record_val, template))
|
||||||
|
}
|
||||||
|
|
||||||
pub fn try_fold_pure(
|
pub fn try_fold_pure(
|
||||||
&self,
|
&self,
|
||||||
callee: &AnalyzedNode,
|
callee: &AnalyzedNode,
|
||||||
|
|||||||
@@ -31,6 +31,7 @@ impl<'a> Inliner<'a> {
|
|||||||
| Value::Bool(_)
|
| Value::Bool(_)
|
||||||
| Value::Text(_)
|
| Value::Text(_)
|
||||||
| Value::Keyword(_)
|
| Value::Keyword(_)
|
||||||
|
| Value::Record(_, _)
|
||||||
| Value::DateTime(_) => true,
|
| Value::DateTime(_) => true,
|
||||||
Value::Object(obj) => {
|
Value::Object(obj) => {
|
||||||
if let Some(closure) = obj.as_any().downcast_ref::<Closure>() {
|
if let Some(closure) = obj.as_any().downcast_ref::<Closure>() {
|
||||||
|
|||||||
@@ -19,6 +19,24 @@ impl SubstitutionMap {
|
|||||||
Self::default()
|
Self::default()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Creates a new SubstitutionMap for an inner scope (like an inlined Lambda).
|
||||||
|
/// Safely inherits only Global substitutions, because Local and Upvalue
|
||||||
|
/// addresses are relative to the specific function frame and would overlap.
|
||||||
|
pub fn new_inner(&self) -> Self {
|
||||||
|
let mut inner = Self::new();
|
||||||
|
for (k, v) in &self.values {
|
||||||
|
if matches!(k, Address::Global(_)) {
|
||||||
|
inner.values.insert(*k, v.clone());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
for (k, v) in &self.ast_substitutions {
|
||||||
|
if matches!(k, Address::Global(_)) {
|
||||||
|
inner.ast_substitutions.insert(*k, v.clone());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
inner
|
||||||
|
}
|
||||||
|
|
||||||
pub fn add_ast_substitution(&mut self, addr: Address, node: AnalyzedNode) {
|
pub fn add_ast_substitution(&mut self, addr: Address, node: AnalyzedNode) {
|
||||||
self.ast_substitutions.insert(addr, node);
|
self.ast_substitutions.insert(addr, node);
|
||||||
}
|
}
|
||||||
|
|||||||
+45
-8
@@ -482,17 +482,17 @@ mod tests {
|
|||||||
#[test]
|
#[test]
|
||||||
fn test_record_optimized_access() {
|
fn test_record_optimized_access() {
|
||||||
let env = Environment::new();
|
let env = Environment::new();
|
||||||
let source = "(.price {:id 1 :price 99.5})";
|
let source_eval = "(.price {:id 1 :price 99.5})";
|
||||||
|
|
||||||
// 1. Check result
|
// 1. Check result (will be fully folded to a constant by the new optimization)
|
||||||
let res = env.run_script(source).unwrap();
|
let res = env.run_script(source_eval).unwrap();
|
||||||
assert_eq!(format!("{}", res), "99.5");
|
assert_eq!(format!("{}", res), "99.5");
|
||||||
|
|
||||||
// 2. Verify optimization to GET_FIELD
|
// 2. Verify optimization to GET_FIELD when the record contains non-constants
|
||||||
let dump = env.dump_ast(source).unwrap();
|
let source_ast = "(fn [id] (.price {:id id :price 99.5}))";
|
||||||
|
let dump = env.dump_ast(source_ast).unwrap();
|
||||||
assert!(dump.contains("GetField: .price"), "Should be optimized to GetField. Dump:\n{}", dump);
|
assert!(dump.contains("GetField: .price"), "Should be optimized to GetField. Dump:\n{}", dump);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_first_class_field_accessor() {
|
fn test_first_class_field_accessor() {
|
||||||
let env = Environment::new();
|
let env = Environment::new();
|
||||||
@@ -512,7 +512,44 @@ mod tests {
|
|||||||
// Optimizer should fold (.x {:x 10}) into 10
|
// Optimizer should fold (.x {:x 10}) into 10
|
||||||
let source = "(.x {:x 10 :y 20})";
|
let source = "(.x {:x 10 :y 20})";
|
||||||
let dump = env.dump_ast(source).unwrap();
|
let dump = env.dump_ast(source).unwrap();
|
||||||
assert!(dump.contains("Constant: 10"), "Should be constant folded. Dump:\n{}", dump);
|
assert!(dump.contains("Constant: 10"), "Should evaluate GetField at compile time.");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_record_literal_constant_folding() {
|
||||||
|
let env = Environment::new();
|
||||||
|
let source = " {:a 1 :b 2} ";
|
||||||
|
let dump = env.dump_ast(source).unwrap();
|
||||||
|
|
||||||
|
// Ensure the record definition itself is folded into a Constant.
|
||||||
|
assert!(dump.contains("Constant: {:a 1, :b 2}"), "Should transform a pure record literal into a constant value.");
|
||||||
|
assert!(!dump.contains("Record {"), "Should not leave a runtime Record node in the AST.");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_record_inlining_in_while_loop() {
|
||||||
|
let env_ast = Environment::new();
|
||||||
|
// Ensure that constants (like the config record) are inherited into inner lambda scopes (like the body of a while loop).
|
||||||
|
let source = r#"
|
||||||
|
(do
|
||||||
|
(def loop-config {:start 0 :limit 10})
|
||||||
|
(def loop-idx 0)
|
||||||
|
(while (< loop-idx (.limit loop-config))
|
||||||
|
(assign loop-idx (+ loop-idx 1)))
|
||||||
|
loop-idx
|
||||||
|
)
|
||||||
|
"#;
|
||||||
|
let dump = env_ast.dump_ast(source).unwrap();
|
||||||
|
|
||||||
|
// The optimizer should inline `loop-config`, resolving `(.limit loop-config)` to `10`.
|
||||||
|
// The GetField and the Get for 'loop-config' should vanish inside the condition.
|
||||||
|
assert!(!dump.contains("GetField: .limit"), "The record field should be completely inlined.");
|
||||||
|
assert!(dump.contains("Constant: 10"), "The limit should be resolved to a constant 10.");
|
||||||
|
|
||||||
|
// The result of running it should obviously still be correct.
|
||||||
|
let env_run = Environment::new();
|
||||||
|
let res = env_run.run_script(source).unwrap();
|
||||||
|
assert_eq!(format!("{}", res), "10");
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
|||||||
Reference in New Issue
Block a user