General code nits by ShaharNaveh · Pull Request #8135 · RustPython/RustPython

ShaharNaveh · 2026-06-20T12:07:06Z

Closes #xxxx
This PR follows our AI policy

Summary

Summary by CodeRabbit

Refactor
- Refactored code generation and bytecode handling to use modernized internal APIs.
- Improved truthiness evaluation for constant values.
- Updated method signatures for improved type safety and clarity.
- Reformatted generic bounds and import statements for consistency.
Chores
- Added dependency to support numeric operations.
- Added compiler linting attributes to improve code quality checks.

coderabbitai · 2026-06-20T12:07:19Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 9062cd97-e49b-43ee-9b3a-15a82f45c661

📥 Commits

Reviewing files that changed from the base of the PR and between fe2a7db and eef5950.

⛔ Files ignored due to path filters (1)

Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (13)

crates/codegen/src/compile.rs
crates/codegen/src/ir.rs
crates/codegen/src/string_parser.rs
crates/compiler-core/Cargo.toml
crates/compiler-core/src/bytecode.rs
crates/vm/src/function/buffer.rs
crates/vm/src/function/builtin.rs
crates/vm/src/function/either.rs
crates/vm/src/function/getset.rs
crates/vm/src/function/method.rs
crates/vm/src/py_io.rs
crates/vm/src/py_serde.rs
crates/vm/src/readline.rs

📝 Walkthrough

Wait, I have some ranges referenced in the layer that are not in my all_range_ids list (range_5282, range_9274, range_1331, range_12402). Let me re-check — these don't appear in the all_range_ids list. Let me re-read the compile.rs ranges more carefully and reassign properly.

Walkthrough

Migrates compile.rs and ir.rs from bytecode::-qualified Instruction::* variant construction and CodeFlags/MakeFunctionFlags references to Opcode/PseudoOpcode-centric APIs. Adds ConstantData::truthiness() to compiler-core backed by num-traits. Applies where-clause reformatting, #[must_use]/const fn annotations, and import consolidation across VM utility files.

Changes

Compiler/codegen opcode API migration

Layer / File(s)	Summary
ConstantData::truthiness() and num-traits dep `crates/compiler-core/Cargo.toml`, `crates/compiler-core/src/bytecode.rs`	Adds `num-traits` dependency, imports `Zero`, and introduces `ConstantData::truthiness() -> bool` with per-variant Python-style truth semantics, replacing the local `constant_truthiness` helpers in `compile.rs` and `ir.rs`.
compile.rs: import rewiring and CodeFlags/MakeFunctionFlags migration `crates/codegen/src/compile.rs`	Updates imports to non-qualified bytecode types and migrates all `CodeFlags`/`MakeFunctionFlags`/`MakeFunctionFlag` construction sites across module init, function flags, async/generator transitions, coroutine setup, docstring handling, type-params scopes, and comprehension scopes.
compile.rs: symbol table lookup and free-variable filtering `crates/codegen/src/compile.rs`	Refactors symbol table entry lookup to let-else form, restructures `free_names` filtering for `FREE_CLASS`, rewrites `get_ref_type` using `current_symbol_table()` and `matches!`, and refactors class-scope special-case symbol predicate.
compile.rs: Opcode/PseudoOpcode emission and assertion sites `crates/codegen/src/compile.rs`	Switches instruction construction from `Instruction::` to `Opcode::/PseudoOpcode::*` at Resume, BuildList/Set/Map, jump, break/continue, scope-entry/exit assertion, and function-attribute wiring sites; replaces `Self::constant_truthiness` with `value.truthiness()`.
compile.rs: test assertions updated to CodeFlags `crates/codegen/src/compile.rs`	Updates all compile.rs unit tests from `bytecode::CodeFlags`-qualified assertions to direct `CodeFlags` identifiers for METHOD, NESTED, FUTURE_ANNOTATIONS, GENERATOR, ASYNC_GENERATOR, and COROUTINE flags.
ir.rs: CFG optimizer migration to Opcode/PseudoOpcode APIs `crates/codegen/src/ir.rs`	Migrates all CFG optimization passes (jump lowering, const folding, list/set/tuple folding, swap threading, static swaps, load-const optimization, NOP removal, superinstruction insertion, pseudo-op conversion) from `Instruction::` construction and `real()` matching to `Opcode::/PseudoOpcode::*` assignment and `real_opcode()`/`pseudo_opcode()` predicates; removes local `constant_truthiness`; makes `Block::is_empty`/`is_pseudo_target` `const fn`; adds derives to `LoadFastInstrFlag` and `StackEffects`.
ir.rs: test updates to opcode-based construction and assertions `crates/codegen/src/ir.rs`	Updates ir.rs unit tests to construct instructions via `Opcode::/PseudoOpcode::.into()` encoding and assert via `real_opcode()`-based `matches!` predicates.
string_parser.rs: EscapedChar derive additions `crates/codegen/src/string_parser.rs`	Adds `Clone, Copy, Debug, Eq, PartialEq` derives to `EscapedChar`.

VM utility cleanups

Layer / File(s)	Summary
VM function/ module: where-clause, #[must_use], and doc cleanups `crates/vm/src/function/buffer.rs`, `crates/vm/src/function/builtin.rs`, `crates/vm/src/function/either.rs`, `crates/vm/src/function/getset.rs`, `crates/vm/src/function/method.rs`	Moves inline generic bounds to `where`-clauses in `buffer.rs`, `builtin.rs`, and `either.rs`; adds `#[must_use]` to `PyMethodDef::const_copy`; reformats `getset.rs` module doc.
VM py_io, py_serde, readline: import and annotation cleanups `crates/vm/src/py_io.rs`, `crates/vm/src/py_serde.rs`, `crates/vm/src/readline.rs`	Consolidates `fmt`/`ops` imports from `core` in `py_io.rs`, removes a `.to_owned()` allocation; marks `PyObjectDeserializer::new` as `#[must_use] const fn`; shortens `ReadlineResult::Io` error type path.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

[RFC] Split oparg type from the opcode enum #6746 — The PR directly implements the opcode/API separation described in the RFC, migrating all Instruction::* variant construction sites in codegen and the CFG optimizer to Opcode/PseudoOpcode-centric forms.

Possibly related PRs

RustPython/RustPython#7757: Both PRs switch instruction recognition/rewriting in ir.rs from Instruction variant matching to Opcode/PseudoOpcode-based APIs via real_opcode()/pseudo_opcode(), directly overlapping in the optimizer layer.
RustPython/RustPython#7942: Both PRs modify constant truthiness handling in codegen — this PR removes the local constant_truthiness helper and adds ConstantData::truthiness(), while that PR changed constant-folding behavior for ConstantData::Tuple.
RustPython/RustPython#7926: Both PRs touch compile.rs symbol/scope handling and class-scope special-name logic, making the changes directly overlapping at the symbol-table lookup and dunder-name classification level.

Suggested reviewers

fanninpm
youknowone

Poem

🐇 Hop, hop, away from bytecode:: haze,
Opcode::Resume now leads the maze!
ConstantData knows when truth is zero,
MakeFunctionFlags — a refactor hero.
The rabbit tidied every where-clause too,
Less noise, more signal — the bytecode flew! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'General code nits' is vague and generic, using non-descriptive language that does not convey meaningful information about the actual changes in the PR.	Consider a more specific title that highlights the main changes, such as 'Refactor bytecode APIs to use Opcode/PseudoOpcode types' or 'Convert generic trait bounds to where-clause syntax'.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 81.32% 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.}

ShaharNaveh added 5 commits June 20, 2026 12:42

General nitpicks

c8e1a5a

Move Compiler::constant_truthiness to a ConstantData method

04cc652

More code cleanup

76a6040

bytecode::CodeFlags -> CodeFlags

93bb0e7

More code clenaups

eef5950

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

General code nits#8135

General code nits#8135
ShaharNaveh wants to merge 5 commits into
RustPython:mainfrom
ShaharNaveh:nits-8

ShaharNaveh commented Jun 20, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 20, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Possibly related PRs

Suggested reviewers

Poem

❌ Failed checks (1 inconclusive)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

ShaharNaveh commented Jun 20, 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 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Possibly related PRs

Suggested reviewers

Poem

❌ Failed checks (1 inconclusive)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

ShaharNaveh commented Jun 20, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 20, 2026 •

edited

Loading