crates/codegen/src/ir.rs (3)

247-256: Consider consolidating NOP removal.

NOPs are removed here after convert_pseudo_ops, but there's also a separate remove_nops() call at Line 195. The early call handles NOPs from constant folding, but this creates a two-phase removal pattern that could be confusing.

---

1266-1269: POP_BLOCK is converted to NOP twice.

POP_BLOCK is converted to Instruction::Nop here in label_exception_targets (Line 1269) and again in convert_pseudo_ops (Lines 1348-1350). The second conversion is redundant but harmless. The comment on Line 1347 acknowledges this.

Consider removing the redundant case in convert_pseudo_ops since it's already handled:

♻️ Proposed simplification

                 // POP_BLOCK → NOP (should already be NOP from label_exception_targets)
                 PseudoInstruction::PopBlock => {
-                    info.instr = Instruction::Nop.into();
+                    // Already converted to NOP in label_exception_targets
+                    debug_assert!(matches!(info.instr.real(), Some(Instruction::Nop)));
                 }

---

1360-1367: Unimplemented panic for unexpected pseudo instructions.

Using unimplemented! will cause a panic if these pseudo-ops are encountered. If these represent compiler bugs rather than user errors, consider using unreachable! with a clearer message, or returning an error.

♻️ Proposed change

                 // These should have been resolved earlier
                 PseudoInstruction::AnnotationsPlaceholder
                 | PseudoInstruction::JumpIfFalse { .. }
                 | PseudoInstruction::JumpIfTrue { .. }
                 | PseudoInstruction::StoreFastMaybeNull(_) => {
-                    unimplemented!(
-                        "Unexpected pseudo instruction in convert_pseudo_ops: {pseudo:?}"
-                    )
+                    unreachable!(
+                        "Pseudo instruction should have been resolved earlier: {pseudo:?}"
+                    )
                 }

crates/codegen/src/ir.rs (1)

1204-1326: Consider asserting consistent except stacks at merge points.

Because visited short-circuits, a block reached with a different except stack will be ignored. A stored entry stack + debug assert would guard invariants.

💡 Suggested defensive check

-    #[derive(Clone)]
+    #[derive(Clone, PartialEq, Eq)]
     struct ExceptEntry {
         handler_block: BlockIdx,
         preserve_lasti: bool,
     }

     let mut visited = vec![false; num_blocks];
     let mut block_stacks: Vec<Option<Vec<ExceptEntry>>> = vec![None; num_blocks];
+    let mut entry_stacks: Vec<Option<Vec<ExceptEntry>>> = vec![None; num_blocks];

     // Entry block
     visited[0] = true;
     block_stacks[0] = Some(Vec::new());
+    entry_stacks[0] = Some(Vec::new());

@@
-                if target != BlockIdx::NULL && !visited[target.idx()] {
-                    visited[target.idx()] = true;
-                    block_stacks[target.idx()] = Some(stack.clone());
-                    todo.push(target);
-                }
+                if target != BlockIdx::NULL {
+                    if let Some(existing) = entry_stacks[target.idx()].as_ref() {
+                        debug_assert_eq!(existing, &stack, "mismatched except stack");
+                    } else {
+                        entry_stacks[target.idx()] = Some(stack.clone());
+                    }
+                    if !visited[target.idx()] {
+                        visited[target.idx()] = true;
+                        block_stacks[target.idx()] = Some(stack.clone());
+                        todo.push(target);
+                    }
+                }

@@
-                if target != BlockIdx::NULL && !visited[target.idx()] {
-                    visited[target.idx()] = true;
-                    block_stacks[target.idx()] = Some(stack.clone());
-                    todo.push(target);
-                }
+                if target != BlockIdx::NULL {
+                    if let Some(existing) = entry_stacks[target.idx()].as_ref() {
+                        debug_assert_eq!(existing, &stack, "mismatched except stack");
+                    } else {
+                        entry_stacks[target.idx()] = Some(stack.clone());
+                    }
+                    if !visited[target.idx()] {
+                        visited[target.idx()] = true;
+                        block_stacks[target.idx()] = Some(stack.clone());
+                        todo.push(target);
+                    }
+                }

crates/codegen/src/ir.rs (1)

863-867: Persist computed start_depth into Block.start_depth.

start_depths is computed but never stored, leaving the new field None and out of sync with the comment. Consider populating it for downstream uses.

♻️ Suggested update

         if DEBUG {
             eprintln!("DONE: {maxdepth}");
         }
+
+        // Persist block entry depths for downstream consumers
+        for (idx, block) in self.blocks.iter_mut().enumerate() {
+            let depth = start_depths[idx];
+            if depth != u32::MAX {
+                block.start_depth = Some(depth);
+            }
+        }
 
         // Fix up handler stack_depth in ExceptHandlerInfo using start_depths

crates/codegen/src/ir.rs (2)

824-838: Use checked_add for handler depth to keep overflow handling consistent.
The rest of the stack-depth computation uses checked arithmetic; this path currently doesn’t.

♻️ Proposed fix

-                        let handler_depth = depth + handler_effect;
+                        let handler_depth = depth
+                            .checked_add(handler_effect)
+                            .ok_or(InternalError::StackOverflow)?;

---

873-889: Consider persisting computed start depths into Block.start_depth.
The field is added but never populated, which can mislead downstream consumers.

♻️ Proposed fix

-        // Fix up handler stack_depth in ExceptHandlerInfo using start_depths
+        // Persist block start depths for downstream consumers (if any)
+        for (i, block) in self.blocks.iter_mut().enumerate() {
+            block.start_depth = (start_depths[i] != u32::MAX).then_some(start_depths[i]);
+        }
+
+        // Fix up handler stack_depth in ExceptHandlerInfo using start_depths