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

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! ✨

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

• 📝 Generate docstrings

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

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

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

• crates/codegen/src/compile.rs
• crates/compiler-core/src/bytecode.rs
• crates/stdlib/src/opcode.rs
• crates/vm/src/frame.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

• 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)

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.

• X
• Mastodon
• Reddit
• LinkedIn

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