Newtype for `[ir::Block]` by ShaharNaveh · Pull Request #8142 · RustPython/RustPython · GitHub

Closes #xxxx
This PR follows our AI policy

Summary

Summary by CodeRabbit

Refactor
- Reorganized internal compiler code generation system for improved code structure and maintainability. No user-facing functionality changes.

📝 Walkthrough

Walkthrough

Introduces a Blocks newtype wrapping Vec<Block> in ir.rs, with BlockIdx-aware Index/IndexMut and standard conversion traits. CodeInfo.blocks is changed from Vec<Block> to Blocks, and every CFG construction, optimization, analysis, and transformation function that previously accepted &mut [Block] or &mut Vec<Block> is updated to accept &mut Blocks. All initialization and append sites in compile.rs are updated accordingly, as are unit tests.

Changes

Blocks Newtype Refactor

Layer / File(s)	Summary
Blocks type definition and BlockIdx conversions crates/codegen/src/ir.rs	Adds pub struct Blocks(Vec<Block>) with try_reserve, push, Deref/DerefMut, Index/IndexMut for usize and BlockIdx, and From conversions. Reworks BlockIdx helpers (as_u32, as_usize, idx) and updates the core::ops import and ConstantPool::Index impl path.
CodeInfo.blocks field and compile.rs callsites crates/codegen/src/ir.rs, crates/codegen/src/compile.rs	Changes CodeInfo.blocks from Vec<Block> to Blocks. Updates all initialization (Blocks::from([Block::default()])), reset (mem::replace), and block-append sites in compile.rs and adds the Block/Blocks imports.
CFG builder and construction pipeline crates/codegen/src/ir.rs	Updates CfgBuilder.blocks, cfg_from_instruction_sequence (returns Blocks), basicblock_append_block_instructions, cfg_to_instruction_sequence, translate_jump_labels_to_targets, and dataflow passes to use Blocks and BlockIdx indexing.
Optimization phases crates/codegen/src/ir.rs	Propagates Blocks through optimize_code_unit, optimize_cfg, optimized_cfg_to_instruction_sequence, unreachable-block removal, optimize_load_const, optimize_basic_block, calculate_stackdepth, insert_superinstructions, and stack-depth helpers.
CFG analysis and transformation pipeline crates/codegen/src/ir.rs	Converts exception handler propagation, warm/cold marking, cold-block relocation, CFG validation, jump threading, jump normalization, redundancy removals, and traversal-stack helpers to Blocks.
Line-number propagation and exception-stack handling crates/codegen/src/ir.rs	Refactors propagate_line_numbers, resolve_line_numbers, duplicate_exits_without_lineno, copy_basicblock, exception-stack helpers (except_stack_top, push/pop_except_block), label_exception_targets, and convert_pseudo_ops to &mut Blocks.
fix_cell_offsets and tests crates/codegen/src/ir.rs	Updates fix_cell_offsets parameter to &mut Blocks and converts all unit test block constructors from vec![...] to Blocks::from([..]) across line-number, jump-threading, and optimization tests.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐇 A wrapper so tidy, a newtype so neat,
Blocks hops in to keep the old Vec off its feet.
With Index<BlockIdx> and Deref in tow,
No more .idx() calls wherever we go.
The bunny refactored from builder to test—
Typed blocks, typed hops, a codegen blessed! 🌿

🚥 Pre-merge checks | ✅ 5 ✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Newtype for [ir::Block]' directly and concisely describes the main change—introducing a newtype wrapper for blocks—which aligns with the primary modifications in the changeset.
Docstring Coverage	✅ Passed	Docstring coverage is 96.04% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches 🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

🧹 Nitpick comments (1)

crates/codegen/src/ir.rs (1)
757-808: 🧹 Nitpick | 🔵 Trivial | 💤 Low value

Consider consistent indexing style.

This function uses blocks[block_idx.idx()] (usize indexing via .idx()) throughout, while other functions like basicblock_append_block_instructions use blocks[block_idx] directly. Both patterns work correctly due to the dual Index implementations, but consistent use of direct BlockIdx indexing would be cleaner and leverage the newtype's purpose.

Not blocking since both patterns are functionally correct.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/codegen/src/ir.rs` around lines 757 - 808, The function
cfg_to_instruction_sequence uses inconsistent indexing style by calling .idx()
on BlockIdx values (like blocks[block_idx.idx()]) while other functions use
direct indexing with the BlockIdx newtype (like blocks[block_idx]). Replace all
instances of blocks[block_idx.idx()], blocks[target_block.idx()], and
blocks[handler.handler_block.idx()] throughout the function to use direct
BlockIdx indexing without calling .idx(), such as blocks[block_idx],
blocks[target_block], and blocks[handler.handler_block], to maintain consistency
with the codebase and properly leverage the BlockIdx newtype's Index trait
implementation.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@crates/codegen/src/ir.rs`:
- Around line 757-808: The function cfg_to_instruction_sequence uses
inconsistent indexing style by calling .idx() on BlockIdx values (like
blocks[block_idx.idx()]) while other functions use direct indexing with the
BlockIdx newtype (like blocks[block_idx]). Replace all instances of
blocks[block_idx.idx()], blocks[target_block.idx()], and
blocks[handler.handler_block.idx()] throughout the function to use direct
BlockIdx indexing without calling .idx(), such as blocks[block_idx],
blocks[target_block], and blocks[handler.handler_block], to maintain consistency
with the codebase and properly leverage the BlockIdx newtype's Index trait
implementation.

ℹ️ Review info ⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 06947e9b-5ad8-4d27-9ce6-c20a4f72b915

📥 Commits

Reviewing files that changed from the base of the PR and between 55b96f9 and e476f3c.

📒 Files selected for processing (2)

crates/codegen/src/compile.rs
crates/codegen/src/ir.rs

ShaharNaveh added 4 commits June 22, 2026 15:30

Basic Blocks newtype

9ba98db

fix tests

f19d2b5

clippy

2ac86e7

cleanup some code

e476f3c

coderabbitai Bot reviewed Jun 22, 2026

View reviewed changes

youknowone approved these changes Jun 22, 2026

View reviewed changes

youknowone merged commit 18dc6b3 into RustPython:main Jun 22, 2026
26 checks passed

coderabbitai Bot mentioned this pull request Jul 1, 2026

Move free standing ir::Block functions to methods #8205

Merged

2 tasks

youknowone pushed a commit that referenced this pull request Sep 16, 2026

Newtype for [ir::Block] (#8142 )

6533a78

Newtype for `[ir::Block]` by ShaharNaveh · Pull Request #8142 · RustPython/RustPython · GitHub

Repository navigation

Newtype for [ir::Block] - #8142

Newtype for [ir::Block]#8142
youknowone merged 4 commits into
RustPython:mainfrom
ShaharNaveh:newtype-ir-blocks

ShaharNaveh commented Jun 22, 2026 •
edited by coderabbitai Bot
Loading

Uh oh!

Uh oh!

coderabbitai Bot commented Jun 22, 2026 •
edited
Loading

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Newtype for `[ir::Block]` by ShaharNaveh · Pull Request #8142 · RustPython/RustPython · GitHub

Repository navigation

Newtype for [ir::Block] - #8142

Newtype for [ir::Block]#8142youknowone merged 4 commits intoRustPython:mainRustPython/RustPython:mainfrom ShaharNaveh:newtype-ir-blocksShaharNaveh/RustPython:newtype-ir-blocksCopy head branch name to clipboard

Conversation

ShaharNaveh commented Jun 22, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Newtype for [ir::Block]#8142
youknowone merged 4 commits into
RustPython:mainfrom
ShaharNaveh:newtype-ir-blocks

ShaharNaveh commented Jun 22, 2026 •
edited by coderabbitai Bot
Loading

coderabbitai Bot commented Jun 22, 2026 •
edited
Loading