diff --git a/src/mir/builder/control_flow/joinir/patterns/trim_loop_lowering.rs b/src/mir/builder/control_flow/joinir/patterns/trim_loop_lowering.rs index c5f45a1e..40887aba 100644 --- a/src/mir/builder/control_flow/joinir/patterns/trim_loop_lowering.rs +++ b/src/mir/builder/control_flow/joinir/patterns/trim_loop_lowering.rs @@ -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, ); diff --git a/src/mir/join_ir/lowering/common/condition_only_emitter.rs b/src/mir/join_ir/lowering/common/condition_only_emitter.rs index 462c2d69..679cabcc 100644 --- a/src/mir/join_ir/lowering/common/condition_only_emitter.rs +++ b/src/mir/join_ir/lowering/common/condition_only_emitter.rs @@ -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, + /// 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" diff --git a/src/mir/join_ir/lowering/step_schedule.rs b/src/mir/join_ir/lowering/step_schedule.rs index 0089733d..b6945a81 100644 --- a/src/mir/join_ir/lowering/step_schedule.rs +++ b/src/mir/join_ir/lowering/step_schedule.rs @@ -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::>() + .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"); }