From 84cd653aeff6b5e2b24ee238d0c1e31f1a274a28 Mon Sep 17 00:00:00 2001 From: tomoaki Date: Sun, 21 Dec 2025 07:07:21 +0900 Subject: [PATCH] refactor(joinir): Phase 260 P0.2 - extract merge_variable_handler.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extracts merge copy emission pattern from joinir_block_converter.rs to eliminate 4x code duplication: - MergeBranch enum (Then/Else) for branch selection - emit_merge_copies() for MergePair-based copy emission - emit_copy_instructions() for direct ValueId pair emission - Comprehensive unit tests for all patterns Pattern reduction: ~20 lines duplicated 4x → single 60-line module 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- .../merge_variable_handler.rs | 168 ++++++++++++++++++ src/mir/join_ir_vm_bridge/mod.rs | 1 + 2 files changed, 169 insertions(+) create mode 100644 src/mir/join_ir_vm_bridge/merge_variable_handler.rs diff --git a/src/mir/join_ir_vm_bridge/merge_variable_handler.rs b/src/mir/join_ir_vm_bridge/merge_variable_handler.rs new file mode 100644 index 00000000..5d008604 --- /dev/null +++ b/src/mir/join_ir_vm_bridge/merge_variable_handler.rs @@ -0,0 +1,168 @@ +//! Merge Variable Handler - Copy emission for if/else merge patterns +//! +//! Phase 260 P0.2: Extracted from joinir_block_converter.rs +//! Eliminates repeated merge copy emission patterns (4x duplication). +//! +//! ## Pattern Before +//! +//! ```ignore +//! for merge in merges { +//! block_obj.instructions.push(MirInstruction::Copy { +//! dst: merge.dst, +//! src: merge.then_val, // or else_val +//! }); +//! block_obj.instruction_spans.push(Span::unknown()); +//! } +//! ``` +//! +//! ## Pattern After +//! +//! ```ignore +//! emit_merge_copies(&mut block_obj, merges, MergeBranch::Then); +//! ``` + +use crate::ast::Span; +use crate::mir::join_ir::MergePair; +use crate::mir::{BasicBlock, MirInstruction, ValueId}; + +/// Which branch of an if/else to extract values from +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum MergeBranch { + /// Use then_val from MergePair + Then, + /// Use else_val from MergePair + Else, +} + +/// Emit Copy instructions for merge variables +/// +/// Adds Copy instructions to the block for each merge pair, +/// selecting the appropriate source value based on the branch. +/// +/// # Arguments +/// +/// * `block` - Target BasicBlock to add instructions to +/// * `merges` - Slice of MergePair describing the merge variables +/// * `branch` - Which branch's values to use (Then or Else) +pub fn emit_merge_copies(block: &mut BasicBlock, merges: &[MergePair], branch: MergeBranch) { + for merge in merges { + let src = match branch { + MergeBranch::Then => merge.then_val, + MergeBranch::Else => merge.else_val, + }; + + block.instructions.push(MirInstruction::Copy { + dst: ValueId(merge.dst.0), + src: ValueId(src.0), + }); + block.instruction_spans.push(Span::unknown()); + } +} + +/// Emit Copy instructions with explicit ValueId conversion +/// +/// Same as emit_merge_copies but uses direct ValueId pairs +/// instead of MergePair struct. +/// +/// # Arguments +/// +/// * `block` - Target BasicBlock to add instructions to +/// * `copies` - Slice of (dst, src) ValueId pairs +pub fn emit_copy_instructions(block: &mut BasicBlock, copies: &[(ValueId, ValueId)]) { + for (dst, src) in copies { + block.instructions.push(MirInstruction::Copy { + dst: *dst, + src: *src, + }); + block.instruction_spans.push(Span::unknown()); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::mir::BasicBlockId; + + #[test] + fn test_emit_merge_copies_then() { + let mut block = BasicBlock::new(BasicBlockId::new(0)); + let merges = vec![ + MergePair { + dst: ValueId(10), + then_val: ValueId(20), + else_val: ValueId(30), + type_hint: None, + }, + MergePair { + dst: ValueId(11), + then_val: ValueId(21), + else_val: ValueId(31), + type_hint: None, + }, + ]; + + emit_merge_copies(&mut block, &merges, MergeBranch::Then); + + assert_eq!(block.instructions.len(), 2); + + if let MirInstruction::Copy { dst, src } = &block.instructions[0] { + assert_eq!(*dst, ValueId(10)); + assert_eq!(*src, ValueId(20)); + } else { + panic!("Expected Copy instruction"); + } + + if let MirInstruction::Copy { dst, src } = &block.instructions[1] { + assert_eq!(*dst, ValueId(11)); + assert_eq!(*src, ValueId(21)); + } else { + panic!("Expected Copy instruction"); + } + } + + #[test] + fn test_emit_merge_copies_else() { + let mut block = BasicBlock::new(BasicBlockId::new(0)); + let merges = vec![MergePair { + dst: ValueId(10), + then_val: ValueId(20), + else_val: ValueId(30), + type_hint: None, + }]; + + emit_merge_copies(&mut block, &merges, MergeBranch::Else); + + assert_eq!(block.instructions.len(), 1); + + if let MirInstruction::Copy { dst, src } = &block.instructions[0] { + assert_eq!(*dst, ValueId(10)); + assert_eq!(*src, ValueId(30)); // else_val + } else { + panic!("Expected Copy instruction"); + } + } + + #[test] + fn test_emit_copy_instructions() { + let mut block = BasicBlock::new(BasicBlockId::new(0)); + let copies = vec![ + (ValueId(1), ValueId(2)), + (ValueId(3), ValueId(4)), + ]; + + emit_copy_instructions(&mut block, &copies); + + assert_eq!(block.instructions.len(), 2); + assert_eq!(block.instruction_spans.len(), 2); + } + + #[test] + fn test_emit_merge_copies_empty() { + let mut block = BasicBlock::new(BasicBlockId::new(0)); + let merges: Vec = vec![]; + + emit_merge_copies(&mut block, &merges, MergeBranch::Then); + + assert!(block.instructions.is_empty()); + } +} diff --git a/src/mir/join_ir_vm_bridge/mod.rs b/src/mir/join_ir_vm_bridge/mod.rs index 83831568..495bcc4f 100644 --- a/src/mir/join_ir_vm_bridge/mod.rs +++ b/src/mir/join_ir_vm_bridge/mod.rs @@ -42,6 +42,7 @@ mod block_allocator; // Phase 260 P0.2: Block ID allocation utility mod bridge; mod joinir_block_converter; mod joinir_function_converter; +mod merge_variable_handler; // Phase 260 P0.2: Merge copy emission utility mod meta; #[cfg(feature = "normalized_dev")] mod normalized_bridge;