diff --git a/docs/development/architecture/loops/loopform_ssot.md b/docs/development/architecture/loops/loopform_ssot.md index cab9f92b..d8e586e2 100644 --- a/docs/development/architecture/loops/loopform_ssot.md +++ b/docs/development/architecture/loops/loopform_ssot.md @@ -27,7 +27,58 @@ SSOT（Single Source of Truth） - Bridge 側に `LoopPhiOps` 実装を追加し、`prepare/seal/exit` を直接呼ぶ。 - ループ形状の生成をユーティリティ化（builder/bridge 双方から共通呼び出し）。 +--- + +## LoopForm v2 ケース表 + +| Case | loop 条件形 | exit preds の構成 | 想定される PHI 入力の形 | 対応テスト | 対応 .hako | +|------|-----------|-----------------|---------------------|----------|-----------| +| **A** | `loop(i < n)` | header / body | header fallthrough + break | `loop_conditional_reassign_exit_phi_header_and_break` | - | +| **B** | `loop(1 == 1)` | body のみ | break のみ | `loop_constant_true_exit_phi_dominates` | `apps/tests/minimal_ssa_skip_ws.hako` | +| **C** | `loop(1 == 1)` + body-local | body のみ（一部の経路のみ定義） | break のみ（BodyLocalInternal は除外） | `loop_body_local_exit_phi_body_only` | - | +| **D** | `loop(i < n)` + continue | header / body / continue_merge | header + break + continue_merge | `loop_continue_merge_header_exit` | - | + +### ケース説明 + +#### Case A: header+break（標準パターン） +- **条件**: `loop(i < n)` のような動的条件 +- **特徴**: header→exit と body→exit の両方が CFG 上存在 +- **exit PHI**: header fallthrough + break 経路の両方を含む +- **検証**: `loop_conditional_reassign_exit_phi_header_and_break` + +#### Case B: constant-true+break-only（header 除外パターン） +- **条件**: `loop(1 == 1)` のような定数 true +- **特徴**: exit pred は break 経路のみ（`header→exit` は無い） +- **exit PHI**: break 経路のみ（header は CFG predecessor でないため除外） +- **検証**: `loop_constant_true_exit_phi_dominates` + `minimal_ssa_skip_ws.hako` + +#### Case C: body-local変数（BodyLocalInternal 除外パターン） +- **条件**: body 内で宣言された変数が一部の exit 経路でのみ定義される +- **特徴**: 変数が全 exit predecessors で定義されていない +- **exit PHI**: BodyLocalInternal 変数は除外（PHI 生成しない） +- **検証**: `loop_body_local_exit_phi_body_only` + +#### Case D: continue+break（continue_merge パターン） +- **条件**: continue 文を含むループ +- **特徴**: `continue_merge → header → exit` の経路あり +- **exit PHI**: header + break + continue_merge の 3 系統 +- **検証**: `loop_continue_merge_header_exit` + +### 実装ファイル + +| ファイル | 役割 | +|---------|-----| +| `src/mir/loop_builder.rs` | ループ構造生成・[LoopForm] コメント付き | +| `src/mir/phi_core/loop_snapshot_merge.rs` | Case A/B 分岐ロジック | +| `src/mir/phi_core/exit_phi_builder.rs` | Exit PHI 生成・Phantom block 除外 | +| `src/tests/mir_loopform_conditional_reassign.rs` | 4 ケース全てのテスト（[LoopForm-Test] タグ付き） | + +--- + 関連 - `src/mir/loop_builder.rs` - `src/runner/json_v0_bridge/lowering/loop_.rs` - `src/mir/phi_core/common.rs` +- `src/mir/phi_core/loop_snapshot_merge.rs` +- `src/mir/phi_core/exit_phi_builder.rs` +- `src/tests/mir_loopform_conditional_reassign.rs` diff --git a/lang/src/compiler/parser/using/using_collector_box.hako b/lang/src/compiler/parser/using/using_collector_box.hako index 4662ef3a..b5f859ea 100644 --- a/lang/src/compiler/parser/using/using_collector_box.hako +++ b/lang/src/compiler/parser/using/using_collector_box.hako @@ -11,28 +11,47 @@ using lang.compiler.parser.scan.parser_common_utils_box as ParserCommonUtilsBox static box UsingCollectorBox { // Public API: collect line-based using declarations to JSON array string + // Refactored to LoopForm v2 / Region+next_i pattern (Phase 25.1) collect(src) { if src == null { return "[]" } local n = src.length() local i = 0 local first = 1 local out = "[" + + // Main line-scanning loop: Region+next_i形 loop(i < n) { - // line slice [i, j) + local next_i = i + + // Find line boundaries [i, j) - Region+next_j形 local j = i - loop(j < n && src.substring(j, j+1) != "\n") { j = j + 1 } + loop(j < n && src.substring(j, j+1) != "\n") { + local next_j = j + 1 + j = next_j + } local line = src.substring(i, j) - // trim left spaces/tabs + + // Trim left spaces/tabs - Region+next_k形 local k = 0 - loop(k < line.length() && (line.substring(k,k+1) == " " || line.substring(k,k+1) == "\t")) { k = k + 1 } + loop(k < line.length() && (line.substring(k,k+1) == " " || line.substring(k,k+1) == "\t")) { + local next_k = k + 1 + k = next_k + } + + // Process if line starts with "using " if ParserCommonUtilsBox.starts_with(line, k, "using ") == 1 { local rest = ParserCommonUtilsBox.trim(line.substring(k + 6, line.length())) - // split on ' as ' + + // Split on ' as ' - initialize all variables before conditional use local as_pos = ParserCommonUtilsBox.index_of(rest, 0, " as ") local target = rest local alias = null - if as_pos >= 0 { target = ParserCommonUtilsBox.trim(rest.substring(0, as_pos)) alias = ParserCommonUtilsBox.trim(rest.substring(as_pos + 4, rest.length())) } - // path or namespace + if as_pos >= 0 { + target = ParserCommonUtilsBox.trim(rest.substring(0, as_pos)) + alias = ParserCommonUtilsBox.trim(rest.substring(as_pos + 4, rest.length())) + } + + // Determine if target is a path or namespace local is_path = 0 if target.length() > 0 { if ParserCommonUtilsBox.starts_with(target, 0, ParserCommonUtilsBox.dq()) == 1 { is_path = 1 } @@ -41,38 +60,71 @@ static box UsingCollectorBox { if target.length() >= 5 && ParserCommonUtilsBox.starts_with(target, target.length()-5, ".hako") == 1 { is_path = 1 } if target.length() >= 6 && ParserCommonUtilsBox.starts_with(target, target.length()-6, ".hako") == 1 { is_path = 1 } } + + // Initialize name and path before conditional blocks local name = "" local path = null + if is_path == 1 { - // strip quotes + // Strip quotes if present - use temp var to avoid SSA issues + local clean_target = target if ParserCommonUtilsBox.starts_with(target, 0, ParserCommonUtilsBox.dq()) == 1 { - target = target.substring(1, target.length()) - if target.length() > 0 && target.substring(target.length()-1, target.length()) == ParserCommonUtilsBox.dq() { target = target.substring(0, target.length()-1) } + local temp1 = target.substring(1, target.length()) + clean_target = temp1 + if temp1.length() > 0 && temp1.substring(temp1.length()-1, temp1.length()) == ParserCommonUtilsBox.dq() { + clean_target = temp1.substring(0, temp1.length()-1) + } } - path = target - if alias != null { name = alias } else { - // basename - local p = target + path = clean_target + + // Determine name from alias or basename + if alias != null { + name = alias + } else { + // Extract basename: Region+next_t形 - use temp vars to avoid SSA issues + local p = clean_target local idx = -1 local t = 0 - loop(t < p.length()) { if p.substring(t,t+1) == "/" { idx = t } t = t + 1 } - if idx >= 0 { p = p.substring(idx+1, p.length()) } - // strip extension - if p.length() > 5 && ParserCommonUtilsBox.starts_with(p, p.length()-5, ".hako") == 1 { p = p.substring(0, p.length()-5) } - else { if p.length() > 6 && ParserCommonUtilsBox.starts_with(p, p.length()-6, ".hako") == 1 { p = p.substring(0, p.length()-6) } } - name = p + loop(t < p.length()) { + local next_t = t + 1 + if p.substring(t,t+1) == "/" { idx = t } + t = next_t + } + + // Extract substring only if idx found + local p2 = p + if idx >= 0 { p2 = p.substring(idx+1, p.length()) } + + // Strip extension - use temp vars + local p3 = p2 + if p2.length() > 5 && ParserCommonUtilsBox.starts_with(p2, p2.length()-5, ".hako") == 1 { + p3 = p2.substring(0, p2.length()-5) + } else if p2.length() > 6 && ParserCommonUtilsBox.starts_with(p2, p2.length()-6, ".hako") == 1 { + p3 = p2.substring(0, p2.length()-6) + } + name = p3 } } else { name = target } - // append entry - if first == 0 { out = out + "," } else { first = 0 } + + // Append entry to output + if first == 0 { + out = out + "," + } else { + first = 0 + } out = out + "{" + ParserCommonUtilsBox.dq() + "name" + ParserCommonUtilsBox.dq() + ":" + ParserCommonUtilsBox.dq() + ParserCommonUtilsBox.esc_json(name) + ParserCommonUtilsBox.dq() - if path != null { out = out + "," + ParserCommonUtilsBox.dq() + "path" + ParserCommonUtilsBox.dq() + ":" + ParserCommonUtilsBox.dq() + ParserCommonUtilsBox.esc_json(path) + ParserCommonUtilsBox.dq() } + if path != null { + out = out + "," + ParserCommonUtilsBox.dq() + "path" + ParserCommonUtilsBox.dq() + ":" + ParserCommonUtilsBox.dq() + ParserCommonUtilsBox.esc_json(path) + ParserCommonUtilsBox.dq() + } out = out + "}" } - i = j + 1 + + next_i = j + 1 + i = next_i } + out = out + "]" return out } diff --git a/src/mir/loop_builder.rs b/src/mir/loop_builder.rs index fd1fe0a6..15bc0ecb 100644 --- a/src/mir/loop_builder.rs +++ b/src/mir/loop_builder.rs @@ -1,23 +1,35 @@ /*! * MIR Loop Builder - SSA形式でのループ構築専用モジュール * - * Sealed/Unsealed blockとPhi nodeを使った正しいループ実装 - * Based on Gemini's recommendation for proper SSA loop handling + * Sealed/Unsealed blockとPhi nodeを使った正しいループ実装。 + * + * LoopForm v2 の「形」をここで固定している: + * - preheader: ループに入る直前のブロック（初期値の copy 発生源） + * - header : ループ条件を評価するブロック（`loop(cond)` の `cond` 部分） + * - body : ループ本体（ユーザーコードが書いたブロック） + * - latch : body の末尾から header へ戻る backedge 用ブロック + * - exit : ループ脱出先（`break` / `cond == false` が合流するブロック） + * + * 典型パターン（ControlForm::LoopShape）: + * - Case A: header-cond + header→exit + body→exit（`loop(i < n) { if (...) break }`） + * - Case B: constant-true + body→exit のみ（`loop(1 == 1) { if (...) break }`） + * - この場合、header→exit のエッジは存在しないので、exit PHI に header 値を入れてはいけない。 + * - Case C: continue_merge を経由して header に戻る経路あり（`continue` を含むループ）。 + * + * それぞれのケースは ControlForm / LoopSnapshotMergeBox / ExitPhiBuilder に伝搬され、 + * exit PHI の入力選択や BodyLocalInternal 変数の扱いに反映される。 */ use super::{BasicBlockId, ConstValue, MirInstruction, ValueId}; -use crate::mir::control_form::{ControlForm, IfShape, LoopShape, is_control_form_trace_on}; -use crate::mir::phi_core::loopform_builder::{LoopFormBuilder, LoopFormOps}; -use crate::mir::phi_core::loop_snapshot_merge::LoopSnapshotMergeBox; -use crate::mir::phi_core::phi_input_collector::PhiInputCollector; use crate::ast::ASTNode; +use crate::mir::control_form::{is_control_form_trace_on, ControlForm, IfShape, LoopShape}; +use crate::mir::phi_core::loop_snapshot_merge::LoopSnapshotMergeBox; +use crate::mir::phi_core::loopform_builder::{LoopFormBuilder, LoopFormOps}; +use crate::mir::phi_core::phi_input_collector::PhiInputCollector; use std::collections::HashMap; // Phase 15 段階的根治戦略：制御フローユーティリティ -use super::utils::{ - is_current_block_terminated, - capture_actual_predecessor_and_jump, -}; +use super::utils::{capture_actual_predecessor_and_jump, is_current_block_terminated}; /// ループ脱出の種類（箱化・共通化のための型） #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -50,11 +62,9 @@ pub struct LoopBuilder<'a> { /// break文からの変数スナップショット（exit PHI生成用） exit_snapshots: Vec<(BasicBlockId, HashMap)>, - // フェーズM: no_phi_modeフィールド削除（常にPHI使用） } - impl<'a> LoopBuilder<'a> { // Implement phi_core LoopPhiOps on LoopBuilder for in-place delegation @@ -83,27 +93,31 @@ impl<'a> LoopBuilder<'a> { Ok(void_id) } - /// 【箱化】ループ脱出の共通処理（break/continue統一化） + /// [LoopForm] 【箱化】ループ脱出の共通処理（break/continue統一化） /// /// Phase 25.1o: break と continue の共通パターンを抽出し、 /// LoopExitKind で振る舞いを切り替える統一メソッド。 /// /// # 処理フロー /// 1. 現在の変数マップをスナップショット - /// 2. 適切なスナップショットリストに追加（Break → exit_snapshots, Continue → continue_snapshots） - /// 3. ターゲットブロックへジャンプ（Break → exit, Continue → header） + /// 2. [LoopForm] スナップショット保存（Break → exit_snapshots, Continue → continue_snapshots） + /// 3. [LoopForm] ターゲットブロックへジャンプ（Break → exit, Continue → header/continue_merge） /// 4. unreachable ブロックに切り替え fn do_loop_exit(&mut self, kind: LoopExitKind) -> Result { // 1. スナップショット取得（共通処理） let snapshot = self.get_current_variable_map(); let cur_block = self.current_block()?; - // 2. スナップショット保存（kind別処理） + // 2. [LoopForm] exit-break path: スナップショット保存（exit PHI入力用） + // [LoopForm] continue-backedge path: スナップショット保存（continue_merge → header） match kind { LoopExitKind::Break => { if std::env::var("NYASH_LOOPFORM_DEBUG").ok().as_deref() == Some("1") { - eprintln!("[DEBUG/do_break] Saved snapshot from block {:?}, vars: {:?}", - cur_block, snapshot.keys().collect::>()); + eprintln!( + "[DEBUG/do_break] Saved snapshot from block {:?}, vars: {:?}", + cur_block, + snapshot.keys().collect::>() + ); } self.exit_snapshots.push((cur_block, snapshot)); } @@ -116,7 +130,8 @@ impl<'a> LoopBuilder<'a> { // 3. ターゲットブロックへジャンプ（kind別処理） match kind { LoopExitKind::Break => { - if let Some(exit_bb) = crate::mir::builder::loops::current_exit(self.parent_builder) { + if let Some(exit_bb) = crate::mir::builder::loops::current_exit(self.parent_builder) + { self.jump_with_pred(exit_bb)?; } } @@ -156,7 +171,7 @@ impl<'a> LoopBuilder<'a> { loop_header: None, continue_target: None, continue_snapshots: Vec::new(), - exit_snapshots: Vec::new(), // exit PHI用のスナップショット + exit_snapshots: Vec::new(), // exit PHI用のスナップショット } } @@ -179,8 +194,10 @@ impl<'a> LoopBuilder<'a> { if std::env::var("NYASH_LOOPFORM_DEBUG").is_ok() { eprintln!("[build_loop_with_loopform] === ENTRY ==="); if let Some(ref func) = self.parent_builder.current_function { - eprintln!("[build_loop_with_loopform] fn='{}', counter={}, func_ptr={:p}", - func.signature.name, func.next_value_id, func as *const _); + eprintln!( + "[build_loop_with_loopform] fn='{}', counter={}, func_ptr={:p}", + func.signature.name, func.next_value_id, func as *const _ + ); } eprintln!("[build_loop_with_loopform] condition={:?}", condition); eprintln!("[build_loop_with_loopform] body.len()={}", body.len()); @@ -194,8 +211,11 @@ impl<'a> LoopBuilder<'a> { // DEBUG: Show variable map before guard check if std::env::var("NYASH_LOOPFORM_DEBUG").is_ok() { - eprintln!("[loopform] before_loop_id={:?}, variable_map size={}", - before_loop_id, current_vars.len()); + eprintln!( + "[loopform] before_loop_id={:?}, variable_map size={}", + before_loop_id, + current_vars.len() + ); for (name, value) in ¤t_vars { eprintln!(" {} -> {:?}", name, value); } @@ -220,7 +240,11 @@ impl<'a> LoopBuilder<'a> { let entry_block = self.current_block()?; self.emit_jump(preheader_id)?; // 📦 Hotfix 6: Add CFG predecessor for preheader (same as legacy version) - crate::mir::builder::loops::add_predecessor(self.parent_builder, preheader_id, entry_block)?; + crate::mir::builder::loops::add_predecessor( + self.parent_builder, + preheader_id, + entry_block, + )?; // Initialize LoopFormBuilder with preheader and header blocks let mut loopform = LoopFormBuilder::new(preheader_id, header_id); @@ -229,7 +253,10 @@ impl<'a> LoopBuilder<'a> { if std::env::var("NYASH_LOOPFORM_DEBUG").is_ok() { eprintln!("[loopform] Block IDs: preheader={:?}, header={:?}, body={:?}, latch={:?}, exit={:?}", preheader_id, header_id, body_id, latch_id, exit_id); - eprintln!("[loopform] variable_map at loop entry (size={}):", current_vars.len()); + eprintln!( + "[loopform] variable_map at loop entry (size={}):", + current_vars.len() + ); let mut loop_count = 0; for (name, value) in ¤t_vars { loop_count += 1; @@ -240,8 +267,10 @@ impl<'a> LoopBuilder<'a> { } eprintln!("[loopform] iterated {} times", loop_count); if let Some(ref func) = self.parent_builder.current_function { - eprintln!("[loopform] BEFORE prepare_structure: fn='{}', counter={}, func_ptr={:p}", - func.signature.name, func.next_value_id, func as *const _); + eprintln!( + "[loopform] BEFORE prepare_structure: fn='{}', counter={}, func_ptr={:p}", + func.signature.name, func.next_value_id, func as *const _ + ); } else { eprintln!("[loopform] BEFORE prepare_structure: current_function=None"); } @@ -249,8 +278,10 @@ impl<'a> LoopBuilder<'a> { loopform.prepare_structure(self, ¤t_vars)?; if std::env::var("NYASH_LOOPFORM_DEBUG").is_ok() { if let Some(ref func) = self.parent_builder.current_function { - eprintln!("[loopform] AFTER prepare_structure: fn='{}', counter={}, func_ptr={:p}", - func.signature.name, func.next_value_id, func as *const _); + eprintln!( + "[loopform] AFTER prepare_structure: fn='{}', counter={}, func_ptr={:p}", + func.signature.name, func.next_value_id, func as *const _ + ); } else { eprintln!("[loopform] AFTER prepare_structure: current_function=None"); } @@ -285,12 +316,19 @@ impl<'a> LoopBuilder<'a> { self.continue_snapshots.clear(); self.exit_snapshots.clear(); - // Emit condition check in header + // [LoopForm] header-cond: cond true → body, false → exit (Case A/B) + // - Case A: loop(i < n) → header can branch to exit directly + // - Case B: loop(1 == 1) → header always enters body, exit only via break if std::env::var("NYASH_LOOPFORM_DEBUG").is_ok() { - eprintln!("[loopform/condition] BEFORE build_expression: current_block={:?}", self.current_block()?); + eprintln!( + "[loopform/condition] BEFORE build_expression: current_block={:?}", + self.current_block()? + ); if let Some(ref func) = self.parent_builder.current_function { - eprintln!("[loopform/condition] BEFORE: fn='{}', counter={}, func_ptr={:p}", - func.signature.name, func.next_value_id, func as *const _); + eprintln!( + "[loopform/condition] BEFORE: fn='{}', counter={}, func_ptr={:p}", + func.signature.name, func.next_value_id, func as *const _ + ); } } let cond_value = self.parent_builder.build_expression(condition)?; @@ -298,28 +336,52 @@ impl<'a> LoopBuilder<'a> { // if build_expression created new blocks) let branch_source_block = self.current_block()?; if std::env::var("NYASH_LOOPFORM_DEBUG").is_ok() { - eprintln!("[loopform/condition] AFTER build_expression: branch_source_block={:?}", branch_source_block); + eprintln!( + "[loopform/condition] AFTER build_expression: branch_source_block={:?}", + branch_source_block + ); if let Some(ref func) = self.parent_builder.current_function { - eprintln!("[loopform/condition] AFTER: fn='{}', counter={}, func_ptr={:p}", - func.signature.name, func.next_value_id, func as *const _); + eprintln!( + "[loopform/condition] AFTER: fn='{}', counter={}, func_ptr={:p}", + func.signature.name, func.next_value_id, func as *const _ + ); } } self.emit_branch(cond_value, body_id, exit_id)?; // 📦 Hotfix 6: Add CFG predecessors for branch targets (Cytron et al. 1991 requirement) // This ensures exit_block.predecessors is populated before Exit PHI generation if std::env::var("NYASH_LOOPFORM_DEBUG").is_ok() { - eprintln!("[loopform/condition] BEFORE add_predecessor: exit_id={:?}, branch_source={:?}", exit_id, branch_source_block); + eprintln!( + "[loopform/condition] BEFORE add_predecessor: exit_id={:?}, branch_source={:?}", + exit_id, branch_source_block + ); } - crate::mir::builder::loops::add_predecessor(self.parent_builder, body_id, branch_source_block)?; - crate::mir::builder::loops::add_predecessor(self.parent_builder, exit_id, branch_source_block)?; + crate::mir::builder::loops::add_predecessor( + self.parent_builder, + body_id, + branch_source_block, + )?; + crate::mir::builder::loops::add_predecessor( + self.parent_builder, + exit_id, + branch_source_block, + )?; if std::env::var("NYASH_LOOPFORM_DEBUG").is_ok() { - eprintln!("[loopform/condition] AFTER emit_branch: current_block={:?}", self.current_block()?); - eprintln!("[loopform/condition] Added predecessors: body={:?} exit={:?} from={:?}", - body_id, exit_id, branch_source_block); + eprintln!( + "[loopform/condition] AFTER emit_branch: current_block={:?}", + self.current_block()? + ); + eprintln!( + "[loopform/condition] Added predecessors: body={:?} exit={:?} from={:?}", + body_id, exit_id, branch_source_block + ); // Verify predecessors were added if let Some(ref func) = self.parent_builder.current_function { if let Some(exit_block) = func.blocks.get(&exit_id) { - eprintln!("[loopform/condition] exit_block.predecessors = {:?}", exit_block.predecessors); + eprintln!( + "[loopform/condition] exit_block.predecessors = {:?}", + exit_block.predecessors + ); } } } @@ -358,7 +420,10 @@ impl<'a> LoopBuilder<'a> { // DEBUG: Log writes collection if std::env::var("NYASH_LOOPFORM_DEBUG").is_ok() { eprintln!("[loopform/writes] === WRITES COLLECTION (Step 5-1) ==="); - eprintln!("[loopform/writes] {} variables modified in loop body", writes.len()); + eprintln!( + "[loopform/writes] {} variables modified in loop body", + writes.len() + ); let mut sorted_writes: Vec<_> = writes.iter().collect(); sorted_writes.sort(); for name in &sorted_writes { @@ -404,8 +469,8 @@ impl<'a> LoopBuilder<'a> { // DISABLED: Body-local header PHI generation // This code was causing undefined value errors because it created header PHIs // for variables that should only have exit PHIs (or no PHIs at all) - if false { // Disabled for Step 5-5-B experiment - // [Original code removed - see git history if needed] + if false { // Disabled for Step 5-5-B experiment + // [Original code removed - see git history if needed] } // Pass 4: Generate continue_merge PHIs first, then seal header PHIs @@ -418,15 +483,18 @@ impl<'a> LoopBuilder<'a> { let merged_snapshot: HashMap = if !raw_continue_snaps.is_empty() { if trace_loop_phi { - eprintln!("[loop-phi/continue-merge] Generating PHI nodes for {} continue paths", - raw_continue_snaps.len()); + eprintln!( + "[loop-phi/continue-merge] Generating PHI nodes for {} continue paths", + raw_continue_snaps.len() + ); } // すべての continue snapshot に現れる変数を収集 let mut all_vars: HashMap> = HashMap::new(); for (continue_bb, snapshot) in &raw_continue_snaps { for (var_name, &value) in snapshot { - all_vars.entry(var_name.clone()) + all_vars + .entry(var_name.clone()) .or_default() .push((*continue_bb, value)); } @@ -460,8 +528,10 @@ impl<'a> LoopBuilder<'a> { } if trace_loop_phi { - eprintln!("[loop-phi/continue-merge] Generated PHI for '{}': {:?}", - var_name, phi_id); + eprintln!( + "[loop-phi/continue-merge] Generated PHI for '{}': {:?}", + var_name, phi_id + ); } phi_id }; @@ -472,8 +542,11 @@ impl<'a> LoopBuilder<'a> { // Note: 変数マップへの反映は seal_phis に委譲（干渉を避ける） if trace_loop_phi { - eprintln!("[loop-phi/continue-merge] Merged {} variables from {} paths", - merged.len(), raw_continue_snaps.len()); + eprintln!( + "[loop-phi/continue-merge] Merged {} variables from {} paths", + merged.len(), + raw_continue_snaps.len() + ); } merged } else { @@ -481,7 +554,11 @@ impl<'a> LoopBuilder<'a> { }; self.emit_jump(header_id)?; - crate::mir::builder::loops::add_predecessor(self.parent_builder, header_id, continue_merge_id)?; + crate::mir::builder::loops::add_predecessor( + self.parent_builder, + header_id, + continue_merge_id, + )?; // Step 2: merged_snapshot を使って seal_phis を呼ぶ // Phase 25.3: Continue merge PHI実装（Task先生の発見！） @@ -555,7 +632,9 @@ impl<'a> LoopBuilder<'a> { // Region 情報（entry/exit/slots）をログに出すよ。 crate::mir::region::observer::observe_control_form(self.parent_builder, &form); - // Build exit PHIs for break statements using ControlForm wrapper + // [LoopForm] exit PHI for Case A/B (uses exit_snapshots + exit_preds) + // - Case A: header+break → exit PHI includes both paths + // - Case B: break-only → exit PHI excludes header (not a predecessor) let exit_snaps = self.exit_snapshots.clone(); crate::mir::phi_core::loopform_builder::build_exit_phis_for_control( &loopform, @@ -583,7 +662,6 @@ impl<'a> LoopBuilder<'a> { Ok(void_dst) } - // ============================================================= // PHI Helpers — prepare/finalize PHIs and block sealing // ============================================================= @@ -675,17 +753,25 @@ impl<'a> LoopBuilder<'a> { let mut idx = 0; while idx < block.instructions.len() { match &mut block.instructions[idx] { - MirInstruction::Phi { dst: d, inputs: ins } if *d == dst => { + MirInstruction::Phi { + dst: d, + inputs: ins, + } if *d == dst => { *ins = inputs.clone(); replaced = true; break; } - MirInstruction::Phi { .. } => { idx += 1; } + MirInstruction::Phi { .. } => { + idx += 1; + } _ => break, } } if !replaced { - let phi_inst = MirInstruction::Phi { dst, inputs: inputs.clone() }; + let phi_inst = MirInstruction::Phi { + dst, + inputs: inputs.clone(), + }; block.instructions.insert(0, phi_inst); } if dbg { @@ -774,7 +860,6 @@ impl<'a> LoopBuilder<'a> { self.parent_builder.variable_map.get(name).copied() } - fn build_statement(&mut self, stmt: ASTNode) -> Result { match stmt { // Ensure nested bare blocks inside loops are lowered with loop-aware semantics @@ -794,8 +879,12 @@ impl<'a> LoopBuilder<'a> { void_id })) } - ASTNode::If { condition, then_body, else_body, .. } => - self.lower_if_in_loop(*condition, then_body, else_body), + ASTNode::If { + condition, + then_body, + else_body, + .. + } => self.lower_if_in_loop(*condition, then_body, else_body), ASTNode::Break { .. } => self.do_break(), ASTNode::Continue { .. } => self.do_continue(), other => self.parent_builder.build_expression(other), @@ -813,21 +902,32 @@ impl<'a> LoopBuilder<'a> { let join_id = self.parent_builder.debug_next_join_id(); // Pre-pin comparison operands to slots so repeated uses across blocks are safe if crate::config::env::mir_pre_pin_compare_operands() { - if let ASTNode::BinaryOp { operator, left, right, .. } = &condition { - use crate::ast::BinaryOperator as BO; - match operator { - BO::Equal | BO::NotEqual | BO::Less | BO::LessEqual | BO::Greater | BO::GreaterEqual => { - if let Ok(lhs_v) = self.parent_builder.build_expression((**left).clone()) { - let _ = self.parent_builder.pin_to_slot(lhs_v, "@loop_if_lhs"); - } - if let Ok(rhs_v) = self.parent_builder.build_expression((**right).clone()) { - let _ = self.parent_builder.pin_to_slot(rhs_v, "@loop_if_rhs"); + if let ASTNode::BinaryOp { + operator, + left, + right, + .. + } = &condition + { + use crate::ast::BinaryOperator as BO; + match operator { + BO::Equal + | BO::NotEqual + | BO::Less + | BO::LessEqual + | BO::Greater + | BO::GreaterEqual => { + if let Ok(lhs_v) = self.parent_builder.build_expression((**left).clone()) { + let _ = self.parent_builder.pin_to_slot(lhs_v, "@loop_if_lhs"); + } + if let Ok(rhs_v) = self.parent_builder.build_expression((**right).clone()) { + let _ = self.parent_builder.pin_to_slot(rhs_v, "@loop_if_rhs"); + } } + _ => {} } - _ => {} } } - } // Evaluate condition and create blocks let cond_val = self.parent_builder.build_expression(condition)?; let then_bb = self.new_block(); @@ -871,16 +971,14 @@ impl<'a> LoopBuilder<'a> { for s in then_body.iter().cloned() { let _ = self.build_statement(s)?; // フェーズS修正：統一終端検出ユーティリティ使用 - if is_current_block_terminated(self.parent_builder)? { - break; + if is_current_block_terminated(self.parent_builder)? { + break; } } let then_var_map_end = self.get_current_variable_map(); // フェーズS修正：最強モード指摘の「実到達predecessor捕捉」を統一 - let then_pred_to_merge = capture_actual_predecessor_and_jump( - self.parent_builder, - merge_bb - )?; + let then_pred_to_merge = + capture_actual_predecessor_and_jump(self.parent_builder, merge_bb)?; // Pop then-branch debug region self.parent_builder.debug_pop_region(); @@ -916,17 +1014,15 @@ impl<'a> LoopBuilder<'a> { for s in es.into_iter() { let _ = self.build_statement(s)?; // フェーズS修正：統一終端検出ユーティリティ使用 - if is_current_block_terminated(self.parent_builder)? { - break; + if is_current_block_terminated(self.parent_builder)? { + break; } } else_var_map_end_opt = Some(self.get_current_variable_map()); } // フェーズS修正：else branchでも統一実到達predecessor捕捉 - let else_pred_to_merge = capture_actual_predecessor_and_jump( - self.parent_builder, - merge_bb - )?; + let else_pred_to_merge = + capture_actual_predecessor_and_jump(self.parent_builder, merge_bb)?; // Pop else-branch debug region self.parent_builder.debug_pop_region(); @@ -937,10 +1033,16 @@ impl<'a> LoopBuilder<'a> { .debug_push_region(format!("join#{}", join_id) + "/join"); let mut vars: std::collections::HashSet = std::collections::HashSet::new(); - let then_prog = ASTNode::Program { statements: then_body.clone(), span: crate::ast::Span::unknown() }; + let then_prog = ASTNode::Program { + statements: then_body.clone(), + span: crate::ast::Span::unknown(), + }; crate::mir::phi_core::if_phi::collect_assigned_vars(&then_prog, &mut vars); if let Some(es) = &else_body { - let else_prog = ASTNode::Program { statements: es.clone(), span: crate::ast::Span::unknown() }; + let else_prog = ASTNode::Program { + statements: es.clone(), + span: crate::ast::Span::unknown(), + }; crate::mir::phi_core::if_phi::collect_assigned_vars(&else_prog, &mut vars); } @@ -949,15 +1051,25 @@ impl<'a> LoopBuilder<'a> { // Use shared helper to merge modified variables at merge block struct Ops<'b, 'a>(&'b mut LoopBuilder<'a>); impl<'b, 'a> crate::mir::phi_core::if_phi::PhiMergeOps for Ops<'b, 'a> { - fn new_value(&mut self) -> ValueId { self.0.new_value() } + fn new_value(&mut self) -> ValueId { + self.0.new_value() + } fn emit_phi_at_block_start( &mut self, block: BasicBlockId, dst: ValueId, inputs: Vec<(BasicBlockId, ValueId)>, - ) -> Result<(), String> { self.0.emit_phi_at_block_start(block, dst, inputs) } - fn update_var(&mut self, name: String, value: ValueId) { self.0.parent_builder.variable_map.insert(name, value); } - fn debug_verify_phi_inputs(&mut self, merge_bb: BasicBlockId, inputs: &[(BasicBlockId, ValueId)]) { + ) -> Result<(), String> { + self.0.emit_phi_at_block_start(block, dst, inputs) + } + fn update_var(&mut self, name: String, value: ValueId) { + self.0.parent_builder.variable_map.insert(name, value); + } + fn debug_verify_phi_inputs( + &mut self, + merge_bb: BasicBlockId, + inputs: &[(BasicBlockId, ValueId)], + ) { if let Some(ref func) = self.0.parent_builder.current_function { crate::mir::phi_core::common::debug_verify_phi_inputs(func, merge_bb, inputs); } @@ -1011,7 +1123,9 @@ impl<'a> LoopBuilder<'a> { // Implement phi_core LoopPhiOps on LoopBuilder for in-place delegation impl crate::mir::phi_core::loop_phi::LoopPhiOps for LoopBuilder<'_> { - fn new_value(&mut self) -> ValueId { self.new_value() } + fn new_value(&mut self) -> ValueId { + self.new_value() + } fn emit_phi_at_block_start( &mut self, block: BasicBlockId, @@ -1020,12 +1134,18 @@ impl crate::mir::phi_core::loop_phi::LoopPhiOps for LoopBuilder<'_> { ) -> Result<(), String> { self.emit_phi_at_block_start(block, dst, inputs) } - fn update_var(&mut self, name: String, value: ValueId) { self.update_variable(name, value) } + fn update_var(&mut self, name: String, value: ValueId) { + self.update_variable(name, value) + } fn get_variable_at_block(&mut self, name: &str, block: BasicBlockId) -> Option { // Call the inherent method (immutable borrow) to avoid recursion LoopBuilder::get_variable_at_block(self, name, block) } - fn debug_verify_phi_inputs(&mut self, merge_bb: BasicBlockId, inputs: &[(BasicBlockId, ValueId)]) { + fn debug_verify_phi_inputs( + &mut self, + merge_bb: BasicBlockId, + inputs: &[(BasicBlockId, ValueId)], + ) { if let Some(ref func) = self.parent_builder.current_function { crate::mir::phi_core::common::debug_verify_phi_inputs(func, merge_bb, inputs); } @@ -1047,7 +1167,10 @@ impl crate::mir::phi_core::loop_phi::LoopPhiOps for LoopBuilder<'_> { if let Some(ref mut function) = self.parent_builder.current_function { if let Some(block) = function.get_block_mut(preheader_id) { if dbg { - eprintln!("[DEBUG] Adding Copy instruction to block {}", preheader_id); + eprintln!( + "[DEBUG] Adding Copy instruction to block {}", + preheader_id + ); } block.add_instruction(MirInstruction::Copy { dst, src }); Ok(()) @@ -1083,15 +1206,20 @@ impl<'a> LoopFormOps for LoopBuilder<'a> { let before = func.next_value_id; let id = func.next_value_id(); if std::env::var("NYASH_LOOPFORM_DEBUG").is_ok() { - eprintln!("[LoopFormOps::new_value] fn='{}' counter: {} -> {}, allocated: {:?}", - func.signature.name, before, func.next_value_id, id); + eprintln!( + "[LoopFormOps::new_value] fn='{}' counter: {} -> {}, allocated: {:?}", + func.signature.name, before, func.next_value_id, id + ); } id } else { // Fallback (should never happen in practice) let id = self.parent_builder.value_gen.next(); if std::env::var("NYASH_LOOPFORM_DEBUG").is_ok() { - eprintln!("[LoopFormOps::new_value] FALLBACK value_gen, allocated: {:?}", id); + eprintln!( + "[LoopFormOps::new_value] FALLBACK value_gen, allocated: {:?}", + id + ); } id }; @@ -1126,13 +1254,16 @@ impl<'a> LoopFormOps for LoopBuilder<'a> { } } - fn get_block_predecessors(&self, block: BasicBlockId) -> std::collections::HashSet { + fn get_block_predecessors( + &self, + block: BasicBlockId, + ) -> std::collections::HashSet { // 📦 Hotfix 6: Get actual CFG predecessors for PHI validation if let Some(ref func) = self.parent_builder.current_function { if let Some(bb) = func.blocks.get(&block) { bb.predecessors.clone() } else { - std::collections::HashSet::new() // Non-existent blocks have no predecessors + std::collections::HashSet::new() // Non-existent blocks have no predecessors } } else { std::collections::HashSet::new() @@ -1154,8 +1285,12 @@ impl<'a> LoopFormOps for LoopBuilder<'a> { let is_param = self.parent_builder.is_value_parameter(value_id); if std::env::var("NYASH_LOOPFORM_DEBUG").is_ok() { - eprintln!("[is_parameter] ValueId({}) -> {} (kind = {:?})", - value_id.0, is_param, self.parent_builder.get_value_kind(value_id)); + eprintln!( + "[is_parameter] ValueId({}) -> {} (kind = {:?})", + value_id.0, + is_param, + self.parent_builder.get_value_kind(value_id) + ); } is_param @@ -1166,7 +1301,8 @@ impl<'a> LoopFormOps for LoopBuilder<'a> { } fn emit_copy(&mut self, dst: ValueId, src: ValueId) -> Result<(), String> { - self.parent_builder.emit_instruction(MirInstruction::Copy { dst, src }) + self.parent_builder + .emit_instruction(MirInstruction::Copy { dst, src }) } fn emit_jump(&mut self, target: BasicBlockId) -> Result<(), String> { @@ -1178,11 +1314,7 @@ impl<'a> LoopFormOps for LoopBuilder<'a> { dst: ValueId, inputs: Vec<(BasicBlockId, ValueId)>, ) -> Result<(), String> { - self.emit_phi_at_block_start( - self.current_block()?, - dst, - inputs - ) + self.emit_phi_at_block_start(self.current_block()?, dst, inputs) } fn update_phi_inputs( @@ -1191,7 +1323,8 @@ impl<'a> LoopFormOps for LoopBuilder<'a> { phi_id: ValueId, inputs: Vec<(BasicBlockId, ValueId)>, ) -> Result<(), String> { - self.parent_builder.update_phi_instruction(block, phi_id, inputs) + self.parent_builder + .update_phi_instruction(block, phi_id, inputs) } fn update_var(&mut self, name: String, value: ValueId) { diff --git a/src/mir/phi_core/exit_phi_builder.rs b/src/mir/phi_core/exit_phi_builder.rs index 1decfc78..dc9644bf 100644 --- a/src/mir/phi_core/exit_phi_builder.rs +++ b/src/mir/phi_core/exit_phi_builder.rs @@ -66,31 +66,31 @@ impl ExitPhiBuilder { /// let exit_builder = ExitPhiBuilder::new(body_builder); /// ``` pub fn new(body_local_builder: BodyLocalPhiBuilder) -> Self { - Self { - body_local_builder, - } + Self { body_local_builder } } - /// Build Exit PHIs + /// [LoopForm] Build Exit PHIs /// /// # Arguments /// * `ops` - LoopFormOps trait implementation /// * `exit_id` - Exit block ID /// * `header_id` - Loop header block ID - /// * `branch_source_block` - Branch source block ID (early exit path) + /// * `branch_source_block` - Branch source block ID (early exit path, Case A) /// * `header_vals` - Header variable values (parameter values) - /// * `exit_snapshots` - Exit predecessor snapshots + /// * `exit_snapshots` - Exit predecessor snapshots (from break statements) /// * `pinned_vars` - Pinned variable names (loop-invariant parameters) /// * `carrier_vars` - Carrier variable names (loop-modified variables) /// /// # Returns /// Result: Ok(()) on success, Err(msg) on failure /// - /// # Process - /// 1. Get exit predecessors (CFG validation) - /// 2. Filter phantom blocks + /// # [LoopForm] Process + /// 1. Get exit predecessors (CFG validation) - determines Case A/B + /// 2. Filter phantom blocks (Step 5-5-H) /// 3. Record definitions in inspector - /// 4. Generate PHI inputs using LoopSnapshotMergeBox::merge_exit_with_classification + /// 4. [LoopForm] Generate PHI inputs using LoopSnapshotMergeBox::merge_exit_with_classification + /// - Case A: header+break paths included + /// - Case B: break paths only (header not a predecessor) /// 5. For each variable, use PhiInputCollector to optimize and generate PHI nodes /// /// # Example @@ -119,18 +119,14 @@ impl ExitPhiBuilder { ) -> Result<(), String> { ops.set_current_block(exit_id)?; - // 1. Exit predecessorsを取得（CFG検証） + // [LoopForm] 1. Exit predecessorsを取得（CFG検証）- Case A/B判定のキー let exit_preds_set = ops.get_block_predecessors(exit_id); let exit_preds: Vec = exit_preds_set.iter().copied().collect(); - // 2. Phantom blockをフィルタリング - let filtered_snapshots = self.filter_phantom_blocks( - exit_snapshots, - &exit_preds_set, - ops, - ); + // [LoopForm] 2. Phantom blockをフィルタリング（Step 5-5-H） + let filtered_snapshots = self.filter_phantom_blocks(exit_snapshots, &exit_preds_set, ops); - // 3. Inspectorに定義を記録 + // [LoopForm] 3. Inspectorに定義を記録（変数分類の基盤） let inspector = self.body_local_builder.inspector_mut(); for pinned_name in pinned_vars { inspector.record_definition(pinned_name, header_id); @@ -145,7 +141,9 @@ impl ExitPhiBuilder { inspector.record_snapshot(branch_source_block, header_vals); } - // 4. LoopSnapshotMergeBoxでPHI入力を生成（static function call） + // [LoopForm] 4. LoopSnapshotMergeBoxでPHI入力を生成（static function call） + // - branch_source が exit pred のときだけ header snapshot を追加（Case A） + // - break-only の場合は header を除外（Case B） let all_vars = LoopSnapshotMergeBox::merge_exit_with_classification( header_id, header_vals, @@ -248,7 +246,11 @@ pub trait LoopFormOps { fn new_value(&mut self) -> ValueId; /// Emit PHI instruction - fn emit_phi(&mut self, phi_id: ValueId, inputs: Vec<(BasicBlockId, ValueId)>) -> Result<(), String>; + fn emit_phi( + &mut self, + phi_id: ValueId, + inputs: Vec<(BasicBlockId, ValueId)>, + ) -> Result<(), String>; /// Update variable binding fn update_var(&mut self, var_name: String, value_id: ValueId); @@ -305,7 +307,10 @@ mod tests { } fn get_block_predecessors(&self, block_id: BasicBlockId) -> HashSet { - self.predecessors.get(&block_id).cloned().unwrap_or_default() + self.predecessors + .get(&block_id) + .cloned() + .unwrap_or_default() } fn block_exists(&self, block_id: BasicBlockId) -> bool { @@ -318,7 +323,11 @@ mod tests { id } - fn emit_phi(&mut self, phi_id: ValueId, inputs: Vec<(BasicBlockId, ValueId)>) -> Result<(), String> { + fn emit_phi( + &mut self, + phi_id: ValueId, + inputs: Vec<(BasicBlockId, ValueId)>, + ) -> Result<(), String> { self.emitted_phis.push((phi_id, inputs)); Ok(()) } @@ -571,7 +580,7 @@ mod tests { let header_id = BasicBlockId(5); let exit_id = BasicBlockId(10); let branch_source = BasicBlockId(7); // Early break - let exit_pred1 = BasicBlockId(8); // After loop body + let exit_pred1 = BasicBlockId(8); // After loop body ops.add_block(header_id); ops.add_block(exit_id); @@ -590,7 +599,7 @@ mod tests { let mut snapshot1 = HashMap::new(); snapshot1.insert("s".to_string(), ValueId(1)); snapshot1.insert("idx".to_string(), ValueId(10)); // Modified - snapshot1.insert("ch".to_string(), ValueId(15)); // Body-local + snapshot1.insert("ch".to_string(), ValueId(15)); // Body-local let exit_snapshots = vec![(exit_pred1, snapshot1)]; let pinned_vars = vec!["s".to_string()]; diff --git a/src/mir/phi_core/loop_snapshot_merge.rs b/src/mir/phi_core/loop_snapshot_merge.rs index b77149b0..c82e329a 100644 --- a/src/mir/phi_core/loop_snapshot_merge.rs +++ b/src/mir/phi_core/loop_snapshot_merge.rs @@ -16,7 +16,7 @@ */ use crate::mir::{BasicBlockId, ValueId}; -use std::collections::{HashMap, BTreeSet, BTreeMap}; +use std::collections::{BTreeMap, BTreeSet, HashMap}; // Option C PHI bug fix: Use box-based classification use super::local_scope_inspector::LocalScopeInspectorBox; @@ -208,6 +208,15 @@ impl LoopSnapshotMergeBox { eprintln!("[Option C] carrier_vars: {:?}", carrier_vars); } + // [LoopForm] Case A/B 分岐: + // - header_id ∈ exit_preds → header fallthrough を exit PHI 入力に含める（Case A） + // - header_id ∉ exit_preds → break-only, header 由来の値を PHI に入れない（Case B） + // + // Header is a valid predecessor only when CFG actually branches to exit from the header. + // 例: loop(1 == 1) { ... break } のように header→exit のエッジが無い場合は、 + // header 値を exit PHI 入力に含めると「非支配ブロックからの値参照」で壊れる。 + let header_in_exit_preds = exit_preds.contains(&header_id); + // LoopVarClassBox でフィルタリング let classifier = LoopVarClassBox::new(); @@ -226,12 +235,16 @@ impl LoopSnapshotMergeBox { pinned_vars, carrier_vars, inspector, - exit_preds, // ← 実際のCFG predecessorsを使用！ + exit_preds, // ← 実際のCFG predecessorsを使用！ ); if debug { - eprintln!("[Option C] var '{}': {:?} needs_exit_phi={}", - var_name, class, class.needs_exit_phi()); + eprintln!( + "[Option C] var '{}': {:?} needs_exit_phi={}", + var_name, + class, + class.needs_exit_phi() + ); let defining_blocks = inspector.get_defining_blocks(&var_name); eprintln!("[Option C] defining_blocks: {:?}", defining_blocks); } @@ -244,9 +257,15 @@ impl LoopSnapshotMergeBox { if !class.needs_exit_phi() || !is_in_all_preds { if debug { if !class.needs_exit_phi() { - eprintln!("[Option C] → SKIP exit PHI for '{}' (class={:?})", var_name, class); + eprintln!( + "[Option C] → SKIP exit PHI for '{}' (class={:?})", + var_name, class + ); } else { - eprintln!("[Option C] → SKIP exit PHI for '{}' (NOT in all preds, class={:?})", var_name, class); + eprintln!( + "[Option C] → SKIP exit PHI for '{}' (NOT in all preds, class={:?})", + var_name, class + ); } } continue; @@ -256,18 +275,26 @@ impl LoopSnapshotMergeBox { let mut inputs: Vec<(BasicBlockId, ValueId)> = Vec::new(); // Header fallthrough（header に存在する変数のみ） - if let Some(&val) = header_vals.get(&var_name) { - inputs.push((header_id, val)); + if header_in_exit_preds { + if let Some(&val) = header_vals.get(&var_name) { + inputs.push((header_id, val)); + } + } else if debug { + eprintln!( + "[Option C] header not a predecessor → skip header input for '{}'", + var_name + ); } // Break snapshots for (bb, snap) in exit_snapshots { - // Step 5-5-H: CRITICAL - Skip phantom blocks from stale snapshots - // After loopform renumbering, snapshots may contain BlockIds that no longer exist. - // ONLY use snapshots from blocks that are actual CFG predecessors. + // Step 5-5-H: CRITICAL - Skip phantom exit preds if !exit_preds.contains(bb) { if debug { - eprintln!("[Option C] ⚠️ SKIP phantom exit pred (not in CFG): {:?} for var '{}'", bb, var_name); + eprintln!( + "[Option C] ⚠️ SKIP phantom exit pred (not in CFG): {:?} for var '{}'", + bb, var_name + ); } continue; }