GeezerVison

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

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