GeezerVison

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

1713-1713: Minor style inconsistency: PopBlock has no fields.

PopBlock is defined as a unit variant (line 1608), so the { .. } pattern is unnecessary. Other unit-like matches in this block don't use this pattern.

Proposed fix

-            Self::PopBlock { .. } => "POP_BLOCK",
+            Self::PopBlock => "POP_BLOCK",

🤖 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` at line 1713, The match arm
for the enum variant PopBlock unnecessarily uses a struct pattern with `{ .. }`;
update the pattern in the match (the arm mapping to "POP_BLOCK") from
Self::PopBlock { .. } to the unit pattern Self::PopBlock so it matches the unit
variant definition of PopBlock in the Instruction enum.

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

1705-1719: Minor style inconsistency in pattern matching.

Line 1713 uses Self::PopBlock { .. } but PopBlock is a unit variant (no fields). While valid Rust, this is inconsistent with line 1707 where Self::AnnotationsPlaceholder (also a unit variant) uses no braces.

Optional: Use consistent pattern style

-            Self::PopBlock { .. } => "POP_BLOCK",
+            Self::PopBlock => "POP_BLOCK",

🤖 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 1705 - 1719,
The pattern match in the name() method mixes unit-variant style and struct-like
style; change the unit variant match Self::PopBlock { .. } to the consistent
unit pattern Self::PopBlock (same style as Self::AnnotationsPlaceholder) inside
the name() function so all unit variants use the same pattern form.

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

1479-1508: Consider extracting common LoadGlobal* formatting logic.

The formatting logic for LoadGlobal, LoadGlobalBuiltin, and LoadGlobalModule is nearly identical. Per coding guidelines, when branches share common logic differing only in value, consider extracting to avoid duplication.

♻️ Suggested refactor

-            Self::LoadGlobal { namei } => {
-                let oparg = namei.get(arg);
-                let name_idx = oparg >> 1;
-                let opname = self.name();
-                if (oparg & 1) != 0 {
-                    write!(f, "{:pad$}({}, NULL + {})", opname, oparg, name(name_idx))
-                } else {
-                    write!(f, "{:pad$}({}, {})", opname, oparg, name(name_idx))
-                }
-            }
-            Self::LoadGlobalBuiltin => {
-                let oparg = u32::from(arg);
-                let name_idx = oparg >> 1;
-                let opname = self.name();
-                if (oparg & 1) != 0 {
-                    write!(f, "{:pad$}({}, NULL + {})", opname, oparg, name(name_idx))
-                } else {
-                    write!(f, "{:pad$}({}, {})", opname, oparg, name(name_idx))
-                }
-            }
-            Self::LoadGlobalModule => {
-                let oparg = u32::from(arg);
-                let name_idx = oparg >> 1;
-                let opname = self.name();
-                if (oparg & 1) != 0 {
-                    write!(f, "{:pad$}({}, NULL + {})", opname, oparg, name(name_idx))
-                } else {
-                    write!(f, "{:pad$}({}, {})", opname, oparg, name(name_idx))
-                }
-            }
+            Self::LoadGlobal { namei } => {
+                let oparg = namei.get(arg);
+                fmt_load_global(f, self.name(), oparg, name(oparg >> 1), pad)
+            }
+            Self::LoadGlobalBuiltin | Self::LoadGlobalModule => {
+                let oparg = u32::from(arg);
+                fmt_load_global(f, self.name(), oparg, name(oparg >> 1), pad)
+            }

Where fmt_load_global is a local closure or helper:

let fmt_load_global = |f: &mut fmt::Formatter<'_>, opname: &str, oparg: u32, resolved_name: &str, pad: usize| -> fmt::Result {
    if (oparg & 1) != 0 {
        write!(f, "{:pad$}({}, NULL + {})", opname, oparg, resolved_name)
    } else {
        write!(f, "{:pad$}({}, {})", opname, oparg, resolved_name)
    }
};

As per coding guidelines: "When branches differ only in a value but share common logic, extract the differing value first, then call the common logic once to avoid duplicate code."

🤖 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 1479 - 1508,
The three match arms Self::LoadGlobal, Self::LoadGlobalBuiltin, and
Self::LoadGlobalModule duplicate identical formatting logic; extract the shared
formatting into a small helper or closure (e.g., fmt_load_global) that accepts
opname (from self.name()), oparg (u32 from arg), resolved_name (result of
name(name_idx)) and pad, then call that helper from each arm after computing
name_idx and converting arg as needed; ensure you reuse the existing write!(f,
...) calls inside the helper so each arm only computes its specific inputs and
invokes fmt_load_global.