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

1072-1076: Please confirm fmt/clippy + clean bytecode build were run.

Bytecode generation paths changed here, so please ensure cargo fmt, cargo clippy --all-targets --all-features, and the clean build step (rm -r target/debug/build/rustpython-* && find . | grep -E "\.pyc$" | xargs rm -r) were executed before finalizing.

As per coding guidelines: Follow the default rustfmt code style by running cargo fmt to format Rust code; Always run clippy to lint Rust code (cargo clippy) before completing tasks and fix any warnings or lints that are introduced by your changes; When modifying bytecode instructions, perform a clean build by running: rm -r target/debug/build/rustpython-* && find . | grep -E "\.pyc$" | xargs rm -r.

crates/vm/src/stdlib/typing.rs (2)

110-118: new_eager stores non-callable value as compute_value — latent risk if cache is ever cleared.

In new_eager, compute_value is set to value.clone(), which is the evaluated result (not a callable). If cached_value were ever None after construction (e.g., due to a future refactor), __value__ would attempt to call a non-callable, producing a confusing TypeError. Currently safe since cached_value is always Some after new_eager, but this is fragile.

Consider storing a sentinel or documenting the invariant:

Suggestion

         /// Create with an eagerly evaluated value (used by constructor)
         pub fn new_eager(name: PyStrRef, type_params: PyTupleRef, value: PyObjectRef) -> Self {
             Self {
                 name,
                 type_params,
-                compute_value: value.clone(),
+                compute_value: value.clone(), // N.B. not callable — cached_value must always be Some for eager aliases
                 cached_value: crate::common::lock::PyMutex::new(Some(value)),
             }
         }

---

272-277: Module attribute fix is reasonable but fragile.

If TypeAliasType is ever not exported as a pyattr (or renamed), the get_attr("TypeAliasType", vm) lookup silently fails and __module__ stays as "_typing". The let _ = on set_attr also silently swallows failures. This is acceptable as a workaround, but a comment explaining why this is needed (pyattr sets __module__ from the containing #[pymodule(name = "_typing")]) would help future maintainers.

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

399-413: Dead !is_class branch in needs_class_symbols computation.

The !is_class disjunct (lines 405-407) can never influence class_symbols_clone because line 409 gates cloning on is_class && needs_class_symbols. When !is_class, class_symbols_clone is always None regardless of needs_class_symbols, and child scopes receive class_entry directly (line 425). The extra condition adds noise without effect.

♻️ Proposed simplification

-        let needs_class_symbols = (is_class
-            && (sub_tables.iter().any(|st| st.can_see_class_scope)
-                || annotation_block
-                    .as_ref()
-                    .is_some_and(|b| b.can_see_class_scope)))
-            || (!is_class
-                && class_entry.is_some()
-                && sub_tables.iter().any(|st| st.can_see_class_scope));
-
-        let class_symbols_clone = if is_class && needs_class_symbols {
+        let needs_class_symbols = is_class
+            && (sub_tables.iter().any(|st| st.can_see_class_scope)
+                || annotation_block
+                    .as_ref()
+                    .is_some_and(|b| b.can_see_class_scope));
+
+        let class_symbols_clone = if needs_class_symbols {
             Some(symbols.clone())
         } else {
             None
         };

crates/vm/src/stdlib/typing.rs (4)

367-377: vm.import("typing", 0) on every iter() call is potentially expensive.

This performs a module import lookup each time a TypeAliasType is iterated (for Unpack semantics). The same pattern appears in unpack_typevartuples. While this follows other patterns in the codebase and the import is cached after first load, it's worth noting.

---

214-246: check_type_params return value is discarded by the caller.

Line 316 calls Self::check_type_params(&tp, vm)?; but ignores the Option<PyTupleRef> return value. The function is only used for its validation side-effect. Consider changing the return type to PyResult<()> to reflect this.

Suggested simplification

         fn check_type_params(
             type_params: &PyTupleRef,
             vm: &VirtualMachine,
-        ) -> PyResult<Option<PyTupleRef>> {
+        ) -> PyResult<()> {
             if type_params.is_empty() {
-                return Ok(None);
+                return Ok(());
             }
             // ... validation logic unchanged ...
-            Ok(Some(type_params.clone()))
+            Ok(())
         }

---

134-143: Avoid double-computation in __value__ via non-atomic check-then-set.

The lock is released after clone() on line 136, then re-acquired on line 141. Two concurrent threads could both observe None, both call compute_value, and both store results, causing wasteful redundant computation. Since compute_value is a Python callable, the double-computation is safe but inefficient.

Use a double-check-lock pattern to eliminate the race:

Suggested pattern

         fn __value__(&self, vm: &VirtualMachine) -> PyResult {
-            let cached = self.cached_value.lock().clone();
-            if let Some(value) = cached {
-                return Ok(value);
-            }
-            let value = self.compute_value.call((), vm)?;
-            *self.cached_value.lock() = Some(value.clone());
-            Ok(value)
+            {
+                let cached = self.cached_value.lock();
+                if let Some(value) = cached.as_ref() {
+                    return Ok(value.clone());
+                }
+            }
+            let value = self.compute_value.call((), vm)?;
+            let mut cached = self.cached_value.lock();
+            if let Some(existing) = cached.as_ref() {
+                Ok(existing.clone())
+            } else {
+                *cached = Some(value.clone());
+                Ok(value)
+            }
         }

This pattern computes compute_value outside the lock (safe for Python re-entry), then verifies another thread hasn't already cached the result before storing.

---

322-326: Use the established caller() pattern for consistency with TypeVar.

TypeAliasType retrieves the module by accessing func_obj.get_attr("__module__", vm), but TypeVar in the same codebase uses the caller() helper which accesses frame.globals.get_item("__name__", vm). A comment in typevar.rs explicitly notes that "RustPython's Frame architecture is different" from CPython and deliberately uses globals instead of func_obj. TypeAliasType should follow this same pattern—either by calling set_module_from_caller() after construction (like TypeVar does in slot_new) or by using the caller() function directly to retrieve the module name, ensuring consistency with RustPython's architectural approach.

crates/vm/src/builtins/genericalias.rs (2)

150-168: List repr: borrow_vec() per-element access may panic on concurrent mutation.

The comment says "Use indexed access so list mutation during repr causes IndexError," but list.borrow_vec() acquires a borrow guard. If repr_item triggers a __repr__ that mutates this same list, the second borrow_vec() call will panic (borrow conflict) rather than returning an IndexError. Consider either borrowing the whole slice once upfront (clone elements) or documenting this difference from CPython's behavior.

Possible safer alternative: clone elements upfront

         if obj.class().is(vm.ctx.types.list_type) {
             let list = obj.downcast_ref::<crate::builtins::PyList>().unwrap();
-            let len = list.borrow_vec().len();
-            let mut parts = Vec::with_capacity(len);
-            // Use indexed access so list mutation during repr causes IndexError
-            for i in 0..len {
-                let item =
-                    list.borrow_vec().get(i).cloned().ok_or_else(|| {
-                        vm.new_index_error("list index out of range".to_owned())
-                    })?;
-                parts.push(repr_item(item, vm)?);
-            }
+            let items: Vec<PyObjectRef> = list.borrow_vec().to_vec();
+            let mut parts = Vec::with_capacity(items.len());
+            for item in items {
+                parts.push(repr_item(item, vm)?);
+            }
             Ok(format!("[{}]", parts.join(", ")))

---

670-721: PyGenericAliasIterator lacks traverse — potential GC concern.

The iterator holds a PyMutex<Option<PyObjectRef>> which references a PyGenericAlias. If the alias participates in a reference cycle, the GC won't be able to trace through this iterator. Consider adding traverse = "manual" or traverse to the #[pyclass] attribute, similar to how other iterators in the codebase handle this.