refactor(joinir): Phase 222.5 - Modularization & Determinism
Phase 222.5-B: ConditionEnv API Unification - Remove deprecated build_loop_param_only() (v1) - Unify build_with_captures() to use JoinValueSpace (v2 API) - Code reduction: -17 lines - Tests: 5/5 PASS Phase 222.5-C: exit_binding.rs Modularization - Split into 4 modules following Phase 33 pattern: - exit_binding_validator.rs (171 lines) - exit_binding_constructor.rs (165 lines) - exit_binding_applicator.rs (163 lines) - exit_binding.rs orchestrator (364 lines, -71 reduction) - Single responsibility per module - Tests: 16/16 PASS Phase 222.5-D: HashMap → BTreeMap for Determinism - Convert 13 critical locations to BTreeMap: - exit_binding (3), carrier_info (2), pattern_pipeline (1) - loop_update_analyzer (2), loop_with_break/continue (2) - pattern4_carrier_analyzer (1), condition_env (2) - Deterministic iteration guaranteed in JoinIR pipeline - Inventory document: phase222-5-d-hashmap-inventory.md - Tests: 849/856 PASS (7 pre-existing failures) - Determinism verified: 3-run consistency test PASS Overall Impact: - Code quality: Single responsibility, function-based design - Determinism: JoinIR pipeline now uses BTreeMap uniformly - Tests: All Phase 222.5 tests passing - Documentation: Complete inventory & implementation plan 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
@ -95,16 +95,12 @@ impl CommonPatternInitializer {
|
||||
})?;
|
||||
|
||||
// Phase 183-2: Delegate to CarrierInfo::from_variable_map for consistency
|
||||
// Convert BTreeMap to HashMap for compatibility
|
||||
let mut variable_map_hashmap = std::collections::HashMap::new();
|
||||
for (k, v) in variable_map {
|
||||
variable_map_hashmap.insert(k.clone(), *v);
|
||||
}
|
||||
// Phase 222.5-D: Direct BTreeMap usage (no conversion needed)
|
||||
|
||||
// Step 2: Use CarrierInfo::from_variable_map as primary initialization method
|
||||
let mut carrier_info = CarrierInfo::from_variable_map(
|
||||
loop_var_name.clone(),
|
||||
&variable_map_hashmap,
|
||||
variable_map,
|
||||
)?;
|
||||
|
||||
// Step 3: Apply exclusions if provided (Pattern 2 specific)
|
||||
|
||||
@ -38,25 +38,6 @@ use std::collections::BTreeMap;
|
||||
pub struct ConditionEnvBuilder;
|
||||
|
||||
impl ConditionEnvBuilder {
|
||||
/// Build ConditionEnv with loop parameter only
|
||||
///
|
||||
/// Used when there are no additional condition-only variables,
|
||||
/// only the loop parameter needs to be in the environment.
|
||||
///
|
||||
/// # Arguments
|
||||
///
|
||||
/// * `loop_var_name` - Loop parameter name
|
||||
///
|
||||
/// # Returns
|
||||
///
|
||||
/// ConditionEnv with only the loop parameter mapped to ValueId(0)
|
||||
#[allow(dead_code)]
|
||||
pub fn build_loop_param_only(loop_var_name: &str) -> ConditionEnv {
|
||||
let mut env = ConditionEnv::new();
|
||||
env.insert(loop_var_name.to_string(), ValueId(0));
|
||||
env
|
||||
}
|
||||
|
||||
/// Phase 201: Build ConditionEnv using JoinValueSpace (disjoint ValueId regions)
|
||||
///
|
||||
/// This method uses JoinValueSpace to allocate ValueIds, ensuring that
|
||||
@ -141,6 +122,10 @@ impl ConditionEnvBuilder {
|
||||
/// Adds captured function-scoped variables to ConditionEnv and generates
|
||||
/// condition_bindings for the boundary builder.
|
||||
///
|
||||
/// # Phase 222.5-B Update
|
||||
///
|
||||
/// Now uses JoinValueSpace for ValueId allocation (via build_loop_param_only_v2).
|
||||
///
|
||||
/// # Behavior
|
||||
///
|
||||
/// - Add captured variables to ConditionEnv.captured field
|
||||
@ -154,25 +139,30 @@ impl ConditionEnvBuilder {
|
||||
/// * `captured` - Function-scoped captured variables with host ValueIds
|
||||
/// * `boundary` - Boundary builder for adding condition_bindings
|
||||
/// * `variable_map` - Host function's variable_map to resolve host ValueIds
|
||||
/// * `space` - JoinValueSpace for unified ValueId allocation (Phase 222.5-B)
|
||||
///
|
||||
/// # Returns
|
||||
///
|
||||
/// ConditionEnv with loop parameter and captured variables
|
||||
/// Tuple of:
|
||||
/// - ConditionEnv with loop parameter and captured variables
|
||||
/// - loop_var_join_id: The JoinIR ValueId allocated for the loop parameter
|
||||
///
|
||||
/// # Example
|
||||
///
|
||||
/// ```ignore
|
||||
/// let captured = analyze_captured_vars(fn_body, loop_ast, scope);
|
||||
/// let mut boundary = JoinInlineBoundaryBuilder::new();
|
||||
/// let env = ConditionEnvBuilder::build_with_captures(
|
||||
/// let mut space = JoinValueSpace::new();
|
||||
/// let (env, loop_var_join_id) = ConditionEnvBuilder::build_with_captures(
|
||||
/// "pos",
|
||||
/// &captured, // Contains "digits"
|
||||
/// &mut boundary,
|
||||
/// &variable_map, // To resolve "digits" → ValueId(42)
|
||||
/// &mut space,
|
||||
/// );
|
||||
/// // env.params: "pos" → ValueId(0)
|
||||
/// // env.captured: "digits" → ValueId(1)
|
||||
/// // boundary.condition_bindings: [ConditionBinding { name: "digits", host_value: ValueId(42), join_value: ValueId(1) }]
|
||||
/// // env.params: "pos" → ValueId(100) (from JoinValueSpace)
|
||||
/// // env.captured: "digits" → ValueId(101)
|
||||
/// // boundary.condition_bindings: [ConditionBinding { name: "digits", host_value: ValueId(42), join_value: ValueId(101) }]
|
||||
/// ```
|
||||
#[allow(dead_code)]
|
||||
pub fn build_with_captures(
|
||||
@ -180,7 +170,8 @@ impl ConditionEnvBuilder {
|
||||
captured: &CapturedEnv,
|
||||
boundary: &mut JoinInlineBoundaryBuilder,
|
||||
variable_map: &BTreeMap<String, ValueId>,
|
||||
) -> ConditionEnv {
|
||||
space: &mut JoinValueSpace,
|
||||
) -> (ConditionEnv, ValueId) {
|
||||
use std::env;
|
||||
|
||||
let debug = env::var("NYASH_CAPTURE_DEBUG").is_ok();
|
||||
@ -189,8 +180,8 @@ impl ConditionEnvBuilder {
|
||||
eprintln!("[capture/env_builder] Building ConditionEnv with {} captured vars", captured.vars.len());
|
||||
}
|
||||
|
||||
// Step 1: Build base ConditionEnv with loop params (existing logic)
|
||||
let mut env = Self::build_loop_param_only(loop_var_name);
|
||||
// Step 1: Build base ConditionEnv with loop params using v2 API (Phase 222.5-B)
|
||||
let (mut env, loop_var_join_id) = Self::build_loop_param_only_v2(loop_var_name, space);
|
||||
|
||||
// Step 2: Add captured vars as ParamRole::Condition
|
||||
for var in &captured.vars {
|
||||
@ -236,7 +227,7 @@ impl ConditionEnvBuilder {
|
||||
eprintln!("[capture/env_builder] Final ConditionEnv: {} params, {} captured", param_count, env.captured.len());
|
||||
}
|
||||
|
||||
env
|
||||
(env, loop_var_join_id)
|
||||
}
|
||||
}
|
||||
|
||||
@ -245,14 +236,6 @@ mod tests {
|
||||
use super::*;
|
||||
use crate::ast::{LiteralValue, Span};
|
||||
|
||||
#[test]
|
||||
fn test_build_loop_param_only() {
|
||||
let env = ConditionEnvBuilder::build_loop_param_only("i");
|
||||
|
||||
assert_eq!(env.len(), 1);
|
||||
assert!(env.get("i").is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_build_for_break_condition_no_extra_vars() {
|
||||
// Condition: i < 10 (only uses loop parameter)
|
||||
|
||||
@ -1,26 +1,38 @@
|
||||
//! Phase 193-4: Exit Binding Builder
|
||||
//! Phase 193-4 / Phase 222.5-C: Exit Binding Builder
|
||||
//!
|
||||
//! Connects JoinIR exit values back to host function's variable_map,
|
||||
//! eliminating hardcoded variable names and ValueId assumptions.
|
||||
//!
|
||||
//! This box fully abstractifies loop exit binding generation for Pattern 3 & 4.
|
||||
//! Phase 222.5-C: Refactored into modular architecture:
|
||||
//! - Validator: Validates CarrierInfo and ExitMeta consistency
|
||||
//! - Constructor: Builds exit bindings and allocates post-loop ValueIds
|
||||
//! - Applicator: Applies bindings to JoinInlineBoundary
|
||||
//!
|
||||
//! This orchestrator coordinates the three modules for a complete workflow.
|
||||
|
||||
use crate::mir::ValueId;
|
||||
use crate::mir::join_ir::lowering::inline_boundary::{
|
||||
JoinInlineBoundary, LoopExitBinding,
|
||||
};
|
||||
use crate::mir::join_ir::lowering::carrier_info::{CarrierInfo, ExitMeta};
|
||||
use std::collections::HashMap;
|
||||
use std::collections::BTreeMap; // Phase 222.5-D: HashMap → BTreeMap for determinism
|
||||
|
||||
// Phase 222.5-C: Import modular components
|
||||
use super::exit_binding_validator::validate_exit_binding;
|
||||
use super::exit_binding_constructor::build_loop_exit_bindings;
|
||||
use super::exit_binding_applicator::{apply_exit_bindings_to_boundary, create_loop_var_exit_binding};
|
||||
|
||||
/// Builder for generating loop exit bindings
|
||||
///
|
||||
/// Phase 193-4: Fully boxifies exit binding generation.
|
||||
/// Phase 222.5-C: Refactored into orchestrator pattern.
|
||||
///
|
||||
/// Eliminates hardcoded variable names and ValueId plumbing scattered across lowerers.
|
||||
#[allow(dead_code)]
|
||||
pub struct ExitBindingBuilder<'a> {
|
||||
carrier_info: &'a CarrierInfo,
|
||||
exit_meta: &'a ExitMeta,
|
||||
variable_map: &'a mut HashMap<String, ValueId>,
|
||||
variable_map: &'a mut BTreeMap<String, ValueId>, // Phase 222.5-D: HashMap → BTreeMap for determinism
|
||||
}
|
||||
|
||||
impl<'a> std::fmt::Debug for ExitBindingBuilder<'a> {
|
||||
@ -28,7 +40,7 @@ impl<'a> std::fmt::Debug for ExitBindingBuilder<'a> {
|
||||
f.debug_struct("ExitBindingBuilder")
|
||||
.field("carrier_info", self.carrier_info)
|
||||
.field("exit_meta", self.exit_meta)
|
||||
.field("variable_map", &"<HashMap>")
|
||||
.field("variable_map", &"<BTreeMap>") // Phase 222.5-D: HashMap → BTreeMap for determinism
|
||||
.finish()
|
||||
}
|
||||
}
|
||||
@ -36,6 +48,8 @@ impl<'a> std::fmt::Debug for ExitBindingBuilder<'a> {
|
||||
impl<'a> ExitBindingBuilder<'a> {
|
||||
/// Create a new ExitBindingBuilder
|
||||
///
|
||||
/// Phase 222.5-C: Delegates validation to validator module.
|
||||
///
|
||||
/// # Arguments
|
||||
///
|
||||
/// * `carrier_info` - Metadata about loop variables and carriers
|
||||
@ -49,34 +63,10 @@ impl<'a> ExitBindingBuilder<'a> {
|
||||
pub fn new(
|
||||
carrier_info: &'a CarrierInfo,
|
||||
exit_meta: &'a ExitMeta,
|
||||
variable_map: &'a mut HashMap<String, ValueId>,
|
||||
variable_map: &'a mut BTreeMap<String, ValueId>, // Phase 222.5-D: HashMap → BTreeMap for determinism
|
||||
) -> Result<Self, String> {
|
||||
// Validate that all carriers in ExitMeta exist in CarrierInfo
|
||||
for (carrier_name, _) in &exit_meta.exit_values {
|
||||
if carrier_name == &carrier_info.loop_var_name {
|
||||
return Err(format!(
|
||||
"Loop variable '{}' should not be in exit_values",
|
||||
carrier_name
|
||||
));
|
||||
}
|
||||
|
||||
if !carrier_info.find_carrier(carrier_name).is_some() {
|
||||
return Err(format!(
|
||||
"Exit carrier '{}' not found in CarrierInfo",
|
||||
carrier_name
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
// Validate that all carriers in CarrierInfo have exit values
|
||||
for carrier in &carrier_info.carriers {
|
||||
if exit_meta.find_binding(&carrier.name).is_none() {
|
||||
return Err(format!(
|
||||
"Carrier '{}' missing in ExitMeta",
|
||||
carrier.name
|
||||
));
|
||||
}
|
||||
}
|
||||
// Phase 222.5-C: Delegate validation to validator module
|
||||
validate_exit_binding(carrier_info, exit_meta)?;
|
||||
|
||||
Ok(Self {
|
||||
carrier_info,
|
||||
@ -87,6 +77,8 @@ impl<'a> ExitBindingBuilder<'a> {
|
||||
|
||||
/// Generate loop exit bindings
|
||||
///
|
||||
/// Phase 222.5-C: Delegates to constructor module.
|
||||
///
|
||||
/// Returns one LoopExitBinding per carrier, in sorted order.
|
||||
/// Updates variable_map with new post-loop ValueIds for each carrier.
|
||||
///
|
||||
@ -95,30 +87,14 @@ impl<'a> ExitBindingBuilder<'a> {
|
||||
/// Vec of LoopExitBinding, one per carrier, sorted by carrier name
|
||||
#[allow(dead_code)]
|
||||
pub fn build_loop_exit_bindings(&mut self) -> Result<Vec<LoopExitBinding>, String> {
|
||||
let mut bindings = Vec::new();
|
||||
|
||||
// Process each carrier in sorted order
|
||||
for carrier in &self.carrier_info.carriers {
|
||||
let join_exit_id = self.exit_meta.find_binding(&carrier.name)
|
||||
.ok_or_else(|| format!("Carrier '{}' missing in ExitMeta", carrier.name))?;
|
||||
|
||||
bindings.push(LoopExitBinding {
|
||||
carrier_name: carrier.name.clone(),
|
||||
join_exit_value: join_exit_id,
|
||||
host_slot: carrier.host_id,
|
||||
});
|
||||
|
||||
// Allocate new ValueId for post-loop carrier value
|
||||
// This represents the carrier variable's value after the loop completes
|
||||
let post_loop_id = self.allocate_new_value_id();
|
||||
self.variable_map.insert(carrier.name.clone(), post_loop_id);
|
||||
}
|
||||
|
||||
Ok(bindings)
|
||||
// Phase 222.5-C: Delegate to constructor module
|
||||
build_loop_exit_bindings(self.carrier_info, self.exit_meta, self.variable_map)
|
||||
}
|
||||
|
||||
/// Apply bindings to JoinInlineBoundary
|
||||
///
|
||||
/// Phase 222.5-C: Delegates to applicator module.
|
||||
///
|
||||
/// Sets exit_bindings (and join_outputs for legacy) based on loop_var + carriers.
|
||||
/// Must be called after build_loop_exit_bindings().
|
||||
///
|
||||
@ -131,70 +107,24 @@ impl<'a> ExitBindingBuilder<'a> {
|
||||
/// Success or error if boundary cannot be updated
|
||||
#[allow(dead_code)]
|
||||
pub fn apply_to_boundary(&self, boundary: &mut JoinInlineBoundary) -> Result<(), String> {
|
||||
// Build explicit exit bindings (loop var + carriers)
|
||||
let mut bindings = Vec::new();
|
||||
bindings.push(self.loop_var_exit_binding());
|
||||
|
||||
let mut join_outputs = vec![self.carrier_info.loop_var_id]; // legacy field for compatibility
|
||||
|
||||
for carrier in &self.carrier_info.carriers {
|
||||
let post_loop_id = self.variable_map.get(&carrier.name).copied().ok_or_else(|| {
|
||||
format!("Post-loop ValueId not found for carrier '{}'", carrier.name)
|
||||
})?;
|
||||
|
||||
let join_exit_id = self.exit_meta.find_binding(&carrier.name).ok_or_else(|| {
|
||||
format!("Exit value not found for carrier '{}'", carrier.name)
|
||||
})?;
|
||||
|
||||
bindings.push(LoopExitBinding {
|
||||
carrier_name: carrier.name.clone(),
|
||||
host_slot: post_loop_id,
|
||||
join_exit_value: join_exit_id,
|
||||
});
|
||||
|
||||
join_outputs.push(join_exit_id);
|
||||
}
|
||||
|
||||
boundary.exit_bindings = bindings;
|
||||
// Deprecated fields kept in sync for legacy consumers
|
||||
let join_outputs_clone = join_outputs.clone();
|
||||
boundary.join_outputs = join_outputs;
|
||||
#[allow(deprecated)]
|
||||
{
|
||||
boundary.host_outputs = join_outputs_clone;
|
||||
}
|
||||
|
||||
Ok(())
|
||||
// Phase 222.5-C: Delegate to applicator module
|
||||
apply_exit_bindings_to_boundary(
|
||||
self.carrier_info,
|
||||
self.exit_meta,
|
||||
self.variable_map,
|
||||
boundary,
|
||||
)
|
||||
}
|
||||
|
||||
/// Get the loop variable exit binding
|
||||
///
|
||||
/// Phase 222.5-C: Delegates to applicator module.
|
||||
///
|
||||
/// The loop variable is always the first exit (index 0).
|
||||
#[allow(dead_code)]
|
||||
pub fn loop_var_exit_binding(&self) -> LoopExitBinding {
|
||||
LoopExitBinding {
|
||||
carrier_name: self.carrier_info.loop_var_name.clone(),
|
||||
join_exit_value: self.carrier_info.loop_var_id, // Loop var maps to itself
|
||||
host_slot: self.carrier_info.loop_var_id,
|
||||
}
|
||||
}
|
||||
|
||||
/// Allocate a new ValueId for a post-loop carrier
|
||||
///
|
||||
/// Phase 193-4: Temporary sequential allocation strategy.
|
||||
/// Future improvement: Delegate to MirBuilder's next_value_id() for proper allocation.
|
||||
#[allow(dead_code)]
|
||||
fn allocate_new_value_id(&self) -> ValueId {
|
||||
// Find the maximum ValueId in current variable_map
|
||||
let max_id = self.variable_map.values()
|
||||
.map(|v| v.0)
|
||||
.max()
|
||||
.unwrap_or(0);
|
||||
|
||||
// Allocate next sequential ID
|
||||
// Note: This is a temporary strategy and should be replaced with
|
||||
// proper ValueId allocation from the builder
|
||||
ValueId(max_id + 1)
|
||||
// Phase 222.5-C: Delegate to applicator module
|
||||
create_loop_var_exit_binding(self.carrier_info)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@ -0,0 +1,163 @@
|
||||
//! Phase 222.5-C: Exit Binding Applicator
|
||||
//!
|
||||
//! Applies exit bindings to JoinInlineBoundary.
|
||||
//! Single-responsibility box for boundary application logic.
|
||||
|
||||
use crate::mir::ValueId;
|
||||
use crate::mir::join_ir::lowering::inline_boundary::{
|
||||
JoinInlineBoundary, LoopExitBinding,
|
||||
};
|
||||
use crate::mir::join_ir::lowering::carrier_info::{CarrierInfo, ExitMeta};
|
||||
use std::collections::BTreeMap; // Phase 222.5-D: HashMap → BTreeMap for determinism
|
||||
|
||||
/// Apply bindings to JoinInlineBoundary
|
||||
///
|
||||
/// Sets exit_bindings (and join_outputs for legacy) based on loop_var + carriers.
|
||||
/// Must be called after build_loop_exit_bindings().
|
||||
///
|
||||
/// Phase 222.5-C: Extracted from ExitBindingBuilder to separate application concerns.
|
||||
///
|
||||
/// # Arguments
|
||||
///
|
||||
/// * `carrier_info` - Metadata about loop variables and carriers
|
||||
/// * `exit_meta` - Exit values from JoinIR lowering
|
||||
/// * `variable_map` - Host function's variable map (must contain post-loop ValueIds)
|
||||
/// * `boundary` - JoinInlineBoundary to update
|
||||
///
|
||||
/// # Returns
|
||||
///
|
||||
/// Success or error if boundary cannot be updated
|
||||
pub fn apply_exit_bindings_to_boundary(
|
||||
carrier_info: &CarrierInfo,
|
||||
exit_meta: &ExitMeta,
|
||||
variable_map: &BTreeMap<String, ValueId>, // Phase 222.5-D: HashMap → BTreeMap for determinism
|
||||
boundary: &mut JoinInlineBoundary,
|
||||
) -> Result<(), String> {
|
||||
// Build explicit exit bindings (loop var + carriers)
|
||||
let mut bindings = Vec::new();
|
||||
bindings.push(create_loop_var_exit_binding(carrier_info));
|
||||
|
||||
let mut join_outputs = vec![carrier_info.loop_var_id]; // legacy field for compatibility
|
||||
|
||||
for carrier in &carrier_info.carriers {
|
||||
let post_loop_id = variable_map.get(&carrier.name).copied().ok_or_else(|| {
|
||||
format!("Post-loop ValueId not found for carrier '{}'", carrier.name)
|
||||
})?;
|
||||
|
||||
let join_exit_id = exit_meta.find_binding(&carrier.name).ok_or_else(|| {
|
||||
format!("Exit value not found for carrier '{}'", carrier.name)
|
||||
})?;
|
||||
|
||||
bindings.push(LoopExitBinding {
|
||||
carrier_name: carrier.name.clone(),
|
||||
host_slot: post_loop_id,
|
||||
join_exit_value: join_exit_id,
|
||||
});
|
||||
|
||||
join_outputs.push(join_exit_id);
|
||||
}
|
||||
|
||||
boundary.exit_bindings = bindings;
|
||||
// Deprecated fields kept in sync for legacy consumers
|
||||
let join_outputs_clone = join_outputs.clone();
|
||||
boundary.join_outputs = join_outputs;
|
||||
#[allow(deprecated)]
|
||||
{
|
||||
boundary.host_outputs = join_outputs_clone;
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Create the loop variable exit binding
|
||||
///
|
||||
/// The loop variable is always the first exit (index 0).
|
||||
///
|
||||
/// Phase 222.5-C: Extracted from ExitBindingBuilder for single-purpose function.
|
||||
///
|
||||
/// # Arguments
|
||||
///
|
||||
/// * `carrier_info` - Metadata about loop variables and carriers
|
||||
///
|
||||
/// # Returns
|
||||
///
|
||||
/// LoopExitBinding for the loop variable
|
||||
pub fn create_loop_var_exit_binding(carrier_info: &CarrierInfo) -> LoopExitBinding {
|
||||
LoopExitBinding {
|
||||
carrier_name: carrier_info.loop_var_name.clone(),
|
||||
join_exit_value: carrier_info.loop_var_id, // Loop var maps to itself
|
||||
host_slot: carrier_info.loop_var_id,
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::mir::join_ir::lowering::carrier_info::CarrierVar;
|
||||
|
||||
#[test]
|
||||
fn test_apply_to_boundary() {
|
||||
let carrier_info = CarrierInfo::with_carriers(
|
||||
"i".to_string(),
|
||||
ValueId(5),
|
||||
vec![CarrierVar {
|
||||
name: "sum".to_string(),
|
||||
host_id: ValueId(10),
|
||||
join_id: None,
|
||||
}],
|
||||
);
|
||||
|
||||
let exit_meta = ExitMeta::single("sum".to_string(), ValueId(15));
|
||||
|
||||
// Simulate post-loop ValueId allocation (as done by constructor)
|
||||
let variable_map = [
|
||||
("i".to_string(), ValueId(5)),
|
||||
("sum".to_string(), ValueId(11)), // Post-loop ValueId
|
||||
]
|
||||
.iter()
|
||||
.cloned()
|
||||
.collect();
|
||||
|
||||
let mut boundary = JoinInlineBoundary {
|
||||
host_inputs: vec![],
|
||||
join_inputs: vec![],
|
||||
exit_bindings: vec![], // Phase 171: Add missing field
|
||||
#[allow(deprecated)]
|
||||
host_outputs: vec![], // legacy, unused in new assertions
|
||||
join_outputs: vec![],
|
||||
#[allow(deprecated)]
|
||||
condition_inputs: vec![], // Phase 171: Add missing field
|
||||
condition_bindings: vec![], // Phase 171-fix: Add missing field
|
||||
expr_result: None, // Phase 33-14: Add missing field
|
||||
loop_var_name: None, // Phase 33-16: Add missing field
|
||||
};
|
||||
|
||||
apply_exit_bindings_to_boundary(&carrier_info, &exit_meta, &variable_map, &mut boundary)
|
||||
.expect("Failed to apply to boundary");
|
||||
|
||||
// Should have loop_var + sum carrier in exit_bindings
|
||||
assert_eq!(boundary.exit_bindings.len(), 2);
|
||||
assert_eq!(boundary.exit_bindings[0].carrier_name, "i");
|
||||
assert_eq!(boundary.exit_bindings[0].host_slot, ValueId(5));
|
||||
assert_eq!(boundary.exit_bindings[0].join_exit_value, ValueId(5));
|
||||
|
||||
assert_eq!(boundary.exit_bindings[1].carrier_name, "sum");
|
||||
// Post-loop carrier id is freshly allocated (10 -> 11)
|
||||
assert_eq!(boundary.exit_bindings[1].host_slot, ValueId(11));
|
||||
assert_eq!(boundary.exit_bindings[1].join_exit_value, ValueId(15));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_loop_var_exit_binding() {
|
||||
let carrier_info = CarrierInfo::with_carriers(
|
||||
"i".to_string(),
|
||||
ValueId(5),
|
||||
vec![],
|
||||
);
|
||||
|
||||
let binding = create_loop_var_exit_binding(&carrier_info);
|
||||
assert_eq!(binding.carrier_name, "i");
|
||||
assert_eq!(binding.host_slot, ValueId(5));
|
||||
assert_eq!(binding.join_exit_value, ValueId(5));
|
||||
}
|
||||
}
|
||||
@ -0,0 +1,165 @@
|
||||
//! Phase 222.5-C: Exit Binding Constructor
|
||||
//!
|
||||
//! Constructs loop exit bindings and allocates post-loop ValueIds.
|
||||
//! Single-responsibility box for binding construction logic.
|
||||
|
||||
use crate::mir::ValueId;
|
||||
use crate::mir::join_ir::lowering::inline_boundary::LoopExitBinding;
|
||||
use crate::mir::join_ir::lowering::carrier_info::{CarrierInfo, ExitMeta};
|
||||
use std::collections::BTreeMap; // Phase 222.5-D: HashMap → BTreeMap for determinism
|
||||
|
||||
/// Generate loop exit bindings
|
||||
///
|
||||
/// Returns one LoopExitBinding per carrier, in sorted order.
|
||||
/// Updates variable_map with new post-loop ValueIds for each carrier.
|
||||
///
|
||||
/// Phase 222.5-C: Extracted from ExitBindingBuilder to separate construction concerns.
|
||||
///
|
||||
/// # Arguments
|
||||
///
|
||||
/// * `carrier_info` - Metadata about loop variables and carriers
|
||||
/// * `exit_meta` - Exit values from JoinIR lowering
|
||||
/// * `variable_map` - Host function's variable map (will be updated with post-loop ValueIds)
|
||||
///
|
||||
/// # Returns
|
||||
///
|
||||
/// Vec of LoopExitBinding, one per carrier, sorted by carrier name
|
||||
pub fn build_loop_exit_bindings(
|
||||
carrier_info: &CarrierInfo,
|
||||
exit_meta: &ExitMeta,
|
||||
variable_map: &mut BTreeMap<String, ValueId>, // Phase 222.5-D: HashMap → BTreeMap for determinism
|
||||
) -> Result<Vec<LoopExitBinding>, String> {
|
||||
let mut bindings = Vec::new();
|
||||
|
||||
// Process each carrier in sorted order
|
||||
for carrier in &carrier_info.carriers {
|
||||
let join_exit_id = exit_meta.find_binding(&carrier.name)
|
||||
.ok_or_else(|| format!("Carrier '{}' missing in ExitMeta", carrier.name))?;
|
||||
|
||||
bindings.push(LoopExitBinding {
|
||||
carrier_name: carrier.name.clone(),
|
||||
join_exit_value: join_exit_id,
|
||||
host_slot: carrier.host_id,
|
||||
});
|
||||
|
||||
// Allocate new ValueId for post-loop carrier value
|
||||
// This represents the carrier variable's value after the loop completes
|
||||
let post_loop_id = allocate_new_value_id(variable_map);
|
||||
variable_map.insert(carrier.name.clone(), post_loop_id);
|
||||
}
|
||||
|
||||
Ok(bindings)
|
||||
}
|
||||
|
||||
/// Allocate a new ValueId for a post-loop carrier
|
||||
///
|
||||
/// Phase 222.5-C: Temporary sequential allocation strategy.
|
||||
/// Future improvement: Delegate to MirBuilder's next_value_id() for proper allocation.
|
||||
///
|
||||
/// # Arguments
|
||||
///
|
||||
/// * `variable_map` - Current variable map to determine next available ValueId
|
||||
///
|
||||
/// # Returns
|
||||
///
|
||||
/// Newly allocated ValueId
|
||||
pub fn allocate_new_value_id(variable_map: &BTreeMap<String, ValueId>) -> ValueId { // Phase 222.5-D: HashMap → BTreeMap for determinism
|
||||
// Find the maximum ValueId in current variable_map
|
||||
let max_id = variable_map.values()
|
||||
.map(|v| v.0)
|
||||
.max()
|
||||
.unwrap_or(0);
|
||||
|
||||
// Allocate next sequential ID
|
||||
// Note: This is a temporary strategy and should be replaced with
|
||||
// proper ValueId allocation from the builder
|
||||
ValueId(max_id + 1)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::mir::join_ir::lowering::carrier_info::CarrierVar;
|
||||
|
||||
#[test]
|
||||
fn test_single_carrier_binding() {
|
||||
let carrier_info = CarrierInfo::with_carriers(
|
||||
"i".to_string(),
|
||||
ValueId(5),
|
||||
vec![CarrierVar {
|
||||
name: "sum".to_string(),
|
||||
host_id: ValueId(10),
|
||||
join_id: None,
|
||||
}],
|
||||
);
|
||||
|
||||
let exit_meta = ExitMeta::single("sum".to_string(), ValueId(15));
|
||||
|
||||
let mut variable_map = [
|
||||
("i".to_string(), ValueId(5)),
|
||||
("sum".to_string(), ValueId(10)),
|
||||
]
|
||||
.iter()
|
||||
.cloned()
|
||||
.collect();
|
||||
|
||||
let bindings = build_loop_exit_bindings(&carrier_info, &exit_meta, &mut variable_map)
|
||||
.expect("Failed to build bindings");
|
||||
|
||||
assert_eq!(bindings.len(), 1);
|
||||
assert_eq!(bindings[0].carrier_name, "sum");
|
||||
assert_eq!(bindings[0].host_slot, ValueId(10));
|
||||
assert_eq!(bindings[0].join_exit_value, ValueId(15));
|
||||
|
||||
// Check that variable_map was updated with new post-loop ValueId
|
||||
assert!(variable_map.contains_key("sum"));
|
||||
let post_loop_id = variable_map["sum"];
|
||||
assert!(post_loop_id.0 > 10); // Should be allocated after max of existing IDs
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_multi_carrier_binding() {
|
||||
let carrier_info = CarrierInfo::with_carriers(
|
||||
"i".to_string(),
|
||||
ValueId(5),
|
||||
vec![
|
||||
CarrierVar {
|
||||
name: "printed".to_string(),
|
||||
host_id: ValueId(11),
|
||||
join_id: None,
|
||||
},
|
||||
CarrierVar {
|
||||
name: "sum".to_string(),
|
||||
host_id: ValueId(10),
|
||||
join_id: None,
|
||||
},
|
||||
],
|
||||
);
|
||||
|
||||
let exit_meta = ExitMeta::multiple(vec![
|
||||
("printed".to_string(), ValueId(14)),
|
||||
("sum".to_string(), ValueId(15)),
|
||||
]);
|
||||
|
||||
let mut variable_map = [
|
||||
("i".to_string(), ValueId(5)),
|
||||
("sum".to_string(), ValueId(10)),
|
||||
("printed".to_string(), ValueId(11)),
|
||||
]
|
||||
.iter()
|
||||
.cloned()
|
||||
.collect();
|
||||
|
||||
let bindings = build_loop_exit_bindings(&carrier_info, &exit_meta, &mut variable_map)
|
||||
.expect("Failed to build bindings");
|
||||
|
||||
assert_eq!(bindings.len(), 2);
|
||||
// Bindings should be sorted by carrier name
|
||||
assert_eq!(bindings[0].carrier_name, "printed");
|
||||
assert_eq!(bindings[1].carrier_name, "sum");
|
||||
|
||||
// Check post-loop ValueIds are allocated
|
||||
assert!(variable_map.contains_key("printed"));
|
||||
assert!(variable_map.contains_key("sum"));
|
||||
}
|
||||
}
|
||||
@ -0,0 +1,171 @@
|
||||
//! Phase 222.5-C: Exit Binding Validator
|
||||
//!
|
||||
//! Validates consistency between CarrierInfo and ExitMeta.
|
||||
//! Single-responsibility box for validation logic.
|
||||
|
||||
use crate::mir::join_ir::lowering::carrier_info::{CarrierInfo, ExitMeta};
|
||||
|
||||
/// Validate CarrierInfo and ExitMeta consistency
|
||||
///
|
||||
/// Checks:
|
||||
/// 1. Loop variable is NOT in exit_values
|
||||
/// 2. All carriers in ExitMeta exist in CarrierInfo
|
||||
/// 3. All carriers in CarrierInfo have exit values in ExitMeta
|
||||
///
|
||||
/// # Arguments
|
||||
///
|
||||
/// * `carrier_info` - Metadata about loop variables and carriers
|
||||
/// * `exit_meta` - Exit values from JoinIR lowering
|
||||
///
|
||||
/// # Returns
|
||||
///
|
||||
/// Ok(()) if validation passes, Err with descriptive message if validation fails
|
||||
pub fn validate_exit_binding(
|
||||
carrier_info: &CarrierInfo,
|
||||
exit_meta: &ExitMeta,
|
||||
) -> Result<(), String> {
|
||||
// Validate that all carriers in ExitMeta exist in CarrierInfo
|
||||
for (carrier_name, _) in &exit_meta.exit_values {
|
||||
if carrier_name == &carrier_info.loop_var_name {
|
||||
return Err(format!(
|
||||
"Loop variable '{}' should not be in exit_values",
|
||||
carrier_name
|
||||
));
|
||||
}
|
||||
|
||||
if !carrier_info.find_carrier(carrier_name).is_some() {
|
||||
return Err(format!(
|
||||
"Exit carrier '{}' not found in CarrierInfo",
|
||||
carrier_name
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
// Validate that all carriers in CarrierInfo have exit values
|
||||
for carrier in &carrier_info.carriers {
|
||||
if exit_meta.find_binding(&carrier.name).is_none() {
|
||||
return Err(format!(
|
||||
"Carrier '{}' missing in ExitMeta",
|
||||
carrier.name
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::mir::join_ir::lowering::carrier_info::CarrierVar;
|
||||
use crate::mir::ValueId;
|
||||
|
||||
#[test]
|
||||
fn test_validate_single_carrier() {
|
||||
let carrier_info = CarrierInfo::with_carriers(
|
||||
"i".to_string(),
|
||||
ValueId(5),
|
||||
vec![CarrierVar {
|
||||
name: "sum".to_string(),
|
||||
host_id: ValueId(10),
|
||||
join_id: None,
|
||||
}],
|
||||
);
|
||||
|
||||
let exit_meta = ExitMeta::single("sum".to_string(), ValueId(15));
|
||||
|
||||
let result = validate_exit_binding(&carrier_info, &exit_meta);
|
||||
assert!(result.is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_validate_multi_carrier() {
|
||||
let carrier_info = CarrierInfo::with_carriers(
|
||||
"i".to_string(),
|
||||
ValueId(5),
|
||||
vec![
|
||||
CarrierVar {
|
||||
name: "printed".to_string(),
|
||||
host_id: ValueId(11),
|
||||
join_id: None,
|
||||
},
|
||||
CarrierVar {
|
||||
name: "sum".to_string(),
|
||||
host_id: ValueId(10),
|
||||
join_id: None,
|
||||
},
|
||||
],
|
||||
);
|
||||
|
||||
let exit_meta = ExitMeta::multiple(vec![
|
||||
("printed".to_string(), ValueId(14)),
|
||||
("sum".to_string(), ValueId(15)),
|
||||
]);
|
||||
|
||||
let result = validate_exit_binding(&carrier_info, &exit_meta);
|
||||
assert!(result.is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_carrier_name_mismatch_error() {
|
||||
let carrier_info = CarrierInfo::with_carriers(
|
||||
"i".to_string(),
|
||||
ValueId(5),
|
||||
vec![CarrierVar {
|
||||
name: "sum".to_string(),
|
||||
host_id: ValueId(10),
|
||||
join_id: None,
|
||||
}],
|
||||
);
|
||||
|
||||
// ExitMeta with non-existent carrier
|
||||
let exit_meta = ExitMeta::single("foo".to_string(), ValueId(15));
|
||||
|
||||
let result = validate_exit_binding(&carrier_info, &exit_meta);
|
||||
assert!(result.is_err());
|
||||
assert!(result.unwrap_err().contains("not found in CarrierInfo"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_missing_carrier_in_exit_meta() {
|
||||
let carrier_info = CarrierInfo::with_carriers(
|
||||
"i".to_string(),
|
||||
ValueId(5),
|
||||
vec![CarrierVar {
|
||||
name: "sum".to_string(),
|
||||
host_id: ValueId(10),
|
||||
join_id: None,
|
||||
}],
|
||||
);
|
||||
|
||||
// ExitMeta is empty
|
||||
let exit_meta = ExitMeta::empty();
|
||||
|
||||
let result = validate_exit_binding(&carrier_info, &exit_meta);
|
||||
assert!(result.is_err());
|
||||
assert!(result.unwrap_err().contains("missing in ExitMeta"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_loop_var_in_exit_meta_error() {
|
||||
let carrier_info = CarrierInfo::with_carriers(
|
||||
"i".to_string(),
|
||||
ValueId(5),
|
||||
vec![CarrierVar {
|
||||
name: "sum".to_string(),
|
||||
host_id: ValueId(10),
|
||||
join_id: None,
|
||||
}],
|
||||
);
|
||||
|
||||
// ExitMeta incorrectly includes loop var
|
||||
let exit_meta = ExitMeta::multiple(vec![
|
||||
("i".to_string(), ValueId(5)),
|
||||
("sum".to_string(), ValueId(15)),
|
||||
]);
|
||||
|
||||
let result = validate_exit_binding(&carrier_info, &exit_meta);
|
||||
assert!(result.is_err());
|
||||
assert!(result.unwrap_err().contains("should not be in exit_values"));
|
||||
}
|
||||
}
|
||||
@ -15,8 +15,11 @@
|
||||
//! - ast_feature_extractor.rs: Pure function module for analyzing loop AST
|
||||
//! - High reusability for Pattern 5-6 and pattern analysis tools
|
||||
//!
|
||||
//! Phase 193-4: Exit Binding Builder
|
||||
//! - exit_binding.rs: Fully boxified exit binding generation
|
||||
//! Phase 193-4 / Phase 222.5-C: Exit Binding Builder
|
||||
//! - exit_binding.rs: Fully boxified exit binding generation (orchestrator)
|
||||
//! - exit_binding_validator.rs: CarrierInfo and ExitMeta validation
|
||||
//! - exit_binding_constructor.rs: Exit binding construction and ValueId allocation
|
||||
//! - exit_binding_applicator.rs: Boundary application logic
|
||||
//! - Eliminates hardcoded variable names and ValueId assumptions
|
||||
//! - Supports both single and multi-carrier loop patterns
|
||||
//!
|
||||
@ -43,6 +46,9 @@ pub(in crate::mir::builder) mod common_init;
|
||||
pub(in crate::mir::builder) mod condition_env_builder;
|
||||
pub(in crate::mir::builder) mod conversion_pipeline;
|
||||
pub(in crate::mir::builder) mod exit_binding;
|
||||
pub(in crate::mir::builder) mod exit_binding_validator; // Phase 222.5-C
|
||||
pub(in crate::mir::builder) mod exit_binding_constructor; // Phase 222.5-C
|
||||
pub(in crate::mir::builder) mod exit_binding_applicator; // Phase 222.5-C
|
||||
pub(in crate::mir::builder) mod loop_scope_shape_builder;
|
||||
pub(in crate::mir::builder) mod pattern_pipeline;
|
||||
pub(in crate::mir::builder) mod pattern1_minimal;
|
||||
|
||||
@ -17,7 +17,7 @@ use crate::ast::ASTNode;
|
||||
use crate::mir::join_ir::lowering::carrier_info::{CarrierInfo, CarrierVar};
|
||||
use crate::mir::join_ir::lowering::continue_branch_normalizer::ContinueBranchNormalizer;
|
||||
use crate::mir::join_ir::lowering::loop_update_analyzer::{LoopUpdateAnalyzer, UpdateExpr};
|
||||
use std::collections::HashMap;
|
||||
use std::collections::BTreeMap; // Phase 222.5-D: HashMap → BTreeMap for determinism
|
||||
|
||||
pub struct Pattern4CarrierAnalyzer;
|
||||
|
||||
@ -86,7 +86,7 @@ impl Pattern4CarrierAnalyzer {
|
||||
pub fn analyze_carrier_updates(
|
||||
loop_body: &[ASTNode],
|
||||
carriers: &[CarrierVar],
|
||||
) -> HashMap<String, UpdateExpr> {
|
||||
) -> BTreeMap<String, UpdateExpr> { // Phase 222.5-D: HashMap → BTreeMap for determinism
|
||||
LoopUpdateAnalyzer::analyze_carrier_updates(loop_body, carriers)
|
||||
}
|
||||
|
||||
|
||||
@ -42,7 +42,7 @@ use crate::mir::join_ir::lowering::loop_update_summary::LoopUpdateSummary; // P
|
||||
use crate::mir::loop_pattern_detection::trim_loop_helper::TrimLoopHelper;
|
||||
use crate::mir::ValueId;
|
||||
use crate::mir::BasicBlockId;
|
||||
use std::collections::{HashMap, BTreeSet};
|
||||
use std::collections::{BTreeMap, BTreeSet}; // Phase 222.5-D: HashMap → BTreeMap for determinism
|
||||
|
||||
use super::common_init::CommonPatternInitializer;
|
||||
use super::loop_scope_shape_builder::LoopScopeShapeBuilder;
|
||||
@ -102,7 +102,7 @@ pub struct PatternPipelineContext {
|
||||
/// Carrier update expressions (variable → UpdateExpr)
|
||||
/// Used by Pattern 2 (multi-carrier) and Pattern 4 (Select-based updates)
|
||||
#[allow(dead_code)]
|
||||
pub carrier_updates: Option<HashMap<String, UpdateExpr>>,
|
||||
pub carrier_updates: Option<BTreeMap<String, UpdateExpr>>, // Phase 222.5-D: HashMap → BTreeMap for determinism
|
||||
|
||||
// === Pattern 2/4: Trim Pattern Support ===
|
||||
|
||||
|
||||
@ -138,9 +138,9 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn test_setup_trim_carrier_binding() {
|
||||
use std::collections::HashMap;
|
||||
use std::collections::BTreeMap; // Phase 222.5-D: HashMap → BTreeMap for determinism
|
||||
let helper = create_test_helper();
|
||||
let mut variable_map = HashMap::new();
|
||||
let mut variable_map = BTreeMap::new();
|
||||
variable_map.insert("is_ws".to_string(), ValueId(42));
|
||||
|
||||
let mut counter = 100u32;
|
||||
@ -166,9 +166,9 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn test_setup_trim_carrier_binding_missing_carrier() {
|
||||
use std::collections::HashMap;
|
||||
use std::collections::BTreeMap; // Phase 222.5-D: HashMap → BTreeMap for determinism
|
||||
let helper = create_test_helper();
|
||||
let variable_map: HashMap<String, ValueId> = HashMap::new(); // Empty!
|
||||
let variable_map: BTreeMap<String, ValueId> = BTreeMap::new(); // Empty!
|
||||
|
||||
let mut counter = 100u32;
|
||||
let mut alloc = || {
|
||||
@ -190,12 +190,12 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn test_add_to_condition_env() {
|
||||
use std::collections::HashMap;
|
||||
use std::collections::BTreeMap; // Phase 222.5-D: HashMap → BTreeMap for determinism
|
||||
let helper = create_test_helper();
|
||||
let mut variable_map = HashMap::new();
|
||||
let mut variable_map = BTreeMap::new();
|
||||
variable_map.insert("is_ws".to_string(), ValueId(42));
|
||||
|
||||
let mut env = HashMap::new();
|
||||
let mut env = BTreeMap::new();
|
||||
|
||||
let mut counter = 100u32;
|
||||
let mut alloc = || {
|
||||
|
||||
Reference in New Issue
Block a user