tomoaki/hakorune
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor(joinir): Phase 255 P2 - loop_invariants + var() unification + Phase 256 prep

Browse Source 
## Task A: Loop Invariants Architecture (Option A - Boundary Extension)

Introduced explicit loop_invariants concept for variables that are:
- Used in loop body but never modified (e.g., substring needle in index_of)
- Need header PHI (all iterations use same value)
- Do NOT need exit PHI (not a LoopState)

Implementation:
- Added `loop_invariants: Vec<(String, ValueId)>` field to JoinInlineBoundary
- Added `with_loop_invariants()` method to JoinInlineBoundaryBuilder
- Modified Pattern 6 to use loop_invariants instead of ConditionOnly misuse
  * s (haystack) and ch (needle) now properly classified
  * exit_bindings simplified to only LoopState carriers
- Extended LoopHeaderPhiBuilder to generate invariant PHIs
  * Entry PHI: from host
  * Latch PHI: self-reference (same value every iteration)
- Updated instruction_rewriter to handle invariant latch incoming

Files Modified:
- src/mir/join_ir/lowering/inline_boundary.rs (structure + builder)
- src/mir/join_ir/lowering/inline_boundary_builder.rs (builder method)
- src/mir/builder/control_flow/joinir/patterns/pattern6_scan_with_init.rs
- src/mir/builder/control_flow/joinir/merge/loop_header_phi_builder.rs
- src/mir/builder/control_flow/joinir/merge/mod.rs
- src/mir/builder/control_flow/joinir/merge/instruction_rewriter.rs

## Task B: Code Quality - var() Helper Unification

Created common module to eliminate var() duplication:
- New module: src/mir/builder/control_flow/joinir/patterns/common/
- Centralized var() helper in ast_helpers.rs
- Updated Pattern 6 and Pattern 2 to use common var()
- Test code (5 occurrences) deferred to Phase 256+ per 80/20 rule

Result: Eliminated 2 duplicate var() functions, 5 test occurrences remain

Files Created:
- src/mir/builder/control_flow/joinir/patterns/common/ast_helpers.rs
- src/mir/builder/control_flow/joinir/patterns/common/mod.rs

Files Modified:
- src/mir/builder/control_flow/joinir/patterns/mod.rs
- src/mir/builder/control_flow/joinir/patterns/pattern6_scan_with_init.rs
- src/mir/builder/control_flow/joinir/patterns/policies/balanced_depth_scan_policy.rs

## Task C: Documentation - PostLoopEarlyReturnPlan Usage Examples

Enhanced post_loop_early_return_plan.rs with:
- Architecture explanation (exit PHI usage prevents DCE)
- Pattern 2 example (Less: balanced_depth_scan)
- Pattern 6 example (NotEqual: index_of)
- Builder defer decision: when 4+ patterns emerge, add builder

Files Modified:
- src/mir/builder/control_flow/joinir/patterns/policies/post_loop_early_return_plan.rs

## Task D: Phase 256 Preparation

Analyzed next failure from smoke tests:
- `StringUtils.split/2` with variable-step loop
- Not constant step (i += len or similar)
- Three implementation options identified

Created Phase 256 README with:
- Minimal reproduction code
- Root cause analysis
- Implementation options with trade-offs
- Clear next steps

Files Created:
- docs/development/current/main/phases/phase-256/README.md

## Verification Results

✅ All tests passing:
- pattern254_p0_index_of_vm.sh: PASS
- No regression in Pattern 1-5
- Smoke test progresses past json_lint_vm to next failure

✅ Architecture achievements:
- Semantic clarity: loop_invariants vs exit_bindings properly separated
- ConditionOnly misuse eliminated
- PHI generation correct for all carrier types
- Code quality: var() duplication reduced
- Next phase clearly defined

## Summary

Phase 255 P2 achieves root-cause fix for multi-param loop support:
- Boundary concept expanded (loop_invariants field)
- Pattern 6 architecture corrected (no ConditionOnly misuse)
- Code quality improved (var() centralized)
- Next pattern (variable-step) is now the challenge

✨ Box-first principles maintained: clean separation, explicit roles, minimal coupling

🧠 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
This commit is contained in:
tomoaki
2025-12-19 23:48:49 +09:00
parent 2d9c6ea3c6
commit 575a5d750f
12 changed files with 408 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
src/mir/builder/control_flow/joinir/patterns/common/ast_helpers.rs Normal file
View File

@ -0,0 +1,25 @@
//! Phase 255 P2: Common AST helpers for pattern lowering
use crate::ast::{ASTNode, Span};
/// Create Variable ASTNode with unknown span
///
/// # Phase 255 P2
///
/// This helper function eliminates duplicate var() implementations across
/// pattern lowering code. Previously, there were 7 copies of this function
/// scattered across different files.
///
/// # Usage
///
/// ```ignore
/// use super::common::var;
///
/// let i_var = var("i"); // ASTNode::Variable { name: "i", span: Span::unknown() }
/// ```
pub(crate) fn var(name: &str) -> ASTNode {
ASTNode::Variable {
name: name.to_string(),
span: Span::unknown(),
}
}

8
src/mir/builder/control_flow/joinir/patterns/common/mod.rs Normal file
View File

@ -0,0 +1,8 @@
//! Phase 255 P2: Common utilities for JoinIR pattern lowering
//!
//! This module provides shared helper functions used across different pattern
//! lowering implementations, eliminating code duplication and ensuring consistency.
mod ast_helpers;
pub(crate) use ast_helpers::var;

4
src/mir/builder/control_flow/joinir/patterns/mod.rs
View File

@ -48,7 +48,11 @@
//! Phase 93/94: Pattern Policies
//! - policies/: Pattern recognition and routing decision (future expansion)
//! - Currently a placeholder directory for future policy box organization
//!
//! Phase 255 P2: Common Utilities
//! - common/: Shared helper functions (var() etc.) to eliminate code duplication
pub(in crate::mir::builder) mod common; // Phase 255 P2: Common AST helpers
pub(in crate::mir::builder) mod ast_feature_extractor;
pub(in crate::mir::builder) mod policies; // Phase 93/94: Pattern routing policies (future expansion)
pub(in crate::mir::builder) mod body_local_policy; // Phase 92 P3: promotion vs slot routing

75
src/mir/builder/control_flow/joinir/patterns/pattern6_scan_with_init.rs
View File

@ -34,6 +34,7 @@
//! - Need to hoist MethodCall to init phase
use super::super::trace;
use super::common::var; // Phase 255 P2: Use shared var() helper
use crate::ast::{ASTNode, BinaryOperator, LiteralValue};
use crate::mir::builder::MirBuilder;
use crate::mir::ValueId;
@ -413,37 +414,29 @@ impl MirBuilder {
let mut join_value_space = JoinValueSpace::new();
let join_module = lower_scan_with_init_minimal(&mut join_value_space);
// Phase 255 P0: Build CarrierInfo for multi-param loop
// Step 1: Create CarrierInfo with 3 variables (s, ch, i)
use crate::mir::join_ir::lowering::carrier_info::{CarrierInfo, CarrierVar, CarrierRole};
let carriers = vec![
// s: haystack (ConditionOnly - header PHI only, no exit PHI)
CarrierVar::with_role(
parts.haystack.clone(),
s_host,
CarrierRole::ConditionOnly,
),
// ch: needle (ConditionOnly - header PHI only, no exit PHI)
CarrierVar::with_role(
parts.needle.clone(),
ch_host,
CarrierRole::ConditionOnly,
),
];
// Phase 255 P2: Build CarrierInfo for loop variable only
// Step 1: Create CarrierInfo with loop variable (i) only
// s and ch are now loop_invariants (not carriers)
use crate::mir::join_ir::lowering::carrier_info::{CarrierInfo, CarrierRole};
let carrier_info = CarrierInfo::with_carriers(
parts.loop_var.clone(), // loop_var_name: "i"
i_host, // loop_var_id (LoopState - header PHI + exit PHI)
carriers, // s, ch only (i is handled as loop_var)
vec![], // Empty carriers - only loop_var
);
// Phase 255 P2: Create loop_invariants for s and ch
let loop_invariants = vec![
(parts.haystack.clone(), s_host), // s: haystack
(parts.needle.clone(), ch_host), // ch: needle
];
if debug {
trace.debug(
"pattern6/lower",
&format!(
"Phase 255 P0: CarrierInfo with {} carriers (s, ch: ConditionOnly, i: LoopState)",
carrier_info.carrier_count()
"Phase 255 P2: CarrierInfo with loop_var only (i: LoopState), {} loop_invariants (s, ch)",
loop_invariants.len()
),
);
}
@ -460,8 +453,8 @@ impl MirBuilder {
];
// Step 3: Build exit_bindings manually
// CRITICAL: ALL carriers (ConditionOnly + LoopState) must be in exit_bindings
// for latch incoming collection, even though ConditionOnly don't participate in exit PHI
// Phase 255 P2: Only LoopState variables (i) need exit bindings
// Loop invariants (s, ch) do NOT need exit bindings
use crate::mir::join_ir::lowering::inline_boundary::LoopExitBinding;
let k_exit_func = join_module.require_function("k_exit", "Pattern 6");
@ -478,37 +471,21 @@ impl MirBuilder {
role: CarrierRole::LoopState,
};
// Phase 255 P0: Add ConditionOnly carriers in alphabetical order [ch, s]
// They don't participate in exit PHI, but need latch incoming collection
let ch_exit_binding = LoopExitBinding {
carrier_name: parts.needle.clone(),
join_exit_value: ValueId(0), // Placeholder - not used for ConditionOnly
host_slot: ch_host,
role: CarrierRole::ConditionOnly,
};
let s_exit_binding = LoopExitBinding {
carrier_name: parts.haystack.clone(),
join_exit_value: ValueId(0), // Placeholder - not used for ConditionOnly
host_slot: s_host,
role: CarrierRole::ConditionOnly,
};
// CRITICAL: Order must match tail call args order: [i, ch, s] (alphabetical)
// Phase 255 P0: loop_step tail call: args = vec![i_plus_1, ch_step_param, s_step_param]
// The loop variable i is first (args[0]), then carriers in alphabetical order (args[1], args[2])
let exit_bindings = vec![i_exit_binding, ch_exit_binding, s_exit_binding];
// Phase 255 P2: Only i (LoopState) in exit_bindings
// s and ch are loop_invariants, not carriers
let exit_bindings = vec![i_exit_binding];
if debug {
trace.debug(
"pattern6/lower",
&format!("Phase 255 P0: Generated {} exit_bindings", exit_bindings.len()),
&format!("Phase 255 P2: Generated {} exit_bindings (i only)", exit_bindings.len()),
);
}
// Step 4: Build boundary with carrier_info
// Step 4: Build boundary with carrier_info and loop_invariants
let boundary = JoinInlineBoundaryBuilder::new()
.with_inputs(join_inputs, host_inputs)
.with_loop_invariants(loop_invariants) // Phase 255 P2: Add loop invariants
.with_exit_bindings(exit_bindings)
.with_loop_var_name(Some(parts.loop_var.clone()))
.with_carrier_info(carrier_info.clone()) // ✅ Key: carrier_info for multi-PHI
@ -581,13 +558,7 @@ impl MirBuilder {
}
}
/// Phase 255 P1: Helper function to create Variable ASTNode
fn var(name: &str) -> ASTNode {
ASTNode::Variable {
name: name.to_string(),
span: crate::ast::Span::unknown(),
}
}
// Phase 255 P2: var() function removed - now using super::common::var
#[cfg(test)]
mod tests {

8
src/mir/builder/control_flow/joinir/patterns/policies/balanced_depth_scan_policy.rs
View File

@ -6,6 +6,7 @@
//! - Fail-fast with tagged reasons when the shape is close but unsupported
use crate::ast::{ASTNode, BinaryOperator, LiteralValue, Span};
use crate::mir::builder::control_flow::joinir::patterns::common::var; // Phase 255 P2: Use shared var() helper
use crate::mir::join_ir::lowering::common::balanced_depth_scan_emitter::BalancedDepthScanRecipe;
use crate::mir::join_ir::lowering::error_tags;
use crate::mir::join_ir::lowering::loop_update_analyzer::{UpdateExpr, UpdateRhs};
@ -449,12 +450,7 @@ fn is_var_plus_int(node: &ASTNode, var_name: &str, n: i64) -> bool {
)
}
fn var(name: &str) -> ASTNode {
ASTNode::Variable {
name: name.to_string(),
span: Span::unknown(),
}
}
// Phase 255 P2: var() function removed - now using crate::mir::builder::control_flow::joinir::patterns::common::var
fn eq_str(left: ASTNode, s: &str) -> ASTNode {
ASTNode::BinaryOp {

56
src/mir/builder/control_flow/joinir/patterns/policies/post_loop_early_return_plan.rs
View File

@ -1,15 +1,67 @@
//! Post-loop early return plan (policy-level SSOT)
//! Phase 255 P2: Post-loop early return plan (policy-level SSOT)
//!
//! # Responsibility
//!
//! Responsibility:
//! - Describe a post-loop guard that emulates an in-loop `return` without making
//! Pattern2 lowering itself return-in-loop aware.
//! - Keep the plan policy-agnostic so multiple Pattern2 families can reuse it.
//!
//! # Architecture
//!
//! Ensures exit PHI values are used (prevents DCE elimination). The post-loop
//! guard creates a conditional that references the exit PHI value, forcing it
//! to be live and preventing dead code elimination.
//!
//! # Usage Patterns
//!
//! ## Pattern 2: Less Than (balanced_depth_scan)
//!
//! Used in `json_cur.find_balanced_*` family functions.
//!
//! ```ignore
//! PostLoopEarlyReturnPlan {
//! cond: BinaryOp { Less, var("i"), var("n") },
//! ret_expr: var("i"),
//! }
//! ```
//!
//! Generated post-loop guard:
//! ```nyash
//! if i < n {
//! return i
//! }
//! ```
//!
//! ## Pattern 6: Not Equal (index_of)
//!
//! Used in `StringUtils.index_of` and similar search functions.
//!
//! ```ignore
//! PostLoopEarlyReturnPlan {
//! cond: BinaryOp { NotEqual, var("i"), Literal(-1) },
//! ret_expr: var("i"),
//! }
//! ```
//!
//! Generated post-loop guard:
//! ```nyash
//! if i != -1 {
//! return i
//! }
//! ```
//!
//! # Builder Decision
//!
//! Deferred to Phase 256+. Currently 2 patterns use this (Pattern 2 and Pattern 6).
//! Direct construction is acceptable. Will create builder when 4+ patterns use it.
use crate::ast::ASTNode;
#[derive(Debug, Clone)]
pub(crate) struct PostLoopEarlyReturnPlan {
/// Condition for the post-loop guard (e.g., `i < n` or `i != -1`)
pub cond: ASTNode,
/// Expression to return if condition is true (e.g., `var("i")`)
pub ret_expr: ASTNode,
}