refactor: Reduce excessive nesting in codegen inline operations #213

New issue

Closed

opened 2026-01-09 04:42:59 +00:00 by navicore · 1 comment

navicore commented

2026-01-09 04:42:59 +00:00

(Migrated from github.com)

Problem

crates/compiler/src/codegen.rs contains functions with 5-6 levels of nesting, making the code difficult to read, test, and modify.

Example: `codegen_inline_binary_op` (lines 4079-4190)

if self.virtual_stack.len() >= 2 {
    // Fast path
    if let (VirtualValue::Int { ... }, VirtualValue::Int { ... }) = (...) {
        if some_condition {
            // 4 levels deep
            match something {
                // 5 levels deep
                Pattern::A => {
                    // 6 levels deep!
                }
            }
        } else {
            // another branch at level 4
        }
    } else {
        // level 3
    }
} else {
    // Slow path - another 4+ levels
}

Impact

Cognitive load: difficult to understand control flow
Testing: hard to test individual branches
Maintenance: changes risk breaking unrelated paths
Code review: reviewers miss edge cases

Affected Functions

codegen_inline_binary_op (lines 4079-4190) - 5-6 levels
codegen_match_statement (lines 5269-5386) - 4-5 levels
codegen_program_with_config (lines 820-1144) - deep initialization nesting

Proposed Solution

1. Extract fast/slow paths to separate methods

fn codegen_inline_binary_op(&mut self, op: BinaryOp) -> Result<()> {
    if self.can_use_virtual_stack_fast_path() {
        self.codegen_inline_binary_op_fast(op)
    } else {
        self.codegen_inline_binary_op_slow(op)
    }
}

2. Use early returns to flatten

fn codegen_inline_binary_op(&mut self, op: BinaryOp) -> Result<()> {
    if self.virtual_stack.len() < 2 {
        return self.codegen_inline_binary_op_slow(op);
    }
    
    let (a, b) = self.pop_two_virtual();
    
    if !a.is_int() || !b.is_int() {
        return self.codegen_inline_binary_op_mixed(op, a, b);
    }
    
    // Now we're at the fast path with minimal nesting
    // ...
}

3. Replace nested matches with lookup tables where applicable

const BINARY_OP_INSTRUCTIONS: &[(BinaryOp, &str)] = &[
    (BinaryOp::Add, "add"),
    (BinaryOp::Sub, "sub"),
    // ...
];

Acceptance Criteria

No function exceeds 3 levels of nesting
Complex branches extracted to named functions
Early returns used to flatten conditionals
All tests pass

Labels

refactor, technical-debt, medium-priority

## Problem `crates/compiler/src/codegen.rs` contains functions with 5-6 levels of nesting, making the code difficult to read, test, and modify. ### Example: `codegen_inline_binary_op` (lines 4079-4190) ```rust if self.virtual_stack.len() >= 2 { // Fast path if let (VirtualValue::Int { ... }, VirtualValue::Int { ... }) = (...) { if some_condition { // 4 levels deep match something { // 5 levels deep Pattern::A => { // 6 levels deep! } } } else { // another branch at level 4 } } else { // level 3 } } else { // Slow path - another 4+ levels } ``` ## Impact - Cognitive load: difficult to understand control flow - Testing: hard to test individual branches - Maintenance: changes risk breaking unrelated paths - Code review: reviewers miss edge cases ## Affected Functions 1. `codegen_inline_binary_op` (lines 4079-4190) - 5-6 levels 2. `codegen_match_statement` (lines 5269-5386) - 4-5 levels 3. `codegen_program_with_config` (lines 820-1144) - deep initialization nesting ## Proposed Solution ### 1. Extract fast/slow paths to separate methods ```rust fn codegen_inline_binary_op(&mut self, op: BinaryOp) -> Result<()> { if self.can_use_virtual_stack_fast_path() { self.codegen_inline_binary_op_fast(op) } else { self.codegen_inline_binary_op_slow(op) } } ``` ### 2. Use early returns to flatten ```rust fn codegen_inline_binary_op(&mut self, op: BinaryOp) -> Result<()> { if self.virtual_stack.len() < 2 { return self.codegen_inline_binary_op_slow(op); } let (a, b) = self.pop_two_virtual(); if !a.is_int() || !b.is_int() { return self.codegen_inline_binary_op_mixed(op, a, b); } // Now we're at the fast path with minimal nesting // ... } ``` ### 3. Replace nested matches with lookup tables where applicable ```rust const BINARY_OP_INSTRUCTIONS: &[(BinaryOp, &str)] = &[ (BinaryOp::Add, "add"), (BinaryOp::Sub, "sub"), // ... ]; ``` ## Acceptance Criteria - [ ] No function exceeds 3 levels of nesting - [ ] Complex branches extracted to named functions - [ ] Early returns used to flatten conditionals - [ ] All tests pass ## Labels refactor, technical-debt, medium-priority

navicore commented

2026-01-10 04:26:17 +00:00

(Migrated from github.com)

https://github.com/navicore/patch-seq/pull/221