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 e4ad3e9b..0f587cbc 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 @@ -75,6 +75,11 @@ impl ConditionEnvBuilder { let loop_var_join_id = space.alloc_param(); env.insert(loop_var_name.to_string(), loop_var_join_id); + // 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.) + // This comment serves as a reminder for future developers. + // For each condition variable, allocate JoinIR-local ValueId and build binding for var_name in &condition_var_names { let host_id = variable_map.get(var_name).copied().ok_or_else(|| { 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 index c37ae70e..8f29be82 100644 --- a/src/mir/builder/control_flow/joinir/patterns/pattern2_with_break.rs +++ b/src/mir/builder/control_flow/joinir/patterns/pattern2_with_break.rs @@ -104,7 +104,7 @@ fn prepare_pattern2_inputs( // Value space + condition env let mut join_value_space = JoinValueSpace::new(); - let (mut env, mut condition_bindings, _loop_var_join_id) = + let (mut env, mut condition_bindings, loop_var_join_id) = ConditionEnvBuilder::build_for_break_condition_v2( condition, &loop_var_name, @@ -113,6 +113,20 @@ fn prepare_pattern2_inputs( &mut join_value_space, )?; + // Phase 79-2: Register loop variable BindingId (dev-only) + #[cfg(feature = "normalized_dev")] + if let Some(loop_var_bid) = builder.binding_map.get(&loop_var_name).copied() { + env.register_loop_var_binding(loop_var_bid, loop_var_join_id); + log_pattern2( + verbose, + "phase79", + format!( + "Registered loop var BindingId: '{}' BindingId({}) → ValueId({})", + loop_var_name, loop_var_bid.0, loop_var_join_id.0 + ), + ); + } + log_pattern2( verbose, "phase201", @@ -369,9 +383,36 @@ fn promote_and_prepare_carriers( carrier.join_id = Some(carrier_join_id); #[cfg(feature = "normalized_dev")] if let Some(binding_id) = carrier.binding_id { - inputs - .env - .register_carrier_binding(binding_id, carrier_join_id); + // Phase 79-2: Use role-specific registration method + use crate::mir::join_ir::lowering::carrier_info::CarrierRole; + match carrier.role { + CarrierRole::ConditionOnly => { + inputs + .env + .register_condition_binding(binding_id, carrier_join_id); + log_pattern2( + verbose, + "phase79", + format!( + "Registered condition-only carrier '{}' BindingId({}) → ValueId({})", + carrier.name, binding_id.0, carrier_join_id.0 + ), + ); + } + CarrierRole::LoopState => { + inputs + .env + .register_carrier_binding(binding_id, carrier_join_id); + log_pattern2( + verbose, + "phase79", + format!( + "Registered loop-state carrier '{}' BindingId({}) → ValueId({})", + carrier.name, binding_id.0, carrier_join_id.0 + ), + ); + } + } } log_pattern2( verbose, diff --git a/src/mir/join_ir/lowering/condition_env.rs b/src/mir/join_ir/lowering/condition_env.rs index a7065cae..ec00a55b 100644 --- a/src/mir/join_ir/lowering/condition_env.rs +++ b/src/mir/join_ir/lowering/condition_env.rs @@ -199,6 +199,65 @@ impl ConditionEnv { self.binding_id_map.insert(binding_id, join_value_id); } + /// Phase 79-2: Register loop variable BindingId → ValueId mapping (dev-only) + /// + /// This method registers the loop variable (e.g., "i", "p") BindingId + /// after header PHI allocation, enabling BindingId-based lookup during + /// condition lowering. + /// + /// # Arguments + /// + /// * `binding_id` - BindingId for the loop variable + /// * `value_id` - JoinIR-local ValueId for loop var (after header PHI) + /// + /// # Example + /// + /// ```ignore + /// // After allocating loop param in header PHI + /// if let Some(loop_var_bid) = builder.binding_map.get(&loop_var_name).copied() { + /// condition_env.register_loop_var_binding(loop_var_bid, loop_param_value_id); + /// } + /// ``` + #[cfg(feature = "normalized_dev")] + pub fn register_loop_var_binding( + &mut self, + binding_id: BindingId, + value_id: ValueId, + ) { + self.binding_id_map.insert(binding_id, value_id); + } + + /// Phase 79-2: Register condition-only variable BindingId → ValueId mapping (dev-only) + /// + /// This method registers condition-only variables (variables used only in + /// loop/break conditions, not in loop body) for BindingId-based lookup. + /// + /// # Arguments + /// + /// * `binding_id` - BindingId for the condition-only variable + /// * `value_id` - JoinIR-local ValueId for this variable + /// + /// # Example + /// + /// ```ignore + /// // After allocating condition-only carriers + /// for carrier in &carrier_info.carriers { + /// if carrier.role == CarrierRole::ConditionOnly { + /// if let Some(bid) = carrier.binding_id { + /// condition_env.register_condition_binding(bid, carrier.join_id); + /// } + /// } + /// } + /// ``` + #[cfg(feature = "normalized_dev")] + pub fn register_condition_binding( + &mut self, + binding_id: BindingId, + value_id: ValueId, + ) { + self.binding_id_map.insert(binding_id, value_id); + } + /// Phase 75: Resolve variable with BindingId priority (dev-only, pilot integration) /// /// Look up variable by BindingId first, falling back to name-based lookup. diff --git a/src/mir/join_ir/lowering/expr_lowerer.rs b/src/mir/join_ir/lowering/expr_lowerer.rs index 1ddd71f5..4b98ea51 100644 --- a/src/mir/join_ir/lowering/expr_lowerer.rs +++ b/src/mir/join_ir/lowering/expr_lowerer.rs @@ -190,8 +190,11 @@ impl<'env, 'builder, S: ScopeManager> ExprLowerer<'env, 'builder, S> { } // 2. Build ConditionEnv from ScopeManager - // This is the key integration point: we translate ScopeManager's view - // into the ConditionEnv format expected by condition_lowerer. + // Phase 79-1: Use BindingId-aware version when available + #[cfg(feature = "normalized_dev")] + let condition_env = scope_resolution::build_condition_env_from_scope_with_binding(self.scope, ast, self.builder)?; + + #[cfg(not(feature = "normalized_dev"))] let condition_env = scope_resolution::build_condition_env_from_scope(self.scope, ast)?; // 3. Delegate to existing condition_lowerer diff --git a/src/mir/join_ir/lowering/expr_lowerer/scope_resolution.rs b/src/mir/join_ir/lowering/expr_lowerer/scope_resolution.rs index 90dc899c..4100dfd4 100644 --- a/src/mir/join_ir/lowering/expr_lowerer/scope_resolution.rs +++ b/src/mir/join_ir/lowering/expr_lowerer/scope_resolution.rs @@ -1,7 +1,45 @@ use super::{ExprLoweringError, ScopeManager}; use crate::ast::ASTNode; use crate::mir::join_ir::lowering::condition_env::ConditionEnv; +#[cfg(feature = "normalized_dev")] +use crate::mir::builder::MirBuilder; +/// Phase 79-1: Build ConditionEnv with BindingId support (dev-only) +/// +/// This is the BindingId-aware version that checks builder.binding_map first. +#[cfg(feature = "normalized_dev")] +pub(crate) fn build_condition_env_from_scope_with_binding( + scope: &S, + ast: &ASTNode, + builder: &MirBuilder, +) -> Result { + let mut env = ConditionEnv::new(); + let mut vars = Vec::new(); + collect_vars(ast, &mut vars); + + for var in vars { + let binding_id = builder.binding_map.get(&var).copied(); + + #[cfg(feature = "normalized_dev")] + if let Some(bid) = binding_id { + if let Some(value_id) = scope.lookup_with_binding(Some(bid), &var) { + env.insert(var.clone(), value_id); + continue; + } + } + + // Fallback to name-based lookup + if let Some(id) = scope.lookup(&var) { + env.insert(var.clone(), id); + } else { + return Err(ExprLoweringError::VariableNotFound(var)); + } + } + + Ok(env) +} + +/// Phase 79-1: Legacy name-only version (for non-dev builds) pub(crate) fn build_condition_env_from_scope( scope: &S, ast: &ASTNode, diff --git a/tests/normalized_joinir_min.rs b/tests/normalized_joinir_min.rs index 23d1769e..be6b946e 100644 --- a/tests/normalized_joinir_min.rs +++ b/tests/normalized_joinir_min.rs @@ -2178,3 +2178,206 @@ fn test_phase70c_merge_relay_same_owner_accepted() { eprintln!("[phase70c/test] Merge relay validation accepted: multiple inner loops → same owner"); } + +// ============================================================================ +// Phase 79-3: BindingId Lookup E2E Tests +// ============================================================================ + +/// Phase 79-3: Test BindingId lookup for DigitPos pattern (digit_pos < 0) +/// +/// This test verifies that BindingId-based variable resolution works +/// during condition lowering for the classic DigitPos break pattern. +#[test] +#[cfg(feature = "normalized_dev")] +fn test_phase79_digitpos_bindingid_lookup_works() { + use nyash_rust::ast::{ASTNode, BinaryOperator, LiteralValue, Span}; + use nyash_rust::mir::builder::MirBuilder; + + // Enable debug logging to verify BindingId hits + std::env::set_var("NYASH_JOINIR_DEBUG", "1"); + + fn var(name: &str) -> ASTNode { + ASTNode::Variable { + name: name.to_string(), + span: Span::unknown(), + } + } + fn lit_i(i: i64) -> ASTNode { + ASTNode::Literal { + value: LiteralValue::Integer(i), + span: Span::unknown(), + } + } + + // Build minimal AST with digit_pos < 0 pattern + let ast = ASTNode::FunctionDeclaration { + name: "test_digitpos".to_string(), + params: vec![], + body: vec![ + ASTNode::Local { + variables: vec!["i".to_string()], + initial_values: vec![Some(Box::new(lit_i(0)))], + span: Span::unknown(), + }, + ASTNode::Loop { + condition: Box::new(ASTNode::BinaryOp { + operator: BinaryOperator::Less, + left: Box::new(var("i")), + right: Box::new(lit_i(10)), + span: Span::unknown(), + }), + body: vec![ + ASTNode::Local { + variables: vec!["digit_pos".to_string()], + initial_values: vec![Some(Box::new(lit_i(-1)))], + span: Span::unknown(), + }, + ASTNode::If { + condition: Box::new(ASTNode::BinaryOp { + operator: BinaryOperator::Less, + left: Box::new(var("digit_pos")), + right: Box::new(lit_i(0)), + span: Span::unknown(), + }), + then_body: vec![ASTNode::Break { + span: Span::unknown(), + }], + else_body: vec![], + span: Span::unknown(), + }, + ], + span: Span::unknown(), + }, + ], + span: Span::unknown(), + return_type: None, + }; + + // Lower to MIR (this will trigger Pattern2 lowering with BindingId lookup) + let mut builder = MirBuilder::new(); + let result = builder.lower_function_declaration(&ast); + + assert!( + result.is_ok(), + "DigitPos pattern should lower successfully via BindingId path" + ); + + eprintln!("[phase79/test] DigitPos BindingId lookup test completed"); +} + +/// Phase 79-3: Test BindingId lookup for Trim pattern (ch == ' ' || ch == '\\t') +/// +/// This test verifies that BindingId-based variable resolution works +/// for the Trim pattern with character comparison conditions. +#[test] +#[cfg(feature = "normalized_dev")] +fn test_phase79_trim_bindingid_lookup_works() { + use nyash_rust::ast::{ASTNode, BinaryOperator, LiteralValue, Span, UnaryOperator}; + use nyash_rust::mir::builder::MirBuilder; + + // Enable debug logging to verify BindingId hits + std::env::set_var("NYASH_JOINIR_DEBUG", "1"); + + fn var(name: &str) -> ASTNode { + ASTNode::Variable { + name: name.to_string(), + span: Span::unknown(), + } + } + fn lit_i(i: i64) -> ASTNode { + ASTNode::Literal { + value: LiteralValue::Integer(i), + span: Span::unknown(), + } + } + fn lit_str(s: &str) -> ASTNode { + ASTNode::Literal { + value: LiteralValue::String(s.to_string()), + span: Span::unknown(), + } + } + + // Build minimal AST with Trim pattern (ch == ' ' || ch == '\t') + let ast = ASTNode::FunctionDeclaration { + name: "test_trim".to_string(), + params: vec![], + body: vec![ + ASTNode::Local { + variables: vec!["s".to_string()], + initial_values: vec![Some(Box::new(lit_str(" hello ")))], + span: Span::unknown(), + }, + ASTNode::Local { + variables: vec!["p".to_string()], + initial_values: vec![Some(Box::new(lit_i(0)))], + span: Span::unknown(), + }, + ASTNode::Loop { + condition: Box::new(ASTNode::MethodCall { + object: Box::new(var("p")), + method: "less".to_string(), + arguments: vec![ASTNode::MethodCall { + object: Box::new(var("s")), + method: "length".to_string(), + arguments: vec![], + span: Span::unknown(), + }], + span: Span::unknown(), + }), + body: vec![ + ASTNode::Local { + variables: vec!["ch".to_string()], + initial_values: vec![Some(Box::new(ASTNode::MethodCall { + object: Box::new(var("s")), + method: "get".to_string(), + arguments: vec![var("p")], + span: Span::unknown(), + }))], + span: Span::unknown(), + }, + ASTNode::If { + condition: Box::new(ASTNode::UnaryOp { + operator: UnaryOperator::Not, + operand: Box::new(ASTNode::BinaryOp { + operator: BinaryOperator::Or, + left: Box::new(ASTNode::BinaryOp { + operator: BinaryOperator::Equal, + left: Box::new(var("ch")), + right: Box::new(lit_str(" ")), + span: Span::unknown(), + }), + right: Box::new(ASTNode::BinaryOp { + operator: BinaryOperator::Equal, + left: Box::new(var("ch")), + right: Box::new(lit_str("\t")), + span: Span::unknown(), + }), + span: Span::unknown(), + }), + span: Span::unknown(), + }), + then_body: vec![ASTNode::Break { + span: Span::unknown(), + }], + else_body: vec![], + span: Span::unknown(), + }, + ], + span: Span::unknown(), + }, + ], + span: Span::unknown(), + return_type: None, + }; + + // Lower to MIR (this will trigger Pattern2 lowering with BindingId lookup) + let mut builder = MirBuilder::new(); + let result = builder.lower_function_declaration(&ast); + + assert!( + result.is_ok(), + "Trim pattern should lower successfully via BindingId path" + ); + + eprintln!("[phase79/test] Trim BindingId lookup test completed"); +}