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

feat(joinir): Phase 135 P0 - ConditionLoweringBox allocator SSOT (ValueId collision fix)

Browse Source 
## Summary
Root cause: ConditionLoweringBox was bypassing ConditionContext.alloc_value (SSOT allocator),
causing ValueId collisions between JoinIR condition params and lowered instructions.

## Changes
1. **ConditionLoweringBox (expr_lowerer.rs)**: Must use ConditionContext.alloc_value
   - Pass &mut ConditionContext to lower_condition (SSOT allocator)
   - Eliminates internal counter usage

2. **Allocator unification (condition_lowerer.rs, method_call_lowerer.rs)**:
   - Accept &mut dyn FnMut() -> ValueId as allocator parameter
   - Ensures all lowering paths use same SSOT allocator

3. **Boundary Copy deduplication (joinir_inline_boundary_injector.rs)**:
   - Deduplicate condition_bindings by dst
   - Fail-Fast if different sources target same dst (MIR SSA violation)

4. **Trim pattern fixes (trim_loop_lowering.rs, trim_pattern_validator.rs, stmts.rs)**:
   - Use builder.next_value_id() instead of value_gen.next() in function context
   - Ensures function-level ValueId allocation respects reserved PHI dsts

## Acceptance
✅ ./target/release/hakorune --verify apps/tests/phase133_json_skip_whitespace_min.hako
✅ Smoke: phase135_trim_mir_verify.sh - MIR SSA validation PASS
✅ Regression: phase132_exit_phi_parity.sh - 3/3 PASS
✅ Regression: phase133_json_skip_whitespace_llvm_exe.sh - compile-only PASS

🤖 Generated with Claude Code

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
This commit is contained in:
nyash-codex
2025-12-15 18:49:08 +09:00
parent e8e4779942
commit d82c332a40
15 changed files with 186 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
src/mir/builder/control_flow/joinir/patterns/trim_loop_lowering.rs
View File

@ -363,7 +363,8 @@ impl TrimLoopLowerer {
use crate::mir::instruction::MirInstruction;
use crate::mir::types::BinaryOp;
let one = emit_integer(builder, 1);
let start_plus_1 = builder.value_gen.next();
// Phase 135 P0: Use function-level ValueId (SSOT)
let start_plus_1 = builder.next_value_id();
builder.emit_instruction(MirInstruction::BinOp {
dst: start_plus_1,
op: BinaryOp::Add,
@ -372,7 +373,8 @@ impl TrimLoopLowerer {
})?;
// Generate: ch0 = s.substring(start, start+1)
let ch0 = builder.value_gen.next();
// Phase 135 P0: Use function-level ValueId (SSOT)
let ch0 = builder.next_value_id();
builder.emit_method_call(
Some(ch0),
s_id,

6
src/mir/builder/control_flow/joinir/patterns/trim_pattern_validator.rs
View File

@ -47,12 +47,14 @@ impl TrimPatternValidator {
let ws_const = emit_string(builder, ws_char.clone());
// eq_check = ch == ws_const
let eq_dst = builder.value_gen.next();
// Phase 135 P0: Use function-level ValueId (SSOT)
let eq_dst = builder.next_value_id();
emit_eq_to(builder, eq_dst, ch_value, ws_const)?;
result_opt = Some(if let Some(prev_result) = result_opt {
// result = prev_result || eq_check
let dst = builder.value_gen.next();
// Phase 135 P0: Use function-level ValueId (SSOT)
let dst = builder.next_value_id();
builder.emit_instruction(MirInstruction::BinOp {
dst,
op: BinaryOp::Or,

18
src/mir/builder/joinir_inline_boundary_injector.rs
View File

@ -178,6 +178,7 @@ impl BoundaryInjector {
// We inject Copy: remapped_join_value = Copy host_value
//
// Phase 177-3 Option B: Use pre-allocated reallocations for PHI collision cases
let mut seen_dst_to_src: BTreeMap<ValueId, ValueId> = BTreeMap::new();
for binding in &boundary.condition_bindings {
// Look up the remapped JoinIR ValueId from value_map
let remapped_join = value_map
@ -191,6 +192,23 @@ impl BoundaryInjector {
.copied()
.unwrap_or(remapped_join);
// Phase 135-P0: Deduplicate condition bindings that alias the same JoinIR ValueId.
//
// Promoters may produce multiple names that map to the same JoinIR ValueId
// (e.g. 'char' and 'is_char_match' sharing a promoted carrier slot).
// Emitting multiple `Copy` instructions to the same `dst` violates MIR SSA and
// fails `--verify` (ValueId defined multiple times).
if let Some(prev_src) = seen_dst_to_src.get(&final_dst).copied() {
if prev_src != binding.host_value {
return Err(format!(
"[BoundaryInjector] condition_bindings conflict: dst {:?} would be assigned from two different sources: {:?} and {:?} (JoinIR {:?}, name '{}')",
final_dst, prev_src, binding.host_value, binding.join_value, binding.name
));
}
continue;
}
seen_dst_to_src.insert(final_dst, binding.host_value);
// Copy instruction: final_dst = Copy host_value
let copy_inst = MirInstruction::Copy {
dst: final_dst,

3
src/mir/builder/stmts.rs
View File

@ -346,7 +346,8 @@ impl super::MirBuilder {
}
last_value = Some(var_id);
}
Ok(last_value.unwrap_or_else(|| self.value_gen.next()))
// Phase 135 P0: Use function-level ValueId (SSOT) - build_local_statement is always in function context
Ok(last_value.unwrap_or_else(|| self.next_value_id()))
}
// Return statement

63
src/mir/join_ir/lowering/condition_lowerer.rs
View File

@ -59,14 +59,11 @@ use super::method_call_lowerer::MethodCallLowerer;
/// &env,
/// )?;
/// ```
pub fn lower_condition_to_joinir<F>(
pub fn lower_condition_to_joinir(
cond_ast: &ASTNode,
alloc_value: &mut F,
alloc_value: &mut dyn FnMut() -> ValueId,
env: &ConditionEnv,
) -> Result<(ValueId, Vec<JoinInst>), String>
where
F: FnMut() -> ValueId,
{
) -> Result<(ValueId, Vec<JoinInst>), String> {
let mut instructions = Vec::new();
let result_value = lower_condition_recursive(cond_ast, alloc_value, env, &mut instructions)?;
Ok((result_value, instructions))
@ -75,15 +72,12 @@ where
/// Recursive helper for condition lowering
///
/// Handles all supported AST node types and emits appropriate JoinIR instructions.
fn lower_condition_recursive<F>(
fn lower_condition_recursive(
cond_ast: &ASTNode,
alloc_value: &mut F,
alloc_value: &mut dyn FnMut() -> ValueId,
env: &ConditionEnv,
instructions: &mut Vec<JoinInst>,
) -> Result<ValueId, String>
where
F: FnMut() -> ValueId,
{
) -> Result<ValueId, String> {
match cond_ast {
// Comparison operations: <, ==, !=, <=, >=, >
ASTNode::BinaryOp {
@ -128,16 +122,14 @@ where
}
/// Lower a comparison operation (e.g., `i < end`)
fn lower_comparison<F>(
fn lower_comparison(
operator: &BinaryOperator,
left: &ASTNode,
right: &ASTNode,
alloc_value: &mut F,
alloc_value: &mut dyn FnMut() -> ValueId,
env: &ConditionEnv,
instructions: &mut Vec<JoinInst>,
) -> Result<ValueId, String>
where
F: FnMut() -> ValueId,
{
// Lower left and right sides
let lhs = lower_value_expression(left, alloc_value, env, instructions)?;
@ -166,15 +158,13 @@ where
}
/// Lower logical AND operation (e.g., `a && b`)
fn lower_logical_and<F>(
fn lower_logical_and(
left: &ASTNode,
right: &ASTNode,
alloc_value: &mut F,
alloc_value: &mut dyn FnMut() -> ValueId,
env: &ConditionEnv,
instructions: &mut Vec<JoinInst>,
) -> Result<ValueId, String>
where
F: FnMut() -> ValueId,
{
// Logical AND: evaluate both sides and combine
let lhs = lower_condition_recursive(left, alloc_value, env, instructions)?;
@ -193,15 +183,13 @@ where
}
/// Lower logical OR operation (e.g., `a || b`)
fn lower_logical_or<F>(
fn lower_logical_or(
left: &ASTNode,
right: &ASTNode,
alloc_value: &mut F,
alloc_value: &mut dyn FnMut() -> ValueId,
env: &ConditionEnv,
instructions: &mut Vec<JoinInst>,
) -> Result<ValueId, String>
where
F: FnMut() -> ValueId,
{
// Logical OR: evaluate both sides and combine
let lhs = lower_condition_recursive(left, alloc_value, env, instructions)?;
@ -220,14 +208,12 @@ where
}
/// Lower NOT operator (e.g., `!cond`)
fn lower_not_operator<F>(
fn lower_not_operator(
operand: &ASTNode,
alloc_value: &mut F,
alloc_value: &mut dyn FnMut() -> ValueId,
env: &ConditionEnv,
instructions: &mut Vec<JoinInst>,
) -> Result<ValueId, String>
where
F: FnMut() -> ValueId,
{
let operand_val = lower_condition_recursive(operand, alloc_value, env, instructions)?;
let dst = alloc_value();
@ -243,13 +229,11 @@ where
}
/// Lower a literal value (e.g., `10`, `true`, `"text"`)
fn lower_literal<F>(
fn lower_literal(
value: &LiteralValue,
alloc_value: &mut F,
alloc_value: &mut dyn FnMut() -> ValueId,
instructions: &mut Vec<JoinInst>,
) -> Result<ValueId, String>
where
F: FnMut() -> ValueId,
{
let dst = alloc_value();
let const_value = match value {
@ -279,15 +263,12 @@ where
///
/// This handles the common case where we need to evaluate a simple value
/// (variable or literal) as part of a comparison.
pub fn lower_value_expression<F>(
pub fn lower_value_expression(
expr: &ASTNode,
alloc_value: &mut F,
alloc_value: &mut dyn FnMut() -> ValueId,
env: &ConditionEnv,
instructions: &mut Vec<JoinInst>,
) -> Result<ValueId, String>
where
F: FnMut() -> ValueId,
{
) -> Result<ValueId, String> {
match expr {
// Variables - look up in ConditionEnv
ASTNode::Variable { name, .. } => env
@ -334,16 +315,14 @@ where
}
/// Lower an arithmetic binary operation (e.g., `i + 1`)
fn lower_arithmetic_binop<F>(
fn lower_arithmetic_binop(
operator: &BinaryOperator,
left: &ASTNode,
right: &ASTNode,
alloc_value: &mut F,
alloc_value: &mut dyn FnMut() -> ValueId,
env: &ConditionEnv,
instructions: &mut Vec<JoinInst>,
) -> Result<ValueId, String>
where
F: FnMut() -> ValueId,
{
let lhs = lower_value_expression(left, alloc_value, env, instructions)?;
let rhs = lower_value_expression(right, alloc_value, env, instructions)?;

8
src/mir/join_ir/lowering/condition_lowering_box.rs
View File

@ -125,7 +125,7 @@ pub trait ConditionLoweringBox<S: ScopeManager> {
fn lower_condition(
&mut self,
condition: &ASTNode,
context: &ConditionContext<S>,
context: &mut ConditionContext<S>,
) -> Result<ValueId, String>;
/// Check if this lowerer supports the given condition pattern
@ -225,7 +225,7 @@ mod tests {
id
};
let context = ConditionContext {
let mut context = ConditionContext {
loop_var_name: "i".to_string(),
loop_var_id: ValueId(1),
scope: &scope,
@ -245,7 +245,7 @@ mod tests {
);
// Lower condition via ConditionLoweringBox trait (Step 2 implemented)
let result = expr_lowerer.lower_condition(&ast, &context);
let result = expr_lowerer.lower_condition(&ast, &mut context);
assert!(result.is_ok(), "i < 10 should lower successfully via trait");
}
@ -280,7 +280,7 @@ mod tests {
id
};
let context = ConditionContext {
let mut context = ConditionContext {
loop_var_name: "i".to_string(),
loop_var_id: ValueId(1),
scope: &scope,

40
src/mir/join_ir/lowering/expr_lowerer.rs
View File

@ -265,11 +265,43 @@ impl<'env, 'builder, S: ScopeManager> ConditionLoweringBox<S> for ExprLowerer<'e
fn lower_condition(
&mut self,
condition: &ASTNode,
_context: &ConditionContext<S>,
context: &mut ConditionContext<S>,
) -> Result<ValueId, String> {
// Delegate to existing lower() method
// ConditionContext is unused because ExprLowerer already has scope access
self.lower(condition).map_err(|e| e.to_string())
// Phase 244+ / Phase 201 SSOT: ValueId allocation must be coordinated by the caller.
//
// JoinIR lowering uses JoinValueSpace as SSOT for ValueId regions.
// If we allocate locally here (e.g. starting from 1000), we can collide with
// other JoinIR value users (main params, carrier slots), and after remapping
// this becomes a MIR-level ValueId collision.
if !ast_support::is_supported_condition(condition) {
return Err(format!("Unsupported condition node: {:?}", condition));
}
// Build ConditionEnv from the provided scope (the caller controls the scope + allocator).
#[cfg(feature = "normalized_dev")]
let condition_env =
scope_resolution::build_condition_env_from_scope_with_binding(context.scope, condition, self.builder)
.map_err(|e| e.to_string())?;
#[cfg(not(feature = "normalized_dev"))]
let condition_env = scope_resolution::build_condition_env_from_scope(context.scope, condition)
.map_err(|e| e.to_string())?;
// Delegate to the well-tested lowerer, but use the caller-provided allocator (SSOT).
let (result_value, instructions) =
lower_condition_to_joinir(condition, &mut *context.alloc_value, &condition_env)
.map_err(|e| e.to_string())?;
self.last_instructions = instructions;
if self.debug {
eprintln!(
"[expr_lowerer/phase244] Lowered condition → ValueId({:?}) (context alloc)",
result_value
);
}
Ok(result_value)
}
/// Phase 244: Check if ExprLowerer supports a given condition pattern

8
src/mir/join_ir/lowering/loop_with_break_minimal/header_break_lowering.rs
View File

@ -51,14 +51,14 @@ pub(crate) fn lower_header_condition(
let mut expr_lowerer =
ExprLowerer::new(&scope_manager, ExprContext::Condition, &mut dummy_builder);
let context = ConditionContext {
let mut context = ConditionContext {
loop_var_name: loop_var_name.to_string(),
loop_var_id,
scope: &scope_manager,
alloc_value,
};
match expr_lowerer.lower_condition(condition, &context) {
match expr_lowerer.lower_condition(condition, &mut context) {
Ok(value_id) => {
let instructions = expr_lowerer.take_last_instructions();
eprintln!(
@ -105,7 +105,7 @@ pub(crate) fn lower_break_condition(
let mut expr_lowerer =
ExprLowerer::new(&scope_manager, ExprContext::Condition, &mut dummy_builder);
let context = ConditionContext {
let mut context = ConditionContext {
loop_var_name: loop_var_name.to_string(),
loop_var_id,
scope: &scope_manager,
@ -113,7 +113,7 @@ pub(crate) fn lower_break_condition(
};
let value_id = expr_lowerer
.lower_condition(break_condition, &context)
.lower_condition(break_condition, &mut context)
.map_err(|e| {
format!(
"[joinir/pattern2/phase244] ConditionLoweringBox failed to lower break condition: {}",

4
src/mir/join_ir/lowering/loop_with_continue_minimal.rs
View File

@ -259,14 +259,14 @@ pub(crate) fn lower_loop_with_continue_minimal(
let mut expr_lowerer =
ExprLowerer::new(&scope_manager, ExprContext::Condition, &mut dummy_builder);
let context = ConditionContext {
let mut context = ConditionContext {
loop_var_name: loop_var_name.clone(),
loop_var_id: i_param,
scope: &scope_manager,
alloc_value: &mut alloc_value,
};
match expr_lowerer.lower_condition(condition, &context) {
match expr_lowerer.lower_condition(condition, &mut context) {
Ok(value_id) => {
let instructions = expr_lowerer.take_last_instructions();
eprintln!("[joinir/pattern4/phase244] Header condition via ConditionLoweringBox: {} instructions", instructions.len());

9
src/mir/join_ir/lowering/method_call_lowerer.rs
View File

@ -81,17 +81,14 @@ impl MethodCallLowerer {
/// &mut instructions,
/// )?;
/// ```
pub fn lower_for_condition<F>(
pub fn lower_for_condition(
recv_val: ValueId,
method_name: &str,
args: &[ASTNode],
alloc_value: &mut F,
alloc_value: &mut dyn FnMut() -> ValueId,
env: &ConditionEnv,
instructions: &mut Vec<JoinInst>,
) -> Result<ValueId, String>
where
F: FnMut() -> ValueId,
{
) -> Result<ValueId, String> {
// Resolve method name to CoreMethodId
// Note: We don't know receiver type at this point, so we try all methods
let method_id = CoreMethodId::iter()