hakorune/docs/development/current/main/phase223-228-refactoring-opportunities.md

Phase 223-228 リファクタリング機会調査レポート

🎯 調査の目的

Phase 223-228 の実装を通じて、以下の観点でリファクタリング機会を探す：

1. 削除可能なレガシーコード - 新しい設計に置き換わったもの
2. 重複コード - 同じ処理が複数箇所に存在するもの
3. 未使用の型/関数 - 削除して構造を簡潔に
4. 箱化のチャンス - 分散している処理を1箇所に集約

📊 調査結果サマリー

✅ 発見した重複コード

カテゴリ	重複箇所	行数	改善優先度
パターン検出	Trim/DigitPos Promoter	1371行	中
ConditionOnly フィルタ	ExitLine 3ファイル	540行	高
ConditionAlias 冗長性	CarrierInfo 連携	-	高
MethodCall Lowering	既に統一化済み	-	✅完了

🎯 重要発見

1. ConditionAlias は必要か？ - CarrierVar.role と promoted_loopbodylocals で代替可能
2. ConditionOnly フィルタ分散 - 4ファイルに同じロジックが存在
3. MethodCallLowerer 成功例 - Phase 224で既に Box化済み（参考にすべき）

---

🔍 詳細調査

A. パターン検出の重複 (優先度: 中)

現状

2つの独立した Promoter:

• loop_body_carrier_promoter.rs (658行) - Trim パターン
• loop_body_digitpos_promoter.rs (713行) - DigitPos パターン

重複している処理:

1. メソッド検出ロジック

   // Trim promoter (line 279)
   fn is_substring_method_call(node: &ASTNode) -> bool {
       matches!(node, ASTNode::MethodCall { method, .. } if method == "substring")
   }
   
   // DigitPos promoter (line 433)
   fn is_substring_method_call(node: &ASTNode) -> bool {
       matches!(node, ASTNode::MethodCall { method, .. } if method == "substring")
   }
   

   → 完全に同じ関数が2箇所に存在！

2. 条件変数抽出ロジック

   • Trim: extract_equality_literals() (line 301)
   • DigitPos: extract_comparison_var() (line 317)
   • 両方とも AST を worklist で走査する同じパターン

3. CarrierInfo 構築

   • 両方とも carrier_info.carriers.push() と condition_aliases.push() を呼ぶ

改善案

Option 1: 共通トレイト抽出 (中リスク)

/// 共通のパターン検出トレイト
pub trait LoopBodyLocalPattern {
    /// パターン名（デバッグ用）
    fn pattern_name() -> &'static str;

    /// 初期化式が許可されるメソッド名
    fn allowed_init_methods() -> &'static [&'static str];

    /// 条件式が許可される比較パターン
    fn allowed_condition_patterns() -> &'static [ConditionPattern];

    /// パターン検出
    fn detect(
        cond_scope: &LoopConditionScope,
        loop_body: &[ASTNode],
        break_cond: Option<&ASTNode>,
    ) -> Option<PromotionCandidate>;
}

impl LoopBodyLocalPattern for TrimPattern { ... }
impl LoopBodyLocalPattern for DigitPosPattern { ... }

メリット:

• 共通ロジックを1箇所に集約
• 新しいパターン追加が容易
• テスト容易性向上

デメリット:

• 既存コードへの影響大（2ファイル全体のリファクタリング）
• 回帰テストが必須
• 実装難度: 中〜高

推奨度: ⭐⭐⭐ (中) - 新しいパターン追加時に検討

---

B. ConditionAlias の必要性検証 (優先度: 高)

現状

ConditionAlias の目的:

pub struct ConditionAlias {
    pub old_name: String,           // "digit_pos"
    pub carrier_name: String,       // "is_digit_pos"
}

条件式で digit_pos を参照した時に is_digit_pos に変換する。

使用箇所:

• 定義: carrier_info.rs (line 96)
• 追加:
  • loop_body_digitpos_promoter.rs (line 203)
  • loop_body_carrier_promoter.rs (line 87)
• 使用: pattern2_with_break.rs (line 356-379)

使用パターン:

// pattern2_with_break.rs (line 356)
for alias in &carrier_info.condition_aliases {
    if let Some(carrier) = carriers_with_join_ids.iter().find(|c| c.name == alias.carrier_name) {
        if let Some(join_id) = carrier.join_id {
            env.insert(alias.old_name.clone(), join_id);
        }
    }
}

問題点

既に promoted_loopbodylocals がある！

pub struct CarrierInfo {
    pub promoted_loopbodylocals: Vec<String>, // ["digit_pos"]
    pub carriers: Vec<CarrierVar>,            // [CarrierVar{name: "is_digit_pos", ...}]
    pub condition_aliases: Vec<ConditionAlias>, // [ConditionAlias{old: "digit_pos", carrier: "is_digit_pos"}]
}

冗長性の分析:

1. promoted_loopbodylocals に "digit_pos" が記録されている
2. carriers に "is_digit_pos" が記録されている（role 付き）
3. condition_aliases で両者をマッピング

→ condition_aliases は promoted_loopbodylocals と carriers の情報から導出可能！

改善案

Option 1: ConditionAlias 削除 (低リスク、高リターン)

// ConditionAlias を削除して、必要な時に動的に解決
fn resolve_promoted_variable(
    var_name: &str,
    carrier_info: &CarrierInfo,
) -> Option<String> {
    // promoted_loopbodylocals に含まれていたら carrier 名を探す
    if carrier_info.promoted_loopbodylocals.contains(&var_name.to_string()) {
        // 命名規則: "digit_pos" → "is_digit_pos"
        // または carriers を検索して一致するものを探す
        let promoted_name = format!("is_{}", var_name);
        if carrier_info.carriers.iter().any(|c| c.name == promoted_name) {
            return Some(promoted_name);
        }
    }
    None
}

メリット:

• データ構造の簡素化 - CarrierInfo のフィールドが1つ減る
• 保守コスト削減 - 3箇所でデータ整合性を保つ必要がなくなる
• confusion 削減 - 「何のためのフィールドか」が明確になる

デメリット:

• 命名規則の依存（"is_" prefix）が必要
• または carriers を線形探索（パフォーマンス無視可能）

影響範囲:

• carrier_info.rs - ConditionAlias 型削除 (line 96)
• pattern2_with_break.rs - resolve 関数に置き換え (line 356)
• 2つの promoter - condition_aliases.push() 削除

推奨度: ⭐⭐⭐⭐⭐ (高) - 今すぐ実施推奨

実装難度: 低 (削除が主、1〜2時間)

---

C. ConditionOnly フィルタの分散 (優先度: 高)

現状

ConditionOnly フィルタが4ファイルに分散:

1. meta_collector.rs (line 131)

   let is_condition_only = if let Some(ci) = carrier_info {
       ci.carriers.iter().any(|c| c.name == *carrier_name && c.role == CarrierRole::ConditionOnly)
   } else {
       false
   };
   

2. reconnector.rs (line 109, 192)

   if binding.role == CarrierRole::ConditionOnly {
       // Skip ConditionOnly carriers (no variable_map update)
       continue;
   }
   

3. instruction_rewriter.rs (line 615)

   if binding.role == crate::mir::join_ir::lowering::carrier_info::CarrierRole::ConditionOnly {
       eprintln!("Skipping ConditionOnly carrier from exit PHI");
       continue;
   }
   

計25箇所の ConditionOnly 参照 (merge/ 全体)

問題点

同じロジックが繰り返される:

• 「ConditionOnly なら exit PHI を skip」
• 「ConditionOnly なら variable_map を更新しない」
• 「ConditionOnly なら exit_bindings に含める（latch 用）が reconnect しない」

→ これらの判断ロジックが4ファイルに散らばっている

改善案

Option 1: ExitLinePolicy Trait (中リスク)

/// Exit line 処理のポリシー箱
pub trait ExitLinePolicy {
    /// exit_bindings に含めるべきか？
    fn should_collect(role: CarrierRole) -> bool {
        true // ConditionOnly も collect（latch incoming 用）
    }

    /// variable_map を更新すべきか？
    fn should_reconnect(role: CarrierRole) -> bool {
        role == CarrierRole::LoopState // LoopState のみ
    }

    /// exit PHI を生成すべきか？
    fn should_create_exit_phi(role: CarrierRole) -> bool {
        role == CarrierRole::LoopState // LoopState のみ
    }
}

/// デフォルト実装
pub struct DefaultExitLinePolicy;
impl ExitLinePolicy for DefaultExitLinePolicy {}

使用例:

// reconnector.rs
if !DefaultExitLinePolicy::should_reconnect(binding.role) {
    continue;
}

// instruction_rewriter.rs
if !DefaultExitLinePolicy::should_create_exit_phi(binding.role) {
    continue;
}

メリット:

• 単一責任 - ロールに基づく判断が1箇所に集約
• テスト容易 - Policy をモックできる
• 拡張性 - 新しいロール追加時に1箇所修正

デメリット:

• 新しい抽象層の追加
• 既存の4ファイルを修正
• 実装難度: 中

Option 2: ExitLineOrchestrator Box (高リスク)

/// ExitLine 処理の統括箱
pub struct ExitLineOrchestrator;

impl ExitLineOrchestrator {
    /// exit_bindings を収集して、reconnect と exit PHI を実行
    pub fn process_exit_line(
        boundary: &JoinInlineBoundary,
        carrier_phis: &BTreeMap<String, ValueId>,
        variable_map: &mut BTreeMap<String, ValueId>,
        blocks: &mut Vec<MirBasicBlock>,
        exit_block_id: BasicBlockId,
    ) -> Result<(), String> {
        // 1. meta_collector の処理
        // 2. reconnector の処理
        // 3. exit PHI builder の処理
        // を統合
    }
}

メリット:

• 最も「箱理論的」
• ExitLine の全処理が1箇所に

デメリット:

• Phase 33 で分離した構造を再統合（後退？）
• 影響範囲が非常に大きい
• 実装難度: 高

推奨度:

• Option 1: ⭐⭐⭐⭐ (高) - Policy Trait を推奨
• Option 2: ⭐⭐ (低) - 統合しすぎ

実装難度:

• Option 1: 中 (2〜3時間)
• Option 2: 高 (1日+)

---

D. MethodCall Lowering 統一化 (優先度: ✅完了)

現状

Phase 224 で既に Box化完了！

• method_call_lowerer.rs - 統一的な MethodCallLowerer Box
• loop_body_local_init.rs - MethodCallLowerer::lower_for_init() を呼ぶ
• condition_lowerer.rs - MethodCallLowerer::lower_for_condition() を呼ぶ

設計の特徴:

• Metadata-Driven - CoreMethodId ベースで判断
• Fail-Fast - whitelist にない method は即エラー
• Context-Aware - for_condition / for_init で異なる whitelist

教訓

これが理想的な Box化！

Before (Phase 223):

// loop_body_local_init.rs
match method {
    "substring" => { /* inline lowering */ }
    "indexOf" => { /* inline lowering */ }
    _ => Err("Unknown method")
}

// condition_lowerer.rs
match method {
    "length" => { /* inline lowering */ }
    _ => Err("Unknown method")
}

After (Phase 224):

// loop_body_local_init.rs
MethodCallLowerer::lower_for_init(recv, method, args, ...)

// condition_lowerer.rs
MethodCallLowerer::lower_for_condition(recv, method, args, ...)

// method_call_lowerer.rs (統一箇所)
impl MethodCallLowerer {
    pub fn lower_for_init(...) -> Result<ValueId, String> {
        let method_id = CoreMethodId::iter().find(|m| m.name() == method_name)?;
        if !method_id.allowed_in_init() {
            return Err("Method not allowed in init");
        }
        // ... 統一的なロジック
    }
}

成果:

• 重複コード削減: 推定 200行+
• 保守性向上: method 追加時に1箇所修正
• テスト容易性: MethodCallLowerer を単独テスト可能

→ A, C の改善案でも同じパターンを採用すべき

---

🎯 推奨アクション

📋 優先順位

項目	優先度	実装難度	リターン	推奨実施時期
B. ConditionAlias 削除	⭐⭐⭐⭐⭐	低	高	Phase 229
C. ExitLinePolicy Trait	⭐⭐⭐⭐	中	高	Phase 230
A. パターン検出共通化	⭐⭐⭐	中	中	Phase 231+

✅ Phase 229 推奨アクション

1. ConditionAlias 削除 (1〜2時間)

Step 1: resolve 関数実装

// carrier_info.rs に追加
impl CarrierInfo {
    /// 昇格された変数の carrier 名を解決
    pub fn resolve_promoted_carrier(&self, old_name: &str) -> Option<&str> {
        if !self.promoted_loopbodylocals.contains(&old_name.to_string()) {
            return None;
        }

        // 命名規則: "digit_pos" → "is_digit_pos"
        let expected_name = format!("is_{}", old_name);
        self.carriers.iter()
            .find(|c| c.name == expected_name)
            .map(|c| c.name.as_str())
    }
}

Step 2: pattern2_with_break.rs 修正

// 削除: for alias in &carrier_info.condition_aliases { ... }

// 追加:
for promoted_var in &carrier_info.promoted_loopbodylocals {
    if let Some(carrier_name) = carrier_info.resolve_promoted_carrier(promoted_var) {
        if let Some(carrier) = carriers_with_join_ids.iter().find(|c| c.name == carrier_name) {
            if let Some(join_id) = carrier.join_id {
                env.insert(promoted_var.clone(), join_id);
            }
        }
    }
}

Step 3: ConditionAlias 型削除

• carrier_info.rs - struct ConditionAlias 削除
• 2つの promoter - condition_aliases.push() 削除

Step 4: テスト

# Trim pattern
cargo test --release test_mir_joinir_funcscanner_trim

# DigitPos pattern
cargo test --release test_loopbodylocal_digitpos

# Pattern 2 integration
cargo test --release test_loop_with_break

期待される成果:

• CarrierInfo のフィールド: 6 → 5
• 保守コスト削減: 3箇所のデータ整合性チェック不要
• confusion 削減: 「condition_aliases は何のため？」問題解消

---

📐 設計ガイドライン

✅ 良い Box化の特徴 (MethodCallLowerer から学ぶ)

1. 単一責任 - 「この Box は X を判断する」が明確
2. Metadata-Driven - ハードコードより設定駆動
3. Fail-Fast - フォールバックより明示的エラー
4. Context-Aware - 用途に応じた API (for_init / for_condition)
5. 独立テスト可能 - 単独で動作検証できる

❌ 避けるべきパターン

1. 情報の重複 - 同じ情報を複数の場所に保存
2. ロジックの分散 - 同じ判断が4箇所に散らばる
3. 型の冗長 - CarrierVar.role があるのに binding.role も持つ
4. 暗黙の依存 - 命名規則に依存しすぎ（"is_" prefix など）

---

🧪 回帰テスト戦略

Phase 229 変更時のテストセット

Level 1: 単体テスト

cargo test --lib carrier_info
cargo test --lib loop_pattern_detection

Level 2: パターンテスト

# Trim pattern
cargo test --release test_mir_joinir_funcscanner_trim

# DigitPos pattern
cargo test --release test_loopbodylocal_digitpos

# Pattern 2 (break)
cargo test --release test_loop_with_break

Level 3: E2E テスト

# Smoke tests
tools/smokes/v2/run.sh --profile quick --filter "loop_*"

# Full MIR test suite
cargo test --release --test '*' 2>&1 | grep -E "(test result|FAILED)"

決定性テスト (重要！):

# 3回実行して ValueId が変わらないことを確認
for i in 1 2 3; do
    echo "=== Run $i ==="
    cargo test --release test_loop_with_break 2>&1 | grep -E "ValueId|test result"
done

---

📝 実装時の注意事項

ConditionAlias 削除時

1. 命名規則の明文化

   • "digit_pos" → "is_digit_pos" の変換ルールをドキュメント化
   • または CarrierVar に original_name: Option<String> フィールド追加

2. エラーメッセージの改善

   if carrier_info.resolve_promoted_carrier(var_name).is_none() {
       return Err(format!(
           "Variable '{}' was promoted but carrier not found. Expected 'is_{}' in carriers.",
           var_name, var_name
       ));
   }
   

3. デバッグログの追加

   eprintln!(
       "[pattern2/resolve] Promoted variable '{}' resolved to carrier '{}'",
       old_name, carrier_name
   );
   

ExitLinePolicy 実装時

1. trait より struct + impl 推奨

   • trait は over-engineering になりがち
   • static メソッドで十分

2. ドキュメント重視

   /// Phase 227: ConditionOnly Filtering Policy
   ///
   /// ## Design Decision
   ///
   /// ConditionOnly carriers are:
   /// - Collected in exit_bindings (for latch incoming)
   /// - NOT reconnected in variable_map (no exit PHI)
   /// - NOT included in exit PHI
   impl ExitLinePolicy {
       pub fn should_reconnect(role: CarrierRole) -> bool { ... }
   }
   

---

📊 期待される改善効果

ConditionAlias 削除 (Phase 229)

• 削減行数: ~50行 (carrier_info.rs, 2 promoters, pattern2)
• 保守コスト削減: CarrierInfo のフィールド管理が簡単に
• confusion 削減: 「なぜ3箇所に情報があるか？」問題解消
• 実装時間: 1〜2時間
• リスク: 低（既存のテストで検証可能）

ExitLinePolicy 実装 (Phase 230)

• 削減行数: ~20行 (重複 if 文の削減)
• 可読性向上: 判断ロジックが1箇所に
• 拡張性: 新しい CarrierRole 追加時に1箇所修正
• 実装時間: 2〜3時間
• リスク: 中（4ファイル修正）

パターン検出共通化 (Phase 231+)

• 削減行数: ~200行+ (重複ロジック統合)
• 拡張性: 新パターン追加が容易
• テスト容易性: 共通部分を単独テスト可能
• 実装時間: 1日+
• リスク: 高（2大ファイルの全面リファクタリング）

---

🎯 まとめ

今すぐやるべきこと (Phase 229)

✅ ConditionAlias 削除 - 低リスク・高リターン・1〜2時間

次にやるべきこと (Phase 230)

⭐ ExitLinePolicy 実装 - 中リスク・高リターン・2〜3時間

後回しでOK (Phase 231+)

💡 パターン検出共通化 - 新パターン追加時に検討

---

📚 参考資料

• Phase 224 MethodCallLowerer: src/mir/join_ir/lowering/method_call_lowerer.rs
• Phase 227 CarrierRole: src/mir/join_ir/lowering/carrier_info.rs
• Phase 33 Exit Line: src/mir/builder/control_flow/joinir/merge/exit_line/

---

調査日: 2025-12-10 調査者: Claude Code Assistant 対象: Phase 223-228 コードベース Status: Active
Scope: Refactoring 機会の整理（JoinIR/ExprLowerer ライン）