GeezerVison

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

757-771: Use named constants for nibble packing to remove magic numbers.

Logic is correct, but replacing 4 and 15 with constants makes the encoding contract clearer and safer to maintain.

Suggested diff

 impl VarNums {
+    const LOW_NIBBLE_MASK: u32 = 0x0F;
+    const HIGH_NIBBLE_SHIFT: u32 = 4;
+
     #[must_use]
     pub const fn idx_1(self) -> VarNum {
-        VarNum::new(self.0 >> 4)
+        VarNum::new(self.0 >> Self::HIGH_NIBBLE_SHIFT)
     }

     #[must_use]
     pub const fn idx_2(self) -> VarNum {
-        VarNum::new(self.0 & 15)
+        VarNum::new(self.0 & Self::LOW_NIBBLE_MASK)
     }

     #[must_use]
     pub const fn indexes(self) -> (VarNum, VarNum) {
         (self.idx_1(), self.idx_2())
     }
 }

🤖 Prompt for AI Agents

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

In `@crates/compiler-core/src/bytecode/oparg.rs` around lines 757 - 771, Replace
the magic numbers in VarNums's nibble packing/unpacking by adding descriptive
consts (e.g., NIBBLE_BITS = 4 and NIBBLE_MASK = (1 << NIBBLE_BITS) - 1) and use
them in the methods idx_1, idx_2, and indexes; update VarNums::idx_1 to shift by
NIBBLE_BITS and VarNums::idx_2 to mask with NIBBLE_MASK while keeping them as
const fn and returning VarNum::new(...) so the encoding contract is explicit and
easier to maintain.

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

1348-1353: Inconsistent display format for StoreFastLoadFast.

StoreFastLoadFast outputs raw index numbers (store_idx, load_idx) instead of variable names, while similar instructions like LoadFastLoadFast, LoadFastBorrowLoadFastBorrow, and StoreFastStoreFast all use varname() to display human-readable variable names.

Consider using varname() for consistency:

♻️ Suggested fix for consistency

 Self::StoreFastLoadFast { var_nums } => {
     let oparg = var_nums.get(arg);
     let (store_idx, load_idx) = oparg.indexes();
-    write!(f, "STORE_FAST_LOAD_FAST")?;
-    write!(f, " ({}, {})", store_idx, load_idx)
+    write!(
+        f,
+        "{:pad$}({}, {})",
+        "STORE_FAST_LOAD_FAST",
+        varname(store_idx),
+        varname(load_idx)
+    )
 }

🤖 Prompt for AI Agents

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

In `@crates/compiler-core/src/bytecode/instruction.rs` around lines 1348 - 1353,
The Display implementation for the enum variant Self::StoreFastLoadFast {
var_nums } prints raw indexes (store_idx, load_idx) rather than human-readable
names; change the second write! call in that match arm to use varname() on the
indexes (the same approach used by LoadFastLoadFast,
LoadFastBorrowLoadFastBorrow, and StoreFastStoreFast) so it prints the variable
names (e.g., var_nums.varname(store_idx), var_nums.varname(load_idx)) instead of
numeric indices while preserving the "STORE_FAST_LOAD_FAST" label.