From 38e810d0710df308c00c11a3c05d22929f7fc1ec Mon Sep 17 00:00:00 2001 From: nyash-codex Date: Wed, 10 Dec 2025 21:53:27 +0900 Subject: [PATCH] refactor(joinir): Phase 229 - Remove redundant ConditionAlias MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace static alias mapping with dynamic condition variable resolution using: - promoted_loopbodylocals as source of truth - Naming conventions: is_ or is__match - Pattern-aware inference during lowering Benefits: - Simpler data structure (6 fields → 5) - Single source of truth - Self-documenting with explicit naming conventions - Fewer maintenance points Net: -25 lines (60 additions, 85 deletions) Tests: 877/884 PASS (no regressions) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- CURRENT_TASK.md | 13 + .../current/main/REFACTORING_INDEX.md | 234 +++++++ .../phase223-228-refactoring-opportunities.md | 637 ++++++++++++++++++ .../current/main/phase229-action-plan.md | 397 +++++++++++ .../joinir/patterns/pattern2_with_break.rs | 65 +- .../patterns/pattern4_carrier_analyzer.rs | 2 - .../joinir/patterns/pattern_pipeline.rs | 2 - src/mir/join_ir/lowering/carrier_info.rs | 47 +- .../loop_body_carrier_promoter.rs | 8 +- .../loop_body_digitpos_promoter.rs | 10 +- 10 files changed, 1329 insertions(+), 86 deletions(-) create mode 100644 docs/development/current/main/REFACTORING_INDEX.md create mode 100644 docs/development/current/main/phase223-228-refactoring-opportunities.md create mode 100644 docs/development/current/main/phase229-action-plan.md diff --git a/CURRENT_TASK.md b/CURRENT_TASK.md index 954635ff..9b7a2ec7 100644 --- a/CURRENT_TASK.md +++ b/CURRENT_TASK.md @@ -106,6 +106,19 @@ - **既存テスト**: 877/884 PASS（Phase 226 非関連 7失敗、pre-existing） - **残課題**: `ValueId(14) undefined` エラー（promotion ロジック関連、Phase 226 範囲外） - **詳細**: Cascading dependency chain: `ch = s.substring(p, p+1)` → `digit_pos = digits.indexOf(ch)` → `if digit_pos < 0 { break }` +- **Phase 227-228 完了** ✅: CarrierRole 導入（LoopState vs ConditionOnly）+ CarrierInit 最適化 + - **CarrierRole 導入**: ConditionOnly carriers の exit PHI 生成をスキップ（latch incoming のみ） + - **CarrierInit::BoolConst(false)**: ConditionOnly carriers の header PHI を `false` 初期化で統一 + - **Pattern4 integration**: continue バグを解決し、`_skip_whitespace` 完全修正 (RC=30) + - **CondVarScope 拡張**: LoopBodyLocalInit - 多段依存追跡（ch → is_ws → cond） + - **統一モデル確立**: Pattern4/Pattern2 で同じ構造（ConditionEnv + ConditionOnly）を再利用 +- **Phase 223-228 リファクタリング調査完了** ✅: 重複コード・レガシー箇所の棚卸 + - **ConditionAlias 冗長性発見**: promoted_loopbodylocals + CarrierVar.role で代替可能 + - **ConditionOnly フィルタ分散**: 4ファイルに同じロジックが存在（ExitLine 処理） + - **MethodCallLowerer 成功例**: Phase 224 で既に統一化済み（参考になる Box化パターン） + - **Phase 229 推奨**: ConditionAlias 削除（低リスク・高リターン・1〜2時間） + - 📄 詳細: `docs/development/current/main/phase223-228-refactoring-opportunities.md` + - 📄 実装計画: `docs/development/current/main/phase229-action-plan.md` ### 2. JsonParser / Trim / selfhost への適用状況 diff --git a/docs/development/current/main/REFACTORING_INDEX.md b/docs/development/current/main/REFACTORING_INDEX.md new file mode 100644 index 00000000..7e053cef --- /dev/null +++ b/docs/development/current/main/REFACTORING_INDEX.md @@ -0,0 +1,234 @@ +# リファクタリング機会 - クイックインデックス + +Phase 223-228 実装を通じて発見されたリファクタリング機会の一覧です。 + +## 📊 サマリー + +| Phase | タイトル | 優先度 | 実装難度 | 推奨時期 | ドキュメント | +|-------|---------|--------|---------|---------|-------------| +| **229** | **ConditionAlias 削除** | ⭐⭐⭐⭐⭐ | 低 | **今すぐ** | [phase229-action-plan.md](phase229-action-plan.md) | +| 230 | ExitLinePolicy Trait | ⭐⭐⭐⭐ | 中 | Phase 229 後 | [phase223-228-refactoring-opportunities.md](phase223-228-refactoring-opportunities.md) | +| 231+ | パターン検出共通化 | ⭐⭐⭐ | 中〜高 | 新パターン追加時 | [phase223-228-refactoring-opportunities.md](phase223-228-refactoring-opportunities.md) | + +## 🎯 Phase 229: ConditionAlias 削除（推奨実施） + +### 問題 + +**ConditionAlias は冗長！** + +```rust +pub struct CarrierInfo { + pub promoted_loopbodylocals: Vec, // ["digit_pos"] + pub carriers: Vec, // [CarrierVar{name: "is_digit_pos", ...}] + pub condition_aliases: Vec, // ← これは promoted_loopbodylocals + carriers から導出可能 +} +``` + +### 解決策 + +**動的解決で十分！** + +```rust +impl CarrierInfo { + pub fn resolve_promoted_carrier(&self, old_name: &str) -> Option<&str> { + if !self.promoted_loopbodylocals.contains(&old_name.to_string()) { + return None; + } + let expected_name = format!("is_{}", old_name); + self.carriers.iter() + .find(|c| c.name == expected_name) + .map(|c| c.name.as_str()) + } +} +``` + +### 効果 + +- **削減**: CarrierInfo フィールド 6 → 5 +- **保守性**: データ整合性チェック 3箇所 → 1箇所 +- **時間**: 1〜2時間 +- **リスク**: 低 + +### 実装計画 + +📄 **[phase229-action-plan.md](phase229-action-plan.md)** - 詳細な実装手順とテスト計画 + +--- + +## 🔧 Phase 230: ExitLinePolicy Trait + +### 問題 + +**ConditionOnly フィルタが4ファイルに分散！** + +- `meta_collector.rs` - collection ロジック +- `reconnector.rs` (2箇所) - reconnect ロジック +- `instruction_rewriter.rs` - exit PHI ロジック + +### 解決策 + +**Policy Trait で集約** + +```rust +pub struct ExitLinePolicy; + +impl ExitLinePolicy { + pub fn should_collect(role: CarrierRole) -> bool { + true // ConditionOnly も collect（latch incoming 用） + } + + pub fn should_reconnect(role: CarrierRole) -> bool { + role == CarrierRole::LoopState // LoopState のみ + } + + pub fn should_create_exit_phi(role: CarrierRole) -> bool { + role == CarrierRole::LoopState // LoopState のみ + } +} +``` + +### 効果 + +- **削減**: ~20行（重複 if 文の削減） +- **可読性**: 判断ロジックが1箇所に +- **拡張性**: 新しい CarrierRole 追加時に1箇所修正 +- **時間**: 2〜3時間 +- **リスク**: 中 + +--- + +## 💡 Phase 231+: パターン検出共通化 + +### 問題 + +**Trim と DigitPos Promoter に重複コード** + +- `loop_body_carrier_promoter.rs` (658行) +- `loop_body_digitpos_promoter.rs` (713行) + +**同じ関数が2箇所に存在**: +- `is_substring_method_call()` - 完全に同一 +- 条件変数抽出ロジック - worklist 走査パターンが同じ + +### 解決策 + +**LoopBodyLocalPattern Trait** + +```rust +pub trait LoopBodyLocalPattern { + fn pattern_name() -> &'static str; + fn allowed_init_methods() -> &'static [&'static str]; + fn detect(...) -> Option; +} + +impl LoopBodyLocalPattern for TrimPattern { ... } +impl LoopBodyLocalPattern for DigitPosPattern { ... } +``` + +### 効果 + +- **削減**: ~200行+（重複ロジック統合） +- **拡張性**: 新パターン追加が容易 +- **テスト容易性**: 共通部分を単独テスト可能 +- **時間**: 1日+ +- **リスク**: 高 + +### 推奨時期 + +**新しいパターン追加時に検討** - 今は後回しでOK + +--- + +## 📚 参考資料 + +### 成功例 + +**Phase 224: MethodCallLowerer 統一化** - 理想的な Box化パターン + +- **Before**: loop_body_local_init.rs と condition_lowerer.rs に重複コード +- **After**: MethodCallLowerer Box に統一 +- **成果**: ~200行削減、保守性向上、テスト容易性向上 + +**特徴**: +- Metadata-Driven（CoreMethodId ベース） +- Fail-Fast（whitelist にない method は即エラー） +- Context-Aware（for_init / for_condition で異なる whitelist） + +### 設計ガイドライン + +✅ **良い Box化**: +1. 単一責任 +2. Metadata-Driven +3. Fail-Fast +4. Context-Aware +5. 独立テスト可能 + +❌ **避けるべき**: +1. 情報の重複 +2. ロジックの分散 +3. 型の冗長 +4. 暗黙の依存 + +--- + +## 🧪 テスト戦略 + +### Phase 229 変更時 + +**Level 1: ビルド確認** +```bash +cargo build --release +``` + +**Level 2: 単体テスト** +```bash +cargo test --lib carrier_info +cargo test --lib loop_pattern_detection +``` + +**Level 3: パターンテスト** +```bash +cargo test --release test_mir_joinir_funcscanner_trim +cargo test --release test_loopbodylocal_digitpos +cargo test --release test_loop_with_break +``` + +**Level 4: 決定性テスト** +```bash +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 +``` + +**Level 5: E2E テスト** +```bash +tools/smokes/v2/run.sh --profile quick --filter "loop_*" +cargo test --release --test '*' +``` + +--- + +## 📋 実装チェックリスト + +Phase 229 実装時： + +- [ ] CarrierInfo::resolve_promoted_carrier() 実装 +- [ ] pattern2_with_break.rs の condition_aliases ループ削除 +- [ ] loop_body_carrier_promoter.rs 修正 +- [ ] loop_body_digitpos_promoter.rs 修正 +- [ ] ConditionAlias 型削除 +- [ ] pattern4_carrier_analyzer.rs 修正 +- [ ] pattern_pipeline.rs 修正 +- [ ] ビルド確認 +- [ ] 単体テスト +- [ ] パターンテスト +- [ ] 決定性テスト +- [ ] E2E テスト +- [ ] CURRENT_TASK.md 更新 + +--- + +**作成日**: 2025-12-10 +**対象**: Phase 223-228 コードベース分析結果 +**推奨次アクション**: Phase 229 ConditionAlias 削除の実施 diff --git a/docs/development/current/main/phase223-228-refactoring-opportunities.md b/docs/development/current/main/phase223-228-refactoring-opportunities.md new file mode 100644 index 00000000..3b97af09 --- /dev/null +++ b/docs/development/current/main/phase223-228-refactoring-opportunities.md @@ -0,0 +1,637 @@ +# 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. **メソッド検出ロジック** + ```rust + // 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: 共通トレイト抽出** (中リスク) + +```rust +/// 共通のパターン検出トレイト +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; +} + +impl LoopBodyLocalPattern for TrimPattern { ... } +impl LoopBodyLocalPattern for DigitPosPattern { ... } +``` + +**メリット**: +- 共通ロジックを1箇所に集約 +- 新しいパターン追加が容易 +- テスト容易性向上 + +**デメリット**: +- 既存コードへの影響大（2ファイル全体のリファクタリング） +- 回帰テストが必須 +- 実装難度: 中〜高 + +**推奨度**: ⭐⭐⭐ (中) - 新しいパターン追加時に検討 + +--- + +### B. ConditionAlias の必要性検証 (優先度: 高) + +#### 現状 + +**ConditionAlias の目的**: +```rust +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) + +**使用パターン**: +```rust +// 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` がある！** + +```rust +pub struct CarrierInfo { + pub promoted_loopbodylocals: Vec, // ["digit_pos"] + pub carriers: Vec, // [CarrierVar{name: "is_digit_pos", ...}] + pub condition_aliases: Vec, // [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 削除** (低リスク、高リターン) + +```rust +// ConditionAlias を削除して、必要な時に動的に解決 +fn resolve_promoted_variable( + var_name: &str, + carrier_info: &CarrierInfo, +) -> Option { + // 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) + ```rust + 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) + ```rust + if binding.role == CarrierRole::ConditionOnly { + // Skip ConditionOnly carriers (no variable_map update) + continue; + } + ``` + +3. **instruction_rewriter.rs** (line 615) + ```rust + 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** (中リスク) + +```rust +/// 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 {} +``` + +**使用例**: +```rust +// 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** (高リスク) + +```rust +/// ExitLine 処理の統括箱 +pub struct ExitLineOrchestrator; + +impl ExitLineOrchestrator { + /// exit_bindings を収集して、reconnect と exit PHI を実行 + pub fn process_exit_line( + boundary: &JoinInlineBoundary, + carrier_phis: &BTreeMap, + variable_map: &mut BTreeMap, + blocks: &mut Vec, + 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): +```rust +// 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): +```rust +// 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 { + 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 関数実装 +```rust +// 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 修正 +```rust +// 削除: 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**: テスト +```bash +# 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: 単体テスト** +```bash +cargo test --lib carrier_info +cargo test --lib loop_pattern_detection +``` + +**Level 2: パターンテスト** +```bash +# 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 テスト** +```bash +# 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)" +``` + +**決定性テスト** (重要！): +```bash +# 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` フィールド追加 + +2. **エラーメッセージの改善** + ```rust + 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. **デバッグログの追加** + ```rust + eprintln!( + "[pattern2/resolve] Promoted variable '{}' resolved to carrier '{}'", + old_name, carrier_name + ); + ``` + +### ExitLinePolicy 実装時 + +1. **trait より struct + impl 推奨** + - trait は over-engineering になりがち + - static メソッドで十分 + +2. **ドキュメント重視** + ```rust + /// 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 コードベース diff --git a/docs/development/current/main/phase229-action-plan.md b/docs/development/current/main/phase229-action-plan.md new file mode 100644 index 00000000..c5199c7b --- /dev/null +++ b/docs/development/current/main/phase229-action-plan.md @@ -0,0 +1,397 @@ +# Phase 229 アクションプラン: ConditionAlias 削除 + +## 🎯 目標 + +**ConditionAlias 型を削除し、CarrierInfo を簡素化する** + +- 削減: CarrierInfo のフィールド 6 → 5 +- 効果: データ整合性チェック箇所 3 → 1 +- 時間: 1〜2時間 +- リスク: 低 + +## 📋 実装手順 + +### Step 1: resolve 関数実装 (15分) + +**ファイル**: `src/mir/join_ir/lowering/carrier_info.rs` + +```rust +impl CarrierInfo { + /// Phase 229: Resolve promoted LoopBodyLocal carrier name + /// + /// # Arguments + /// + /// * `old_name` - Original LoopBodyLocal variable name (e.g., "digit_pos") + /// + /// # Returns + /// + /// * `Some(carrier_name)` - If this variable was promoted (e.g., "is_digit_pos") + /// * `None` - If not a promoted variable + /// + /// # Design + /// + /// Uses naming convention: "var_name" → "is_var_name" + /// Alternative: Linear search in carriers with original_name field + pub fn resolve_promoted_carrier(&self, old_name: &str) -> Option<&str> { + // Check if this variable was promoted + if !self.promoted_loopbodylocals.contains(&old_name.to_string()) { + return None; + } + + // Naming convention: "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()) + } +} +``` + +**追加箇所**: line 420 付近（CarrierInfo impl ブロックの最後） + +--- + +### Step 2: pattern2_with_break.rs 修正 (20分) + +**ファイル**: `src/mir/builder/control_flow/joinir/patterns/pattern2_with_break.rs` + +**削除** (line 354-391): +```rust +// Phase 224-D: Add condition aliases to ConditionEnv +// This allows promoted variables to be referenced by their original names in conditions +for alias in &carrier_info.condition_aliases { + // ... 約40行削除 +} +``` + +**追加** (line 354): +```rust +// Phase 229: Resolve promoted LoopBodyLocal variables dynamically +// This replaces condition_aliases with on-demand resolution +for promoted_var in &carrier_info.promoted_loopbodylocals { + if let Some(carrier_name) = carrier_info.resolve_promoted_carrier(promoted_var) { + // Find carrier's join_id in carriers list (BEFORE filtering, with join_ids) + if let Some(carrier) = carriers_with_join_ids.iter().find(|c| c.name == carrier_name) { + if let Some(join_id) = carrier.join_id { + // Add dynamic mapping: old_name → carrier's join_id + env.insert(promoted_var.clone(), join_id); + eprintln!( + "[pattern2/phase229] Resolved promoted variable '{}' → carrier '{}' (join_id={:?})", + promoted_var, carrier_name, join_id + ); + } else { + eprintln!( + "[pattern2/phase229] WARNING: Promoted carrier '{}' has no join_id yet!", + carrier_name + ); + } + } else { + eprintln!( + "[pattern2/phase229] WARNING: Promoted carrier '{}' not found in carriers list", + carrier_name + ); + } + } +} +``` + +--- + +### Step 3: Promoter 修正 (20分) + +#### 3-1. Trim Promoter + +**ファイル**: `src/mir/loop_pattern_detection/loop_body_carrier_promoter.rs` + +**削除** (line 87-90): +```rust +carrier_info.condition_aliases.push(crate::mir::join_ir::lowering::carrier_info::ConditionAlias { + old_name: trimmed_var_name.to_string(), + carrier_name: carrier_var_name.clone(), +}); +``` + +**コメント追加** (line 87): +```rust +// Phase 229: promoted_loopbodylocals already records this promotion +// No need for condition_aliases - resolved dynamically via CarrierInfo::resolve_promoted_carrier() +``` + +#### 3-2. DigitPos Promoter + +**ファイル**: `src/mir/loop_pattern_detection/loop_body_digitpos_promoter.rs` + +**削除** (line 203-206): +```rust +carrier_info.condition_aliases.push(crate::mir::join_ir::lowering::carrier_info::ConditionAlias { + old_name: var_name.to_string(), + carrier_name: carrier_var_name.clone(), +}); +``` + +**コメント追加** (line 203): +```rust +// Phase 229: promoted_loopbodylocals already records this promotion +// No need for condition_aliases - resolved dynamically via CarrierInfo::resolve_promoted_carrier() +``` + +--- + +### Step 4: ConditionAlias 型削除 (10分) + +**ファイル**: `src/mir/join_ir/lowering/carrier_info.rs` + +**削除** (line 85-101): +```rust +/// Phase 224-D: Condition alias for promoted LoopBodyLocal variables +/// +/// Maps old variable names to their promoted carrier names for condition resolution. +/// ... +#[derive(Debug, Clone)] +pub struct ConditionAlias { + pub old_name: String, + pub carrier_name: String, +} +``` + +**削除** (line 195): +```rust +pub condition_aliases: Vec, +``` + +**削除** (各 constructor で): +- line 259: `condition_aliases: Vec::new(),` +- line 320: `condition_aliases: Vec::new(),` +- line 348: `condition_aliases: Vec::new(),` +- line 410-414: `merge_other()` の condition_aliases マージコード + +--- + +### Step 5: pattern4_carrier_analyzer.rs 修正 (5分) + +**ファイル**: `src/mir/builder/control_flow/joinir/patterns/pattern4_carrier_analyzer.rs` + +**削除** (line 76): +```rust +condition_aliases: all_carriers.condition_aliases.clone(), // Phase 224-D +``` + +**削除** (line 299): +```rust +condition_aliases: Vec::new(), // Phase 224-D +``` + +--- + +### Step 6: pattern_pipeline.rs 修正 (5分) + +**ファイル**: `src/mir/builder/control_flow/joinir/patterns/pattern_pipeline.rs` + +**削除** (line 412): +```rust +condition_aliases: Vec::new(), // Phase 224-D +``` + +**削除** (line 452): +```rust +condition_aliases: Vec::new(), // Phase 224-D +``` + +--- + +## ✅ テスト手順 + +### Level 1: ビルド確認 (5分) + +```bash +cargo build --release 2>&1 | tee /tmp/phase229-build.log +# エラーがないことを確認 +``` + +### Level 2: 単体テスト (5分) + +```bash +# CarrierInfo resolve テスト（手動確認） +cargo test --lib carrier_info -- --nocapture + +# Pattern detection テスト +cargo test --lib loop_pattern_detection -- --nocapture +``` + +### Level 3: パターンテスト (10分) + +```bash +# Trim pattern +cargo test --release test_mir_joinir_funcscanner_trim 2>&1 | tee /tmp/phase229-trim.log + +# DigitPos pattern +cargo test --release test_loopbodylocal_digitpos 2>&1 | tee /tmp/phase229-digitpos.log + +# Pattern 2 (break) integration +cargo test --release test_loop_with_break 2>&1 | tee /tmp/phase229-pattern2.log +``` + +### Level 4: 決定性テスト (10分) + +```bash +# 3回実行して ValueId が変わらないことを確認 +for i in 1 2 3; do + echo "=== Run $i ===" | tee -a /tmp/phase229-determinism.log + cargo test --release test_loop_with_break 2>&1 | grep -E "ValueId|test result" | tee -a /tmp/phase229-determinism.log +done + +# 3回の出力が同一であることを確認 +``` + +### Level 5: E2E テスト (20分) + +```bash +# Smoke tests (loop 関連のみ) +tools/smokes/v2/run.sh --profile quick --filter "loop_*" 2>&1 | tee /tmp/phase229-smoke.log + +# Full MIR test suite +cargo test --release --test '*' 2>&1 | grep -E "(test result|FAILED)" | tee /tmp/phase229-full.log +``` + +--- + +## 🎯 成功条件 + +### ビルド +- ✅ cargo build --release が成功 +- ✅ 警告 0 件（unused imports, dead code など） + +### テスト +- ✅ Trim pattern テストが PASS +- ✅ DigitPos pattern テストが PASS +- ✅ Pattern 2 integration テストが PASS +- ✅ 決定性テスト（3回実行で同じ ValueId） + +### コード品質 +- ✅ CarrierInfo のフィールド数: 6 → 5 +- ✅ condition_aliases 参照: 15箇所 → 0箇所 +- ✅ 新しいドキュメントコメント追加 + +--- + +## 📝 コミットメッセージ + +``` +refactor(joinir): Phase 229 - Remove redundant ConditionAlias + +Problem: +- ConditionAlias duplicates information already in promoted_loopbodylocals +- Maintaining 3 data structures (promoted_loopbodylocals, carriers, condition_aliases) is error-prone +- confusion: "Why do we need condition_aliases when we have promoted_loopbodylocals?" + +Solution: +- Add CarrierInfo::resolve_promoted_carrier() for dynamic resolution +- Remove ConditionAlias type and all usages +- Use promoted_loopbodylocals + naming convention ("var" → "is_var") + +Impact: +- CarrierInfo fields: 6 → 5 +- Maintenance cost: 3 data structures → 2 +- No functional changes (all tests pass) + +Files changed: +- carrier_info.rs: Added resolve_promoted_carrier(), removed ConditionAlias +- pattern2_with_break.rs: Dynamic resolution instead of condition_aliases +- loop_body_carrier_promoter.rs: Removed condition_aliases.push() +- loop_body_digitpos_promoter.rs: Removed condition_aliases.push() +- pattern4_carrier_analyzer.rs: Removed condition_aliases fields +- pattern_pipeline.rs: Removed condition_aliases fields + +Tests: +- ✅ test_mir_joinir_funcscanner_trim +- ✅ test_loopbodylocal_digitpos +- ✅ test_loop_with_break +- ✅ Determinism test (3 runs with same ValueId) +``` + +--- + +## 🚨 リスク管理 + +### 低リスク要因 + +1. **既存のテストで検証可能** + - Trim pattern テスト + - DigitPos pattern テスト + - Pattern 2 integration テスト + +2. **影響範囲が明確** + - CarrierInfo とその使用箇所のみ + - MIR generation ロジックは変更なし + +3. **後方互換性** + - promoted_loopbodylocals は残る + - CarrierVar.role は残る + - 外部インターフェース変更なし + +### もしもの時の対処 + +**ビルドエラー**: +```bash +# condition_aliases の参照が残っていたら +rg "condition_aliases" src/mir --type rust +# 漏れを修正 +``` + +**テスト失敗**: +```bash +# ログを確認 +cat /tmp/phase229-pattern2.log | grep -E "ERROR|FAILED|panic" + +# resolve が失敗していたら +# → 命名規則の確認（"is_" prefix） +# → promoted_loopbodylocals に記録されているか確認 +``` + +**決定性失敗**: +```bash +# 3回実行で ValueId が異なる場合 +# → Phase 229 の変更は関係なし（元々の問題） +# → git log で最近の HashMap 変更をチェック +``` + +--- + +## 📊 見積もり + +| タスク | 時間 | +|-------|-----| +| Step 1: resolve 実装 | 15分 | +| Step 2: pattern2 修正 | 20分 | +| Step 3: Promoter 修正 | 20分 | +| Step 4-6: 型削除 | 20分 | +| テスト実行 | 50分 | +| ドキュメント更新 | 15分 | +| **合計** | **2時間20分** | + +--- + +## ✅ チェックリスト + +実装時にこのリストを確認： + +- [ ] Step 1: CarrierInfo::resolve_promoted_carrier() 実装 +- [ ] Step 2: pattern2_with_break.rs の condition_aliases ループ削除 +- [ ] Step 3-1: loop_body_carrier_promoter.rs の condition_aliases.push() 削除 +- [ ] Step 3-2: loop_body_digitpos_promoter.rs の condition_aliases.push() 削除 +- [ ] Step 4: ConditionAlias 型削除 +- [ ] Step 5: pattern4_carrier_analyzer.rs 修正 +- [ ] Step 6: pattern_pipeline.rs 修正 +- [ ] Level 1: ビルド確認 +- [ ] Level 2: 単体テスト +- [ ] Level 3: パターンテスト（Trim, DigitPos, Pattern 2） +- [ ] Level 4: 決定性テスト（3回実行） +- [ ] Level 5: E2E テスト（Smoke + Full） +- [ ] コミットメッセージ作成 +- [ ] CURRENT_TASK.md 更新 + +--- + +**作成日**: 2025-12-10 +**対象**: Phase 229 実装 +**前提**: Phase 227-228 完了 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 fb3ad270..6cb0dfd5 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 @@ -348,43 +348,42 @@ impl MirBuilder { carrier.name, carrier_join_id); } - // Phase 224-D: Save carriers with join_ids BEFORE filtering - let carriers_with_join_ids = carrier_info.carriers.clone(); + // Phase 229: Dynamic condition variable resolution for promoted variables + // Instead of using condition_aliases, we dynamically resolve promoted variables + // using promoted_loopbodylocals + naming convention + for promoted_var in &carrier_info.promoted_loopbodylocals { + // Try both naming conventions: + // 1. DigitPos pattern: "var" → "is_var" + // 2. Trim pattern: "var" → "is_var_match" - // Phase 224-D: Add condition aliases to ConditionEnv - // This allows promoted variables to be referenced by their original names in conditions - for alias in &carrier_info.condition_aliases { - // Check if the carrier_name matches the loop_var_name (promoted as main carrier) - if alias.carrier_name == carrier_info.loop_var_name { - // Use loop variable's join_id from env - if let Some(join_id) = env.get(&carrier_info.loop_var_name) { - env.insert(alias.old_name.clone(), join_id); - eprintln!( - "[pattern2/phase224d] Added condition alias '{}' → loop_var '{}' (join_id={:?})", - alias.old_name, carrier_info.loop_var_name, join_id - ); - } - } else { - // Find the carrier's join_id in the carriers list (BEFORE filtering, with join_ids) - if let Some(carrier) = carriers_with_join_ids.iter().find(|c| c.name == alias.carrier_name) { - if let Some(join_id) = carrier.join_id { - // Add alias mapping: old_name → carrier's join_id - env.insert(alias.old_name.clone(), join_id); + let candidate_names = vec![ + format!("is_{}", promoted_var), // DigitPos pattern + format!("is_{}_match", promoted_var), // Trim pattern + ]; + + for carrier_name in candidate_names { + // Check if it's the loop variable + if carrier_name == carrier_info.loop_var_name { + if let Some(join_id) = env.get(&carrier_info.loop_var_name) { + env.insert(promoted_var.clone(), join_id); eprintln!( - "[pattern2/phase224d] Added condition alias '{}' → carrier '{}' (join_id={:?})", - alias.old_name, alias.carrier_name, join_id - ); - } else { - eprintln!( - "[pattern2/phase224d] WARNING: Carrier '{}' has no join_id yet!", - alias.carrier_name + "[pattern2/phase229] Dynamically resolved promoted '{}' → loop_var '{}' (join_id={:?})", + promoted_var, carrier_info.loop_var_name, join_id ); + break; // Found resolution + } + } + + // Otherwise check carriers + if let Some(carrier) = carrier_info.carriers.iter().find(|c| c.name == carrier_name) { + if let Some(join_id) = carrier.join_id { + env.insert(promoted_var.clone(), join_id); + eprintln!( + "[pattern2/phase229] Dynamically resolved promoted '{}' → carrier '{}' (join_id={:?})", + promoted_var, carrier_name, join_id + ); + break; // Found resolution } - } else { - eprintln!( - "[pattern2/phase224d] WARNING: Carrier '{}' not found in carriers list!", - alias.carrier_name - ); } } } diff --git a/src/mir/builder/control_flow/joinir/patterns/pattern4_carrier_analyzer.rs b/src/mir/builder/control_flow/joinir/patterns/pattern4_carrier_analyzer.rs index b2c84602..833eaa72 100644 --- a/src/mir/builder/control_flow/joinir/patterns/pattern4_carrier_analyzer.rs +++ b/src/mir/builder/control_flow/joinir/patterns/pattern4_carrier_analyzer.rs @@ -73,7 +73,6 @@ impl Pattern4CarrierAnalyzer { carriers: updated_carriers, trim_helper: all_carriers.trim_helper.clone(), promoted_loopbodylocals: all_carriers.promoted_loopbodylocals.clone(), // Phase 224 - condition_aliases: all_carriers.condition_aliases.clone(), // Phase 224-D }) } @@ -296,7 +295,6 @@ mod tests { ], trim_helper: None, promoted_loopbodylocals: Vec::new(), // Phase 224 - condition_aliases: Vec::new(), // Phase 224-D }; // Analyze carriers diff --git a/src/mir/builder/control_flow/joinir/patterns/pattern_pipeline.rs b/src/mir/builder/control_flow/joinir/patterns/pattern_pipeline.rs index 4ec2c346..bdc10a05 100644 --- a/src/mir/builder/control_flow/joinir/patterns/pattern_pipeline.rs +++ b/src/mir/builder/control_flow/joinir/patterns/pattern_pipeline.rs @@ -409,7 +409,6 @@ mod tests { ], trim_helper: None, promoted_loopbodylocals: Vec::new(), // Phase 224 - condition_aliases: Vec::new(), // Phase 224-D }, loop_scope: LoopScopeShapeBuilder::empty_body_locals( BasicBlockId(0), @@ -449,7 +448,6 @@ mod tests { whitespace_chars: vec![" ".to_string(), "\t".to_string()], }), promoted_loopbodylocals: Vec::new(), // Phase 224 - condition_aliases: Vec::new(), // Phase 224-D }, loop_scope: LoopScopeShapeBuilder::empty_body_locals( BasicBlockId(0), diff --git a/src/mir/join_ir/lowering/carrier_info.rs b/src/mir/join_ir/lowering/carrier_info.rs index 2c267e5b..52ff84f2 100644 --- a/src/mir/join_ir/lowering/carrier_info.rs +++ b/src/mir/join_ir/lowering/carrier_info.rs @@ -77,28 +77,9 @@ pub enum CarrierInit { BoolConst(bool), } -/// Phase 224-D: Alias for promoted LoopBodyLocal in condition expressions -/// -/// When a LoopBodyLocal variable is promoted to a carrier, the original variable -/// name must still be resolvable in condition expressions for backward compatibility. -/// -/// # Example -/// -/// ```ignore -/// // Original variable: "digit_pos" -/// // Promoted carrier: "is_digit_pos" -/// ConditionAlias { -/// old_name: "digit_pos".to_string(), -/// carrier_name: "is_digit_pos".to_string(), -/// } -/// ``` -#[derive(Debug, Clone)] -pub struct ConditionAlias { - /// Original variable name (e.g., "digit_pos") - pub old_name: String, - /// Promoted carrier name (e.g., "is_digit_pos") - pub carrier_name: String, -} +// Phase 229: ConditionAlias removed - redundant with promoted_loopbodylocals +// The naming convention (old_name → "is_" or "is__match") +// is sufficient to resolve promoted variables dynamically. /// Information about a single carrier variable #[derive(Debug, Clone)] @@ -186,13 +167,13 @@ pub struct CarrierInfo { /// These variables were originally LoopBodyLocal but have been promoted to carriers /// during condition promotion (e.g., DigitPosPromoter). The lowerer should skip /// LoopBodyLocal checks for these variables. - pub promoted_loopbodylocals: Vec, - /// Phase 224-D: Condition aliases for promoted LoopBodyLocal variables /// - /// Maps old variable names to their promoted carrier names for condition resolution. - /// This allows break/continue conditions to reference promoted variables by their - /// original names (e.g., `if digit_pos < 0` still works after promotion to "is_digit_pos"). - pub condition_aliases: Vec, + /// Phase 229: Naming convention for promoted carriers: + /// - DigitPos pattern: "var" → "is_var" (e.g., "digit_pos" → "is_digit_pos") + /// - Trim pattern: "var" → "is_var_match" (e.g., "ch" → "is_ch_match") + /// + /// Condition variable resolution dynamically infers the carrier name from this list. + pub promoted_loopbodylocals: Vec, } impl CarrierInfo { @@ -256,7 +237,6 @@ impl CarrierInfo { carriers, trim_helper: None, // Phase 171-C-5: No Trim pattern by default promoted_loopbodylocals: Vec::new(), // Phase 224: No promoted variables by default - condition_aliases: Vec::new(), // Phase 224-D: No aliases by default }) } @@ -317,7 +297,6 @@ impl CarrierInfo { carriers, trim_helper: None, // Phase 171-C-5: No Trim pattern by default promoted_loopbodylocals: Vec::new(), // Phase 224: No promoted variables by default - condition_aliases: Vec::new(), // Phase 224-D: No aliases by default }) } @@ -345,7 +324,6 @@ impl CarrierInfo { carriers, trim_helper: None, // Phase 171-C-5: No Trim pattern by default promoted_loopbodylocals: Vec::new(), // Phase 224: No promoted variables by default - condition_aliases: Vec::new(), // Phase 224-D: No aliases by default } } @@ -406,13 +384,6 @@ impl CarrierInfo { self.promoted_loopbodylocals.push(promoted_var.clone()); } } - - // Phase 224-D: Merge condition_aliases (deduplicate by old_name) - for alias in &other.condition_aliases { - if !self.condition_aliases.iter().any(|a| a.old_name == alias.old_name) { - self.condition_aliases.push(alias.clone()); - } - } } /// Phase 171-C-5: Get Trim pattern helper diff --git a/src/mir/loop_pattern_detection/loop_body_carrier_promoter.rs b/src/mir/loop_pattern_detection/loop_body_carrier_promoter.rs index ad4c3d94..c3b02276 100644 --- a/src/mir/loop_pattern_detection/loop_body_carrier_promoter.rs +++ b/src/mir/loop_pattern_detection/loop_body_carrier_promoter.rs @@ -83,11 +83,9 @@ impl TrimPatternInfo { // Phase 171-C-5: Attach TrimLoopHelper for pattern-specific lowering logic carrier_info.trim_helper = Some(TrimLoopHelper::from_pattern_info(self)); - // Phase 224-D: Record condition alias for promoted variable - carrier_info.condition_aliases.push(crate::mir::join_ir::lowering::carrier_info::ConditionAlias { - old_name: self.var_name.clone(), - carrier_name: self.carrier_name.clone(), - }); + // Phase 229: Record promoted variable (no need for condition_aliases) + // Dynamic resolution uses promoted_loopbodylocals + naming convention + carrier_info.promoted_loopbodylocals.push(self.var_name.clone()); carrier_info } diff --git a/src/mir/loop_pattern_detection/loop_body_digitpos_promoter.rs b/src/mir/loop_pattern_detection/loop_body_digitpos_promoter.rs index cae5e732..fb263f92 100644 --- a/src/mir/loop_pattern_detection/loop_body_digitpos_promoter.rs +++ b/src/mir/loop_pattern_detection/loop_body_digitpos_promoter.rs @@ -199,18 +199,16 @@ impl DigitPosPromoter { vec![promoted_carrier], ); - // Phase 224-D: Record condition alias for promoted variable - carrier_info.condition_aliases.push(crate::mir::join_ir::lowering::carrier_info::ConditionAlias { - old_name: var_in_cond.clone(), - carrier_name: carrier_name.clone(), - }); + // Phase 229: Record promoted variable (no need for condition_aliases) + // Dynamic resolution uses promoted_loopbodylocals + naming convention + carrier_info.promoted_loopbodylocals.push(var_in_cond.clone()); eprintln!( "[digitpos_promoter] A-4 DigitPos pattern promoted: {} → {}", var_in_cond, carrier_name ); eprintln!( - "[digitpos_promoter] Phase 224-D: Added condition alias '{}' → '{}'", + "[digitpos_promoter] Phase 229: Recorded promoted variable '{}' (carrier: '{}')", var_in_cond, carrier_name );