From 37172791eaf9fcd6242d28ceb099319ac92b3f08 Mon Sep 17 00:00:00 2001 From: tomoaki Date: Wed, 31 Dec 2025 06:06:07 +0900 Subject: [PATCH] phase29ap(p6): extend pattern2 plan subsets for stdlib --- docs/development/current/main/10-Now.md | 2 +- docs/development/current/main/30-Backlog.md | 2 +- .../design/coreplan-migration-roadmap-ssot.md | 2 +- .../current/main/phases/phase-29ap/README.md | 24 +- .../joinir/patterns/condition_env_builder.rs | 2 +- .../control_flow/joinir/patterns/mod.rs | 2 - .../pattern2_break_condition_policy_router.rs | 4 +- .../joinir/patterns/pattern2_steps/mod.rs | 4 +- .../joinir/patterns/pattern2_with_break.rs | 471 ------------ .../control_flow/joinir/patterns/router.rs | 27 +- .../plan/extractors/pattern2_break.rs | 111 ++- .../plan/facts/pattern2_break_facts.rs | 671 ++++++++++++++++++ .../control_flow/plan/normalizer/helpers.rs | 66 +- .../lowering/loop_patterns/with_break.rs | 2 +- 14 files changed, 838 insertions(+), 552 deletions(-) delete mode 100644 src/mir/builder/control_flow/joinir/patterns/pattern2_with_break.rs diff --git a/docs/development/current/main/10-Now.md b/docs/development/current/main/10-Now.md index ccb7aa96..061a7662 100644 --- a/docs/development/current/main/10-Now.md +++ b/docs/development/current/main/10-Now.md @@ -3,7 +3,7 @@ ## Current Focus - Phase: `docs/development/current/main/phases/phase-29ap/README.md` -- Next: Phase 29ap P5 (planned; see `docs/development/current/main/phases/phase-29ap/README.md`) +- Next: Phase 29ap P7 (planned; see `docs/development/current/main/phases/phase-29ap/README.md`) ## Gate (SSOT) diff --git a/docs/development/current/main/30-Backlog.md b/docs/development/current/main/30-Backlog.md index 1a768142..d746e5c2 100644 --- a/docs/development/current/main/30-Backlog.md +++ b/docs/development/current/main/30-Backlog.md @@ -5,7 +5,7 @@ Scope: 「次にやる候補」を短く列挙するメモ。入口は `docs/dev ## Active -- Phase 29ap: `docs/development/current/main/phases/phase-29ap/README.md` (Next: P5 planned) +- Phase 29ap: `docs/development/current/main/phases/phase-29ap/README.md` (Next: P7 planned) - JoinIR regression gate SSOT: `docs/development/current/main/phases/phase-29ae/README.md` - CorePlan hardening (docs-first): `docs/development/current/main/phases/phase-29al/README.md` diff --git a/docs/development/current/main/design/coreplan-migration-roadmap-ssot.md b/docs/development/current/main/design/coreplan-migration-roadmap-ssot.md index 85bb9ada..175e3fdd 100644 --- a/docs/development/current/main/design/coreplan-migration-roadmap-ssot.md +++ b/docs/development/current/main/design/coreplan-migration-roadmap-ssot.md @@ -34,7 +34,7 @@ Related: ## 1.1 Current (active) - Active phase: `docs/development/current/main/phases/phase-29ap/README.md` -- Next step: Phase 29ap P5 (planned) +- Next step: Phase 29ap P7 (planned) ## 2. すでに固めた SSOT（再発防止の土台） diff --git a/docs/development/current/main/phases/phase-29ap/README.md b/docs/development/current/main/phases/phase-29ap/README.md index c75bdc5d..a2b0ad45 100644 --- a/docs/development/current/main/phases/phase-29ap/README.md +++ b/docs/development/current/main/phases/phase-29ap/README.md @@ -57,12 +57,30 @@ Gate (SSOT): - Scope: - Remove Pattern8 from `LOOP_PATTERNS` so plan/composer stays SSOT for normal loops. - - Keep legacy table as last resort for Pattern2/4/9 only. + - Keep legacy table as last resort for Pattern6_NestedLoopMinimal / Pattern4 / Pattern9 only. - Guardrails: - No change to logs or error strings. - Legacy routing remains a last-resort fallback. +## P5: Remove Pattern2 from JoinIR legacy table ✅ + +- Scope: + - Remove Pattern2 (with break) from `LOOP_PATTERNS`. + - Keep plan/composer as the only routing path for Pattern2. +- Guardrails: + - No change to logs or error strings. + - Fallback remains `Ok(None)` for non-matching plan cases. + +## P6: stdlib trim_start/trim_end subset (Pattern2BreakFacts) ✅ + +- Scope: + - Add a conservative Pattern2BreakFacts subset for `trim_start`/`trim_end`. + - Normalize `not is_whitespace(...)` into `is_whitespace(...) == false` in facts. + - Restore quick smoke by ensuring stdlib loops are handled by plan/composer. +- Guardrails: + - No new logs or error strings. + - Subset only: if shape deviates, return `Ok(None)`. + ## Next (planned) -- P5: Router pattern-name branching reduction (planner outcome + composer SSOT) -- P6: Facts/Feature expansion if needed +- P7: Legacy table shrink (Pattern9 removal) or leave as-is with justification diff --git a/src/mir/builder/control_flow/joinir/patterns/condition_env_builder.rs b/src/mir/builder/control_flow/joinir/patterns/condition_env_builder.rs index bbaf0897..891ba6b6 100644 --- a/src/mir/builder/control_flow/joinir/patterns/condition_env_builder.rs +++ b/src/mir/builder/control_flow/joinir/patterns/condition_env_builder.rs @@ -77,7 +77,7 @@ impl ConditionEnvBuilder { // Phase 79-2: Register loop variable BindingId (dev-only) // NOTE: We don't have access to builder.binding_map here, so this registration - // needs to happen at the call site (in pattern2_with_break.rs, pattern3_with_if_phi.rs, etc.) + // needs to happen at the call site (pattern2 lowerer/orchestrator, pattern3_with_if_phi.rs, etc.) // This comment serves as a reminder for future developers. // For each condition variable, allocate JoinIR-local ValueId and build binding diff --git a/src/mir/builder/control_flow/joinir/patterns/mod.rs b/src/mir/builder/control_flow/joinir/patterns/mod.rs index 56c794af..383afc37 100644 --- a/src/mir/builder/control_flow/joinir/patterns/mod.rs +++ b/src/mir/builder/control_flow/joinir/patterns/mod.rs @@ -2,7 +2,6 @@ //! //! Phase 2: Extracted from control_flow.rs //! - Pattern 1: Simple While Loop (pattern1_minimal.rs) -//! - Pattern 2: Loop with Conditional Break (pattern2_with_break.rs) //! - Pattern 3: Loop with If-Else PHI (pattern3_with_if_phi.rs) //! - Pattern 4: Loop with Continue (pattern4_with_continue.rs) [Phase 194+] //! @@ -78,7 +77,6 @@ pub(in crate::mir::builder) mod exit_binding_constructor; // Phase 222.5-C pub(in crate::mir::builder) mod exit_binding_validator; // Phase 222.5-C pub(in crate::mir::builder) mod loop_scope_shape_builder; pub(in crate::mir::builder) mod pattern1_minimal; -pub(in crate::mir::builder) mod pattern2_with_break; pub(in crate::mir::builder) mod pattern3_with_if_phi; pub(in crate::mir::builder) mod pattern4_carrier_analyzer; pub(in crate::mir::builder) mod pattern4_with_continue; diff --git a/src/mir/builder/control_flow/joinir/patterns/pattern2_break_condition_policy_router.rs b/src/mir/builder/control_flow/joinir/patterns/pattern2_break_condition_policy_router.rs index 62dacccf..fea204fb 100644 --- a/src/mir/builder/control_flow/joinir/patterns/pattern2_break_condition_policy_router.rs +++ b/src/mir/builder/control_flow/joinir/patterns/pattern2_break_condition_policy_router.rs @@ -1,7 +1,7 @@ //! Pattern2 break-condition routing (policy → break_cond + allow-list + flags) //! -//! This box exists to keep `pattern2_with_break.rs` focused on orchestration and -//! JoinIR wiring. It applies Pattern2-specific policies to derive: +//! This box exists to keep the Pattern2 lowering orchestrator focused on wiring. +//! It applies Pattern2-specific policies to derive: //! - a normalized `break when true` condition node //! - an allow-list of body-local variables permitted in conditions (Phase 92 P3) //! - flags that affect later lowering (e.g. schedule and promotion heuristics) diff --git a/src/mir/builder/control_flow/joinir/patterns/pattern2_steps/mod.rs b/src/mir/builder/control_flow/joinir/patterns/pattern2_steps/mod.rs index 746ba070..c5f6fb3c 100644 --- a/src/mir/builder/control_flow/joinir/patterns/pattern2_steps/mod.rs +++ b/src/mir/builder/control_flow/joinir/patterns/pattern2_steps/mod.rs @@ -1,7 +1,7 @@ //! Phase 106: Pattern2 Step Boxes (SSOT) //! -//! Goal: keep `pattern2_with_break.rs` and the orchestrator thin by splitting -//! the Pattern2 pipeline into explicit steps with clear boundaries. +//! Goal: keep the Pattern2 lowering orchestrator thin by splitting the pipeline +//! into explicit steps with clear boundaries. //! //! Each step should have a single responsibility and fail-fast with a clear tag //! at the step boundary. diff --git a/src/mir/builder/control_flow/joinir/patterns/pattern2_with_break.rs b/src/mir/builder/control_flow/joinir/patterns/pattern2_with_break.rs deleted file mode 100644 index 50cd7457..00000000 --- a/src/mir/builder/control_flow/joinir/patterns/pattern2_with_break.rs +++ /dev/null @@ -1,471 +0,0 @@ -//! Pattern 2: Loop with Conditional Break minimal lowerer - -use super::super::trace; -use crate::ast::ASTNode; -use crate::mir::builder::MirBuilder; -use crate::mir::ValueId; - -/// Phase 194: Detection function for Pattern 2 -/// -/// Phase 282 P4: Updated to ExtractionBased detection with safety valve -/// -/// Pattern 2 matches: -/// - Pattern kind is Pattern2Break (safety valve, O(1) early rejection) -/// - Extraction validates: 比較条件/loop(true) + HAS break + NO continue/return -pub(crate) fn can_lower(builder: &MirBuilder, ctx: &super::router::LoopPatternContext) -> bool { - use crate::mir::loop_pattern_detection::LoopPatternKind; - - // Step 1: Early rejection guard (safety valve, O(1)) - if ctx.pattern_kind != LoopPatternKind::Pattern2Break { - if ctx.debug { - trace::trace().debug( - "pattern2/can_lower", - &format!("reject: pattern_kind={:?}", ctx.pattern_kind), - ); - } - return false; - } - - // Step 2: ExtractionBased validation (SSOT, deep check) - use super::extractors::pattern2::extract_loop_with_break_parts; - - let parts = match extract_loop_with_break_parts(ctx.condition, ctx.body) { - Ok(Some(p)) => p, - Ok(None) => { - if ctx.debug { - trace::trace().debug( - "pattern2/can_lower", - "reject: not Pattern2 (no break or has continue/return)", - ); - } - return false; - } - Err(e) => { - if ctx.debug { - trace::trace().debug("pattern2/can_lower", &format!("error: {}", e)); - } - return false; - } - }; - - if ctx.debug { - trace::trace().debug( - "pattern2/can_lower", - &format!( - "accept: extractable (break_count={}, is_loop_true={}) (Phase 282 P4)", - parts.break_count, parts.is_loop_true - ), - ); - } - - // Step 3: Extract loop variable (hybrid for loop(true)) - use super::loop_true_counter_extractor::LoopTrueCounterExtractorBox; - - let loop_var_name = if parts.is_loop_true { - // loop(true): Use LoopTrueCounterExtractorBox - match LoopTrueCounterExtractorBox::extract_loop_counter_from_body( - ctx.body, - &builder.variable_ctx.variable_map, - ) { - Ok((name, _)) => name, - Err(_) => { - if ctx.debug { - trace::trace().debug( - "pattern2/can_lower", - "reject: loop(true) but no counter found", - ); - } - return false; - } - } - } else { - // Normal loop: Use condition-based extraction - match builder.extract_loop_variable_from_condition(ctx.condition) { - Ok(name) => name, - Err(_) => { - // Try parts.loop_var as fallback - if parts.loop_var.is_empty() { - if ctx.debug { - trace::trace().debug( - "pattern2/can_lower", - "reject: cannot extract loop variable", - ); - } - return false; - } - parts.loop_var.clone() - } - } - }; - - // Step 4: Carrier update validation (SSOT) - use super::common_init::CommonPatternInitializer; - match CommonPatternInitializer::check_carrier_updates_allowed( - ctx.body, - &loop_var_name, - &builder.variable_ctx.variable_map, - ) { - true => true, - false => { - if ctx.debug { - trace::trace().debug( - "pattern2/can_lower", - "reject: carrier updates not allowed (method calls)", - ); - } - false - } - } -} - -/// Phase 194: Lowering function for Pattern 2 -/// -/// Phase 282 P4: Re-extracts for SSOT (no caching from can_lower) -/// -/// Wrapper around cf_loop_pattern2_with_break_impl to match router signature -pub(crate) fn lower( - builder: &mut MirBuilder, - ctx: &super::router::LoopPatternContext, -) -> Result, String> { - use super::extractors::pattern2::extract_loop_with_break_parts; - - // Re-extract (SSOT principle - no caching from can_lower) - let parts = extract_loop_with_break_parts(ctx.condition, ctx.body)? - .ok_or_else(|| "[pattern2] Not a loop with break pattern in lower()".to_string())?; - - if ctx.debug { - trace::trace().debug( - "pattern2/lower", - &format!( - "loop_var={}, break_count={}, is_loop_true={} (Phase 282 P4)", - parts.loop_var, parts.break_count, parts.is_loop_true - ), - ); - } - - // Call existing orchestrator implementation (completely unchanged) - // Note: ctx.skeleton already provided by router if needed - builder.cf_loop_pattern2_with_break_impl( - ctx.condition, - ctx.body, - ctx.func_name, - ctx.debug, - ctx.fn_body, - ctx.skeleton, // Use existing ctx.skeleton (no new env var) - ) -} - -impl MirBuilder { - /// Phase 179-B: Pattern 2 (Loop with Conditional Break) minimal lowerer - /// - /// **Refactored**: Now uses PatternPipelineContext for unified preprocessing - /// **Phase 200-C**: Added fn_body parameter for capture analysis - /// **Phase 92 P0-3**: Added skeleton parameter for ConditionalStep support - /// - /// # Pipeline (Phase 179-B) - /// 1. Build preprocessing context → PatternPipelineContext - /// 2. Pattern 2-specific processing (Trim, carrier updates, break condition) - /// 3. Call JoinIR lowerer → JoinModule - /// 4. Create boundary from context → JoinInlineBoundary - /// 5. Merge MIR blocks → JoinIRConversionPipeline - /// - /// Note: Pattern 2 has complex Trim pattern logic that remains inline - /// for now. Future Phase 180+ will move Trim logic to dedicated module. - #[allow(dead_code)] - pub(in crate::mir::builder) fn cf_loop_pattern2_with_break( - &mut self, - condition: &ASTNode, - _body: &[ASTNode], - _func_name: &str, - debug: bool, - ) -> Result, String> { - // Phase 200-C: Delegate to impl function with fn_body=None for backward compatibility - // Phase 92 P0-3: skeleton=None for backward compatibility - self.cf_loop_pattern2_with_break_impl(condition, _body, _func_name, debug, None, None) - } - - /// Phase 200-C: Pattern 2 implementation with optional fn_body for capture analysis - /// Phase 92 P0-3: Added skeleton parameter for ConditionalStep support - fn cf_loop_pattern2_with_break_impl( - &mut self, - condition: &ASTNode, - _body: &[ASTNode], - _func_name: &str, - debug: bool, - fn_body: Option<&[ASTNode]>, - skeleton: Option<&crate::mir::loop_canonicalizer::LoopSkeleton>, - ) -> Result, String> { - super::pattern2_lowering_orchestrator::Pattern2LoweringOrchestrator::run( - self, - condition, - _body, - _func_name, - debug, - fn_body, - skeleton, - ) - } -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::ast::{BinaryOperator, LiteralValue, Span}; - use crate::mir::builder::control_flow::joinir::patterns::router::LoopPatternContext; - use crate::mir::loop_pattern_detection::LoopPatternKind; - - fn span() -> Span { - Span::unknown() - } - - fn var(name: &str) -> ASTNode { - ASTNode::Variable { - name: name.to_string(), - span: span(), - } - } - - fn lit_i(value: i64) -> ASTNode { - ASTNode::Literal { - value: LiteralValue::Integer(value), - span: span(), - } - } - - fn bin(op: BinaryOperator, left: ASTNode, right: ASTNode) -> ASTNode { - ASTNode::BinaryOp { - operator: op, - left: Box::new(left), - right: Box::new(right), - span: span(), - } - } - - #[test] - #[cfg(feature = "normalized_dev")] - fn phase78_promoted_binding_is_recorded_for_digitpos() { - use super::super::pattern_pipeline::{build_pattern_context, PatternVariant}; - use crate::ast::Span; - use crate::mir::ValueId; - use super::super::pattern2_inputs_facts_box::Pattern2InputsFactsBox; - use super::super::pattern2_steps::apply_policy_step_box::ApplyPolicyStepBox; - - let mut builder = MirBuilder::new(); - builder - .variable_ctx - .variable_map - .insert("i".to_string(), ValueId(1)); - builder - .variable_ctx - .variable_map - .insert("len".to_string(), ValueId(2)); - builder - .variable_ctx - .variable_map - .insert("s".to_string(), ValueId(3)); - builder - .variable_ctx - .variable_map - .insert("digits".to_string(), ValueId(4)); - builder - .variable_ctx - .variable_map - .insert("result".to_string(), ValueId(5)); - - let condition = bin(BinaryOperator::Less, var("i"), var("len")); - - let local_ch = ASTNode::Local { - variables: vec!["ch".to_string()], - initial_values: vec![Some(Box::new(ASTNode::MethodCall { - object: Box::new(var("s")), - method: "substring".to_string(), - arguments: vec![], - span: Span::unknown(), - }))], - span: Span::unknown(), - }; - let local_digit_pos = ASTNode::Local { - variables: vec!["digit_pos".to_string()], - initial_values: vec![Some(Box::new(ASTNode::MethodCall { - object: Box::new(var("digits")), - method: "indexOf".to_string(), - arguments: vec![var("ch")], - span: Span::unknown(), - }))], - span: Span::unknown(), - }; - - let break_if = ASTNode::If { - condition: Box::new(bin(BinaryOperator::Less, var("digit_pos"), lit_i(0))), - then_body: vec![ASTNode::Break { - span: Span::unknown(), - }], - else_body: None, - span: Span::unknown(), - }; - - let body = vec![ - local_ch, - local_digit_pos, - break_if, - ASTNode::Assignment { - target: Box::new(var("result")), - value: Box::new(bin( - BinaryOperator::Add, - bin(BinaryOperator::Multiply, var("result"), lit_i(10)), - var("digit_pos"), - )), - span: Span::unknown(), - }, - ASTNode::Assignment { - target: Box::new(var("i")), - value: Box::new(bin(BinaryOperator::Add, var("i"), lit_i(1))), - span: Span::unknown(), - }, - ]; - - let ctx = build_pattern_context(&mut builder, &condition, &body, PatternVariant::Pattern2) - .expect("build_pattern_context"); - let facts = - Pattern2InputsFactsBox::analyze(&builder, &condition, &body, None, &ctx, false) - .expect("Pattern2InputsFactsBox::analyze"); - let mut inputs = ApplyPolicyStepBox::apply(&condition, &body, facts) - .expect("ApplyPolicyStepBox::apply"); - PromoteStepBox::promote_and_prepare_carriers(&mut builder, &condition, &body, &mut inputs, false, false) - .expect("promote_and_prepare_carriers"); - - assert!( - inputs - .carrier_info - .promoted_loopbodylocals - .contains(&"digit_pos".to_string()), - "digit_pos should be recorded as promoted" - ); - assert_eq!( - inputs.carrier_info.promoted_bindings.len(), - 1, - "promoted_bindings should contain exactly one mapping" - ); - - let (original_bid, promoted_bid) = inputs - .carrier_info - .promoted_bindings - .iter() - .next() - .map(|(k, v)| (*k, *v)) - .unwrap(); - - let promoted_carrier = inputs - .carrier_info - .find_carrier("is_digit_pos") - .expect("promoted carrier exists"); - - assert_eq!( - promoted_carrier.binding_id, - Some(promoted_bid), - "CarrierVar.binding_id should be set for promoted carrier" - ); - - let promoted_join_id = promoted_carrier.join_id.expect("join_id allocated"); - assert_eq!( - inputs.env.binding_id_map.get(&promoted_bid).copied(), - Some(promoted_join_id), - "ConditionEnv should register promoted binding_id -> join_id" - ); - assert_eq!( - inputs.env.get("digit_pos"), - Some(promoted_join_id), - "Name-based alias (digit_pos -> is_digit_pos) should be installed for legacy paths" - ); - - // Ensure the mapping is not degenerate. - assert_ne!(original_bid, promoted_bid); - } - - #[test] - fn parse_number_like_loop_is_routed_to_pattern2() { - let condition = bin(BinaryOperator::Less, var("p"), var("len")); - let break_cond = bin(BinaryOperator::Less, var("digit_pos"), lit_i(0)); - - let body = vec![ - ASTNode::If { - condition: Box::new(break_cond), - then_body: vec![ASTNode::Break { span: span() }], - else_body: None, - span: span(), - }, - ASTNode::Assignment { - target: Box::new(var("p")), - value: Box::new(bin(BinaryOperator::Add, var("p"), lit_i(1))), - span: span(), - }, - // num_str = num_str + ch (string append is allowed by CommonPatternInitializer) - ASTNode::Assignment { - target: Box::new(var("num_str")), - value: Box::new(bin(BinaryOperator::Add, var("num_str"), var("ch"))), - span: span(), - }, - ]; - - let ctx = LoopPatternContext::new(&condition, &body, "parse_number_like", true, false); - let builder = MirBuilder::new(); - - assert_eq!(ctx.pattern_kind, LoopPatternKind::Pattern2Break); - assert!( - can_lower(&builder, &ctx), - "Pattern2 lowerer should accept JsonParser-like break loop" - ); - } - - #[test] - fn test_atoi_loop_routed_to_pattern2() { - // Phase 246-EX Step 2: _atoi loop router integration test - // loop(i < len) { - // if digit_pos < 0 { break } - // result = result * 10 + digit_pos // NumberAccumulation - // i = i + 1 - // } - - let condition = bin(BinaryOperator::Less, var("i"), var("len")); - let break_cond = bin(BinaryOperator::Less, var("digit_pos"), lit_i(0)); - - // result = result * 10 + digit_pos (NumberAccumulation pattern) - let mul_expr = bin(BinaryOperator::Multiply, var("result"), lit_i(10)); - let result_update_value = bin(BinaryOperator::Add, mul_expr, var("digit_pos")); - - let body = vec![ - ASTNode::If { - condition: Box::new(break_cond), - then_body: vec![ASTNode::Break { span: span() }], - else_body: None, - span: span(), - }, - ASTNode::Assignment { - target: Box::new(var("result")), - value: Box::new(result_update_value), - span: span(), - }, - ASTNode::Assignment { - target: Box::new(var("i")), - value: Box::new(bin(BinaryOperator::Add, var("i"), lit_i(1))), - span: span(), - }, - ]; - - let ctx = LoopPatternContext::new(&condition, &body, "_atoi", true, false); - let builder = MirBuilder::new(); - - // Verify pattern classification - assert_eq!( - ctx.pattern_kind, - LoopPatternKind::Pattern2Break, - "_atoi loop should be classified as Pattern2Break" - ); - - // Verify Pattern2 lowerer accepts it - assert!( - can_lower(&builder, &ctx), - "Pattern2 lowerer should accept _atoi loop with NumberAccumulation" - ); - } -} diff --git a/src/mir/builder/control_flow/joinir/patterns/router.rs b/src/mir/builder/control_flow/joinir/patterns/router.rs index 6421fb41..f7404344 100644 --- a/src/mir/builder/control_flow/joinir/patterns/router.rs +++ b/src/mir/builder/control_flow/joinir/patterns/router.rs @@ -180,7 +180,7 @@ fn lower_via_plan( /// - Cons: Extraction can be expensive (but amortized over lowering) /// /// ## StructureBased (Feature Classification) -/// - Used by: Pattern2, Pattern4 (legacy) +/// - Used by: Pattern4 (legacy) /// - Strategy: Check pattern_kind (from LoopPatternContext), plus optional structural checks /// - Pros: Fast classification, reuses centralized feature detection /// - Cons: Two sources of truth (classify + structural checks) @@ -188,7 +188,7 @@ fn lower_via_plan( /// ## Rationale for Dual Strategy: /// - Pattern6/7: Complex extraction logic (variable step, carrier tracking) /// → ExtractionBased avoids duplication between detect and extract -/// - Other patterns: Simple structural features (break/continue/if-phi) +/// - Other patterns: Simple structural features (continue) /// → StructureBased leverages centralized LoopFeatures classification /// /// This documentation prevents bugs like Phase 272 P0.2's Pattern7 issue @@ -201,7 +201,7 @@ pub(crate) enum CanLowerStrategy { ExtractionBased, /// Structure-based detection: Check pattern_kind from LoopPatternContext - /// Used by Pattern2, Pattern4 + /// Used by Pattern4 StructureBased, } @@ -222,14 +222,14 @@ pub(crate) struct LoopPatternEntry { /// /// **IMPORTANT**: Patterns are tried in array order (SSOT). /// Array order defines priority - earlier entries are tried first. -/// Pattern4 → Pattern9 → Pattern2 +/// Pattern6_NestedLoopMinimal → Pattern4 → Pattern9 /// /// # Current Patterns (Structure-based detection, established Phase 131-11+) /// /// Pattern detection strategies (updated Phase 286): /// - Pattern6/7: ExtractionBased (Plan line, Phase 273+) /// - Pattern8/9: ExtractionBased (extraction functions, Plan line Phase 286+) -/// - Pattern2/4: StructureBased (LoopFeatures classification, legacy) +/// - Pattern4: StructureBased (LoopFeatures classification, legacy) /// /// - Pattern 4: Loop with Continue (loop_continue_pattern4.hako) /// - Detection: pattern_kind == Pattern4Continue @@ -237,13 +237,10 @@ pub(crate) struct LoopPatternEntry { /// /// - Pattern3: moved to plan-based routing (planner+composer SSOT) /// -/// - Pattern 2: Loop with Conditional Break (joinir_min_loop.hako) -/// - Detection: pattern_kind == Pattern2Break -/// - Structure: has_break && !has_continue -/// /// Note: func_name is now only used for debug logging, not pattern detection /// Phase 286: Pattern5 removed (migrated to Plan-based routing) /// Phase 29ap P4: Pattern8 removed (migrated to Plan-based routing) +/// Phase 29ap P5: Pattern2 removed (migrated to Plan-based routing) pub(crate) static LOOP_PATTERNS: &[LoopPatternEntry] = &[ LoopPatternEntry { name: "Pattern6_NestedLoopMinimal", // Phase 188.3: 1-level nested loop @@ -264,11 +261,6 @@ pub(crate) static LOOP_PATTERNS: &[LoopPatternEntry] = &[ detect: super::pattern9_accum_const_loop::can_lower, lower: super::pattern9_accum_const_loop::lower, }, - LoopPatternEntry { - name: "Pattern2_WithBreak", - detect: super::pattern2_with_break::can_lower, - lower: super::pattern2_with_break::lower, - }, ]; /// Try all registered patterns in priority order. @@ -282,14 +274,14 @@ pub(crate) static LOOP_PATTERNS: &[LoopPatternEntry] = &[ /// This router uses multiple detection strategies: /// - Plan-based (Pattern6/7): extract_*_plan() → DomainPlan (Phase 273+ SSOT) /// - Extraction-based (Pattern8/9): extract_*() functions (already implemented) -/// - Structure-based (Pattern2/4): ctx.pattern_kind classification (legacy) +/// - Structure-based (Pattern4): ctx.pattern_kind classification (legacy) /// /// # Plan Line SSOT for Pattern6/7 (Phase 273+) /// /// This function implements the following routing strategy: /// 1. Try Plan-based Pattern6 (extract_scan_with_init_plan) → DomainPlan /// 2. Try Plan-based Pattern7 (extract_split_scan_plan) → DomainPlan -/// 3. Fall through to legacy Pattern2/4/9 table for other patterns +/// 3. Fall through to legacy Pattern4/9 table for other patterns /// /// The Plan line (Extractor → Normalizer → Verifier → Lowerer) is the /// current operational SSOT for Pattern6/7. Legacy patterns (1/2/4/8/9) use @@ -304,7 +296,8 @@ pub(crate) static LOOP_PATTERNS: &[LoopPatternEntry] = &[ /// SSOT Entry Points: /// - Pattern6: src/mir/builder/control_flow/plan/normalizer.rs (ScanWithInit normalization) /// - Pattern7: src/mir/builder/control_flow/plan/normalizer.rs (SplitScan normalization) -/// - Pattern2/4/9: src/mir/builder/control_flow/joinir/patterns/pattern*.rs (direct lowering) +/// - Pattern4/9: src/mir/builder/control_flow/joinir/patterns/pattern*.rs (direct lowering) +/// - Pattern6_NestedLoopMinimal: src/mir/builder/control_flow/joinir/patterns/pattern6_nested_minimal.rs pub(crate) fn route_loop_pattern( builder: &mut MirBuilder, ctx: &LoopPatternContext, diff --git a/src/mir/builder/control_flow/plan/extractors/pattern2_break.rs b/src/mir/builder/control_flow/plan/extractors/pattern2_break.rs index d2cc74f4..a9004a16 100644 --- a/src/mir/builder/control_flow/plan/extractors/pattern2_break.rs +++ b/src/mir/builder/control_flow/plan/extractors/pattern2_break.rs @@ -179,17 +179,18 @@ pub(crate) fn extract_pattern2_plan( return Ok(None); // Multiple breaks → legacy fallback } - // Step 4: Body structure must be: if { ... break } + carrier_update + loop_increment - // Expected structure: - // body[0] = If { then_body contains break } - // body[1] = carrier update assignment - // body[2] = loop_var update assignment + // Step 4: Body structure must be one of: + // A) if { ... break } + carrier_update + loop_increment + // B) local seg = s.substring(i, i + 1) + if { ... break } + loop_increment if body.len() != 3 { return Ok(None); // Unexpected body length → legacy fallback } - // Step 4.1: First element must be an If with break in then_body - let (break_condition, carrier_update_in_break) = match &body[0] { + let has_leading_local = match_local_substring_char(&body[0], &parts.loop_var); + let break_stmt_idx = if has_leading_local { 1 } else { 0 }; + + // Step 4.1: Break guard (If with break in then_body) + let (break_condition, carrier_update_in_break) = match &body[break_stmt_idx] { ASTNode::If { condition: if_cond, then_body, @@ -230,19 +231,26 @@ pub(crate) fn extract_pattern2_plan( (break_cond, carrier_update) } - _ => return Ok(None), // First element not If → legacy + _ => return Ok(None), // Break guard missing → legacy }; - // Step 4.2: Extract carrier update in body (second element) - let (carrier_var, carrier_update_in_body) = match &body[1] { - ASTNode::Assignment { target, value, .. } => { - let carrier_name = match target.as_ref() { - ASTNode::Variable { name, .. } => name.clone(), - _ => return Ok(None), - }; - (carrier_name, value.as_ref().clone()) + // Step 4.2: Extract carrier update in body (optional) + let (carrier_var, carrier_update_in_body) = if has_leading_local { + let loop_increment = + super::common_helpers::extract_loop_increment_plan(body, &parts.loop_var)? + .ok_or_else(|| "Pattern2: missing loop increment".to_string())?; + (parts.loop_var.clone(), loop_increment.clone()) + } else { + match &body[1] { + ASTNode::Assignment { target, value, .. } => { + let carrier_name = match target.as_ref() { + ASTNode::Variable { name, .. } => name.clone(), + _ => return Ok(None), + }; + (carrier_name, value.as_ref().clone()) + } + _ => return Ok(None), } - _ => return Ok(None), }; // Step 4.3: Extract loop increment (third element) @@ -251,7 +259,7 @@ pub(crate) fn extract_pattern2_plan( None => return Ok(None), // No loop increment found → legacy }; - // Step 5: Validate loop condition is ` < ` (same as Pattern1) + // Step 5: Validate loop condition is ` < ` or ` < ` if !validate_loop_condition_for_plan(condition, &parts.loop_var) { return Ok(None); } @@ -290,23 +298,82 @@ fn validate_loop_condition_for_plan(cond: &ASTNode, loop_var: &str) -> bool { return false; } - // Right must be integer literal - if !matches!( + // Right must be integer literal or `.length()` + if matches!( right.as_ref(), ASTNode::Literal { value: LiteralValue::Integer(_), .. } ) { - return false; + return true; } - true + matches!( + right.as_ref(), + ASTNode::MethodCall { object, method, arguments, .. } + if method == "length" + && arguments.is_empty() + && matches!(object.as_ref(), ASTNode::Variable { .. }) + ) } else { false } } +fn match_local_substring_char(stmt: &ASTNode, loop_var: &str) -> bool { + let ASTNode::Local { + variables, + initial_values, + .. + } = stmt + else { + return false; + }; + if variables.len() != 1 || initial_values.len() != 1 { + return false; + } + let Some(expr) = initial_values[0].as_ref() else { + return false; + }; + let ASTNode::MethodCall { + object, + method, + arguments, + .. + } = expr.as_ref() + else { + return false; + }; + if method != "substring" || arguments.len() != 2 { + return false; + } + if !matches!(object.as_ref(), ASTNode::Variable { .. }) { + return false; + } + if !matches!(&arguments[0], ASTNode::Variable { name, .. } if name == loop_var) { + return false; + } + match &arguments[1] { + ASTNode::BinaryOp { + operator: BinaryOperator::Add, + left, + right, + .. + } => { + matches!(left.as_ref(), ASTNode::Variable { name, .. } if name == loop_var) + && matches!( + right.as_ref(), + ASTNode::Literal { + value: LiteralValue::Integer(1), + .. + } + ) + } + _ => false, + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/mir/builder/control_flow/plan/facts/pattern2_break_facts.rs b/src/mir/builder/control_flow/plan/facts/pattern2_break_facts.rs index b7671009..e6e3ed7e 100644 --- a/src/mir/builder/control_flow/plan/facts/pattern2_break_facts.rs +++ b/src/mir/builder/control_flow/plan/facts/pattern2_break_facts.rs @@ -26,6 +26,16 @@ pub(in crate::mir::builder) fn try_extract_pattern2_break_facts( condition: &ASTNode, body: &[ASTNode], ) -> Result, Freeze> { + if let Some(parse_int) = try_extract_pattern2_break_parse_integer_subset(condition, body) { + return Ok(Some(parse_int)); + } + + if let Some(trim_whitespace) = + try_extract_pattern2_break_trim_whitespace_subset(condition, body) + { + return Ok(Some(trim_whitespace)); + } + if let Some(realworld) = try_extract_pattern2_break_realworld_subset(condition, body) { return Ok(Some(realworld)); } @@ -86,6 +96,490 @@ pub(in crate::mir::builder) fn try_extract_pattern2_break_facts( })) } +fn try_extract_pattern2_break_parse_integer_subset( + condition: &ASTNode, + body: &[ASTNode], +) -> Option { + let loop_var = extract_loop_var_for_len_condition(condition)?; + + let counts = count_control_flow(body, ControlFlowDetector::default()); + if counts.break_count != 1 || counts.continue_count > 0 || counts.return_count > 0 { + return None; + } + + if body.len() != 5 { + return None; + } + + // local ch = s.substring(i, i + 1) + let (ch_var, haystack_var, ch_expr) = match_local_substring_char(&body[0], &loop_var)?; + + // local d = this.index_of(digits, ch) + let (digits_var, _d_var) = match_local_this_index_of(&body[1], &ch_var)?; + + // if d < 0 { break } + let break_var = match_break_if_less_than_zero(&body[2])?; + + // acc = acc * 10 + d + let carrier_var = match_acc_update_mul10_plus_d(&body[3], &break_var)?; + + // i = i + 1 + let loop_increment = extract_loop_increment_at_end(body, &loop_var)?; + + // Rebuild break_condition and carrier_update_in_body without relying on the local `d`. + // This avoids requiring a local binding in PlanNormalizer while keeping semantics. + let index_expr = ASTNode::MethodCall { + object: Box::new(ASTNode::This { span: Span::unknown() }), + method: "index_of".to_string(), + arguments: vec![var(&digits_var), ch_expr], + span: Span::unknown(), + }; + let break_condition = ASTNode::BinaryOp { + operator: BinaryOperator::Less, + left: Box::new(index_expr.clone()), + right: Box::new(lit_int(0)), + span: Span::unknown(), + }; + let carrier_update_in_body = ASTNode::BinaryOp { + operator: BinaryOperator::Add, + left: Box::new(ASTNode::BinaryOp { + operator: BinaryOperator::Multiply, + left: Box::new(var(&carrier_var)), + right: Box::new(lit_int(10)), + span: Span::unknown(), + }), + right: Box::new(index_expr), + span: Span::unknown(), + }; + + // Also confirm the substring source matches the haystack var and loop var. + // (We don't need to store them, but the match helps prevent accidental overlap.) + let _ = &haystack_var; + + Some(Pattern2BreakFacts { + loop_var, + carrier_var, + loop_condition: condition.clone(), + break_condition, + carrier_update_in_break: None, + carrier_update_in_body, + loop_increment, + }) +} + +fn match_local_substring_char( + stmt: &ASTNode, + loop_var: &str, +) -> Option<(String, String, ASTNode)> { + let ASTNode::Local { + variables, + initial_values, + .. + } = stmt else { + return None; + }; + if variables.len() != 1 || initial_values.len() != 1 { + return None; + } + let ch_var = variables[0].clone(); + let Some(expr) = initial_values[0].as_ref() else { + return None; + }; + let ASTNode::MethodCall { + object, + method, + arguments, + .. + } = expr.as_ref() else { + return None; + }; + if method != "substring" || arguments.len() != 2 { + return None; + } + let ASTNode::Variable { name: haystack_var, .. } = object.as_ref() else { + return None; + }; + if !matches!(&arguments[0], ASTNode::Variable { name, .. } if name == loop_var) { + return None; + } + // end = i + 1 + match &arguments[1] { + ASTNode::BinaryOp { + operator: BinaryOperator::Add, + left, + right, + .. + } => { + if !matches!(left.as_ref(), ASTNode::Variable { name, .. } if name == loop_var) { + return None; + } + if !matches!( + right.as_ref(), + ASTNode::Literal { + value: LiteralValue::Integer(1), + .. + } + ) { + return None; + } + } + _ => return None, + } + + Some((ch_var, haystack_var.clone(), expr.as_ref().clone())) +} + +fn match_local_this_index_of(stmt: &ASTNode, ch_var: &str) -> Option<(String, String)> { + let ASTNode::Local { + variables, + initial_values, + .. + } = stmt else { + return None; + }; + if variables.len() != 1 || initial_values.len() != 1 { + return None; + } + let d_var = variables[0].clone(); + let Some(expr) = initial_values[0].as_ref() else { + return None; + }; + let ASTNode::MethodCall { + object, + method, + arguments, + .. + } = expr.as_ref() else { + return None; + }; + if method != "index_of" || arguments.len() != 2 { + return None; + } + if !matches!(object.as_ref(), ASTNode::This { .. } | ASTNode::Me { .. }) { + return None; + } + let ASTNode::Variable { name: digits_var, .. } = &arguments[0] else { + return None; + }; + if !matches!(&arguments[1], ASTNode::Variable { name, .. } if name == ch_var) { + return None; + } + + Some((digits_var.clone(), d_var)) +} + +fn match_break_if_less_than_zero(stmt: &ASTNode) -> Option { + let (cond, update_opt) = extract_break_if_parts(stmt)?; + if update_opt.is_some() { + return None; + } + let ASTNode::BinaryOp { + operator: BinaryOperator::Less, + left, + right, + .. + } = cond else { + return None; + }; + let ASTNode::Variable { name, .. } = left.as_ref() else { + return None; + }; + if !matches!( + right.as_ref(), + ASTNode::Literal { + value: LiteralValue::Integer(0), + .. + } + ) { + return None; + } + Some(name.clone()) +} + +fn match_acc_update_mul10_plus_d(stmt: &ASTNode, d_var: &str) -> Option { + let ASTNode::Assignment { target, value, .. } = stmt else { + return None; + }; + let ASTNode::Variable { name: carrier_var, .. } = target.as_ref() else { + return None; + }; + let ASTNode::BinaryOp { + operator: BinaryOperator::Add, + left, + right, + .. + } = value.as_ref() else { + return None; + }; + + // left: acc * 10 + match left.as_ref() { + ASTNode::BinaryOp { + operator: BinaryOperator::Multiply, + left: mul_lhs, + right: mul_rhs, + .. + } => { + if !matches!(mul_lhs.as_ref(), ASTNode::Variable { name, .. } if name == carrier_var) { + return None; + } + if !matches!( + mul_rhs.as_ref(), + ASTNode::Literal { + value: LiteralValue::Integer(10), + .. + } + ) { + return None; + } + } + _ => return None, + } + + // right: d + if !matches!(right.as_ref(), ASTNode::Variable { name, .. } if name == d_var) { + return None; + } + + Some(carrier_var.clone()) +} + +fn try_extract_pattern2_break_trim_whitespace_subset( + condition: &ASTNode, + body: &[ASTNode], +) -> Option { + let loop_var = extract_trim_loop_var(condition)?; + + let counts = count_control_flow(body, ControlFlowDetector::default()); + if counts.break_count != 1 || counts.continue_count > 0 || counts.return_count > 0 { + return None; + } + + if body.len() != 2 { + return None; + } + + let break_condition = extract_trim_break_condition(&body[0], &loop_var)?; + let loop_increment = extract_trim_loop_increment(&body[1], &loop_var)?; + + Some(Pattern2BreakFacts { + loop_var: loop_var.clone(), + carrier_var: loop_var, + loop_condition: condition.clone(), + break_condition, + carrier_update_in_break: None, + carrier_update_in_body: loop_increment.clone(), + loop_increment, + }) +} + +fn extract_trim_loop_var(condition: &ASTNode) -> Option { + let ASTNode::BinaryOp { + operator, + left, + right, + .. + } = condition + else { + return None; + }; + let ASTNode::Variable { name: loop_var, .. } = left.as_ref() else { + return None; + }; + + match operator { + BinaryOperator::Less | BinaryOperator::LessEqual => { + if matches!( + right.as_ref(), + ASTNode::MethodCall { object, method, arguments, .. } + if method == "length" + && arguments.is_empty() + && matches!(object.as_ref(), ASTNode::Variable { .. }) + ) { + return Some(loop_var.clone()); + } + } + BinaryOperator::GreaterEqual => { + if matches!( + right.as_ref(), + ASTNode::Literal { + value: LiteralValue::Integer(0), + .. + } + ) { + return Some(loop_var.clone()); + } + } + _ => {} + } + + None +} + +fn extract_trim_break_condition(stmt: &ASTNode, loop_var: &str) -> Option { + let ASTNode::If { + condition, + then_body, + else_body, + .. + } = stmt + else { + return None; + }; + if else_body.is_some() { + return None; + } + if then_body.len() != 1 || !matches!(then_body[0], ASTNode::Break { .. }) { + return None; + } + + let whitespace_call = match condition.as_ref() { + ASTNode::UnaryOp { operator, operand, .. } => { + use crate::ast::UnaryOperator; + if !matches!(operator, UnaryOperator::Not) { + return None; + } + match_is_whitespace_call(operand.as_ref(), loop_var)? + } + ASTNode::BinaryOp { + operator: BinaryOperator::Equal, + left, + right, + .. + } => { + if matches!( + right.as_ref(), + ASTNode::Literal { + value: LiteralValue::Bool(false), + .. + } + ) { + match_is_whitespace_call(left.as_ref(), loop_var)? + } else if matches!( + left.as_ref(), + ASTNode::Literal { + value: LiteralValue::Bool(false), + .. + } + ) { + match_is_whitespace_call(right.as_ref(), loop_var)? + } else { + return None; + } + } + _ => return None, + }; + + Some(ASTNode::BinaryOp { + operator: BinaryOperator::Equal, + left: Box::new(whitespace_call), + right: Box::new(lit_bool(false)), + span: Span::unknown(), + }) +} + +fn match_is_whitespace_call(expr: &ASTNode, loop_var: &str) -> Option { + let ASTNode::MethodCall { + object, + method, + arguments, + .. + } = expr + else { + return None; + }; + if method != "is_whitespace" || arguments.len() != 1 { + return None; + } + let normalized_object = match object.as_ref() { + ASTNode::This { .. } => ASTNode::This { span: Span::unknown() }, + ASTNode::Me { .. } => ASTNode::This { span: Span::unknown() }, + _ => return None, + }; + if !matches_substring_at_loop_var(&arguments[0], loop_var) { + return None; + } + Some(ASTNode::MethodCall { + object: Box::new(normalized_object), + method: method.clone(), + arguments: arguments.clone(), + span: Span::unknown(), + }) +} + +fn matches_substring_at_loop_var(expr: &ASTNode, loop_var: &str) -> bool { + let ASTNode::MethodCall { + object, + method, + arguments, + .. + } = expr + else { + return false; + }; + if method != "substring" || arguments.len() != 2 { + return false; + } + if !matches!(object.as_ref(), ASTNode::Variable { .. }) { + return false; + } + if !matches!(&arguments[0], ASTNode::Variable { name, .. } if name == loop_var) { + return false; + } + match &arguments[1] { + ASTNode::BinaryOp { + operator: BinaryOperator::Add, + left, + right, + .. + } => { + matches!(left.as_ref(), ASTNode::Variable { name, .. } if name == loop_var) + && matches!( + right.as_ref(), + ASTNode::Literal { + value: LiteralValue::Integer(1), + .. + } + ) + } + _ => false, + } +} + +fn extract_trim_loop_increment(stmt: &ASTNode, loop_var: &str) -> Option { + let ASTNode::Assignment { target, value, .. } = stmt else { + return None; + }; + let ASTNode::Variable { name, .. } = target.as_ref() else { + return None; + }; + if name != loop_var { + return None; + } + let ASTNode::BinaryOp { + operator: BinaryOperator::Add | BinaryOperator::Subtract, + left, + right, + .. + } = value.as_ref() + else { + return None; + }; + if !matches!(left.as_ref(), ASTNode::Variable { name, .. } if name == loop_var) { + return None; + } + if !matches!( + right.as_ref(), + ASTNode::Literal { + value: LiteralValue::Integer(1), + .. + } + ) { + return None; + } + Some(value.as_ref().clone()) +} + fn try_extract_pattern2_break_realworld_subset( condition: &ASTNode, body: &[ASTNode], @@ -729,6 +1223,13 @@ fn lit_int(value: i64) -> ASTNode { } } +fn lit_bool(value: bool) -> ASTNode { + ASTNode::Literal { + value: LiteralValue::Bool(value), + span: Span::unknown(), + } +} + fn lit_str(value: &str) -> ASTNode { ASTNode::Literal { value: LiteralValue::String(value.to_string()), @@ -838,6 +1339,15 @@ mod tests { } } + fn this_method_call(method: &str, args: Vec) -> ASTNode { + ASTNode::MethodCall { + object: Box::new(ASTNode::This { span: Span::unknown() }), + method: method.to_string(), + arguments: args, + span: Span::unknown(), + } + } + fn binop(operator: BinaryOperator, left: ASTNode, right: ASTNode) -> ASTNode { ASTNode::BinaryOp { operator, @@ -847,6 +1357,60 @@ mod tests { } } + #[test] + fn extract_pattern2_break_parse_integer_subset() { + let condition = binop( + BinaryOperator::Less, + v("i"), + method_call("s", "length", vec![]), + ); + let body = vec![ + local( + "ch", + method_call( + "s", + "substring", + vec![v("i"), binop(BinaryOperator::Add, v("i"), lit_int(1))], + ), + ), + ASTNode::Local { + variables: vec!["d".to_string()], + initial_values: vec![Some(Box::new(ASTNode::MethodCall { + object: Box::new(ASTNode::This { span: Span::unknown() }), + method: "index_of".to_string(), + arguments: vec![v("digits"), v("ch")], + span: Span::unknown(), + }))], + span: Span::unknown(), + }, + ASTNode::If { + condition: Box::new(binop(BinaryOperator::Less, v("d"), lit_int(0))), + then_body: vec![ASTNode::Break { span: Span::unknown() }], + else_body: None, + span: Span::unknown(), + }, + assign( + "acc", + binop( + BinaryOperator::Add, + binop(BinaryOperator::Multiply, v("acc"), lit_int(10)), + v("d"), + ), + ), + assign("i", binop(BinaryOperator::Add, v("i"), lit_int(1))), + ]; + + let facts = try_extract_pattern2_break_facts(&condition, &body) + .expect("Ok") + .expect("Some facts"); + assert_eq!(facts.loop_var, "i"); + assert_eq!(facts.carrier_var, "acc"); + assert!(matches!( + facts.break_condition, + ASTNode::BinaryOp { operator: BinaryOperator::Less, .. } + )); + } + #[test] fn extract_pattern2_break_realworld_subset() { let condition = lit_bool(true); @@ -927,6 +1491,113 @@ mod tests { } } + #[test] + fn extract_pattern2_break_trim_whitespace_subset_start() { + use crate::ast::UnaryOperator; + + let condition = binop( + BinaryOperator::Less, + v("i"), + method_call("s", "length", vec![]), + ); + let body = vec![ + ASTNode::If { + condition: Box::new(ASTNode::UnaryOp { + operator: UnaryOperator::Not, + operand: Box::new(this_method_call( + "is_whitespace", + vec![method_call( + "s", + "substring", + vec![v("i"), binop(BinaryOperator::Add, v("i"), lit_int(1))], + )], + )), + span: Span::unknown(), + }), + then_body: vec![ASTNode::Break { span: Span::unknown() }], + else_body: None, + span: Span::unknown(), + }, + assign("i", binop(BinaryOperator::Add, v("i"), lit_int(1))), + ]; + + let facts = try_extract_pattern2_break_facts(&condition, &body) + .expect("Ok") + .expect("Some facts"); + assert_eq!(facts.loop_var, "i"); + assert_eq!(facts.carrier_var, "i"); + assert!(matches!( + facts.break_condition, + ASTNode::BinaryOp { operator: BinaryOperator::Equal, .. } + )); + } + + #[test] + fn extract_pattern2_break_trim_whitespace_subset_end() { + use crate::ast::UnaryOperator; + + let condition = binop(BinaryOperator::GreaterEqual, v("i"), lit_int(0)); + let body = vec![ + ASTNode::If { + condition: Box::new(ASTNode::UnaryOp { + operator: UnaryOperator::Not, + operand: Box::new(this_method_call( + "is_whitespace", + vec![method_call( + "s", + "substring", + vec![v("i"), binop(BinaryOperator::Add, v("i"), lit_int(1))], + )], + )), + span: Span::unknown(), + }), + then_body: vec![ASTNode::Break { span: Span::unknown() }], + else_body: None, + span: Span::unknown(), + }, + assign("i", binop(BinaryOperator::Subtract, v("i"), lit_int(1))), + ]; + + let facts = try_extract_pattern2_break_facts(&condition, &body) + .expect("Ok") + .expect("Some facts"); + assert_eq!(facts.loop_var, "i"); + assert_eq!(facts.carrier_var, "i"); + assert!(matches!( + facts.loop_increment, + ASTNode::BinaryOp { operator: BinaryOperator::Subtract, .. } + )); + } + + #[test] + fn extract_pattern2_break_trim_whitespace_subset_rejects_missing_not() { + let condition = binop( + BinaryOperator::Less, + v("i"), + method_call("s", "length", vec![]), + ); + let body = vec![ + ASTNode::If { + condition: Box::new(this_method_call( + "is_whitespace", + vec![method_call( + "s", + "substring", + vec![v("i"), binop(BinaryOperator::Add, v("i"), lit_int(1))], + )], + )), + then_body: vec![ASTNode::Break { span: Span::unknown() }], + else_body: None, + span: Span::unknown(), + }, + assign("i", binop(BinaryOperator::Add, v("i"), lit_int(1))), + ]; + + let facts = try_extract_pattern2_break_facts(&condition, &body) + .expect("Ok"); + assert!(facts.is_none()); + } + #[test] fn extract_pattern2_break_loopbodylocal_trim_seg_subset() { let condition = binop( diff --git a/src/mir/builder/control_flow/plan/normalizer/helpers.rs b/src/mir/builder/control_flow/plan/normalizer/helpers.rs index 9e919507..57957175 100644 --- a/src/mir/builder/control_flow/plan/normalizer/helpers.rs +++ b/src/mir/builder/control_flow/plan/normalizer/helpers.rs @@ -215,27 +215,6 @@ impl super::PlanNormalizer { Ok((value_id, vec![const_effect])) } ASTNode::MethodCall { object, method, arguments, .. } => { - let object_name = match object.as_ref() { - ASTNode::Variable { name, .. } => name.clone(), - _ => { - return Err(format!( - "[normalizer] Method call on non-variable object: {:?}", - object - )) - } - }; - - let object_id = if let Some(&phi_dst) = phi_bindings.get(&object_name) { - phi_dst - } else if let Some(&value_id) = builder.variable_ctx.variable_map.get(&object_name) { - value_id - } else { - return Err(format!( - "[normalizer] Method call object {} not found", - object_name - )); - }; - let mut arg_ids = Vec::new(); let mut arg_effects = Vec::new(); for arg in arguments { @@ -247,13 +226,44 @@ impl super::PlanNormalizer { let result_id = builder.next_value_id(); builder.type_ctx.value_types.insert(result_id, MirType::Integer); - arg_effects.push(CoreEffectPlan::MethodCall { - dst: Some(result_id), - object: object_id, - method: method.clone(), - args: arg_ids, - effects: EffectMask::PURE.add(Effect::Io), - }); + match object.as_ref() { + ASTNode::Variable { name, .. } => { + let object_id = if let Some(&phi_dst) = phi_bindings.get(name) { + phi_dst + } else if let Some(&value_id) = builder.variable_ctx.variable_map.get(name) { + value_id + } else { + return Err(format!( + "[normalizer] Method call object {} not found", + name + )); + }; + arg_effects.push(CoreEffectPlan::MethodCall { + dst: Some(result_id), + object: object_id, + method: method.clone(), + args: arg_ids, + effects: EffectMask::PURE.add(Effect::Io), + }); + } + ASTNode::This { .. } | ASTNode::Me { .. } => { + let Some(box_name) = builder.comp_ctx.current_static_box.clone() else { + return Err("[normalizer] this.method() without current_static_box".to_string()); + }; + let func = format!("{}.{}/{}", box_name, method, arguments.len()); + arg_effects.push(CoreEffectPlan::GlobalCall { + dst: Some(result_id), + func, + args: arg_ids, + }); + } + _ => { + return Err(format!( + "[normalizer] Method call on unsupported object: {:?}", + object + )); + } + } Ok((result_id, arg_effects)) } diff --git a/src/mir/join_ir/lowering/loop_patterns/with_break.rs b/src/mir/join_ir/lowering/loop_patterns/with_break.rs index dadcdcb6..8680dce6 100644 --- a/src/mir/join_ir/lowering/loop_patterns/with_break.rs +++ b/src/mir/join_ir/lowering/loop_patterns/with_break.rs @@ -107,7 +107,7 @@ pub fn lower_loop_with_break_to_joinir( // // Related code: // - Router callsite: loop_pattern_router.rs:148 - // - Actual implementation: control_flow::cf_loop_pattern2_with_break() + // - Actual implementation: Plan/Composer route (PlanLowerer) // - Minimal lowerer: loop_with_break_minimal::lower_loop_with_break_minimal() eprintln!("[loop_patterns] Pattern 2: Lowering delegated to control_flow.rs (stub)");