Oparg resume depth by ShaharNaveh · Pull Request #7515 · RustPython/RustPython

ShaharNaveh

1280-1282: Centralize ResumeContext creation.

The extra RESUME bit is now part of the emitted bytecode, but these sites still encode it with bare booleans, and the open TODOs on Line 7384 and Line 7609 show how easy that invariant is to lose. A tiny helper / named constructor would keep the manual OpArg::new(...) path and the yield/await sites aligned.

♻️ Suggested direction

+    const fn resume_context(location: oparg::ResumeLocation) -> oparg::ResumeContext {
+        match location {
+            oparg::ResumeLocation::AtFuncStart => oparg::ResumeContext::new(location, false),
+            oparg::ResumeLocation::AfterYield => oparg::ResumeContext::new(location, true),
+            oparg::ResumeLocation::AfterYieldFrom | oparg::ResumeLocation::AfterAwait => {
+                oparg::ResumeContext::new(location, false)
+            }
+        }
+    }

Then reuse Self::resume_context(...) at the four Instruction::Resume call sites.

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."

Also applies to: 7204-7211, 7382-7385, 7607-7610

🤖 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 1280 - 1282, The code constructs
ResumeContext inline with OpArg::new(...) (e.g., using
oparg::ResumeContext::new(oparg::ResumeLocation::AtFuncStart, false).into()) at
multiple Instruction::Resume emission sites; centralize this by adding a helper
named constructor (e.g., Self::resume_context(location, resume_bit)) that
returns the properly encoded OpArg/ResumeContext and replace the four raw sites
(the Instruction::Resume call locations) to call Self::resume_context(...)
instead of duplicating the inline construction so the RESUME bit handling is
consistent and the TODOs at the referenced locations are resolved.

1280-1282: Pin the new RESUME variants with local disassembly tests.

This PR changes the emitted oparg for scope entry and suspension points. Adding a few display_expand_code_objects() snapshots here for a plain function, direct yield, yield from, await, and a genexpr would make future opcode-arg refactors much safer.

If helpful, I can sketch the minimal snapshot cases.

Also applies to: 7204-7211, 7382-7385, 7607-7610

🤖 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 1280 - 1282, Add local
disassembly snapshot tests that pin the new RESUME variants: for each place you
construct an OpArg::new(oparg::ResumeContext::new(...)) (e.g., the Emit sites
using oparg::ResumeLocation::AtFuncStart), add display_expand_code_objects()
snapshots for a plain function, a direct yield, a yield from, an await, and a
generator-expression so future opcode-arg refactors are caught; repeat the same
snapshot additions for the other ResumeContext construction sites in this module
so all RESUME variants are covered.