GeezerVison

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

Inline comments:
In `@crates/macros/Cargo.toml`:
- Around line 14-17: The syn dependency in Cargo.toml lacks the "full" feature
required by the procedural macro because the code uses syn types Item, ItemEnum,
ItemStruct and Spanned; update the syn dependency entry to enable the full
feature (i.e., change the syn = { workspace = true } line to include features =
["full"] ) so the procedural macro can compile with those types available.

In `@crates/macros/src/newtype_oparg.rs`:
- Around line 128-141: The code currently calls
attr.parse_nested_meta(...).unwrap(), which panics on bad attribute syntax;
change this to propagate a syn::Result error instead: make the closure passed to
attr.parse_nested_meta return syn::Result<()> (using ? for nested parsing so
unknown/invalid attributes produce a syn::Error), and replace the surrounding
.map/.unwrap pattern with a fallible loop or an iterator collected into Result
(e.g., iterate variants, parse attributes with ? and push VariantInfo into
variants_info), so any parse error from parse_nested_meta bubbles up as a
compile-time error rather than panicking; update handling of display and
catch_all inside the closure to use ? for meta.value().parse and return
Err(meta.error(...)) as syn::Error.

---

Nitpick comments:
In `@crates/vm/src/frame.rs`:
- Around line 5467-5469: Compute the bytecode::Invert value once into a local
named invert (e.g., let invert =
bytecode::Invert::try_from(u32::from(arg)).unwrap_or(bytecode::Invert::No));
then replace the duplicate try_from/unwrap_or calls in the branches that handle
inversion with that single variable and apply the shared boolean inversion logic
one time (use a single match or if let on invert to decide whether to flip the
computed boolean value). Update all locations that currently re-parse invert
(the occurrences around the current match/branches handling inversion) to use
this extracted invert variable and a common inversion step so the branches only
differ by the value used, not the inversion logic.

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

Inline comments:
In `@crates/macros/src/newtype_oparg.rs`:
- Around line 171-179: The current use of Vec::pop_if only checks/removes the
last variant so a #[oparg(catch_all)] not at the end remains in variants_info
and later produces invalid match arms; locate the catch_all by searching
variants_info for vinfo.catch_all (e.g., using iter().position or partition),
remove it explicitly (variants_info.remove(idx) or partition into (catch_all,
others)), assign it to catch_all, and then keep the existing duplicate-check
(ensure no other vinfo.catch_all remains) before proceeding; this fixes the
issue where vinfo.discriminant is None for a non-removed catch_all and prevents
generating bad match arms.

---

Nitpick comments:
In `@crates/macros/src/newtype_oparg.rs`:
- Around line 30-33: The generated newtype currently emits a wrapper struct
(`#ident`) without a representation attribute; modify the generator in
newtype_oparg.rs (where `let output = quote! { ... }` is built) to insert
`#[repr(transparent)]` on the struct before the `#[derive(...)]` so the emitted
type has ABI-compatible layout with the inner `u32`; update the quote!
invocation that produces `#(`#attrs`)* #[derive(...)] `#vis` `#struct_token`
`#ident`(u32)#semi_token` to include the `#[repr(transparent)]` attribute (keeping
`#(`#attrs`)*` intact) so the produced wrapper benefits from FFI/ABI safety.

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

Inline comments:
In `@crates/compiler-core/src/bytecode/oparg.rs`:
- Around line 124-133: The ConvertValueOparg enum now treats None = 0 which
breaks deserialization of legacy .pyc bytes where None was stored as 255; update
the enum deserialization to accept the legacy discriminant by adding a catch-all
variant or mapping for 255 so TryFrom<u32> no longer returns InvalidBytecode for
old files: modify the ConvertValueOparg definition (the enum with
#[newtype_oparg]) to include a #[oparg(catch_all)] variant that converts legacy
value 255 to the current None case (or otherwise upgrade it), ensuring
TryFrom<u32> will map legacy discriminants instead of erroring; alternatively,
if you prefer a breaking change, bump FORMAT_VERSION and add version-gated
deserialization in the TryFrom/u32 logic to reject or migrate old bytecode, but
do not leave TryFrom returning InvalidBytecode for 255.

---

Nitpick comments:
In `@crates/compiler-core/src/bytecode/oparg.rs`:
- Around line 119-573: This change touches bytecode oparg encodings (e.g.,
ConvertValueOparg, MakeFunctionFlag, ComparisonOperator, UnpackExArgs), so
perform a clean build sweep to avoid stale artifacts: delete previous build
artifacts for the rustpython crates and any Python .pyc files, then rebuild (so
the new oparg encoding/decoding is recompiled and tested); after cleaning, run
the full bytecode build/test to verify instructions like
Instruction::ConvertValue and Decode/Encode paths pick up the updated enums and
TryFrom/From impls.

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

Inline comments:
In `@crates/macros/src/newtype_oparg.rs`:
- Around line 96-144: In TryFrom<syn::Variant> for VariantInfo, add validation
so when catch_all is true the variant has exactly one unnamed (tuple) field;
inspect variant.fields and if it's not Fields::Unnamed with exactly one field
return a syn::Error using ident.span() with a clear message (e.g.,
"`#[oparg(catch_all)]` must be a single unnamed field like `Variant(T)`"); keep
this check alongside the existing discriminant/display checks so the macro only
accepts catch_all variants of the form Variant(T).