LoadClassDeref -> LoadFromDictOrDeref by ShaharNaveh · Pull Request #6692 · RustPython/RustPython

ShaharNaveh · 2026-01-10T12:42:38Z

Summary by CodeRabbit

Refactor
- Optimized class scope variable access in the compiler bytecode generation.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2026-01-10T12:42:43Z

📝 Walkthrough

Walkthrough

This PR removes the LoadClassDeref opcode instruction and replaces all occurrences with LoadFromDictOrDeref across the compiler and VM. The functionality remains unchanged; this is a consolidation replacing a specialized instruction variant with a more general one.

Changes

Cohort / File(s)	Summary
Instruction Definition & Bytecode `crates/compiler-core/src/bytecode.rs`	Removed `LoadClassDeref(Arg<NameIdx>) = 132` enum variant. Updated TryFrom decoding, stack effect handling, and disassembly formatting to reference `LoadFromDictOrDeref` instead.
Instruction Emission `crates/codegen/src/compile.rs`	Changed class-scope name loading to emit `Instruction::LoadFromDictOrDeref` instead of `Instruction::LoadClassDeref`.
Instruction Execution `crates/vm/src/frame.rs`	Updated ExecutingFrame instruction handler to match on `LoadFromDictOrDeref` instead of `LoadClassDeref`; internal logic unchanged.
Opcode Classification `crates/stdlib/src/opcode.rs`	Updated `Opcode::has_free` to check for `LoadFromDictOrDeref` instead of `LoadClassDeref`.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Refactor compile_function #6001: Main PR that replaces the specialized LoadClassDeref instruction with LoadFromDictOrDeref across compiler and VM layers with overlapping refactoring scope.
Sort Instruction enum & related match arms #6322: Concurrent changes to Instruction enum and its execution/decoding match arms, including related opcode variant handling.

Suggested reviewers

youknowone

Poem

🐰 A class deref bows out with grace,
LoadFromDict takes its place,
One instruction now, instead of two,
The bytecode's cleaner—hooray, it's true! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and accurately describes the main change across all modified files: replacing the LoadClassDeref instruction variant with LoadFromDictOrDeref throughout the codebase.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing touches

📝 Generate docstrings

📜 Recent review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9e225f5 and 07e7f46.

⛔ Files ignored due to path filters (1)

Lib/_opcode_metadata.py is excluded by !Lib/**

📒 Files selected for processing (4)

crates/codegen/src/compile.rs
crates/compiler-core/src/bytecode.rs
crates/stdlib/src/opcode.rs
crates/vm/src/frame.rs

🧰 Additional context used

📓 Path-based instructions (1)

**/*.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.rs: Follow the default rustfmt code style by running cargo fmt to format Rust code
Always run clippy to lint code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust

Files:

crates/stdlib/src/opcode.rs
crates/vm/src/frame.rs
crates/codegen/src/compile.rs
crates/compiler-core/src/bytecode.rs

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

GitHub Check: Run snippets and cpython tests (macos-latest)
GitHub Check: Run snippets and cpython tests (ubuntu-latest)
GitHub Check: Run snippets and cpython tests (windows-2025)

🔇 Additional comments (6)

crates/compiler-core/src/bytecode.rs (3)

845-846: Opcode/API change looks consistent; consider bytecode backwards-compat expectations.

Line 845: Adding Instruction::LoadFromDictOrDeref(Arg<NameIdx>) = 89 aligns with the CPython opcode space and matches the PR objective, but removing the old LoadClassDeref opcode means any previously-serialized RustPython bytecode containing that opcode will now fail to decode (InvalidBytecode). If RustPython ever persists bytecode between runs, consider a one-release decode shim mapping the old opcode value to LoadFromDictOrDeref during unmarshalling.

1806-1814: Stack effect update for LoadFromDictOrDeref is correct.

Line 1813: LoadFromDictOrDeref(_) => 1 matches “pushes one value” and keeps stack-depth computation consistent with the VM implementation.

2123-2124: Disassembly output updated correctly.

Line 2123: Printing as LOAD_FROM_DICT_OR_DEREF and using cell_name formatting matches the opcode’s cell/free indexing semantics.

crates/stdlib/src/opcode.rs (1)

104-116: has_free() correctly accounts for LoadFromDictOrDeref.

Line 111: Treating Instruction::LoadFromDictOrDeref(_) as using co_freevars/co_cellvars is consistent with the opcode’s operand being an index into the cell/free table.

crates/vm/src/frame.rs (1)

1115-1130: VM execution path matches dict-or-deref semantics; add/confirm a regression test for shadowing.

Lines 1115-1129: The implementation does “locals dict lookup first, then cell/free fallback”, which matches the intended behavior for class-scope name resolution. I’d add/confirm a test that covers shadowing (class namespace key present vs missing) and the unbound-cell path to ensure behavior didn’t subtly drift during the rename.

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

1560-1568: The replacement is a valid drop-in substitute; all concerns are verified.

Both LoadDeref and LoadFromDictOrDeref use identical operand type (Arg<NameIdx>) and stack effect (1). The VM handler correctly implements class scope semantics by checking the locals dict first via self.locals.mapping().subscript(name, vm), then falling back to the closure cell/free variable. No references to the old LoadClassDeref remain, and the new instruction is properly integrated in opcode tables and bytecode definitions across the codebase.

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

youknowone

good catch!

$@ShaharNaveh$

LoadClassDeref -> LoadFromDictOrDeref

07e7f46

$@ShaharNaveh$

ShaharNaveh marked this pull request as ready for review January 10, 2026 12:53

$@ShaharNaveh$

ShaharNaveh requested a review from youknowone January 10, 2026 13:44

$youknowone$

youknowone approved these changes Jan 10, 2026

View reviewed changes

$@youknowone$

youknowone merged commit 6ff7b3e into RustPython:main Jan 10, 2026
13 checks passed

terryluan12 pushed a commit to terryluan12/RustPython that referenced this pull request Jan 15, 2026

$@ShaharNaveh$ $@terryluan12$

LoadClassDeref -> LoadFromDictOrDeref (RustPython#6692)

07a7873

$@coderabbitai$ coderabbitai Bot mentioned this pull request Jan 18, 2026

LoadClosure as pseudo op #6789

Merged

$@coderabbitai$ coderabbitai Bot mentioned this pull request Jan 29, 2026

More Load instructions #6886

Merged

Navigation Menu

Uh oh!

LoadClassDeref -> LoadFromDictOrDeref - #6692

LoadClassDeref -> LoadFromDictOrDeref#6692
youknowone merged 1 commit into
RustPython:mainfrom
ShaharNaveh:bytecode-remove-loadclassderef

$@ShaharNaveh$ ShaharNaveh commented Jan 10, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jan 10, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Uh oh!

$@youknowone$ youknowone left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

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

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jan 10, 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 PRs

Suggested reviewers

Poem

Uh oh!

youknowone 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$ ShaharNaveh commented Jan 10, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jan 10, 2026 •

edited

Loading

$@youknowone$ youknowone left a comment