Use rust idioms for accessing a `Vec` in `codegen/src/compile.rs` by ShaharNaveh · Pull Request #6326 · RustPython/RustPython

coderabbitai

612-629: Avoid unsafe / unwrap_unchecked in push_symbol_table

The unwrap_unchecked here is correct under the stated invariant, but it introduces unsafe for a very small gain. Since this isn’t an ultra‑hot path and the stack cannot be empty after push, you can keep the code safe and aligned with the PR’s intent (idiomatic Vec access) by reusing current_symbol_table instead of unsafe:

         let table = current_table.sub_tables.remove(0);

         // Push the next table onto the stack
         self.symbol_table_stack.push(table);
-        // SAFETY: We just pushed, so it can't be empty
-        unsafe { self.symbol_table_stack.last().unwrap_unchecked() }
+        self.current_symbol_table()

This preserves behavior, removes unsafe, and keeps all Vec access in one helper. As per coding guidelines, safer patterns are preferred when possible.

1193-1204: Make parent lookup in TypeParams scope avoid len() - 2 arithmetic

The TypeParams parent lookup now uses Vec::get(self.symbol_table_stack.len() - 2).expect(...). Logically this matches the existing invariant (“there must be a parent”), but len() - 2 can underflow in debug builds if that invariant is ever broken, yielding a less helpful panic than your custom expect message.

You can stay within the “idiomatic Vec access” goal and avoid manual index math by using an iterator from the back to get “the element before last”:

-            // If not found and we're in TypeParams scope, try parent scope
-            let symbol = if symbol.is_none() && is_typeparams {
-                self.symbol_table_stack
-                    .get(self.symbol_table_stack.len() - 2) // Try to get parent index
-                    .expect("Symbol has no parent! This is a compiler bug.")
-                    .lookup(name.as_ref())
-            } else {
-                symbol
-            };
+            // If not found and we're in TypeParams scope, try parent scope
+            let symbol = if symbol.is_none() && is_typeparams {
+                let parent = self
+                    .symbol_table_stack
+                    .iter()
+                    .rev()
+                    .nth(1) // second-from-last element is the parent
+                    .expect("Symbol has no parent! This is a compiler bug.");
+                parent.lookup(name.as_ref())
+            } else {
+                symbol
+            };

This keeps the same invariant but avoids explicit len() - 2 arithmetic on the Vec.