Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.pre-commit-config.yaml:
- Around line 46-47: The hook `files` regex currently uses
`tools/opcode_metadata/*` which doesn't match files like
`tools/opcode_metadata/opcodes.py`; update both occurrences of the `files`
pattern in the pre-commit config (the lines with `files:` that currently contain
`tools/opcode_metadata/*`) to use `tools/opcode_metadata/.*` so the
opcode-metadata hooks trigger on changes under that directory (keep
`pass_filenames: false` as-is).
In `@crates/compiler-core/src/bytecode/instruction.rs`:
- Around line 213-233: The macro arm that detects label fields only matches bare
"Label" but opcode variants use "oparg::Label", so add explicit macro arms to
match oparg::Label (mirror the existing Label arms) — e.g. add (`@match`
$self:expr, $name:ident, [$variant:ident { $field:ident : oparg::Label } ,
$($rest:tt)*]) and the final-arm variant (`@match` $self:expr, $name:ident,
[$variant:ident { $field:ident : oparg::Label }]) with the same bodies that
return Some(*$field); this ensures define_opcodes! macro recognizes oparg::Label
fields and preserves CodeObject::label_targets().
In `@tools/opcode_metadata/cpython.py`:
- Around line 17-18: The module currently imports SKIP_PROPERTIES, Analysis,
Family, Properties, analyze_files and get_stack_effect only for downstream
re-export but lint flags them as unused; make this explicit by adding an __all__
=
("SKIP_PROPERTIES","Analysis","Family","Properties","analyze_files","get_stack_effect")
at top (or alternatively append targeted "# noqa: F401" to those import lines)
so the re-exports are intentional and ruff/CI no longer reports F401. Ensure the
symbol names match the imported identifiers exactly (SKIP_PROPERTIES, Analysis,
Family, Properties, analyze_files, get_stack_effect).
In `@tools/opcode_metadata/generate_py_opcode_metadata.py`:
- Around line 12-15: Remove the unused imports that trigger F401 by deleting
"typing" from the top-level imports and removing "to_pascal_case" from the
from-utils import list in generate_py_opcode_metadata.py; update the import line
to only keep used symbols (OpcodeInfo, DEFAULT_INPUT, ROOT, get_conf,
to_upper_snake_case) so the module passes ruff/flake8 linting.
- Around line 84-85: Replace the lambda assigned to key with a small local def
to satisfy E731: create a function (e.g., def sort_key(op): return
(op.cpython_name not in PRIORITY_OPMAP, op.id)) and pass sort_key into
sorted(opcodes, key=sort_key) instead of using key = lambda ...; keep references
to PRIORITY_OPMAP and op.id/op.cpython_name unchanged.
In `@tools/opcode_metadata/generate_rs_opcode_metadata.py`:
- Line 23: The name OpcodeDef used as a type annotation is undefined; either
import the correct type that represents entries returned by
OpcodeInfo.iter_infos (e.g., add an import for OpcodeDef from the module that
defines it) or change the annotation to the actual type produced by
OpcodeInfo.iter_infos (for example Iterator[OpcodeInfo] or list[OpcodeInfo]) and
add the necessary typing imports (from typing import Iterator, List) so
ruff/F821 stops complaining; update the annotation on the variable/parameter
named info (or wherever OpcodeDef is used) accordingly.
- Around line 4-16: Remove the unused imports in the top import block: delete
"collections", "os", "sys", "typing", "tomllib" (if not used elsewhere), and the
unused names from the cpython from-import ("Analysis", "get_analysis",
"get_stack_effect"), and remove "to_pascal_case" from the utils import; keep
only the actually used symbols (e.g., dataclasses, io, pathlib, subprocess,
OpcodeInfo, DEFAULT_INPUT, ROOT, get_conf). Update the import lines in
generate_rs_opcode_metadata.py accordingly and run ruff to verify no F401s
remain.
In `@tools/opcode_metadata/opcodes.py`:
- Around line 35-37: The comparison that skips the family root uses raw
member.name against the normalized family_name, causing mismatches; update the
filter to compare the normalized member_name (the result of
to_pascal_case(member.name)) to family_name (or normalize family_name with
to_pascal_case) so the family opcode is correctly excluded—adjust the logic
around member_name, member.name, family_name and the call to to_pascal_case in
opcodes.py.
In `@tools/opcode_metadata/utils.py`:
- Line 25: Replace the PEP 695 type-alias statement so it parses on Python 3.11:
change the declaration "type OverrideConfs = dict[str, Override]" to an
assignment-style alias "OverrideConfs = dict[str, Override]" (keep the same name
OverrideConfs and the referenced Override type), ensuring any necessary imports
for Override remain unchanged.
---
Nitpick comments:
In `@tools/opcode_metadata/cpython.py`:
- Around line 5-8: The except KeyError block that raises ValueError for missing
CPYTHON_ROOT should chain the original KeyError so the original context is
preserved; modify the except clause to capture the exception (e.g., except
KeyError as err) and re-raise the ValueError using "raise ValueError(\"Missing
environment variable 'CPYTHON_ROOT'\") from err" so the CPYTHON_ROOT resolution
code (pathlib.Path(os.environ["CPYTHON_ROOT"]).expanduser().resolve()) preserves
the original error context for lint B904.
In `@tools/opcode_metadata/opcodes.py`:
- Around line 95-98: The warnings.warn call that emits "Could not get
instruction metadata for {rust_name} from CPython or override conf" should
include a stacklevel so the warning points at the caller, not this helper;
update the warnings.warn call to pass stacklevel=2 (e.g., warnings.warn(...,
stacklevel=2)) referencing the existing rust_name variable and the warnings.warn
invocation so linting (ruff) and caller attribution are correct.