From cc68327ab634a70fb8dba34952ac7c8c9b897e70 Mon Sep 17 00:00:00 2001 From: nyash-codex Date: Mon, 8 Dec 2025 03:48:18 +0900 Subject: [PATCH] feat(joinir): Phase 171-172 Issue 5 - ConditionEnvBuilder unified construction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eliminates 40-50 lines of duplicated ConditionEnv + ConditionBinding construction in Pattern 2. **Changes**: - New: `condition_env_builder.rs` with factory methods - `build_for_break_condition()`: Extract condition variables, allocate JoinIR ValueIds, create bindings - `build_loop_param_only()`: Simple env with only loop parameter - Updated Pattern 2 to use unified builder - Includes 4 unit tests covering all usage scenarios **Impact**: - Pattern 2: 47 lines → 11 lines (36-line reduction, ~77%) - Preserved allocator closure for Trim pattern additions - Maintains mutability for downstream Trim pattern code **Test**: - cargo build --release: ✅ PASS - cargo test --release: ✅ 724/804 PASS (+4 improvements) - Unit tests: ✅ 4/4 PASS Phase 171-172 Stage 1: Total 66 lines reduced so far (Issue 4: 30 lines + Issue 5: 36 lines) --- .../joinir/patterns/condition_env_builder.rs | 248 ++++++++++++++++++ .../control_flow/joinir/patterns/mod.rs | 2 + .../joinir/patterns/pattern2_with_break.rs | 51 +--- 3 files changed, 263 insertions(+), 38 deletions(-) create mode 100644 src/mir/builder/control_flow/joinir/patterns/condition_env_builder.rs diff --git a/src/mir/builder/control_flow/joinir/patterns/condition_env_builder.rs b/src/mir/builder/control_flow/joinir/patterns/condition_env_builder.rs new file mode 100644 index 00000000..d2d3af4e --- /dev/null +++ b/src/mir/builder/control_flow/joinir/patterns/condition_env_builder.rs @@ -0,0 +1,248 @@ +//! ConditionEnvBuilder - Unified ConditionEnv construction +//! +//! Phase 171-172: Issue 5 +//! +//! Provides unified construction methods for ConditionEnv and ConditionBindings +//! used in Pattern 2 (break condition analysis). +//! +//! # Responsibility +//! +//! - Extract condition variables from AST nodes +//! - Allocate JoinIR-local ValueIds for condition-only variables +//! - Build ConditionEnv mapping (variable name → JoinIR ValueId) +//! - Create ConditionBindings for host↔JoinIR value mapping +//! +//! # Usage +//! +//! ```rust +//! // Standard break condition analysis +//! let (env, bindings) = ConditionEnvBuilder::build_for_break_condition( +//! break_condition, +//! &loop_var_name, +//! &variable_map, +//! loop_var_id, +//! )?; +//! ``` + +use crate::ast::ASTNode; +use crate::mir::join_ir::lowering::condition_env::{ConditionBinding, ConditionEnv}; +use crate::mir::join_ir::lowering::condition_to_joinir::extract_condition_variables; +use crate::mir::ValueId; +use std::collections::BTreeMap; + +pub struct ConditionEnvBuilder; + +impl ConditionEnvBuilder { + /// Build ConditionEnv and ConditionBindings from break condition + /// + /// This extracts all variables used in the break condition (excluding the loop parameter) + /// and creates: + /// 1. ConditionEnv: Maps variable names to JoinIR-local ValueIds + /// 2. ConditionBindings: Records HOST ValueId ↔ JoinIR ValueId mappings for merge + /// + /// # Arguments + /// + /// * `break_condition` - AST node for the break condition + /// * `loop_var_name` - Loop parameter name (excluded from condition-only variables) + /// * `variable_map` - HOST function's variable_map (for looking up HOST ValueIds) + /// * `loop_var_id` - HOST ValueId for the loop parameter + /// + /// # Returns + /// + /// Tuple of: + /// - ConditionEnv: Variable name → JoinIR ValueId mapping + /// - Vec: HOST↔JoinIR value mappings for merge + /// + /// # Errors + /// + /// Returns error if a condition variable is not found in variable_map + pub fn build_for_break_condition( + break_condition: &ASTNode, + loop_var_name: &str, + variable_map: &BTreeMap, + loop_var_id: ValueId, + ) -> Result<(ConditionEnv, Vec), String> { + // Extract all variables used in the condition (excluding loop parameter) + let condition_var_names = extract_condition_variables( + break_condition, + &[loop_var_name.to_string()], + ); + + let mut env = ConditionEnv::new(); + let mut bindings = Vec::new(); + + // Add loop parameter to env (ValueId(0) in JoinIR space) + // The loop parameter is NOT a condition binding (it's a join_input instead) + env.insert(loop_var_name.to_string(), ValueId(0)); + + // Create a local allocator for JoinIR-local ValueIds for condition-only variables + let mut join_value_counter = 1u32; // Start from 1 (0 is reserved for loop param) + + // For each condition variable, allocate JoinIR-local ValueId and build binding + for var_name in &condition_var_names { + let host_id = variable_map + .get(var_name) + .copied() + .ok_or_else(|| { + format!( + "Condition variable '{}' not found in variable_map. \ + Loop condition references undefined variable.", + var_name + ) + })?; + + let join_id = ValueId(join_value_counter); + join_value_counter += 1; + + env.insert(var_name.clone(), join_id); + bindings.push(ConditionBinding { + name: var_name.clone(), + host_value: host_id, + join_value: join_id, + }); + } + + Ok((env, bindings)) + } + + /// 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) + 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 + } +} + +#[cfg(test)] +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) + let condition = ASTNode::BinaryOp { + operator: crate::ast::BinaryOperator::Less, + left: Box::new(ASTNode::Variable { + name: "i".to_string(), + span: Span::unknown(), + }), + right: Box::new(ASTNode::Literal { + value: LiteralValue::Integer(10), + span: Span::unknown(), + }), + span: Span::unknown(), + }; + + let mut variable_map = BTreeMap::new(); + variable_map.insert("i".to_string(), ValueId(100)); + + let (env, bindings) = ConditionEnvBuilder::build_for_break_condition( + &condition, + "i", + &variable_map, + ValueId(100), + ) + .unwrap(); + + // Should have loop parameter in env + assert!(env.get("i").is_some()); + + // Should have no condition-only bindings + assert_eq!(bindings.len(), 0); + } + + #[test] + fn test_build_for_break_condition_with_extra_var() { + // Condition: i < max (uses loop parameter + extra variable) + let condition = ASTNode::BinaryOp { + operator: crate::ast::BinaryOperator::Less, + left: Box::new(ASTNode::Variable { + name: "i".to_string(), + span: Span::unknown(), + }), + right: Box::new(ASTNode::Variable { + name: "max".to_string(), + span: Span::unknown(), + }), + span: Span::unknown(), + }; + + let mut variable_map = BTreeMap::new(); + variable_map.insert("i".to_string(), ValueId(100)); + variable_map.insert("max".to_string(), ValueId(200)); + + let (env, bindings) = ConditionEnvBuilder::build_for_break_condition( + &condition, + "i", + &variable_map, + ValueId(100), + ) + .unwrap(); + + // Should have loop parameter in env + assert!(env.get("i").is_some()); + + // Should have "max" in env with JoinIR-local ValueId + assert!(env.get("max").is_some()); + + // Should have one condition-only binding for "max" + assert_eq!(bindings.len(), 1); + assert_eq!(bindings[0].name, "max"); + assert_eq!(bindings[0].host_value, ValueId(200)); + assert!(bindings[0].join_value.0 > 0); // JoinIR-local ID + } + + #[test] + fn test_build_for_break_condition_undefined_variable() { + // Condition: i < undefined_var + let condition = ASTNode::BinaryOp { + operator: crate::ast::BinaryOperator::Less, + left: Box::new(ASTNode::Variable { + name: "i".to_string(), + span: Span::unknown(), + }), + right: Box::new(ASTNode::Variable { + name: "undefined_var".to_string(), + span: Span::unknown(), + }), + span: Span::unknown(), + }; + + let mut variable_map = BTreeMap::new(); + variable_map.insert("i".to_string(), ValueId(100)); + // "undefined_var" is NOT in variable_map + + let result = ConditionEnvBuilder::build_for_break_condition( + &condition, + "i", + &variable_map, + ValueId(100), + ); + + // Should return error + assert!(result.is_err()); + assert!(result + .unwrap_err() + .contains("undefined_var")); + } +} diff --git a/src/mir/builder/control_flow/joinir/patterns/mod.rs b/src/mir/builder/control_flow/joinir/patterns/mod.rs index 0de6b8fe..0f853662 100644 --- a/src/mir/builder/control_flow/joinir/patterns/mod.rs +++ b/src/mir/builder/control_flow/joinir/patterns/mod.rs @@ -26,9 +26,11 @@ //! //! Phase 171-172: Refactoring Infrastructure //! - loop_scope_shape_builder.rs: Unified LoopScopeShape initialization (Issue 4) +//! - condition_env_builder.rs: Unified ConditionEnv construction (Issue 5) pub(in crate::mir::builder) mod ast_feature_extractor; 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 loop_scope_shape_builder; diff --git a/src/mir/builder/control_flow/joinir/patterns/pattern2_with_break.rs b/src/mir/builder/control_flow/joinir/patterns/pattern2_with_break.rs index d11a5517..2684fcd5 100644 --- a/src/mir/builder/control_flow/joinir/patterns/pattern2_with_break.rs +++ b/src/mir/builder/control_flow/joinir/patterns/pattern2_with_break.rs @@ -160,51 +160,26 @@ impl MirBuilder { // Phase 195: Use unified trace trace::trace().varmap("pattern2_start", &self.variable_map); - // Phase 171-fix: Build ConditionEnv and ConditionBindings - use crate::mir::join_ir::lowering::condition_to_joinir::{ - extract_condition_variables, ConditionEnv, ConditionBinding, - }; + // Phase 171-172: Use ConditionEnvBuilder for unified construction (Issue 5) + use super::condition_env_builder::ConditionEnvBuilder; + use crate::mir::join_ir::lowering::condition_env::ConditionBinding; + let (mut env, mut condition_bindings) = ConditionEnvBuilder::build_for_break_condition( + condition, + &loop_var_name, + &self.variable_map, + loop_var_id, + )?; - let condition_var_names = extract_condition_variables(condition, &[loop_var_name.clone()]); - let mut env = ConditionEnv::new(); - let mut condition_bindings = Vec::new(); - - // Phase 171-fix: Add loop parameter to env (ValueId(0) in JoinIR space) - // The loop parameter is NOT a condition binding (it's a join_input instead) - env.insert(loop_var_name.clone(), crate::mir::ValueId(0)); - - // Create a local allocator for JoinIR-local ValueIds for condition-only variables - let mut join_value_counter = 1u32; // Start from 1 (0 is reserved for loop param) + // Create allocator for additional JoinIR-local ValueIds (needed for Trim pattern) + let mut join_value_counter = env.len() as u32; let mut alloc_join_value = || { let id = crate::mir::ValueId(join_value_counter); join_value_counter += 1; id }; - // For each condition variable, allocate JoinIR-local ValueId and build binding - for var_name in &condition_var_names { - let host_id = self.variable_map.get(var_name) - .copied() - .ok_or_else(|| { - format!( - "[cf_loop/pattern2] Condition variable '{}' not found in variable_map. \ - Loop condition references undefined variable.", - var_name - ) - })?; - - let join_id = alloc_join_value(); // Allocate JoinIR-local ValueId - - env.insert(var_name.clone(), join_id); - condition_bindings.push(ConditionBinding { - name: var_name.clone(), - host_value: host_id, - join_value: join_id, - }); - } - - // Phase 171-fix Debug: Log condition bindings - eprintln!("[cf_loop/pattern2] Phase 171-fix: ConditionEnv contains {} variables:", env.len()); + // Debug: Log condition bindings + eprintln!("[cf_loop/pattern2] Phase 171-172: ConditionEnv contains {} variables:", env.len()); eprintln!(" Loop param '{}' → JoinIR ValueId(0)", loop_var_name); if !condition_bindings.is_empty() { eprintln!(" {} condition-only bindings:", condition_bindings.len());