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

ShaharNaveh · 2026-06-22T12:32:20Z

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.

coderabbitai · 2026-06-22T12:32:36Z

📝 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.}

coderabbitai

🧹 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

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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!

coderabbitai Bot commented Jun 22, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

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

ShaharNaveh commented Jun 22, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 22, 2026 •

edited

Loading