From bfbc9b26bf293689a7d377992f93a863327bdf54 Mon Sep 17 00:00:00 2001 From: tomoaki Date: Tue, 23 Dec 2025 08:14:27 +0900 Subject: [PATCH] fix(joinir): Phase 283 P0 - Pattern3 Undefined ValueId Bug Fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix: extract_if_condition() moved after local_cond_env construction (loop_with_if_phi_if_sum.rs:175) - Root cause: condition extraction before i_param/sum_param creation - Result: i % 2 referenced caller's ConditionEnv with unmapped ValueId - Fail-Fast: Add condition_bindings validation in merge (mod.rs) - Fixture: Update loop_if_phi.hako for C2 compatibility (sum.toString()) - Verified: VM execution outputs sum=9 ✅ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 --- apps/tests/loop_if_phi.hako | 3 +- docs/development/current/main/10-Now.md | 11 ++--- docs/development/current/main/30-Backlog.md | 15 +++--- .../current/main/phases/phase-283/README.md | 47 +++++++++++++++++++ .../builder/control_flow/joinir/merge/mod.rs | 44 ++++++++++++++--- .../lowering/loop_with_if_phi_if_sum.rs | 25 ++++++---- 6 files changed, 114 insertions(+), 31 deletions(-) create mode 100644 docs/development/current/main/phases/phase-283/README.md diff --git a/apps/tests/loop_if_phi.hako b/apps/tests/loop_if_phi.hako index e5d92d54..a729c709 100644 --- a/apps/tests/loop_if_phi.hako +++ b/apps/tests/loop_if_phi.hako @@ -7,8 +7,7 @@ static box Main { if (i % 2 == 1) { sum = sum + i } else { sum = sum + 0 } i = i + 1 } - console.println("sum=" + sum) + console.println("sum=" + sum.toString()) return 0 } } - diff --git a/docs/development/current/main/10-Now.md b/docs/development/current/main/10-Now.md index 28b252cc..9e7749dd 100644 --- a/docs/development/current/main/10-Now.md +++ b/docs/development/current/main/10-Now.md @@ -2,16 +2,15 @@ ## Current Focus (next) -- Phase 281（design-first）: `docs/development/current/main/phases/phase-281/README.md` - - P1 planned: Pattern6（early-exit）を compose に寄せる（挙動不変で段階移行） - - SSOT: `docs/development/current/main/design/edgecfg-fragments.md`, `src/mir/builder/control_flow/edgecfg/api/compose.rs` -- Phase 282（planned, design-first）: Router shrinkage - - pattern番号を “症状ラベル（テスト名）” に降格し、合成SSOTへ寄せた後に router を縮退させる +- Phase 283（bugfix）: JoinIR if-condition remap fix: `docs/development/current/main/phases/phase-283/README.md` + - 目的: Pattern3（if-sum）で `i % 2 == 1` が undefined `ValueId` を生成する不具合を根治する + - SSOT: `src/mir/join_ir/lowering/loop_with_if_phi_if_sum.rs`（ConditionEnv の正しい使用位置） ## Recently Completed (2025-12-23) +- Phase 282（Router shrinkage + extraction-based migration P0–P5）: `docs/development/current/main/phases/phase-282/README.md` - Phase 280（Frag composition SSOT positioning）: `docs/development/current/main/phases/phase-280/README.md` -- Phase 281 P0（Pattern7 → compose::if_）: `docs/development/current/main/phases/phase-281/README.md` +- Phase 281（Pattern6/7 compose adoption, P0–P3）: `docs/development/current/main/phases/phase-281/README.md` - Phase 273（Plan line SSOT）: `docs/development/current/main/phases/phase-273/README.md` - Phase 275 P0（A1/B2/C2 coercion SSOT）: `docs/development/current/main/phases/phase-275/README.md` - Phase 276 P0（quick wins / type_helper SSOT）: `docs/development/current/main/phases/phase-276/README.md` diff --git a/docs/development/current/main/30-Backlog.md b/docs/development/current/main/30-Backlog.md index f6728d97..2ec08248 100644 --- a/docs/development/current/main/30-Backlog.md +++ b/docs/development/current/main/30-Backlog.md @@ -8,12 +8,9 @@ Related: ## 直近（JoinIR/selfhost） -- **Phase 281（planned）: Pattern6 を compose へ段階吸収** - - 入口: `docs/development/current/main/phases/phase-281/README.md` - - P0（Pattern7）✅ 完了、次は P1（Pattern6 early-exit） - -- **Phase 282（planned）: Router shrinkage** - - 目的: pattern番号を “症状ラベル” に縮退させ、合成SSOTへ寄せた後に router の分岐を減らす +- **Phase 282（planned, design-first）: Router shrinkage** + - 入口: `docs/development/current/main/phases/phase-282/README.md` + - 目的: pattern番号を “症状ラベル” に縮退し、router の責務を「抽出の配線」へ収束させる （✅ done）**Phase 279 P0**: Type propagation pipeline SSOT 統一（lifecycle / JoinIR / LLVM の二重化解消） - 完了: `docs/development/current/main/phases/phase-279/README.md` @@ -27,6 +24,12 @@ Related: - （✅ done）**Phase 273**: Plan line SSOT（Pattern6/7） - 完了: `docs/development/current/main/phases/phase-273/README.md` +- （✅ done）**Phase 280**: Frag composition SSOT positioning + - 完了: `docs/development/current/main/phases/phase-280/README.md` + +- （✅ done）**Phase 281**: compose adoption（Pattern6/7, P0–P3） + - 完了: `docs/development/current/main/phases/phase-281/README.md` + - （✅ done）**Phase 277/278**: PHI strict + env var 収束 - 完了: `docs/development/current/main/phases/phase-277/README.md` - 完了: `docs/development/current/main/phases/phase-278/README.md` diff --git a/docs/development/current/main/phases/phase-283/README.md b/docs/development/current/main/phases/phase-283/README.md new file mode 100644 index 00000000..09eab6b3 --- /dev/null +++ b/docs/development/current/main/phases/phase-283/README.md @@ -0,0 +1,47 @@ +# Phase 283: JoinIR If-Condition ValueId Remap Fix + +Status: **P0 ✅ complete (2025-12-23)** + +Goal: +- Fix a Pattern3 (if-sum) JoinIR lowering bug where complex if-conditions like `i % 2 == 1` could produce MIR that uses an undefined `ValueId`. + +SSOT References: +- JoinIR lowering (if-sum): `src/mir/join_ir/lowering/loop_with_if_phi_if_sum.rs` +- Condition value lowering: `src/mir/join_ir/lowering/condition_lowerer.rs` +- MIR verification (undefined values): `src/mir/verification/ssa.rs` +- Test fixture: `apps/tests/loop_if_phi.hako` + +## Problem + +Observed error (VM): +- `use of undefined value ValueId(...)` inside the loop body, originating from the modulo expression in the if-condition. + +The symptom appeared in `apps/tests/loop_if_phi.hako`: +```hako +if (i % 2 == 1) { ... } else { ... } +``` + +## Root Cause + +In `lower_if_sum_pattern`, the if-condition was extracted (and its JoinIR value-expression lowered) **before** `i_param` / `sum_param` existed, so `Variable("i")` inside complex expressions resolved through the caller-provided `ConditionEnv`. + +That early `ConditionEnv` mapping could not be remapped by the boundary/merger, resulting in JoinIR ValueIds that became undefined host ValueIds in MIR. + +## Fix (P0) + +- Move `extract_if_condition(...)` so it runs **after** `local_cond_env` is constructed and shadowed: + - `local_cond_env.insert(loop_var, i_param)` + - `local_cond_env.insert(update_var, sum_param)` + - (and any condition bindings remapped to `main_params`) + +This ensures complex expressions like `i % 2` resolve `i` to `i_param` (a loop_step param that the merger remaps), preventing undefined ValueIds. + +## Fixture Note (Coercion SSOT) + +After Phase 275 (C2), implicit `"String" + Integer` is a TypeError. The fixture uses: +- `sum.toString()` instead of `"sum=" + sum` or `str(sum)`. + +## Acceptance + +- `apps/tests/loop_if_phi.hako` runs on VM without undefined ValueId errors and prints `sum=9`. + diff --git a/src/mir/builder/control_flow/joinir/merge/mod.rs b/src/mir/builder/control_flow/joinir/merge/mod.rs index 0b712180..cdab3357 100644 --- a/src/mir/builder/control_flow/joinir/merge/mod.rs +++ b/src/mir/builder/control_flow/joinir/merge/mod.rs @@ -239,7 +239,24 @@ pub(in crate::mir::builder) fn merge_joinir_mir_blocks( .flat_map(|params| params.iter().copied()) .collect(); + // Phase 283 P0 DEBUG: Log condition_bindings count + trace.stderr_if( + &format!( + "[cf_loop/joinir] Phase 283 P0 DEBUG: Processing {} condition_bindings", + boundary.condition_bindings.len() + ), + debug, + ); + for binding in &boundary.condition_bindings { + trace.stderr_if( + &format!( + "[cf_loop/joinir] Phase 283 P0 DEBUG: Checking binding '{}' join={:?}", + binding.name, binding.join_value + ), + debug, + ); + if all_params.contains(&binding.join_value) { trace.stderr_if( &format!( @@ -249,14 +266,27 @@ pub(in crate::mir::builder) fn merge_joinir_mir_blocks( debug, ); } else { - trace.stderr_if( - &format!( - "[cf_loop/joinir] Phase 171-fix: Adding condition binding '{}' JoinIR {:?} to used_values", + // Phase 283 P0 FIX: Ensure remapper has valid mapping (Fail-Fast) + if let Some(host_id) = builder.variable_ctx.variable_map.get(&binding.name) { + // Variable exists in host context - map join_value to existing host_id + trace.stderr_if( + &format!( + "[cf_loop/joinir] Phase 283 P0: ✅ Condition binding '{}' JoinIR {:?} → host {:?}", + binding.name, binding.join_value, host_id + ), + debug, + ); + remapper.set_value(binding.join_value, *host_id); + used_values.insert(binding.join_value); + } else { + // Fail-Fast: No host ValueId found → surface root cause immediately + return Err(format!( + "[merge/phase2.1] Condition variable '{}' (join={:?}) has no host ValueId in variable_map. \ + This indicates the value was not properly supplied by boundary builder or cond_env. \ + Check: (1) boundary builder supplies all condition vars, (2) cond_env correctly tracks host ValueIds.", binding.name, binding.join_value - ), - debug, - ); - used_values.insert(binding.join_value); + )); + } } } diff --git a/src/mir/join_ir/lowering/loop_with_if_phi_if_sum.rs b/src/mir/join_ir/lowering/loop_with_if_phi_if_sum.rs index 98df3493..fba2b2d4 100644 --- a/src/mir/join_ir/lowering/loop_with_if_phi_if_sum.rs +++ b/src/mir/join_ir/lowering/loop_with_if_phi_if_sum.rs @@ -102,16 +102,6 @@ pub fn lower_if_sum_pattern( &format!("{} {:?} ValueId({})", loop_var, loop_op, loop_limit_val.0) ); - // Step 2: Extract if condition info (e.g., i > 0 → var="i", op=Gt, value=ValueId) - // Phase 220-D: Now returns ValueId and instructions - // Phase 242-EX-A: Now supports complex LHS - let (if_var, if_op, if_lhs_val, if_value_val, if_value_insts) = - extract_if_condition(if_stmt, &mut alloc_value, cond_env)?; - trace.log( - "if-cond", - &format!("{} {:?} ValueId({})", if_var, if_op, if_value_val.0) - ); - // Step 3: Extract then-branch update (e.g., sum = sum + 1 → var="sum", addend=) // Phase 256.7: update_addend is now ASTNode (supports variables like separator) let (update_var, update_addend_ast) = extract_then_update(if_stmt)?; @@ -182,6 +172,21 @@ pub fn lower_if_sum_pattern( } let cond_env = &local_cond_env; // Shadow the original cond_env + // Step 2: Extract if condition info (e.g., i > 0 → var="i", op=Gt, value=ValueId) + // + // IMPORTANT (Phase 283 P0): Extract using the local_cond_env. + // The pre-loop extraction phase runs before `i_param`/`sum_param` exist, so + // `i % 2 == 1` would resolve `i` via the caller's ConditionEnv and produce + // JoinIR ValueIds that the boundary cannot remap → undefined ValueId in MIR. + // + // By extracting here, Variable("i") resolves to `i_param` and remains remappable. + let (if_var, if_op, if_lhs_val, if_value_val, if_value_insts) = + extract_if_condition(if_stmt, &mut alloc_value, cond_env)?; + trace.log( + "if-cond", + &format!("{} {:?} ValueId({})", if_var, if_op, if_value_val.0) + ); + // === main() function === let mut main_func = JoinFunction::new(main_id, "main".to_string(), main_params.clone());