refactor(phi): Phase 57 PHI code reduction
## Changes ### 57-2-alt: Remove redundant ConservativeMerge call - phi.rs called ConservativeMerge::analyze twice (once directly, once via merge_all_vars) - Now merge_all_vars returns changed_vars, eliminating redundancy ### 57-3: Delete PhiMergeOps trait (dead code, 17 lines) - PhiMergeOps trait was defined but never called - Only impl in loop_builder.rs was unused - PhiBuilderOps has replaced it ### 57-4: Add responsibility comments to infer_type_from_phi - Document that it's the "last resort" for type inference - Specify deletion conditions (JoinIR type annotations) ## Cumulative PHI reduction - Phase 38: 90 lines - Phase 40-4.1: 35 lines - Phase 41-1: 99 lines - Phase 47: 33 lines - Phase 57: 17 lines (PhiMergeOps) + optimization - Total: 365+ lines removed 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
nyash-codex
@ -21,18 +21,15 @@ impl MirBuilder {
|
||||
skip_var: Option<&str>,
|
||||
) -> Result<(), String> {
|
||||
// 📦 Phase 25.1q: Use PhiMergeHelper for unified PHI insertion
|
||||
// 📦 Phase 57: ConservativeMerge 冗長呼び出し削除
|
||||
// - 以前: ここで ConservativeMerge::analyze を呼び、merge_all_vars 内でも呼んでいた（2回）
|
||||
// - 現在: merge_all_vars が changed_vars を返すので、1回で済む
|
||||
use std::collections::HashSet;
|
||||
|
||||
let conservative = crate::mir::phi_core::conservative::ConservativeMerge::analyze(
|
||||
pre_if_snapshot,
|
||||
then_map_end,
|
||||
else_map_end_opt,
|
||||
);
|
||||
let changed_set: HashSet<String> = conservative.changed_vars.iter().cloned().collect();
|
||||
|
||||
// Use PhiMergeHelper for unified variable merging
|
||||
let mut helper =
|
||||
super::phi_merge::PhiMergeHelper::new(self, then_exit_block_opt, else_exit_block_opt);
|
||||
let changed_set: HashSet<String> =
|
||||
helper.merge_all_vars(pre_if_snapshot, then_map_end, else_map_end_opt, skip_var)?;
|
||||
|
||||
// Ensure pinned synthetic slots ("__pin$...") have a block-local definition at the merge,
|
||||
|
||||
@ -155,14 +155,19 @@ impl<'a> PhiMergeHelper<'a> {
|
||||
/// * `skip_var` - Optional variable name to skip (already merged elsewhere)
|
||||
///
|
||||
/// # Returns
|
||||
/// Ok(()) on success, Err(String) on failure
|
||||
/// Ok(changed_vars) - Set of variables that were changed, for pin handling
|
||||
///
|
||||
/// # Phase 57 改善
|
||||
///
|
||||
/// 戻り値を `()` から `HashSet<String>` に変更。
|
||||
/// これにより `phi.rs` での冗長な `ConservativeMerge::analyze` 呼び出しを削除可能に。
|
||||
pub fn merge_all_vars(
|
||||
&mut self,
|
||||
pre_if_snapshot: &BTreeMap<String, ValueId>, // Phase 25.1: BTreeMap化
|
||||
then_map_end: &BTreeMap<String, ValueId>, // Phase 25.1: BTreeMap化
|
||||
else_map_end_opt: &Option<BTreeMap<String, ValueId>>, // Phase 25.1: BTreeMap化
|
||||
skip_var: Option<&str>,
|
||||
) -> Result<(), String> {
|
||||
) -> Result<std::collections::HashSet<String>, String> {
|
||||
// Use Conservative strategy from conservative module
|
||||
let conservative = crate::mir::phi_core::conservative::ConservativeMerge::analyze(
|
||||
pre_if_snapshot,
|
||||
@ -232,6 +237,7 @@ impl<'a> PhiMergeHelper<'a> {
|
||||
self.merge_variable(name.clone(), then_v, else_v)?;
|
||||
}
|
||||
|
||||
Ok(())
|
||||
// Phase 57: 変更された変数セットを返す（phi.rsでの冗長呼び出し削除用）
|
||||
Ok(conservative.changed_vars)
|
||||
}
|
||||
}
|
||||
|
||||
@ -1068,35 +1068,10 @@ impl<'a> LoopBuilder<'a> {
|
||||
);
|
||||
|
||||
// Phase 26-E: PhiBuilderBox 統合
|
||||
// Ops構造体: PhiMergeOps（Legacy）と PhiBuilderOps（新）の両対応
|
||||
// Phase 57: PhiMergeOps impl 削除（デッドコード、2025-11-29）
|
||||
// - PhiBuilderOps に統一され、PhiMergeOps は不要になった
|
||||
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 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)],
|
||||
) {
|
||||
if let Some(ref func) = self.0.parent_builder.current_function {
|
||||
crate::mir::phi_core::common::debug_verify_phi_inputs(func, merge_bb, inputs);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Phase 26-E: PhiBuilderOps trait 実装（箱理論統一）
|
||||
impl<'b, 'a> crate::mir::phi_core::phi_builder_box::PhiBuilderOps for Ops<'b, 'a> {
|
||||
fn new_value(&mut self) -> ValueId {
|
||||
|
||||
@ -36,10 +36,16 @@
|
||||
* - **置換**: conservative.rs::ConservativeMerge::analyze 内にインライン化
|
||||
* - **理由**: callsite 1箇所のみ、if_phi.rs責務縮小
|
||||
*
|
||||
* ## ✅ Phase 57で削除済み（デッドコード、17行）
|
||||
*
|
||||
* - `PhiMergeOps` trait (17行) → **削除完了 2025-11-29**
|
||||
* - **理由**: 完全にデッドコード（trait定義あるがメソッド呼び出しゼロ）
|
||||
* - **置換**: PhiBuilderOps に統一済み
|
||||
*
|
||||
* ## 現在の状態
|
||||
*
|
||||
* - **if_phi.rs**: 321行 → 288行（Phase 47後、33行削減）
|
||||
* - **累計削減**: 348行（Phase 38: 90行, Phase 40-4.1: 35行, Phase 41-1: 99行, Phase 47: 33行）+ conservative 48行
|
||||
* - **if_phi.rs**: 288行 → 271行（Phase 57後、17行削減）
|
||||
* - **累計削減**: 365行（Phase 38: 90行, Phase 40-4.1: 35行, Phase 41-1: 99行, Phase 47: 33行, Phase 57: 17行）+ conservative 48行
|
||||
*
|
||||
* ## 参照ドキュメント
|
||||
*
|
||||
@ -55,6 +61,28 @@ use std::collections::BTreeMap; // Phase 25.1: 決定性確保
|
||||
|
||||
/// Infer return type by scanning for a Phi that defines `ret_val` and
|
||||
/// verifying that all incoming values have the same type in `types`.
|
||||
///
|
||||
/// # Phase 57 責務明示（削除予定だが現時点では保持）
|
||||
///
|
||||
/// ## 責務
|
||||
///
|
||||
/// 型情報が Unknown のブランチを最低限救う「最後の砦」。
|
||||
/// PHI 命令の incoming values から統一された型を推論する。
|
||||
///
|
||||
/// ## 使用箇所
|
||||
///
|
||||
/// - `lifecycle.rs:281, 296` - 関数の return 型推論
|
||||
///
|
||||
/// ## 将来の削除条件
|
||||
///
|
||||
/// JoinIR 側に型情報（`Option<MirType>`）を持たせた後、
|
||||
/// この関数は不要になる。現時点では lifecycle.rs が依存しているため保持。
|
||||
///
|
||||
/// ## 削除時の移行先
|
||||
///
|
||||
/// - JoinIR の型アノテーション（未実装）
|
||||
/// - または MIR Builder の型伝播強化
|
||||
///
|
||||
pub fn infer_type_from_phi(
|
||||
function: &MirFunction,
|
||||
ret_val: ValueId,
|
||||
@ -230,23 +258,23 @@ fn extract_vars_from_json_stmt(
|
||||
// - ロジックをconservative.rs内にインライン化完了
|
||||
// - if_phi.rsの責務縮小（JoinIR移行準備）
|
||||
|
||||
/// Operations required for emitting a PHI or direct binding at a merge point.
|
||||
pub trait PhiMergeOps {
|
||||
fn new_value(&mut self) -> ValueId;
|
||||
fn emit_phi_at_block_start(
|
||||
&mut self,
|
||||
block: crate::mir::BasicBlockId,
|
||||
dst: ValueId,
|
||||
inputs: Vec<(crate::mir::BasicBlockId, ValueId)>,
|
||||
) -> Result<(), String>;
|
||||
fn update_var(&mut self, name: String, value: ValueId);
|
||||
fn debug_verify_phi_inputs(
|
||||
&mut self,
|
||||
_merge_bb: crate::mir::BasicBlockId,
|
||||
_inputs: &[(crate::mir::BasicBlockId, ValueId)],
|
||||
) {
|
||||
}
|
||||
}
|
||||
// ========================================
|
||||
// Phase 57: PhiMergeOps trait 削除（2025-11-29）
|
||||
// ========================================
|
||||
// 削除日: 2025-11-29
|
||||
// 削除行数: 17行
|
||||
// 理由: 完全にデッドコード
|
||||
// - trait定義はあったが、どのメソッドも呼ばれていない
|
||||
// - 唯一のimpl（loop_builder.rs）もメソッドが使われていなかった
|
||||
// - PhiBuilderOps に統一され、PhiMergeOps は不要に
|
||||
//
|
||||
// 旧定義:
|
||||
// pub trait PhiMergeOps {
|
||||
// fn new_value(&mut self) -> ValueId;
|
||||
// fn emit_phi_at_block_start(...);
|
||||
// fn update_var(...);
|
||||
// fn debug_verify_phi_inputs(...);
|
||||
// }
|
||||
|
||||
// ========================================
|
||||
// Phase 41-1削除済み（2025-11-29）
|
||||
|
||||
Reference in New Issue
Block a user