Actionable comments posted: 1

vm/src/frame.rs (1)

1269-1280: Use PyTupleRef::try_from_object for a cleaner & more permissive tuple-conversion

The new None || tuple handling looks correct 👍.
You can remove the manual downcast boilerplate (and transparently accept tuple sub-classes, mirroring CPython) by leaning on the existing helper:

-    let type_params: PyTupleRef = if vm.is_none(&type_params_obj) {
-        // If None, use empty tuple (matching CPython's behavior)
-        vm.ctx.empty_tuple.clone()
-    } else {
-        type_params_obj
-            .downcast()
-            .map_err(|_| vm.new_type_error("Type params must be a tuple."))?
-    };
+    let type_params: PyTupleRef = if vm.is_none(&type_params_obj) {
+        // If None, use the shared empty tuple (CPython behaviour)
+        vm.ctx.empty_tuple.clone()
+    } else {
+        PyTupleRef::try_from_object(vm, type_params_obj)?
+    };

This keeps semantics identical while reducing code and aligning the acceptance rules with CPython’s (i.e. tuple subclasses are valid).

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

1031-1076: LGTM! Solid PEP 695 TypeAlias implementation with minor optimization opportunity.

The TypeAlias compilation logic correctly handles both cases:

• With type parameters: Proper symbol table management, type params compiled first (making them available for value expression), and correct stack order via Rotate2
• Without type parameters: Clean path that uses None instead of empty tuple for CPython compatibility

The stack manipulation logic is sound and the comments clearly explain the PEP 695 compliance requirements.

Minor optimization: Consider avoiding the extra string clone:

                let name_string = name.id.to_string();
-                self.emit_load_const(ConstantData::Str {
-                    value: name_string.clone().into(),
-                });
+                self.emit_load_const(ConstantData::Str {
+                    value: name_string.as_str().into(),
+                });

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Reviewing files that changed from the base of the PR and between 07a04ac and 5efda1b.

• compiler/codegen/src/compile.rs (1 hunks)
• vm/src/builtins/dict.rs (1 hunks)
• vm/src/builtins/memory.rs (2 hunks)
• vm/src/builtins/tuple.rs (0 hunks)
• vm/src/builtins/type.rs (4 hunks)
• vm/src/builtins/union.rs (3 hunks)
• vm/src/exceptions.rs (1 hunks)
• vm/src/frame.rs (1 hunks)
• vm/src/protocol/object.rs (1 hunks)
• vm/src/types/structseq.rs (1 hunks)

• vm/src/builtins/tuple.rs

Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.138Z
Learning: Minimize modifications to CPython standard library files in the Lib/ directory; bug fixes should be made through Rust code modifications whenever possible.

Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.138Z
Learning: Follow Rust best practices for error handling and memory management in all RustPython Rust code.

Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.138Z
Learning: Use the macro system (such as 'pyclass', 'pymodule', 'pyfunction') when implementing Python functionality in Rust for RustPython.

Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.138Z
Learning: When implementing a Python module in Rust for RustPython, use the #[pymodule] attribute and expose functions and classes using #[pyfunction], #[pyclass], and #[pymethod] as appropriate.

• GitHub Check: Run snippets and cpython tests (windows-latest)
• GitHub Check: Check Rust code with rustfmt and clippy
• GitHub Check: Ensure compilation on various targets
• GitHub Check: Run rust tests (windows-latest)

vm/src/builtins/type.rs (6)

161-168: New utility function for MRO-based subtype checking looks good.

The is_subtype_with_mro function provides a clean way to check subtype relationships by iterating through the MRO. The implementation correctly uses reference equality with item.is(b).

---

209-213: Proper implementation of CPython's PyType_Check equivalent.

The check method correctly implements downcasting to check if an object is an instance of PyType (or its subclass), matching CPython's PyType_Check macro behavior.

---

457-459: Well-implemented subtype checking method.

The is_subtype method properly uses the new is_subtype_with_mro utility with the MRO read guard. The implementation is clean and follows Rust patterns.

---

461-465: Correct implementation of exact type checking.

The check_exact method properly implements CPython's PyType_CheckExact behavior using downcast_ref_if_exact, which ensures the object is exactly a PyType and not a subclass.

---

470-470: Minor cleanup in type signature.

Removing the fully qualified path crate:: from the type parameter is a good cleanup - the unqualified PyObject is clearer and more consistent.

---

1244-1248: Excellent improvement to avoid recursion in subclass checking.

The change to use subclass.real_is_subclass(self.as_object(), vm) instead of calling fast_issubclass directly is a significant improvement. This matches CPython's behavior by using _PyObject_RealIsSubclass and prevents potential infinite recursion through __subclasscheck__.

vm/src/types/structseq.rs (1)

100-100: Proper replacement of fast_getitem with direct indexing.

The change from zelf.fast_getitem(i.into()) to zelf[i as usize].to_owned() correctly follows the systematic removal of the fast_getitem method. Direct indexing is more idiomatic and the .to_owned() maintains the same ownership semantics.

vm/src/protocol/object.rs (1)

407-407: Correct replacement of fast_getitem with direct indexing.

The change from tuple.fast_getitem(0).clone() to tuple[0].clone() properly replaces the method call with direct indexing. This is part of the systematic removal of fast_getitem and maintains the same semantics.

vm/src/builtins/memory.rs (2)

213-219: LGTM: Safe direct indexing replacement

The replacement of fast_getitem with direct indexing x[0].to_owned() is correct and safe. The bounds check if x.len() == 1 ensures the indexing won't panic, and the semantics are preserved.

---

1068-1074: LGTM: Consistent refactoring pattern

Same safe replacement pattern as the unpack_single method. Direct indexing with proper bounds checking maintains correctness while improving performance and consistency.

vm/src/exceptions.rs (1)

1669-1671: LGTM: Safe tuple element access

The replacement of fast_getitem with direct indexing location_tuple[i].to_owned() is safe due to the bounds check if location_tup_len > i. This change maintains consistency with the broader codebase refactoring effort.

vm/src/builtins/dict.rs (1)

238-243: LGTM: Clean refactoring from fast_getitem to direct indexing.

The replacement of fast_getitem calls with direct tuple indexing is correct and maintains the same semantics. The tuple has already been validated to have length 2, making direct indexing safe.

vm/src/builtins/union.rs (3)

43-46: LGTM: Useful addition for internal access.

The new get_args method provides direct access to the internal args tuple, aligning with CPython's _Py_union_args attribute. This is a reasonable addition for internal VM use.

---

202-202: LGTM: Correct refactoring from fast_getitem to direct indexing.

The replacement of args.fast_getitem(0) with args[0].to_owned() is safe since this code path is only reached when args.len() == 1.

---

220-220: LGTM: Consistent tuple access pattern.

The change from new_args.fast_getitem(0) to new_args[0].to_owned() maintains the same semantics and is safe in this context where we know the tuple is non-empty.