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

6732-6735: ⚠️ Potential issue | 🟡 Minor

Reword the Compare fast-path comment.

Rich comparisons can return arbitrary objects. Skipping TO_BOOL here is only safe because PopJumpIf* performs truthiness conversion itself.

Suggested comment update

-                // Compare already produces a bool; everything else needs TO_BOOL
+                // Rich comparisons may return non-bool objects; skipping TO_BOOL
+                // here is only correct because PopJumpIf* performs truthiness
+                // conversion itself.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/codegen/src/compile.rs` around lines 6732 - 6735, Update the comment
above the Compare fast-path to note that rich comparisons may return arbitrary
objects and that we only skip emitting Instruction::ToBool for
ast::Expr::Compare because the subsequent conditional jump opcodes
(PopJumpIfTrue / PopJumpIfFalse / other PopJumpIf* handlers) perform the
truthiness conversion themselves; reference the match on expression
(ast::Expr::Compare), the emit!(self, Instruction::ToBool) call, and the
PopJumpIf* jump handlers to make the reason explicit.

---

4626-4649: ⚠️ Potential issue | 🟠 Major

__static_attributes__ is still always written as ().

This file only initializes static_attributes; it never populates it before this block runs. As written, every class body overwrites __static_attributes__ with an empty tuple, and it does so after compiling body, so it also clobbers an explicit class-body assignment.

Suggested guard until the collector is wired up

-        // Emit __static_attributes__ tuple
-        {
-            let attrs: Vec<String> = self
-                .code_stack
-                .last()
-                .unwrap()
-                .static_attributes
-                .as_ref()
-                .map(|s| s.iter().cloned().collect())
-                .unwrap_or_default();
-            self.emit_load_const(ConstantData::Tuple {
-                elements: attrs
-                    .into_iter()
-                    .map(|s| ConstantData::Str { value: s.into() })
-                    .collect(),
-            });
-            let static_attrs_name = self.name("__static_attributes__");
-            emit!(
-                self,
-                Instruction::StoreName {
-                    namei: static_attrs_name
-                }
-            );
-        }
+        if let Some(attrs) = self
+            .code_stack
+            .last()
+            .and_then(|info| info.static_attributes.as_ref())
+            .filter(|attrs| !attrs.is_empty())
+        {
+            self.emit_load_const(ConstantData::Tuple {
+                elements: attrs
+                    .iter()
+                    .cloned()
+                    .map(|s| ConstantData::Str { value: s.into() })
+                    .collect(),
+            });
+            let static_attrs_name = self.name("__static_attributes__");
+            emit!(self, Instruction::StoreName { namei: static_attrs_name });
+        }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/codegen/src/compile.rs` around lines 4626 - 4649, The code
unconditionally emits and stores __static_attributes__ as an empty tuple because
static_attributes is never populated; fix by guarding the emit: only call
emit_load_const + Instruction::StoreName for __static_attributes__ when
self.code_stack.last().unwrap().static_attributes is Some(vec) and the vec is
non-empty (i.e., contains attributes), so it doesn't overwrite an explicit
class-body assignment or write an empty tuple; use the existing
name("__static_attributes__"), emit_load_const, and the Instruction::StoreName
path when the Option has data.

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

2512-2512: Encode the new 1-based container depth in one place.

These five handlers now rely on the same subtle rule: the opcode arg is 1-based, nth_value() is 0-based, and the lookup happens after popping the transient operand(s). Repeating raw - 1 here makes that contract easy to miss or accidentally revert later; a tiny helper or short comment would make the parity rule much clearer.

Also applies to: 2522-2522, 2864-2864, 3195-3195, 3205-3205

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/frame.rs` at line 2512, Create a small helper to centralize the
1-based→0-based conversion and use it from the handlers instead of repeating `-
1`; for example add a method on the same type (e.g., fn arg_to_index(&self, arg:
u32) -> usize or fn nth_arg_value(&self, arg: u32) -> Value) and replace
occurrences like `let obj = self.nth_value(i.get(arg) - 1);` in the handlers
referenced (the sites at the current diff and the ones you noted: the uses at
~2522, ~2864, ~3195, ~3205) to call that helper so the parity rule is enforced
in one place and clearly named. Ensure the helper documents that opcode args are
1-based and nth_value is 0-based.