diff --git a/src/mir/builder/control_flow/joinir/patterns/loop_scope_shape_builder.rs b/src/mir/builder/control_flow/joinir/patterns/loop_scope_shape_builder.rs new file mode 100644 index 00000000..fad37b2c --- /dev/null +++ b/src/mir/builder/control_flow/joinir/patterns/loop_scope_shape_builder.rs @@ -0,0 +1,211 @@ +//! LoopScopeShapeBuilder - Unified LoopScopeShape initialization +//! +//! Phase 171-172: Issue 4 +//! +//! Provides unified construction methods for LoopScopeShape across all 4 patterns. +//! This eliminates the 50-60 lines of duplicated initialization code in each pattern. +//! +//! # Responsibility +//! +//! - Provide factory methods for creating LoopScopeShape with common configurations +//! - Extract body_locals from loop body AST when needed +//! - Maintain consistent initialization defaults across patterns +//! +//! # Usage +//! +//! ```rust +//! // Pattern 1, 3: Empty body_locals (condition-only analysis) +//! let scope = LoopScopeShapeBuilder::empty_body_locals( +//! header, body, latch, exit, pinned +//! ); +//! +//! // Pattern 2, 4: Extract body_locals from AST +//! let scope = LoopScopeShapeBuilder::with_body_locals( +//! header, body, latch, exit, pinned, loop_body +//! ); +//! ``` + +use crate::ast::ASTNode; +use crate::mir::join_ir::lowering::loop_scope_shape::LoopScopeShape; +use crate::mir::BasicBlockId; +use std::collections::{BTreeMap, BTreeSet}; + +pub struct LoopScopeShapeBuilder; + +impl LoopScopeShapeBuilder { + /// Create LoopScopeShape with empty body_locals + /// + /// Used by Pattern 1 and Pattern 3 which don't require body-local variable analysis. + /// + /// # Arguments + /// + /// * `header` - Header block ID + /// * `body` - Body block ID + /// * `latch` - Latch block ID + /// * `exit` - Exit block ID + /// * `pinned` - Pinned variables (typically empty for current patterns) + pub fn empty_body_locals( + header: BasicBlockId, + body: BasicBlockId, + latch: BasicBlockId, + exit: BasicBlockId, + pinned: BTreeSet, + ) -> LoopScopeShape { + LoopScopeShape { + header, + body, + latch, + exit, + pinned, + carriers: BTreeSet::new(), + body_locals: BTreeSet::new(), + exit_live: BTreeSet::new(), + progress_carrier: None, + variable_definitions: BTreeMap::new(), + } + } + + /// Create LoopScopeShape with body_locals extracted from AST + /// + /// Used by Pattern 2 and Pattern 4 which require body-local variable classification. + /// This is critical for Trim pattern support and carrier promotion analysis. + /// + /// # Arguments + /// + /// * `header` - Header block ID + /// * `body` - Body block ID + /// * `latch` - Latch block ID + /// * `exit` - Exit block ID + /// * `pinned` - Pinned variables (typically empty for current patterns) + /// * `loop_body` - AST nodes of the loop body for local variable extraction + pub fn with_body_locals( + header: BasicBlockId, + body: BasicBlockId, + latch: BasicBlockId, + exit: BasicBlockId, + pinned: BTreeSet, + loop_body: &[ASTNode], + ) -> LoopScopeShape { + let body_locals = Self::extract_body_locals(loop_body); + LoopScopeShape { + header, + body, + latch, + exit, + pinned, + carriers: BTreeSet::new(), + body_locals, + exit_live: BTreeSet::new(), + progress_carrier: None, + variable_definitions: BTreeMap::new(), + } + } + + /// Extract local variables defined in loop body + /// + /// Scans the loop body AST for `local` declarations and collects variable names. + /// This is used for proper variable classification in LoopConditionScopeBox analysis. + /// + /// # Arguments + /// + /// * `body` - AST nodes of the loop body + /// + /// # Returns + /// + /// Set of variable names declared with `local` keyword in the loop body + fn extract_body_locals(body: &[ASTNode]) -> BTreeSet { + let mut locals = BTreeSet::new(); + for stmt in body { + if let ASTNode::Local { variables, .. } = stmt { + for var_name in variables { + locals.insert(var_name.clone()); + } + } + } + locals + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_empty_body_locals() { + let scope = LoopScopeShapeBuilder::empty_body_locals( + BasicBlockId(1), + BasicBlockId(2), + BasicBlockId(3), + BasicBlockId(4), + BTreeSet::new(), + ); + + assert_eq!(scope.header, BasicBlockId(1)); + assert_eq!(scope.body, BasicBlockId(2)); + assert_eq!(scope.latch, BasicBlockId(3)); + assert_eq!(scope.exit, BasicBlockId(4)); + assert!(scope.body_locals.is_empty()); + assert!(scope.carriers.is_empty()); + assert!(scope.pinned.is_empty()); + } + + #[test] + fn test_with_body_locals_extracts_local_variables() { + use crate::ast::Span; + let body = vec![ + ASTNode::Local { + variables: vec!["x".to_string(), "y".to_string()], + initial_values: vec![], + span: Span::unknown(), + }, + ASTNode::Local { + variables: vec!["z".to_string()], + initial_values: vec![], + span: Span::unknown(), + }, + ]; + + let scope = LoopScopeShapeBuilder::with_body_locals( + BasicBlockId(1), + BasicBlockId(2), + BasicBlockId(3), + BasicBlockId(4), + BTreeSet::new(), + &body, + ); + + assert_eq!(scope.body_locals.len(), 3); + assert!(scope.body_locals.contains("x")); + assert!(scope.body_locals.contains("y")); + assert!(scope.body_locals.contains("z")); + } + + #[test] + fn test_extract_body_locals_ignores_non_local_nodes() { + use crate::ast::Span; + let body = vec![ + ASTNode::Local { + variables: vec!["a".to_string()], + initial_values: vec![], + span: Span::unknown(), + }, + // Use a different AST node type that doesn't declare locals + ASTNode::Literal { + value: crate::ast::LiteralValue::Integer(42), + span: Span::unknown(), + }, + ]; + + let scope = LoopScopeShapeBuilder::with_body_locals( + BasicBlockId(0), + BasicBlockId(0), + BasicBlockId(0), + BasicBlockId(0), + BTreeSet::new(), + &body, + ); + + assert_eq!(scope.body_locals.len(), 1); + assert!(scope.body_locals.contains("a")); + } +} diff --git a/src/mir/builder/control_flow/joinir/patterns/mod.rs b/src/mir/builder/control_flow/joinir/patterns/mod.rs index 3929dfb6..0de6b8fe 100644 --- a/src/mir/builder/control_flow/joinir/patterns/mod.rs +++ b/src/mir/builder/control_flow/joinir/patterns/mod.rs @@ -23,11 +23,15 @@ //! Phase 33-22: Common Pattern Infrastructure //! - common_init.rs: CommonPatternInitializer for unified initialization //! - conversion_pipeline.rs: JoinIRConversionPipeline for unified conversion flow +//! +//! Phase 171-172: Refactoring Infrastructure +//! - loop_scope_shape_builder.rs: Unified LoopScopeShape initialization (Issue 4) pub(in crate::mir::builder) mod ast_feature_extractor; pub(in crate::mir::builder) mod common_init; 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; pub(in crate::mir::builder) mod pattern1_minimal; pub(in crate::mir::builder) mod pattern2_with_break; pub(in crate::mir::builder) mod pattern3_with_if_phi; diff --git a/src/mir/builder/control_flow/joinir/patterns/pattern1_minimal.rs b/src/mir/builder/control_flow/joinir/patterns/pattern1_minimal.rs index 5e823d63..ddc9a159 100644 --- a/src/mir/builder/control_flow/joinir/patterns/pattern1_minimal.rs +++ b/src/mir/builder/control_flow/joinir/patterns/pattern1_minimal.rs @@ -70,21 +70,16 @@ impl MirBuilder { // Phase 195: Use unified trace trace::trace().varmap("pattern1_start", &self.variable_map); - // Create a minimal LoopScopeShape (Phase 188: hardcoded for loop_min_while.hako) + // Phase 171-172: Use LoopScopeShapeBuilder for unified initialization (Issue 4) // Pattern 1 lowerer ignores the scope anyway, so this is just a placeholder - use crate::mir::join_ir::lowering::loop_scope_shape::LoopScopeShape; - let scope = LoopScopeShape { - header: BasicBlockId(0), - body: BasicBlockId(0), - latch: BasicBlockId(0), - exit: BasicBlockId(0), - pinned: BTreeSet::new(), - carriers: BTreeSet::new(), - body_locals: BTreeSet::new(), - exit_live: BTreeSet::new(), - progress_carrier: None, - variable_definitions: BTreeMap::new(), - }; + use super::loop_scope_shape_builder::LoopScopeShapeBuilder; + let scope = LoopScopeShapeBuilder::empty_body_locals( + BasicBlockId(0), + BasicBlockId(0), + BasicBlockId(0), + BasicBlockId(0), + BTreeSet::new(), + ); // Call Pattern 1 lowerer let join_module = match lower_simple_while_minimal(scope) { 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 41ed582d..d11a5517 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 @@ -215,32 +215,17 @@ impl MirBuilder { eprintln!(" No condition-only variables"); } - // Create a minimal LoopScopeShape (Phase 188: hardcoded for joinir_min_loop.hako) - // Phase 171-impl-Trim: Extract body_locals from loop body AST for proper variable classification - use crate::mir::join_ir::lowering::loop_scope_shape::LoopScopeShape; - let mut body_locals = BTreeSet::new(); - - // Extract local variable declarations from loop body - for stmt in _body { - if let ASTNode::Local { variables, .. } = stmt { - for var_name in variables { - body_locals.insert(var_name.clone()); - } - } - } - - let scope = LoopScopeShape { - header: BasicBlockId(0), - body: BasicBlockId(0), - latch: BasicBlockId(0), - exit: BasicBlockId(0), - pinned: BTreeSet::new(), - carriers: BTreeSet::new(), - body_locals, // Phase 171-impl-Trim: Now populated from AST - exit_live: BTreeSet::new(), - progress_carrier: None, - variable_definitions: BTreeMap::new(), - }; + // Phase 171-172: Use LoopScopeShapeBuilder for unified initialization (Issue 4) + // Extract body_locals from loop body AST for proper variable classification (Trim support) + use super::loop_scope_shape_builder::LoopScopeShapeBuilder; + let scope = LoopScopeShapeBuilder::with_body_locals( + BasicBlockId(0), + BasicBlockId(0), + BasicBlockId(0), + BasicBlockId(0), + BTreeSet::new(), + _body, + ); // Phase 170-B: Extract break condition from loop body use super::ast_feature_extractor; diff --git a/src/mir/builder/control_flow/joinir/patterns/pattern3_with_if_phi.rs b/src/mir/builder/control_flow/joinir/patterns/pattern3_with_if_phi.rs index ea67d61a..d57cde8c 100644 --- a/src/mir/builder/control_flow/joinir/patterns/pattern3_with_if_phi.rs +++ b/src/mir/builder/control_flow/joinir/patterns/pattern3_with_if_phi.rs @@ -78,21 +78,16 @@ impl MirBuilder { // Phase 195: Use unified trace trace::trace().varmap("pattern3_start", &self.variable_map); - // Create a minimal LoopScopeShape (Phase 188: hardcoded for loop_if_phi.hako) + // Phase 171-172: Use LoopScopeShapeBuilder for unified initialization (Issue 4) // Pattern 3 lowerer ignores the scope anyway, so this is just a placeholder - use crate::mir::join_ir::lowering::loop_scope_shape::LoopScopeShape; - let scope = LoopScopeShape { - header: BasicBlockId(0), - body: BasicBlockId(0), - latch: BasicBlockId(0), - exit: BasicBlockId(0), - pinned: BTreeSet::new(), - carriers: BTreeSet::new(), - body_locals: BTreeSet::new(), - exit_live: BTreeSet::new(), - progress_carrier: None, - variable_definitions: BTreeMap::new(), - }; + use super::loop_scope_shape_builder::LoopScopeShapeBuilder; + let scope = LoopScopeShapeBuilder::empty_body_locals( + BasicBlockId(0), + BasicBlockId(0), + BasicBlockId(0), + BasicBlockId(0), + BTreeSet::new(), + ); // Call Pattern 3 lowerer let join_module = match lower_loop_with_if_phi_pattern(scope) { diff --git a/src/mir/builder/control_flow/joinir/patterns/pattern4_with_continue.rs b/src/mir/builder/control_flow/joinir/patterns/pattern4_with_continue.rs index 94f1cdfb..181acd37 100644 --- a/src/mir/builder/control_flow/joinir/patterns/pattern4_with_continue.rs +++ b/src/mir/builder/control_flow/joinir/patterns/pattern4_with_continue.rs @@ -196,32 +196,17 @@ impl MirBuilder { // Phase 195: Use unified trace trace::trace().varmap("pattern4_start", &self.variable_map); - // Create a minimal LoopScopeShape (Phase 195: hardcoded for loop_continue_pattern4.hako) - // Phase 171-impl-Trim: Extract body_locals from loop body AST for proper variable classification - use crate::mir::join_ir::lowering::loop_scope_shape::LoopScopeShape; - let mut body_locals = BTreeSet::new(); - - // Extract local variable declarations from loop body - for stmt in body_to_analyze { - if let ASTNode::Local { variables, .. } = stmt { - for var_name in variables { - body_locals.insert(var_name.clone()); - } - } - } - - let scope = LoopScopeShape { - header: BasicBlockId(0), - body: BasicBlockId(0), - latch: BasicBlockId(0), - exit: BasicBlockId(0), - pinned: BTreeSet::new(), - carriers: BTreeSet::new(), - body_locals, // Phase 171-impl-Trim: Now populated from AST - exit_live: BTreeSet::new(), - progress_carrier: None, - variable_definitions: BTreeMap::new(), - }; + // Phase 171-172: Use LoopScopeShapeBuilder for unified initialization (Issue 4) + // Extract body_locals from loop body AST for proper variable classification + use super::loop_scope_shape_builder::LoopScopeShapeBuilder; + let scope = LoopScopeShapeBuilder::with_body_locals( + BasicBlockId(0), + BasicBlockId(0), + BasicBlockId(0), + BasicBlockId(0), + BTreeSet::new(), + body_to_analyze, + ); // Phase 171-C-3: LoopBodyCarrierPromoter integration // Check if LoopConditionScopeBox detects LoopBodyLocal variables, and attempt promotion