Files
hakorune/docs/development/current/main/phase242-ex-legacy-lowerer-analysis.md
nyash-codex d4f90976da refactor(joinir): Phase 244 - ConditionLoweringBox trait unification
Unify condition lowering logic across Pattern 2/4 with trait-based API.

New infrastructure:
- condition_lowering_box.rs: ConditionLoweringBox trait + ConditionContext (293 lines)
- ExprLowerer implements ConditionLoweringBox trait (+51 lines)

Pattern migrations:
- Pattern 2 (loop_with_break_minimal.rs): Use trait API
- Pattern 4 (loop_with_continue_minimal.rs): Use trait API

Benefits:
- Unified condition lowering interface
- Extensible for future lowering strategies
- Clean API boundary between patterns and lowering logic
- Zero code duplication

Test results: 911/911 PASS (+2 new tests)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
2025-12-11 02:35:31 +09:00

12 KiB

Phase 242-EX: Pattern3 Legacy Lowerer Analysis

Summary

Recommendation: Option A - Delete Legacy Lowerer

The Pattern3 legacy lowerer (loop_with_if_phi_minimal.rs) contains hardcoded carrier names ("sum", "count") that violate the by-name hardcoding prohibition. However, investigation reveals that the entire lowerer is a Phase 188 PoC hardcoded for one specific test case, making partial fixes insufficient.

Problem Analysis

Hardcoded Carrier Names

File: src/mir/join_ir/lowering/loop_with_if_phi_minimal.rs:423-424

// Phase 213: Build ExitMeta for dynamic exit binding generation
let mut exit_values = vec![];
exit_values.push(("sum".to_string(), sum_final));      // ← hardcoded "sum"
exit_values.push(("count".to_string(), count_final));  // ← hardcoded "count"

Full Scope of Hardcoding

The legacy lowerer is completely hardcoded for loop_if_phi.hako:

  1. Loop condition: i <= 5 (lines 218-243)
  2. If condition: i % 2 == 1 (lines 256-289)
  3. If branches: sum + i vs sum + 0 (lines 291-335)
  4. Counter updates: count + 1 vs count + 0 (lines 301-345)
  5. Carrier names: "sum", "count" (lines 423-424)
  6. Carrier count: Exactly 2 carriers (hardcoded allocation)

Architecture Issue

AST-based Path (Pattern 3):

  • Uses LoopScopeShapeBuilder::empty_body_locals()carriers: BTreeSet::new()
  • CarrierInfo is populated from variable_map
  • But LoopScopeShape.carriers remains empty

LoopForm-based Path:

  • Uses LoopScopeShapeBuilder::from_loopform_intake() → populates carriers from LoopFormIntake
  • This path is for structured loop lowering, not applicable to AST-based MIR building

Result: scope.carriers is empty → Option B (extract from LoopScopeShape) fails.

Option Evaluation

Status: Feasible if if-sum mode can be enhanced.

Current if-sum Mode Rejection Logic

File: src/mir/builder/control_flow/joinir/patterns/pattern_pipeline.rs:194-210

pub fn is_if_sum_pattern(&self) -> bool {
    // (a) Pattern check: must be SimpleComparison
    let pattern = analyze_condition_pattern(condition);
    if pattern != ConditionPattern::SimpleComparison {
        // Complex condition → legacy mode (PoC lowering)
        return false;  // ← Rejects i % 2 == 1
    }
    // ...
}

Reason for Rejection: i % 2 == 1 has BinaryOp in LHS → ConditionPattern::Complex

File: src/mir/join_ir/lowering/condition_pattern.rs:79-125

pub fn analyze_condition_pattern(cond: &ASTNode) -> ConditionPattern {
    match cond {
        ASTNode::BinaryOp { operator, left, right, .. } => {
            // Check LHS/RHS patterns
            let left_is_var = matches!(left.as_ref(), ASTNode::Variable { .. });
            let right_is_literal = matches!(right.as_ref(), ASTNode::Literal { .. });

            if left_is_var && right_is_literal {
                return ConditionPattern::SimpleComparison;  // e.g., i > 0
            }

            // Complex LHS/RHS (e.g., i % 2 == 1) → ConditionPattern::Complex
            ConditionPattern::Complex  // ← i % 2 is BinaryOp, not Variable
        }
        _ => ConditionPattern::Complex,
    }
}

Enhancement Strategy

Approach: Extend if-sum mode to lower complex if conditions dynamically.

  1. Accept complex conditions: Remove SimpleComparison check
  2. Lower condition AST: Use existing AST-based lowering infrastructure
  3. Generate Select instruction: Use condition result to select between branches

Implementation Changes:

  • condition_pattern.rs: Add ConditionPattern::ComplexComparison (still a comparison, but with expressions)
  • loop_with_if_phi_if_sum.rs: Lower complex condition expressions using lower_expression()
  • pattern_pipeline.rs: Accept ComplexComparison in is_if_sum_pattern()

Benefits:

  • Removes 438-line PoC file
  • Unified lowering path (no legacy/if-sum split)
  • No by-name hardcoding
  • Supports arbitrary if conditions (not just i % 2 == 1)

Risks:

  • Need to handle arbitrary expressions in if condition
  • May require additional ValueId allocation in condition lowering

Option B: Extract from LoopScopeShape NOT VIABLE

Status: Not feasible due to empty carriers.

Problem: LoopScopeShape.carriers is empty in AST-based path.

Why it fails:

// pattern_pipeline.rs:283-291
let loop_scope = match variant {
    PatternVariant::Pattern1 | PatternVariant::Pattern3 => {
        LoopScopeShapeBuilder::empty_body_locals(
            BasicBlockId(0), BasicBlockId(0), BasicBlockId(0), BasicBlockId(0),
            BTreeSet::new(),  // ← Empty pinned set
        )  // ← Returns LoopScopeShape { carriers: BTreeSet::new(), .. }
    }
    // ...
}

Alternative: Extract from ctx.carrier_info.carriers instead.

Implementation:

// pattern3_with_if_phi.rs:262 (in lower_pattern3_legacy)
let carrier_names: Vec<String> = ctx.carrier_info.carriers
    .iter()
    .map(|c| c.name.clone())
    .collect();

// Pass to legacy lowerer
let (join_module, fragment_meta) = lower_loop_with_if_phi_pattern(
    carrier_names,  // New parameter
    &mut join_value_space
)?;

// loop_with_if_phi_minimal.rs:421-424
let mut exit_values = vec![];
for (i, carrier_name) in carrier_names.iter().enumerate() {
    let value_id = match i {
        0 => sum_final,
        1 => count_final,
        _ => return Err(format!("Legacy lowerer only supports 2 carriers")),
    };
    exit_values.push((carrier_name.clone(), value_id));
}

Why still not recommended:

  • Only fixes carrier names, not loop/if conditions
  • Still a PoC implementation (hardcoded logic remains)
  • Better to invest in Option A (proper generalization)

Status: Same issues as Option B.

Implementation: Change legacy lowerer signature to accept CarrierInfo.

Why not recommended:

  • Same fundamental issue: only fixes names, not logic
  • Requires signature changes across multiple files
  • Still doesn't generalize the PoC lowering

Test Impact Analysis

Tests Using Legacy Lowerer

Only 1 test: apps/tests/loop_if_phi.hako

static box Main {
  main(args) {
    local console = new ConsoleBox()
    local i = 1
    local sum = 0
    loop(i <= 5) {
      if (i % 2 == 1) { sum = sum + i } else { sum = sum + 0 }
      i = i + 1
    }
    console.println("sum=" + sum)
    return 0
  }
}

Why it uses legacy mode: i % 2 == 1 is a complex condition (BinaryOp in LHS).

Other Pattern 3 tests: None found using rg --type rust 'loop_if_phi\.hako'.

Tests Using If-Sum Mode

Tests using Pattern 3 if-sum mode: Minimal (mostly phase212_if_sum_min.hako).

Pattern 3 detection: File src/mir/loop_pattern_detection/loop_patterns.rs

pub fn detect_pattern3_if_phi(body: &[ASTNode]) -> bool {
    // Has if-else with PHI, no break/continue
    has_if_else_phi(body) && !has_break_continue(body)
}

Implementation Plan

Phase 242-EX-A1: Extend Condition Pattern Analysis

File: src/mir/join_ir/lowering/condition_pattern.rs

  1. Add ConditionPattern::ComplexComparison:

    pub enum ConditionPattern {
        SimpleComparison,      // var CmpOp literal (e.g., i > 0)
        ComplexComparison,     // expr CmpOp expr (e.g., i % 2 == 1)
        Complex,               // Non-comparison (e.g., a && b)
    }
    
  2. Update analyze_condition_pattern():

    pub fn analyze_condition_pattern(cond: &ASTNode) -> ConditionPattern {
        match cond {
            ASTNode::BinaryOp { operator, .. } => {
                let is_comparison = matches!(operator, /* ... */);
                if !is_comparison {
                    return ConditionPattern::Complex;
                }
                // Complex comparison (expr CmpOp expr)
                ConditionPattern::ComplexComparison
            }
            _ => ConditionPattern::Complex,
        }
    }
    

Phase 242-EX-A2: Accept Complex Comparisons in If-Sum Mode

File: src/mir/builder/control_flow/joinir/patterns/pattern_pipeline.rs

pub fn is_if_sum_pattern(&self) -> bool {
    // ...
    let pattern = analyze_condition_pattern(condition);
    if pattern == ConditionPattern::Complex {
        // Non-comparison (e.g., a && b) → legacy mode
        return false;
    }
    // Accept both SimpleComparison and ComplexComparison
    // ...
}

Phase 242-EX-A3: Lower Complex Conditions in If-Sum Lowerer

File: src/mir/join_ir/lowering/loop_with_if_phi_if_sum.rs

Current: Only lowers simple comparisons via ConditionEnv.

Enhanced: Lower arbitrary condition expressions:

// For complex conditions (e.g., i % 2 == 1):
// 1. Lower LHS expression (i % 2)
let lhs_vid = lower_expression_to_joinir(if_condition.lhs, env, body)?;
// 2. Lower RHS expression (1)
let rhs_vid = lower_expression_to_joinir(if_condition.rhs, env, body)?;
// 3. Generate Compare instruction
let cond_vid = alloc_value();
body.push(JoinInst::Compute(MirLikeInst::Compare {
    dst: cond_vid,
    op: map_compare_op(if_condition.operator),
    lhs: lhs_vid,
    rhs: rhs_vid,
}));

Infrastructure needed:

  • lower_expression_to_joinir() helper (similar to lower_comparison())
  • Handle BinaryOp, Variable, Literal recursively
  • Allocate temporary ValueIds for intermediate results

Phase 242-EX-A4: Delete Legacy Lowerer

Files to delete:

  • src/mir/join_ir/lowering/loop_with_if_phi_minimal.rs (entire file, 438 lines)

Files to modify:

  • src/mir/builder/control_flow/joinir/patterns/pattern3_with_if_phi.rs:
    • Remove lower_pattern3_legacy() function
    • Remove dual-mode dispatch in cf_loop_pattern3_with_if_phi()
    • Always use if-sum mode
  • src/mir/builder/control_flow/joinir/patterns/mod.rs:
    • Remove pub mod loop_with_if_phi_minimal;

Phase 242-EX-A5: Verify Tests

# Build
cargo build --release

# Test original case
./target/release/hakorune apps/tests/loop_if_phi.hako
# Expected output: sum=9

# Run full test suite
cargo test --release
# Expected: 909 tests pass

Success Criteria

  1. cargo build --release success
  2. All 909 tests pass
  3. loop_if_phi.hako runs correctly (output: sum=9)
  4. No hardcoded "sum"/"count" references
  5. Legacy lowerer deleted (438 lines removed)
  6. If-sum mode handles complex conditions

Timeline Estimate

  • Phase 242-EX-A1: 30 minutes (condition pattern analysis)
  • Phase 242-EX-A2: 15 minutes (pattern acceptance)
  • Phase 242-EX-A3: 2-3 hours (complex condition lowering)
  • Phase 242-EX-A4: 15 minutes (deletion)
  • Phase 242-EX-A5: 30 minutes (testing)

Total: 3.5-4.5 hours

References

  • Phase 188: Original legacy lowerer implementation
  • Phase 195: Multi-carrier support (hardcoded "sum", "count")
  • Phase 213: ExitMeta-based exit binding generation
  • Phase 219: AST-based if-sum mode introduction
  • Phase 222: Condition normalization
  • Phase 241: Removed hardcoded 'sum' check from loop body analyzer

Core Legacy Lowerer

  • src/mir/join_ir/lowering/loop_with_if_phi_minimal.rs (438 lines)

Callers

  • src/mir/builder/control_flow/joinir/patterns/pattern3_with_if_phi.rs:248-358 (legacy path)

Condition Analysis

  • src/mir/join_ir/lowering/condition_pattern.rs (pattern detection)
  • src/mir/builder/control_flow/joinir/patterns/pattern_pipeline.rs:194-229 (is_if_sum_pattern)

If-Sum Mode

  • src/mir/join_ir/lowering/loop_with_if_phi_if_sum.rs (AST-based lowerer)

Tests

  • apps/tests/loop_if_phi.hako (only test using legacy mode) Status: Historical
    Scope: 旧 lowerer 分析メモ(現役 ExprLowerer ラインとは別枠)