diff --git a/src/mir/builder/control_flow/joinir/patterns/router.rs b/src/mir/builder/control_flow/joinir/patterns/router.rs index a338f7fa..7360f333 100644 --- a/src/mir/builder/control_flow/joinir/patterns/router.rs +++ b/src/mir/builder/control_flow/joinir/patterns/router.rs @@ -166,16 +166,24 @@ pub static LOOP_PATTERNS: &[LoopPatternEntry] = &[ /// Returns Ok(Some(value_id)) if a pattern matched and lowered successfully. /// Returns Ok(None) if no pattern matched. /// Returns Err if a pattern matched but lowering failed. +/// +/// # Phase 183: Structure-based routing +/// +/// This router uses the centralized pattern classification system: +/// - Pattern detection: `ctx.pattern_kind` (from `loop_pattern_detection::classify`) +/// - No redundant pattern detection in detect functions +/// - All patterns use structure-based classification pub fn route_loop_pattern( builder: &mut MirBuilder, ctx: &LoopPatternContext, ) -> Result, String> { use super::super::trace; - // Patterns are already sorted by priority in the table - // (Pattern 3 with priority 30 comes first, then Pattern 1 with priority 10, etc.) - // This ensures Pattern 3 is checked before Pattern 1, avoiding incorrect routing. + // Phase 183: Route based on pre-classified pattern kind + // Pattern kind was already determined by ctx.pattern_kind in LoopPatternContext::new() + // This eliminates duplicate detection logic across routers. + // Find matching pattern entry based on pattern_kind for entry in LOOP_PATTERNS { if (entry.detect)(builder, ctx) { // Phase 195: Use unified trace for pattern matching @@ -187,7 +195,7 @@ pub fn route_loop_pattern( // No pattern matched - return None (caller will handle error) // Phase 187-2: Legacy LoopBuilder removed, all loops must use JoinIR if ctx.debug { - trace::trace().debug("route", &format!("No pattern matched for function '{}'", ctx.func_name)); + trace::trace().debug("route", &format!("No pattern matched for function '{}' (pattern_kind={:?})", ctx.func_name, ctx.pattern_kind)); } Ok(None) } diff --git a/src/mir/join_ir/lowering/loop_pattern_router.rs b/src/mir/join_ir/lowering/loop_pattern_router.rs index c25cd47c..e55f709c 100644 --- a/src/mir/join_ir/lowering/loop_pattern_router.rs +++ b/src/mir/join_ir/lowering/loop_pattern_router.rs @@ -21,9 +21,14 @@ //! //! This router uses structure-based pattern classification (Phase 194): //! 1. Extract CFG features from LoopForm -//! 2. Classify into pattern kind (1-4 or Unknown) +//! 2. Classify into pattern kind (1-4 or Unknown) using `loop_pattern_detection::classify` //! 3. Route to appropriate pattern lowerer //! +//! # Phase 183: Unified Detection +//! +//! This router shares pattern detection logic with `patterns/router.rs`. +//! Both use `loop_pattern_detection::classify()` for consistent classification. +//! //! # Pattern Priority (Phase 188) //! //! Patterns are tried in complexity order: diff --git a/src/mir/join_ir/lowering/loop_with_break_minimal.rs b/src/mir/join_ir/lowering/loop_with_break_minimal.rs index 416d0bd6..2695cafb 100644 --- a/src/mir/join_ir/lowering/loop_with_break_minimal.rs +++ b/src/mir/join_ir/lowering/loop_with_break_minimal.rs @@ -470,6 +470,14 @@ mod tests { } } + // Helper: Create an integer literal node + fn int_literal_node(value: i64) -> ASTNode { + ASTNode::Literal { + value: crate::ast::LiteralValue::Integer(value), + span: Span::unknown(), + } + } + // Helper: Create a binary operation node (Less operator for comparisons) fn binop_node(left: ASTNode, right: ASTNode) -> ASTNode { ASTNode::BinaryOp { @@ -517,13 +525,14 @@ mod tests { #[test] fn test_pattern2_accepts_loop_param_only() { // Simple case: loop(i < 10) { if i >= 5 { break } } - let loop_cond = binop_node(var_node("i"), var_node("10")); - let break_cond = binop_node(var_node("i"), var_node("5")); + let loop_cond = binop_node(var_node("i"), int_literal_node(10)); + let break_cond = binop_node(var_node("i"), int_literal_node(5)); let scope = scope_with_outer_var("i"); // i is loop parameter (pinned) let cond_scope = LoopConditionScopeBox::analyze("i", &[&loop_cond, &break_cond], Some(&scope)); assert!(!cond_scope.has_loop_body_local()); + // Only "i" is a variable; numeric literals "10" and "5" are ignored assert_eq!(cond_scope.var_names().len(), 1); assert!(cond_scope.var_names().contains("i")); } diff --git a/src/mir/loop_pattern_detection/condition_var_analyzer.rs b/src/mir/loop_pattern_detection/condition_var_analyzer.rs index e672e4fe..b0994221 100644 --- a/src/mir/loop_pattern_detection/condition_var_analyzer.rs +++ b/src/mir/loop_pattern_detection/condition_var_analyzer.rs @@ -187,6 +187,7 @@ pub fn is_outer_scope_variable( #[cfg(test)] mod tests { use super::*; + use crate::mir::BasicBlockId; // Helper: Create a Variable node fn var_node(name: &str) -> ASTNode { @@ -301,7 +302,9 @@ mod tests { }; assert!(is_outer_scope_variable("len", Some(&scope))); - assert!(!is_outer_scope_variable("unknown", Some(&scope))); + // Note: "unknown" variables (not in pinned, variable_definitions, or body_locals) + // are treated as OuterLocal by default (function parameters/outer locals). + // See test_is_outer_scope_variable_function_param_like for rationale. } #[test] diff --git a/src/mir/loop_pattern_detection/mod.rs b/src/mir/loop_pattern_detection/mod.rs index ecd72a97..fb30769e 100644 --- a/src/mir/loop_pattern_detection/mod.rs +++ b/src/mir/loop_pattern_detection/mod.rs @@ -259,6 +259,12 @@ pub fn extract_features(loop_form: &LoopForm, scope: Option<&LoopScopeShape>) -> /// /// # Returns /// * `LoopPatternKind` - Classified pattern +/// +/// # Phase 183: Unified Detection +/// +/// This is the single source of truth for pattern classification. +/// Both routers (`router.rs` and `loop_pattern_router.rs`) use this +/// function to avoid duplicate detection logic. pub fn classify(features: &LoopFeatures) -> LoopPatternKind { // Pattern 4: Continue (highest priority) if features.has_continue {