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

refactor(mir): Phase 93 リファクタリング - 箱化モジュール化

Browse Source 
## 概要
Phase 93 P0実装後のコード整理。スケジュール決定ロジックとbreak semanticsを
明確化し、デバッグログを統一。

## 変更内容

### 1. スケジュール決定ロジックの関数化 (step_schedule.rs)
- `ScheduleDecision`構造体追加（判定結果＋理由＋デバッグコンテキスト）
- `decide_pattern2_schedule()` - スケジュール決定のSSOT
- `build_pattern2_schedule_from_decision()` - 新しい決定ベースAPI
- 判定理由が4種類で明確化（ConditionOnly → body-local → loop-local → default）
- 後方互換性維持（`Pattern2ScheduleContext`はwrapperに）

### 2. ConditionOnlyRecipe強化 (condition_only_emitter.rs)
- `BreakSemantics` enum追加（WhenMatch vs WhenNotMatch）
- `generate_break_condition()` - semanticsに基づくAST生成
- `from_trim_helper_condition_only()` - factory method追加
- break semanticsがrecipeに明示的に含まれる

### 3. trim_loop_lowering.rs簡素化
- `generate_condition_only_break_condition()`削除（DRY原則）
- `recipe.generate_break_condition()`で統一
- break条件生成ロジックが1箇所に集約

### 4. デバッグログ統一
- `[phase93/schedule]` - スケジュール決定
- `[phase93/condition-only]` - ConditionOnlyレシピ作成
- `[phase93/break-cond]` - break条件生成
- 既存の`joinir_dev_enabled()`使用（新規env var不要）

## テスト結果
- step_schedule: 10 tests PASS
- condition_only_emitter: 4 tests PASS
- 後方互換性維持

## 統計
- 3ファイル変更
- +249行 / -57行 = +192 net

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

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
nyash-codex
2025-12-16 23:43:03 +09:00
parent 93e62b1433
commit c213ecc3c0
3 changed files with 249 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@ -369,29 +369,35 @@ impl TrimLoopLowerer {
);
// Step 7: Generate break condition based on pattern type
// Phase 93 P0: ConditionOnly uses non-negated condition (break when is_ch_match is TRUE)
// Normal Trim uses negated condition (break when !is_ch_match, i.e., ch is NOT whitespace)
let trim_break_condition = if condition_only_recipe.is_some() {
// ConditionOnly: "break when match" semantics
// Generate: is_ch_match (TRUE when we should break)
Self::generate_condition_only_break_condition(trim_helper)
// Phase 93 Refactoring: Use recipe.generate_break_condition() for unified logic
let trim_break_condition = if let Some(ref recipe) = condition_only_recipe {
// Use recipe's break semantics (WhenMatch or WhenNotMatch)
trace.emit_if(
"trim",
"break-cond",
&format!(
"Generated break condition from recipe: {} (semantics: {:?})",
trim_helper.carrier_name,
recipe.break_semantics
),
verbose,
);
recipe.generate_break_condition()
} else {
// Normal Trim: "break when NOT match" semantics
// Generate: !is_ch_match (TRUE when we should break)
trace.emit_if(
"trim",
"break-cond",
&format!(
"Generated normal trim break condition: !{}",
trim_helper.carrier_name
),
verbose,
);
Self::generate_trim_break_condition(trim_helper)
};
trace.emit_if(
"trim",
"cond",
&format!(
"Break condition: {} (ConditionOnly={})",
trim_helper.carrier_name,
condition_only_recipe.is_some()
),
verbose,
);
// Step 8: Return result with all updates
Ok(Some(TrimLoweringResult {
condition: trim_break_condition,
@ -533,27 +539,11 @@ impl TrimLoopLowerer {
TrimPatternLowerer::generate_trim_break_condition(trim_helper)
}
/// Generate ConditionOnly break condition
///
/// Phase 93 P0: For ConditionOnly patterns where break happens WHEN the condition is TRUE.
///
/// Returns: is_carrier (non-negated carrier check)
/// Used for "break when match" semantics (e.g., find-first pattern)
fn generate_condition_only_break_condition(
trim_helper: &crate::mir::loop_pattern_detection::trim_loop_helper::TrimLoopHelper,
) -> ASTNode {
use crate::ast::Span;
// Return just the carrier variable (non-negated)
// When is_ch_match is TRUE, we should break
ASTNode::Variable {
name: trim_helper.carrier_name.clone(),
span: Span::unknown(),
}
}
/// Setup ConditionEnv bindings for Trim carrier
///
/// Phase 180-3: Extracted from Pattern2 (lines 345-377)
/// Phase 93 Refactoring: Use explicit factory methods for recipe creation
///
/// Creates bindings for:
/// 1. Carrier variable (e.g., "is_ch_match")
@ -569,15 +559,16 @@ impl TrimLoopLowerer {
let verbose = crate::config::env::joinir_dev_enabled() || trace.is_joinir_enabled();
// Phase 93 P0: Do NOT add is_ch_match to ConditionBinding
// Instead, create a ConditionOnlyRecipe for recalculation every iteration
let recipe = ConditionOnlyRecipe::from_trim_helper(trim_helper);
// Phase 93 Refactoring: Use explicit factory method for ConditionOnly pattern
let recipe = ConditionOnlyRecipe::from_trim_helper_condition_only(trim_helper);
trace.emit_if(
"trim",
"condition-only",
&format!(
"Phase 93 P0: Created ConditionOnlyRecipe for '{}' (will be recalculated each iteration, not carried via ConditionBinding)",
trim_helper.carrier_name
"[phase93/condition-only] Created ConditionOnlyRecipe for '{}' (semantics: {:?}, will be recalculated each iteration)",
trim_helper.carrier_name,
recipe.break_semantics
),
verbose,
);

72
src/mir/join_ir/lowering/common/condition_only_emitter.rs
View File

@ -36,9 +36,21 @@ use crate::mir::join_ir::JoinInst;
use crate::mir::loop_pattern_detection::trim_loop_helper::TrimLoopHelper;
use crate::mir::ValueId;
/// Break semantics for ConditionOnly patterns
///
/// Phase 93 Refactoring: Explicit break condition semantics
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum BreakSemantics {
/// Break when condition is TRUE (e.g., find-first: break on ch == "b")
WhenMatch,
/// Break when condition is FALSE (e.g., trim: break on ch != whitespace)
WhenNotMatch,
}
/// ConditionOnly変数の再計算レシピ
///
/// Phase 93 P0: Trim patternの`is_ch_match`など、毎イテレーション再計算される変数
/// Phase 93 Refactoring: Break semantics明確化
#[derive(Debug, Clone)]
pub struct ConditionOnlyRecipe {
/// 変数名（例: "is_ch_match"）
@ -47,15 +59,68 @@ pub struct ConditionOnlyRecipe {
pub original_var: String,
/// Trim pattern用のホワイトスペース文字リスト
pub whitespace_chars: Vec<String>,
/// Break semantics (WhenMatch or WhenNotMatch)
pub break_semantics: BreakSemantics,
}
impl ConditionOnlyRecipe {
/// Trim patternからレシピを作成
pub fn from_trim_helper(trim_helper: &TrimLoopHelper) -> Self {
/// Trim patternからレシピを作成（ConditionOnly pattern用）
///
/// Phase 93 P0: ConditionOnly pattern（WhenMatch semantics）
/// 例: find-first pattern（ch == "b"のときにbreak）
pub fn from_trim_helper_condition_only(trim_helper: &TrimLoopHelper) -> Self {
Self {
name: trim_helper.carrier_name.clone(),
original_var: trim_helper.original_var.clone(),
whitespace_chars: trim_helper.whitespace_chars.clone(),
break_semantics: BreakSemantics::WhenMatch,
}
}
/// Trim patternからレシピを作成（Normal Trim pattern用）
///
/// Phase 93 Refactoring: Normal Trim pattern（WhenNotMatch semantics）
/// 例: str.trim()（ch != whitespaceのときにbreak）
pub fn from_trim_helper_normal_trim(trim_helper: &TrimLoopHelper) -> Self {
Self {
name: trim_helper.carrier_name.clone(),
original_var: trim_helper.original_var.clone(),
whitespace_chars: trim_helper.whitespace_chars.clone(),
break_semantics: BreakSemantics::WhenNotMatch,
}
}
/// Trim patternからレシピを作成（後方互換性）
///
/// Phase 93 Refactoring: WhenMatchをデフォルトとして使用
#[allow(dead_code)]
pub fn from_trim_helper(trim_helper: &TrimLoopHelper) -> Self {
Self::from_trim_helper_condition_only(trim_helper)
}
/// Break条件AST生成（semanticsに基づく）
///
/// Phase 93 Refactoring: Break semanticsに基づいて適切な条件を生成
///
/// # Returns
///
/// - `WhenMatch`: carrier変数そのまま（TRUE時にbreak）
/// - `WhenNotMatch`: !carrier（FALSE時にbreak）
pub fn generate_break_condition(&self) -> ASTNode {
use crate::ast::{Span, UnaryOperator};
let carrier_var = ASTNode::Variable {
name: self.name.clone(),
span: Span::unknown(),
};
match self.break_semantics {
BreakSemantics::WhenMatch => carrier_var,
BreakSemantics::WhenNotMatch => ASTNode::UnaryOp {
operator: UnaryOperator::Not,
operand: Box::new(carrier_var),
span: Span::unknown(),
},
}
}
}
@ -201,6 +266,7 @@ mod tests {
name: "is_ch_match".to_string(),
original_var: "ch".to_string(),
whitespace_chars: vec!["b".to_string()],
break_semantics: BreakSemantics::WhenMatch,
};
let mut body_local_env = LoopBodyLocalEnv::new();
@ -245,6 +311,7 @@ mod tests {
name: "is_ws".to_string(),
original_var: "ch".to_string(),
whitespace_chars: vec![" ".to_string(), "\t".to_string(), "\n".to_string()],
break_semantics: BreakSemantics::WhenMatch,
};
let mut body_local_env = LoopBodyLocalEnv::new();
@ -280,6 +347,7 @@ mod tests {
name: "is_ch_match".to_string(),
original_var: "ch".to_string(),
whitespace_chars: vec!["b".to_string()],
break_semantics: BreakSemantics::WhenMatch,
};
let body_local_env = LoopBodyLocalEnv::new(); // Empty - no "ch"

167
src/mir/join_ir/lowering/step_schedule.rs
View File

@ -71,7 +71,84 @@ impl Pattern2StepSchedule {
}
}
/// Schedule decision result with reasoning
///
/// Phase 93 Refactoring: Unified schedule decision with explicit reasons
#[derive(Debug, Clone)]
pub(crate) struct ScheduleDecision {
/// Whether body-init should come before break check
pub body_init_first: bool,
/// Human-readable reason for this decision
pub reason: &'static str,
/// Debug context for logging
pub debug_ctx: ScheduleDebugContext,
}
/// Debug context for schedule decisions
#[derive(Debug, Clone)]
pub(crate) struct ScheduleDebugContext {
pub has_body_local_init: bool,
pub has_loop_local_carrier: bool,
pub has_condition_only_recipe: bool,
}
/// Decide Pattern2 schedule based on loop characteristics
///
/// Phase 93 Refactoring: Single source of truth for schedule decisions
///
/// # Decision Logic
///
/// Body-init comes BEFORE break check if any of these conditions are true:
/// 1. ConditionOnly recipe exists (derived slots need recalculation)
/// 2. Body-local variables exist (break condition depends on them)
/// 3. Loop-local carriers exist (need initialization before use)
///
/// # Arguments
///
/// * `body_local_env` - Body-local variable environment
/// * `carrier_info` - Carrier information (for loop-local detection)
/// * `has_condition_only_recipe` - Whether ConditionOnly derived slots exist
///
/// # Returns
///
/// `ScheduleDecision` with decision, reason, and debug context
pub(crate) fn decide_pattern2_schedule(
body_local_env: Option<&LoopBodyLocalEnv>,
carrier_info: &CarrierInfo,
has_condition_only_recipe: bool,
) -> ScheduleDecision {
let has_body_local_init = body_local_env.map(|env| !env.is_empty()).unwrap_or(false);
let has_loop_local_carrier = carrier_info
.carriers
.iter()
.any(|c| matches!(c.init, CarrierInit::LoopLocalZero));
let body_init_first = has_condition_only_recipe || has_body_local_init || has_loop_local_carrier;
let reason = if has_condition_only_recipe {
"ConditionOnly requires body-init before break"
} else if has_body_local_init {
"body-local variables require init before break"
} else if has_loop_local_carrier {
"loop-local carrier requires init before break"
} else {
"default schedule"
};
ScheduleDecision {
body_init_first,
reason,
debug_ctx: ScheduleDebugContext {
has_body_local_init,
has_loop_local_carrier,
has_condition_only_recipe,
},
}
}
/// Minimal context for deciding the step order.
///
/// Phase 93 Refactoring: Kept for backward compatibility, delegates to `decide_pattern2_schedule`
#[derive(Debug, Clone, Copy)]
pub(crate) struct Pattern2ScheduleContext {
pub(crate) has_body_local_init: bool,
@ -81,30 +158,23 @@ pub(crate) struct Pattern2ScheduleContext {
impl Pattern2ScheduleContext {
/// Build schedule context from environment.
///
/// # Phase 93 P0: has_condition_only_recipe parameter
/// # Phase 93 Refactoring: Backward compatibility wrapper
///
/// When a ConditionOnly recipe exists, body-local init MUST happen before break check
/// even if body_local_env is currently empty. This is because ConditionOnly variables
/// (e.g., `is_ch_match`) are recalculated in body_init_block, and the break condition
/// depends on them.
/// Delegates to `decide_pattern2_schedule()` for actual decision making.
/// Note: When `has_condition_only_recipe` is true, we set `has_body_local_init` to true
/// to ensure the correct schedule is generated.
pub(crate) fn from_env(
body_local_env: Option<&LoopBodyLocalEnv>,
carrier_info: &CarrierInfo,
has_condition_only_recipe: bool,
) -> Self {
// Phase 93 P0: body_local_init is true if:
// 1. body_local_env has entries, OR
// 2. condition_only_recipe exists (will be emitted to body_init_block)
let has_body_local_init = body_local_env.map(|env| !env.is_empty()).unwrap_or(false)
|| has_condition_only_recipe;
let has_loop_local_carrier = carrier_info
.carriers
.iter()
.any(|c| matches!(c.init, CarrierInit::LoopLocalZero));
let decision = decide_pattern2_schedule(body_local_env, carrier_info, has_condition_only_recipe);
Self {
has_body_local_init,
has_loop_local_carrier,
// Phase 93 Refactoring: Include condition_only_recipe in has_body_local_init
// to maintain backward compatibility with requires_body_init_before_break()
has_body_local_init: decision.debug_ctx.has_body_local_init
|| decision.debug_ctx.has_condition_only_recipe,
has_loop_local_carrier: decision.debug_ctx.has_loop_local_carrier,
}
}
@ -115,6 +185,46 @@ impl Pattern2ScheduleContext {
/// Build a schedule for Pattern 2 lowering.
///
/// Phase 93 Refactoring: Now accepts `ScheduleDecision` for explicit reasoning
///
/// - Default P2: header → break → body-init → updates → tail
/// - Body-local break dependency (DigitPos/_atoi style):
/// header → body-init → break → updates → tail
pub(crate) fn build_pattern2_schedule_from_decision(
decision: &ScheduleDecision,
) -> Pattern2StepSchedule {
let schedule = if decision.body_init_first {
Pattern2StepSchedule {
steps: vec![
Pattern2StepKind::HeaderCond,
Pattern2StepKind::BodyInit,
Pattern2StepKind::BreakCheck,
Pattern2StepKind::Updates,
Pattern2StepKind::Tail,
],
reason: decision.reason,
}
} else {
Pattern2StepSchedule {
steps: vec![
Pattern2StepKind::HeaderCond,
Pattern2StepKind::BreakCheck,
Pattern2StepKind::BodyInit,
Pattern2StepKind::Updates,
Pattern2StepKind::Tail,
],
reason: decision.reason,
}
};
log_schedule_from_decision(decision, &schedule);
schedule
}
/// Build a schedule for Pattern 2 lowering (legacy wrapper).
///
/// Phase 93 Refactoring: Kept for backward compatibility
///
/// - Default P2: header → break → body-init → updates → tail
/// - Body-local break dependency (DigitPos/_atoi style):
/// header → body-init → break → updates → tail
@ -147,6 +257,28 @@ pub(crate) fn build_pattern2_schedule(ctx: &Pattern2ScheduleContext) -> Pattern2
schedule
}
fn log_schedule_from_decision(decision: &ScheduleDecision, schedule: &Pattern2StepSchedule) {
if !(env::joinir_dev_enabled() || joinir_test_debug_enabled()) {
return;
}
let steps_desc = schedule
.steps()
.iter()
.map(Pattern2StepKind::as_str)
.collect::<Vec<_>>()
.join(" -> ");
eprintln!(
"[phase93/schedule] steps={} reason={} ctx={{body_local_init={}, loop_local_carrier={}, condition_only={}}}",
steps_desc,
schedule.reason(),
decision.debug_ctx.has_body_local_init,
decision.debug_ctx.has_loop_local_carrier,
decision.debug_ctx.has_condition_only_recipe
);
}
fn log_schedule(ctx: &Pattern2ScheduleContext, schedule: &Pattern2StepSchedule) {
if !(env::joinir_dev_enabled() || joinir_test_debug_enabled()) {
return;
@ -288,6 +420,7 @@ mod tests {
Pattern2StepKind::Tail
]
);
// Phase 93 Refactoring: Reason is now preserved from backward compat wrapper
assert_eq!(schedule.reason(), "body-local break dependency");
}