Great! We are supporting now the majority of pseudoes

Actionable comments posted: 1

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

769-813: load_args_for_super: consider loading the “first arg” by position (slot 0) rather than by name.
Right now it relies on metadata.varnames.first() being the first parameter. That’s probably true, but it’s also the fragile link that makes the Zero-arg gate (above) so important. If you keep the current approach, the tightened eligibility check is effectively mandatory.

---

3511-3518: get_ref_type: class-scope special-case seems correct, but please verify it matches symbol-table reality.
Returning Cell for __class__ / __classdict__ in CompilerScope::Class fixes the closure-loading path, but it assumes those cells exist whenever referenced. A debug-assert (or a quick unit test) ensuring the class code object actually has the expected cellvar/freevar would make this safer.

---

6334-6356: Super-method fast-path: add at least one regression test that exercises “should NOT optimize”.
Suggested cases:

• Nested function inside a method with super() and no args (should raise RuntimeError: super(): no arguments, not UnboundLocalError).
• super = something_else at module scope and super().m() inside a class method (should behave identically to baseline).
  Even a bytecode snapshot test asserting the absence of LoadZeroSuperMethod in the nested-no-arg case would prevent regressions.

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

101-114: LGTM! Clean encoding/decoding implementation.

The bit-packing logic correctly encodes the three parameters into a u32 and the decode function properly reverses the operation. The implementation follows the same pattern as encode_load_attr_arg above.

Optional: Consider adding bit layout documentation

For future maintainability, consider documenting the bit layout more explicitly in the function docstrings:

-/// Encode LOAD_SUPER_ATTR oparg: bit 0 = load_method, bit 1 = has_class, bits 2+ = name index.
+/// Encode LOAD_SUPER_ATTR oparg into a 32-bit value.
+///
+/// Bit layout:
+/// - Bit 0: load_method (whether to load as method with NULL)
+/// - Bit 1: has_class (whether __class__ cell is available)
+/// - Bits 2-31: name_idx (index into names table)
 #[inline]
 pub const fn encode_load_super_attr_arg(name_idx: u32, load_method: bool, has_class: bool) -> u32 {

---

952-962: Clarify the "Zero" naming convention.

The pseudo opcodes LoadZeroSuperAttr and LoadZeroSuperMethod use a "Zero" prefix that isn't immediately clear. Does this refer to has_class=false, or does it mean zero-argument super()?

Consider adding a comment to clarify:

 // ===== LOAD_SUPER_* Pseudo Opcodes (136-138) =====
 // These are converted to LoadSuperAttr during bytecode finalization
+// "Zero" variants indicate has_class=false (no __class__ cell available)
 LoadSuperMethod {
     idx: Arg<NameIdx>,
 } = 136, // CPython uses pseudo-op 260

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Reviewing files that changed from the base of the PR and between 3909b18 and 959baf3.

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

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

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

• encode_exception_table (47-59)
• encode_load_attr_arg (89-91)
• encode_load_super_attr_arg (103-105)
• marker (551-553)

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

• decode_load_super_attr_arg (109-114)
• get (466-472)
• get (569-571)

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

• get (22-88)

crates/codegen/src/symboltable.rs (4)

246-251: LGTM - Clean wrapper preserving external API.

The wrapper correctly discards the top-level newfree set while allowing internal propagation. The comment clearly explains the rationale.

---

261-272: LGTM - Clean implementation of drop_class_free.

The function correctly uses HashSet::remove to both check presence and remove in a single operation, setting the appropriate flags. This aligns with CPython's drop_class_free() semantics.

---

340-416: LGTM - Free variable propagation logic is correct.

The implementation follows CPython's analyze_block() pattern:

1. Child scopes are analyzed first, collecting their free variables
2. Current scope's Free/FREE_CLASS symbols are added
3. Class-specific handling removes __class__/__classdict__
4. The set is returned for parent scope propagation

The flow ensures proper closure variable tracking across nested scopes.

---

496-502: LGTM - Proper handling of implicit __class__ in class scope.

This correctly handles the case where __class__ is implicitly available in class scope for super() calls. By treating it as implicitly declared when encountered in a class scope, the symbol table properly tracks the closure variable for nested method access.

crates/codegen/src/ir.rs (2)

9-9: LGTM - Import aligns with existing pattern.

---

215-238: LGTM - Correct flag encoding for all super instruction variants.

The encoding matches the encode_load_super_attr_arg(name_idx, load_method, has_class) signature:

• LoadSuperMethod: method=true, class=true (0b11)
• LoadZeroSuperAttr: method=false, class=false (0b00)
• LoadZeroSuperMethod: method=true, class=false (0b01)
• LoadSuperAttr: method=false, class=true (0b10)

All four variants correctly transform to the unified LoadSuperAttr instruction with encoded flags.

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

1107-1114: LGTM - Correct dispatch pattern for super instructions.

The dispatch correctly:

1. Routes LoadSuperAttr to the new handler method
2. Marks pseudo instructions (LoadSuperMethod, LoadZeroSuperAttr, LoadZeroSuperMethod) as unreachable since they're converted during IR finalization

This mirrors the existing pattern for LoadAttr/LoadAttrMethod.

---

2534-2570: LGTM - Correct super attribute loading implementation.

The implementation correctly:

1. Decodes the oparg to extract (name_idx, load_method, has_class)
2. Pops [self, class, super] from stack in the expected order
3. Constructs the super object with or without class context based on has_class flag
4. For method loads, pushes [func, self_obj] enabling the method call optimization
5. For attribute access, pushes just the attribute value

The method vs attribute distinction mirrors load_attr's handling pattern.

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

78-88: SuperCallType<'a> looks like a good internal representation for the optimization.

---

5888-5909: No issues found - stack contract, semantics, and shadowing handling are correct.

The super-attribute optimization is properly implemented:

• load_args_for_super() correctly prepares the stack as [global_super, class, self]
• VM load_super_attr() correctly pops these three values and handles both 0-arg and 2-arg super() calls via the has_class flag
• The has_class flag in the encoded oparg correctly determines whether to call super with (class, self) or ()
• Compile-time check in can_optimize_super_call() ensures optimization only applies when super is SymbolScope::GlobalImplicit (not locally redefined)
• Runtime behavior is semantically correct even if super is globally shadowed: compile_name("super", NameUsage::Load) emits the appropriate LoadGlobal/LoadName instruction, which loads whatever super is at runtime, and the VM calls it with the correct arguments
• Pseudo instructions (LoadSuperMethod, LoadZeroSuperAttr, LoadZeroSuperMethod) are properly converted to LoadSuperAttr with the correct flag encoding during IR lowering

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

1027-1030: LGTM! Correctly extends custom opcodes.

The new pseudo opcodes are properly registered in the custom_ops list for deserialization, following the existing pattern.

---

2157-2169: LGTM! Comprehensive instruction formatting.

The display logic correctly decodes and presents the encoded oparg with all relevant flags, making bytecode inspection straightforward. The implementation follows the same pattern as LoadAttr above.

---

1994-2003: The stack effect calculation is correct; no action needed.

The has_class flag is decoded but correctly excluded from the stack effect calculation because it does not affect stack depth. All LoadSuperAttr variants unconditionally pop 3 items from the stack ([super, class, self]), and the has_class flag only determines how the super object is initialized—with arguments (class, self) when true, or with no arguments when false. The load_method flag is the only factor affecting the number of items pushed (2 if true, 1 if false), making the calculation -3 + 2 or -3 + 1 correct regardless of has_class.

Actionable comments posted: 2

In @crates/codegen/src/compile.rs:
- Around line 676-773: In can_optimize_super_call: tighten the symbol checks to
match later assumptions by (1) treating missing "super" the same as a
non-GlobalImplicit symbol (i.e., if table.lookup("super") returns None, return
None instead of assuming a builtin) so compile_name won't fail later, and (2)
allow "__class__" to be treated as eligible when its symbol.scope is
SymbolScope::Free OR SymbolScope::Cell OR its flags contain
SymbolFlags::FREE_CLASS (i.e., add Cell to the accepted scopes) so zero-arg
super optimizations in nested class closures remain possible; update the
lookup/if conditions around table.lookup("super") and table.lookup("__class__")
in can_optimize_super_call accordingly.

In @crates/compiler-core/src/bytecode/instruction.rs:
- Around line 247-257: The new pseudo-opcodes LoadSuperMethod,
LoadZeroSuperAttr, and LoadZeroSuperMethod (opcodes 136–138) are missing from
the validation in the Instruction::try_from implementation and will be rejected
during deserialization; update the try_from() validation by adding these opcode
values to the custom_ops set/array used to permit non-CPython opcodes (the one
currently listing 119-124, 128-131, 134-135) so that opcodes 136, 137, and 138
are treated as valid and map to the corresponding enum variants.

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

3517-3538: get_ref_type special-casing __class__/__classdict__: add a guard/assert to avoid silently inventing cells.
Line 3519-3524: forcing Cell is correct only if the class body actually has those implicit cells (e.g., ste.needs_class_closure / ste.needs_classdict). If that coupling is ever broken, this can create new cellvars via get_cell_var_index() and make closure layouts diverge from symboltable intent. A debug_assert!(table.lookup(name).is_some() || ...) (or checking the relevant ste flag if accessible) would make failures obvious.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Reviewing files that changed from the base of the PR and between 959baf3 and a27ac1c.

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

• crates/codegen/src/compile.rs
• crates/codegen/src/ir.rs
• crates/codegen/src/symboltable.rs
• crates/compiler-core/src/bytecode.rs
• crates/compiler-core/src/bytecode/instruction.rs
• crates/vm/src/frame.rs

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

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

• decode_load_super_attr_arg (113-118)

• GitHub Check: Run snippets and cpython tests (ubuntu-latest)
• GitHub Check: Run snippets and cpython tests (windows-2025)
• GitHub Check: Run snippets and cpython tests (macos-latest)
• GitHub Check: Run tests under miri
• GitHub Check: Check the WASM package and demo
• GitHub Check: Ensure compilation on various targets

crates/codegen/src/symboltable.rs (5)

24-24: LGTM - HashSet import

Standard library import for tracking free variables. Appropriate choice for deduplicating variable names during propagation.

---

246-251: LGTM - Clean wrapper pattern

The top-level wrapper correctly discards the newfree set since it's only needed for internal propagation between nested scopes. The external API remains unchanged.

---

253-272: LGTM - Correct class-specific free variable handling

The updated drop_class_free correctly removes __class__ and __classdict__ from the propagated free variable set and sets the appropriate flags. This aligns with CPython's implementation.

---

342-416: LGTM - Sound free variable propagation logic

The refactored analyze_symbol_table correctly implements upward propagation of free variables:

1. Collects from child scopes first (lines 356-360)
2. Merges inlined comprehension symbols with proper filtering (lines 369-395)
3. Adds current scope's Free/FREE_CLASS symbols (lines 403-405)
4. Handles class-specific cells via drop_class_free (line 412)

The propagation flow is well-structured and follows compiler best practices.

---

497-502: The implicit __class__ handling in found_in_outer_scope and explicit registration at class scope entry work correctly together. No conflicts exist: explicit registration adds __class__ to the class scope, implicit lookup finds it for nested function references, and drop_class_free() ensures proper handling via the needs_class_closure flag rather than treating it as a free variable. This correctly implements Python's semantics for __class__ availability in methods via implicit closure cells.

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

1107-1114: LGTM - Correct routing for super optimization instructions

The LoadSuperAttr instruction is correctly routed to the new load_super_attr method, and the pseudo-instructions (LoadSuperMethod, LoadZeroSuperAttr, LoadZeroSuperMethod) are appropriately marked as unreachable since they should be converted to concrete instructions during compilation.

---

2534-2570: Stack order, decode function, and has_class semantics are all correct.

1. Stack order (lines 2538-2541) is correct. The compiler emits [global_super, class, self] on the stack (documented in compile.rs:5897), and the pop sequence correctly retrieves them in LIFO order.

2. decode_load_super_attr_arg correctly extracts (name_idx, load_method, has_class) using bit fields: bit 0 for load_method, bit 1 for has_class, remaining bits for name_idx.

3. has_class flag semantics:

   • has_class=true: Explicit super(class, self) form (SuperCallType::TwoArg from compile.rs)
   • has_class=false: Implicit super() form (SuperCallType::ZeroArg from compile.rs), which loads __class__ from the cell and the first function parameter, then calls with no explicit args.

4. Method vs attribute loading correctly implements the two-value protocol: methods push [function, self], attributes push [value, NULL].

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

78-88: SuperCallType shape looks right for keeping 0-arg vs 2-arg logic explicit.
One small nit: since TwoArg stores &Expr, it’s easy to accidentally outlive the AST; keeping the enum private (as you did) is the right containment.

---

775-819: load_args_for_super(ZeroArg) should be resilient to varnames ordering assumptions.
Line 806-815: using info.metadata.varnames.first() relies on varnames always starting with the “first parameter”. If that invariant is guaranteed by SymbolTable::scan_* + enter_function, it’s fine; otherwise, it could load a local that isn’t the first argument (breaking semantics for 0-arg super). Consider asserting that the chosen name is actually the first positional (or vararg) parameter when available.

---

5894-5915: Super-attr emission looks consistent, but verify opcode contract + naming consistency.

• Line 5901-5908: LoadSuperAttr { arg: idx } vs LoadZeroSuperAttr { idx } is a bit asymmetrical; if both ultimately carry a NameIdx, consider aligning field names to reduce footguns when adding more pseudo-ops.
• Please confirm the VM-side implementation consumes exactly [global_super, class, self] and preserves Python behavior when super is not the builtin super (e.g., shadowed at module scope).

---

6340-6362: Super-method emission: add tests for both optimized + fallback paths.
Given this changes call lowering, I’d like to see disassembly snapshots for at least:

• super().m() and super().attr
• super(C, self).m() and .attr
• negative cases: super().__class__, super(*xs).m, super(x=y).m, and local super = ... shadowing inside the function.

---

676-819: Please ensure cargo fmt + cargo clippy are clean for this PR.
As per coding guidelines for **/*.rs.

Also applies to: 5894-5915, 6340-6362

crates/compiler-core/src/bytecode/instruction.rs (4)

9-9: LGTM: Import is correctly added for super instruction decoding.

The decode_load_super_attr_arg import is properly utilized in both stack_effect (line 606) and fmt_dis (line 795) methods for decoding LoadSuperAttr arguments.

---

603-612: LGTM: Stack effect calculations are correct.

The stack effects properly reflect the super instruction semantics:

• Method loads (load_method=true): pop 3 (super, class, self), push 2 (method, self_or_null) = net -1
• Attribute loads (load_method=false): pop 3 (super, class, self), push 1 (attr) = net -2

The use of decode_load_super_attr_arg to dynamically determine the effect for LoadSuperAttr is appropriate, while the pseudo instructions have fixed effects that match their intended pre-conversion behavior.

---

793-805: LGTM: Display logic correctly formats super instructions.

The formatting properly handles:

• LoadSuperAttr: Decodes the encoded argument to display name, method flag, and class flag
• Pseudo instructions: Use straightforward name-based formatting

The display output will clearly distinguish between different super instruction variants and show all relevant flags, which aids debugging and bytecode inspection.

---

177-179: Verify that LoadSuperAttr encoding/decoding is properly tested.

The LoadSuperAttr instruction now uses a complex encoded argument (name_idx, load_method, has_class flags packed into a u32). Ensure that:

1. The encoding in the code generation phase correctly packs these values
2. The decoding via decode_load_super_attr_arg correctly unpacks them
3. Edge cases are tested (e.g., maximum name_idx values, all flag combinations)

The relevant decoding function signature is:

pub const fn decode_load_super_attr_arg(oparg: u32) -> (u32, bool, bool)

Verify the encoding is compatible by checking the code generation logic in crates/codegen/src/compile.rs and ensuring bidirectional correctness.

Actionable comments posted: 0

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

676-771: Shadowing detection for super looks too broad; it may disable optimization on harmless module-level reads.
Line 724-728 currently disables optimization if the module symbol table contains any entry for "super". If the symbol table records mere uses (not bindings), this could unintentionally block optimization in modules that only read super at top-level (no assignment/import). Consider restricting this check to “bound in module scope” cases (assignment/import/param), not any lookup() hit.

Proposed tightening (illustrative)

-        if let Some(top_table) = self.symbol_table_stack.first()
-            && top_table.lookup("super").is_some()
-        {
-            return None;
-        }
+        if let Some(top_table) = self.symbol_table_stack.first()
+            && let Some(sym) = top_table.lookup("super")
+            // Only treat as shadowing if the module actually *binds* `super`
+            && sym.scope != SymbolScope::GlobalImplicit
+        {
+            return None;
+        }

---

716-723: Allow global super (GlobalExplicit) inside the function without losing the optimization.
Requiring symbol.scope == SymbolScope::GlobalImplicit (Line 718-722) likely rejects global super even when it still resolves to the builtin at runtime (as long as the module doesn’t bind super). This is safe-but-unnecessary conservatism.

Proposed tweak

-        if let Some(symbol) = table.lookup("super")
-            && symbol.scope != SymbolScope::GlobalImplicit
+        if let Some(symbol) = table.lookup("super")
+            && !matches!(symbol.scope, SymbolScope::GlobalImplicit | SymbolScope::GlobalExplicit)
         {
             return None;
         }

crates/compiler-core/src/bytecode/instruction.rs (1)

247-257: Consider adding doc comments to clarify the "Zero" naming convention.

The "Zero" prefix indicates zero-argument super() calls, but this isn't immediately clear from the variant names alone. Adding doc comments would improve readability:

/// Pseudo opcode for super().method() - zero-argument super with method load
LoadSuperMethod { idx: Arg<NameIdx> } = 136,
/// Pseudo opcode for super().attr - zero-argument super with attribute load  
LoadZeroSuperAttr { idx: Arg<NameIdx> } = 137,
/// Pseudo opcode for super().method() - zero-argument super with method load
LoadZeroSuperMethod { idx: Arg<NameIdx> } = 138,

Note: There appears to be some naming inconsistency - LoadSuperMethod (line 249) doesn't have "Zero" in its name, while LoadZeroSuperMethod (line 255) does. Verify this is intentional and not a duplicate.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Reviewing files that changed from the base of the PR and between a27ac1c and 44a1b1d.

• crates/codegen/src/compile.rs
• crates/compiler-core/src/bytecode/instruction.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/codegen/src/compile.rs
• crates/compiler-core/src/bytecode/instruction.rs

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

• decode_load_super_attr_arg (113-118)

• GitHub Check: Run snippets and cpython tests (windows-2025)
• GitHub Check: Run rust tests (windows-2025)
• GitHub Check: Run rust tests (ubuntu-latest)
• GitHub Check: Run snippets and cpython tests (ubuntu-latest)
• GitHub Check: Run rust tests (macos-latest)
• GitHub Check: Check Rust code with clippy
• GitHub Check: Run snippets and cpython tests (macos-latest)
• GitHub Check: Check the WASM package and demo
• GitHub Check: Run snippets and cpython tests on wasm-wasi
• GitHub Check: Ensure compilation on various targets
• GitHub Check: Run tests under miri

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

78-88: SuperCallType<'a> is a clean minimal carrier for the optimization paths.
This keeps the “what did we detect?” separate from the “how do we emit?” logic.

---

3514-3536: get_ref_type class-scope override for __class__ / __classdict__ seems aligned with the comment intent.
Using table.typ == CompilerScope::Class matches “only class body, not methods” and avoids relying on possibly-misclassified symbol entries.

---

6334-6360: Super-method fast path integrates cleanly with compile_call_helper’s calling convention.
The optimized branch preserves the existing “method call pushes [callable, self_or_null] before args” flow, so downstream call emission stays consistent.

---

5891-5913: Opcode contract is complete and correct end-to-end.

The LoadZeroSuperAttr, LoadZeroSuperMethod, and LoadSuperMethod pseudo instructions are properly converted to LoadSuperAttr with appropriate flags during IR lowering (crates/codegen/src/ir.rs:210–240). The encoding/decoding functions (crates/compiler-core/src/bytecode.rs:107–118) correctly handle the bit-packed argument, and VM execution in load_super_attr (crates/vm/src/frame.rs:2534+) properly decodes and processes all flag combinations. The unreachable! markers in the VM (frame.rs:1110–1112) ensure pseudo instructions don't escape to runtime.

---

773-817: No changes needed. The code correctly assumes metadata.varnames.first() is the first parameter because parameters are always added to varnames before locals (see symboltable.rs), and can_optimize_super_call() ensures the ZeroArg path is only taken when argcount > 0 or posonlyargcount > 0. Existing tests in syntax_class.py verify super() works correctly with various parameter configurations.

crates/compiler-core/src/bytecode/instruction.rs (4)

9-9: LGTM: Import addition is correct.

The decode_load_super_attr_arg import is necessary for decoding the LoadSuperAttr oparg in stack effect calculations and display formatting.

---

353-356: LGTM: Pseudo opcodes properly registered.

The new super-related pseudo opcodes are correctly added to the custom_ops array for TryFrom<u8> conversion, following the established pattern.

---

607-616: Stack effects correctly implemented.

The stack effects align with CPython 3.12+ semantics: LOAD_SUPER_ATTR pops three values, and the low bit signals a method load (pushing two values) vs attribute load (pushing one value).

The implementation correctly:

• Pops 3 items (super, class, self) regardless of has_class flag
• Uses load_method to determine push count: 2 for methods (method + NULL/self), 1 for attributes
• Provides fixed stack effects for pseudo opcodes during pre-finalization phase

Based on web search results confirming CPython LOAD_SUPER_ATTR behavior.

---

796-808: LGTM: Display formatting correctly decodes and presents instruction details.

The display implementation properly:

• Decodes the packed LoadSuperAttr argument to show the name index, method flag, and class flag
• Renders pseudo opcodes with their respective name identifiers
• Maintains consistency with other instruction display patterns

Actionable comments posted: 1

In @crates/codegen/src/compile.rs:
- Around line 676-773: can_optimize_super_call rejects 0-arg super() when
__class__ is not SymbolScope::Free or lacks FREE_CLASS, but load_args_for_super
also accepts SymbolScope::Cell; update can_optimize_super_call to treat
SymbolScope::Cell as acceptable (i.e., allow optimization when
table.lookup("__class__") returns a symbol whose scope is Free or Cell or whose
flags contain FREE_CLASS) so eligibility matches load_args_for_super; reference
functions can_optimize_super_call and load_args_for_super and the symbol/name
"__class__" and SymbolScope::Cell/FREE_CLASS.

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

5893-5915: Add disassembly snapshot tests for super() optimization cases.

The codebase uses assert_dis_snapshot! for compiler tests (examples: test_if_ors, test_if_ands), but there is no test coverage for the super() attribute access and method call optimizations. Given the semantic complexity (super shadowing, class cell availability, side effects), add targeted snapshot tests that verify LoadSuperAttr and LoadZeroSuperAttr instructions are emitted only for the intended cases and fallback behavior is correct.

---

775-819: Document first-parameter assumption in zero-arg super() optimization.

The code assumes info.metadata.varnames.first() is the first parameter when handling zero-arg super(). While this is safe—can_optimize_super_call() checks argcount > 0 before returning ZeroArg, and the symbol table builder documents "Parameters are always added to varnames first" (symboltable.rs:1957)—adding an explicit assertion or comment at this usage site (line 808) would clarify the assumption for future maintainers and serve as a guard against refactors to the symbol table builder.

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

2534-2541: Clarify stack order in comment for readability.

The comment "Pop [super, class, self] from stack" describes the logical stack layout (bottom to top), but the actual pop order is reversed (self, then class, then super) due to LIFO. Consider rephrasing to: "Pop [self, class, super] from stack (TOS to bottom)" to make the actual pop order clearer for maintainers.

📝 Suggested comment improvement

-        // Pop [super, class, self] from stack
+        // Pop stack (TOS to bottom): [self, class, super]
+        // Stack layout before pop (bottom to top): [super, class, self]
         let self_obj = self.pop_value();
         let class = self.pop_value();
         let global_super = self.pop_value();

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Reviewing files that changed from the base of the PR and between 44a1b1d and 270df4b.

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

• crates/codegen/src/compile.rs
• crates/codegen/src/ir.rs
• crates/codegen/src/symboltable.rs
• crates/compiler-core/src/bytecode.rs
• crates/compiler-core/src/bytecode/instruction.rs
• crates/vm/src/frame.rs

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

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

• decode_load_super_attr_arg (113-118)
• get (470-476)
• get (573-575)

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

• get (22-88)

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

• encode_exception_table (51-63)
• encode_load_attr_arg (93-95)
• encode_load_super_attr_arg (107-109)
• marker (555-557)

• GitHub Check: Run snippets and cpython tests (ubuntu-latest)
• GitHub Check: Run rust tests (windows-2025)
• GitHub Check: Run rust tests (ubuntu-latest)
• GitHub Check: Run snippets and cpython tests (macos-latest)
• GitHub Check: Run rust tests (macos-latest)
• GitHub Check: Ensure compilation on various targets
• GitHub Check: Run snippets and cpython tests (windows-2025)
• GitHub Check: Check Rust code with clippy
• GitHub Check: Check the WASM package and demo
• GitHub Check: Run tests under miri

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

78-88: SuperCallType looks like the right minimal carrier for the optimization paths.
Minor: consider Copy only if you expect to pass this around a lot; Clone is fine since it holds references.

---

6339-6362: The stack contract is correctly implemented across LoadSuperMethod, LoadZeroSuperMethod, and LoadAttrMethod.

All three instructions push [method, self_or_null] in identical form when used in method-call context. LoadSuperMethod and LoadZeroSuperMethod are converted to LoadSuperAttr with the method flag set (lines 216–231 in crates/codegen/src/ir.rs), and both VM implementations (load_attr and load_super_attr in crates/vm/src/frame.rs) push the same stack shape: either [func, self] or [attr, null]. Using compile_call_helper(0, args) is correct in both the super-call fast-path and the normal method-call path.

---

3516-3537: The __class__/__classdict__ shortcut in get_ref_type() is safe; the guarantees are implicit in symbol table analysis.

The concern about fabricating cells is valid in principle, but the guarantee holds because __class__ can only appear in a child scope's freevars if it was explicitly referenced there. When a child method references __class__, the symbol table analysis propagates it to the class scope's newfree set, triggering drop_class_free() to set needs_class_closure = true and insert the cellvar into the cache. Thus, whenever get_ref_type() is called from make_closure() and returns Cell for __class__, the cellvar is guaranteed to exist in the parent's cellvar list.

The shortcut is correct but implicit—consider documenting this dependency on symbol table analysis guarantees to prevent future regressions.

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

215-220: Verify encoding matches runtime decoding expectations.

The encoding encode_load_super_attr_arg(idx.get(info.arg), true, true) sets load_method=true and has_class=true. Ensure this matches the decoding logic in frame.rs (line 2535) where decode_load_super_attr_arg expects the same bit layout: bit 0 for load_method, bit 1 for has_class, and bits 2+ for name_idx.

Based on the relevant snippet from crates/compiler-core/src/bytecode.rs (lines 106-108), the encoding is (name_idx << 2) | ((has_class as u32) << 1) | (load_method as u32), which matches the expected decoding.

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

2551-2563: Validate method binding to original instance.

When load_method=true and a Function is found, the code pushes func and self_obj (the original instance). This is correct because super() should bind methods to the original self, not the super proxy object. The implementation correctly follows Python's super() semantics.

---

2543-2549: The review comment's characterization of has_class=false semantics is incorrect.

When has_class=false, the code does not indicate shadowed super. The compiler's can_optimize_super_call() explicitly detects super shadowing and returns None, preventing any LoadZeroSuperAttr emission. The has_class=false case only occurs when:

1. super is NOT shadowed (verified as GlobalImplicit in both current and top-level scopes)
2. __class__ is guaranteed available as a free or cell variable
3. The function has at least one positional parameter

The compiler's logic correctly ensures has_class=false is emitted only for non-shadowed, optimizable super() calls with implicit __class__ lookup. No verification issue exists; the frame.rs code is semantically correct and properly depends on the compiler's prior validation.

crates/codegen/src/symboltable.rs (4)

246-251: Confirm intentional discarding of top-level free variables.

The top-level analyze_symbol_table function now discards the newfree HashSet returned by the analyzer. This is correct because free variables are only meaningful within nested scopes, and the top-level (module) scope cannot have free variables by definition. The comment clarifies this design decision.

---

342-416: Validate free variable propagation algorithm.

The analysis correctly implements free variable propagation:

1. Lines 350-360: Collects free variables from child scopes via recursion
2. Lines 398-406: Adds current scope's free variables (Free symbols and FREE_CLASS flags) to newfree
3. Lines 411-413: Calls drop_class_free for class scopes to handle implicit cells
4. Line 415: Returns newfree to propagate upward

This follows the standard compiler symbol table analysis pattern and matches CPython's approach.

---

497-502: Confirm class implicit declaration in class scopes.

The special case for __class__ in class scopes (lines 497-502) treats it as implicitly declared, setting decl_depth = Some(i). This is correct because:

• __class__ is automatically available in class scopes for super() to function
• Methods referencing super() need __class__ as a free variable from the class scope
• The symbol table must recognize this implicit declaration when resolving names

This aligns with Python's super() implementation and the changes to drop_class_free.

---

261-272: Verify drop_class_free removes correct implicit cells.

The function correctly removes __class__ and __classdict__ from the newfree set when they're found, setting the corresponding flags (needs_class_closure and needs_classdict) on the symbol table. This prevents these implicit cells from being propagated as regular free variables to parent scopes.

The compiler properly creates these implicit cells during code generation when the flags are set—inserting them into cellvar_cache in the class scope. Methods accessing super() correctly resolve __class__ via the get_ref_type() call, which returns either SymbolScope::Cell or SymbolScope::Free, allowing the super() optimization to load the class reference from the closure. Error handling catches the edge case where the cell is unexpectedly missing.

Actionable comments posted: 0

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

78-88: SuperCallType is a good, minimal carrier for the optimization decision.
Minor: consider #[derive(Copy)] as well (it’s just references + an enum), if you end up passing it around more.

---

676-773: Super optimization gate looks sane; please add “shadowing” regression tests.
The symbol-table checks (current scope + module scope) are conservative in a good way; I’d still want tests covering: local super = ..., global super, and module-level super = ... to ensure we always fall back.

---

5895-5918: Attribute path: optimized super opcodes are cleanly integrated with a safe fallback.
One nit: LoadSuperAttr { arg: idx } vs LoadZeroSuperAttr { idx } is a bit confusing—if these are both “name idx”, aligning field names would reduce cognitive load (unless the IR encoding truly needs arg).

---

6338-6373: Call path: super().method() optimization composes correctly with existing CALL protocol.
Please ensure end-to-end stack contract is covered by tests (compiler disassembly snapshot or VM execution) for both super().m() and super(C, self).m(), including error cases. Also, per guidelines, make sure CI runs cargo fmt + cargo clippy cleanly.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Reviewing files that changed from the base of the PR and between 270df4b and 158d15e.

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

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

• decode_load_super_attr_arg (113-118)

• GitHub Check: Run snippets and cpython tests (ubuntu-latest)
• GitHub Check: Run rust tests (windows-2025)
• GitHub Check: Run rust tests (macos-latest)
• GitHub Check: Run snippets and cpython tests (windows-2025)
• GitHub Check: Run snippets and cpython tests (macos-latest)
• GitHub Check: Check the WASM package and demo
• GitHub Check: Run rust tests (ubuntu-latest)
• GitHub Check: Check Rust code with clippy
• GitHub Check: Run tests under miri
• GitHub Check: Run snippets and cpython tests on wasm-wasi
• GitHub Check: Ensure compilation on various targets

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

3518-3540: get_ref_type(): special-casing class/classdict in class scope is the right shape.
This should unblock closure loading for implicit class cells even when they’re not present as normal symbols.

---

775-821: Remove major issue classification — varnames parameter ordering is guaranteed and documented.

The varnames invariant cited in the code comment is architecturally enforced by symboltable.rs: scan_parameters explicitly processes posonlyargs, args, kwonlyargs, vararg, and kwarg in order before any body analysis that adds locals. The code at line 1957 documents this guarantee ("Parameters are always added to varnames first"), and register_name() adds all parameters via .push() before any assigned variables. This ordering is never modified after symbol table transfer to compile context.

Since can_optimize_super_call() restricts super() ZeroArg to actual function/lambda scopes (via in_func() check, which excludes comprehensions), and lambdas build varnames identically to regular functions, the assumption that varnames.first() yields the first positional parameter is safe and correct under the current architecture.

The suggested refactoring to explicitly store first_positional_param in CompileContext could be a valid optional improvement for code clarity and defensiveness against future refactoring, but the current code contains no semantic bug.

Likely an incorrect or invalid review comment.

crates/compiler-core/src/bytecode/instruction.rs (5)

9-9: LGTM: Import added for super-opcode decoding.

The import of decode_load_super_attr_arg is correctly placed and necessary for the stack effect and display logic added later in the file.

---

356-359: LGTM: Pseudo-opcodes properly registered.

The new super-related pseudo opcodes are correctly added to the custom_ops list for bytecode deserialization, following the established pattern.

---

799-811: LGTM: Display formatting correctly implemented.

The display logic properly:

• Decodes and shows all components of LoadSuperAttr (name, method flag, class flag)
• Renders pseudo opcodes with their instruction names and attribute names
• Maintains consistency with the file's formatting patterns

---

247-260: No action needed—all four variant combinations are intentionally handled.

The missing variant (has_class=true, load_method=false) is already covered by the LoadSuperAttr pseudo-opcode, which is emitted for 2-arg super() attribute access and later converted with the appropriate flags in ir.rs. The design intentionally reuses the generic LoadSuperAttr for the 2-arg case while providing dedicated pseudo-opcodes (LoadZeroSuperAttr, LoadSuperMethod, LoadZeroSuperMethod) for other combinations. This is well-documented in the compiler codegen.

---

610-619: Stack effects are internally consistent and match VM implementation.

The stack effect logic correctly implements:

• All variants pop 3 items (super/class/self)
• load_method variants push 2 (method + self_or_null)
• Attribute variants push 1 (value)

"Zero" variants (0-arg super()) also pop 3 because the compiler loads __class__ from cell variables to maintain uniform stack protocol. The has_class flag only affects how the VM invokes super(), not the stack shape.