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

26-30: Consider re-exporting the replacement resume types here too.

ResumeType used to be available from rustpython_compiler_core::bytecode::*. Removing it without adding ResumeContext / ResumeLocation makes downstream code change module paths as well as type names, even though the rest of the oparg surface is still re-exported from this module.

Possible re-export update

-        MakeFunctionFlag, MakeFunctionFlags, NameIdx, OpArg, OpArgByte, OpArgState, OpArgType,
-        RaiseKind, SpecialMethod, UnpackExArgs,
+        MakeFunctionFlag, MakeFunctionFlags, NameIdx, OpArg, OpArgByte, OpArgState, OpArgType,
+        RaiseKind, ResumeContext, ResumeLocation, SpecialMethod, UnpackExArgs,

🤖 Prompt for AI Agents

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

In `@crates/compiler-core/src/bytecode.rs` around lines 26 - 30, The bytecode
re-exports removed ResumeType and now downstream code breaks; update the oparg
re-exports to include the new resume types by adding public re-exports for
ResumeContext and ResumeLocation from the oparg module (and to preserve
compatibility optionally add a pub type alias ResumeType = ResumeContext so
existing code that expects ResumeType continues to compile). Ensure you
reference the symbols ResumeContext, ResumeLocation (and ResumeType if creating
the alias) in the bytecode re-export list alongside the other OpArg items.

crates/codegen/src/compile.rs (2)

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.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

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

799-803: ⚠️ Potential issue | 🟡 Minor

Fix the ResumeContext intra-doc links.

Context::location and Context::is_exception_depth1 do not exist here, so rustdoc will not resolve these links.

📝 Suggested doc fix

-    /// 1. [`Context::location`]: Indicates where the instruction occurs.
-    /// 2. [`Context::is_exception_depth1`]: Is the instruction is at except-depth 1.
+    /// 1. [`ResumeContext::location`]: Indicates where the instruction occurs.
+    /// 2. [`ResumeContext::is_exception_depth1`]: Whether the instruction is at except-depth 1.

🤖 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 799 - 803, The
intra-doc links point to non-existent symbols `Context::location` and
`Context::is_exception_depth1`; update them to reference the actual type/fields
on the resume context (e.g. `ResumeContext::location` and
`ResumeContext::is_exception_depth1`) so rustdoc can resolve the links and keep
the `Instruction::Resume` reference unchanged.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

crates/codegen/src/compile.rs (1)

7599-7602: ⚠️ Potential issue | 🟠 Major

Stop hardcoding AfterYield depth in the generator-expression path.

Line 7601 still passes true here, so this one AfterYield site keeps deciding exception depth in codegen even though the rest of the file now delegates that to IR. It also diverges from the plain yield case at Line 7377.

🔧 Suggested fix

                         emit!(
                             compiler,
                             Instruction::Resume {
-                                context: oparg::ResumeContext::new(
-                                    oparg::ResumeLocation::AfterYield,
-                                    true, // TODO: Is this always true?
-                                )
+                                context: oparg::ResumeContext::from(
+                                    oparg::ResumeLocation::AfterYield,
+                                )
                             }
                         );

🤖 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 7599 - 7602, The
generator-expression path is still hardcoding oparg::ResumeContext::new(...,
oparg::ResumeLocation::AfterYield, true) which forces exception depth decision
in codegen; change it to delegate depth to the IR like the plain yield path does
by removing the literal true and using the IR-provided depth/flag (the same
value used when constructing ResumeContext for the plain yield case) so
ResumeContext::new(...) receives the IR-determined depth/flag instead of true,
and ensure you use oparg::ResumeLocation::AfterYield as before.

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

853-856: Make ResumeContext construction explicit.

ResumeLocation no longer determines the whole oparg once exception depth comes from IR. This From impl hard-codes false, so a stray .into() at a new emission site can silently drop the depth bit. Prefer ResumeContext::new(location, is_exception_depth1) at call sites.

🤖 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 853 - 856, The
blanket impl "impl From<ResumeLocation> for ResumeContext" hard-codes the
exception-depth bit to false and allows stray .into() calls to drop the depth;
remove this From<ResumeLocation> for ResumeContext implementation and update
call sites to construct explicitly via ResumeContext::new(location,
is_exception_depth1) (using the correct boolean for exception depth) so the
depth bit is never implicitly lost; search for any uses of .into() or
ResumeLocation -> ResumeContext conversions and replace them with explicit
ResumeContext::new(...) calls.

crates/codegen/src/compile.rs (1)

7203-7205: Extract the ResumeLocation before building the context.

Lines 7203-7205 only vary by ResumeLocation; ResumeContext::from(...) is the shared part.

♻️ Suggested refactor

+        let resume_location = if is_await {
+            oparg::ResumeLocation::AfterAwait
+        } else {
+            oparg::ResumeLocation::AfterYieldFrom
+        };
+
         emit!(
             self,
             Instruction::Resume {
-                context: if is_await {
-                    oparg::ResumeContext::from(oparg::ResumeLocation::AfterAwait)
-                } else {
-                    oparg::ResumeContext::from(oparg::ResumeLocation::AfterYieldFrom)
-                }
+                context: oparg::ResumeContext::from(resume_location)
             }
         );

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/codegen/src/compile.rs` around lines 7203 - 7205, Extract the
differing ResumeLocation into a local variable (e.g. let resume_loc =
oparg::ResumeLocation::AfterAwait or AfterYieldFrom based on the branch
condition) and then call oparg::ResumeContext::from(resume_loc) once instead of
repeating oparg::ResumeContext::from(...) in each branch; update the branches to
only set the resume_loc and perform the shared oparg::ResumeContext::from call
after the conditional.

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

798-839: ⚠️ Potential issue | 🟠 Major

ResumeContext still accepts reserved RESUME bits as valid.

Because this type comes from newtype_oparg!, it gets From<u32>/from_u32, and Rust's blanket TryFrom<U> for T where U: Into<T> makes OpArgType::try_from effectively infallible here. Bits outside LOCATION_MASK | DEPTH1_MASK are therefore silently preserved and then ignored by location() / is_exception_depth1() instead of rejecting malformed bytecode. ResumeContext needs a hand-written definition or constructor path that drops the generic From<u32> decode path and validates raw values explicitly.

🤖 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 798 - 839, The
ResumeContext type must reject raw u32 values with reserved bits set instead of
relying on the auto-derived From<u32> from newtype_oparg!, so replace the
infallible decode path with a validating constructor/TryFrom that ensures (value
& !(ResumeContext::LOCATION_MASK | ResumeContext::DEPTH1_MASK)) == 0.
Concretely: stop exposing/using the generic From<u32>/from_u32 for decoding
(either remove that impl or shadow it by adding a TryFrom<u32> implementation
that returns Err on reserved bits), add a public const fn (e.g. try_from_raw or
an impl TryFrom<u32> for ResumeContext) that validates the mask and returns
Err/None on invalid values, and update ResumeContext::new (and any callsites) to
construct via the new validated path while keeping From<ResumeContext>->u32 for
encoding if needed; keep location(), is_exception_depth1() as-is since they rely
on validated internal state.

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

706-718: This renames new(u32) for every public oparg newtype, not just Label.

newtype_oparg! is also used by ConstIdx, VarNum, VarNums, LoadAttr, LoadSuperAttr, Label, and ResumeContext, so if compiler-core is consumed outside the workspace this is a much broader source break than the PR description suggests. If that scope is not intentional, I'd keep the rename local to the types that need future typed constructors or plan a transition for downstream callers.

🤖 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 706 - 718, The macro
newtype_oparg! currently renames the public constructor from new(u32) to
from_u32 for all generated types (ConstIdx, VarNum, VarNums, LoadAttr,
LoadSuperAttr, Label, ResumeContext), which is a breaking API change; restore a
pub const fn new(value: u32) -> Self in the macro so all generated newtypes keep
their original public constructor, and if you want a typed alternative for Label
add an extra from_u32 or from_label constructor only where Label is defined
rather than changing the macro output for every type.

crates/codegen/src/compile.rs (1)

7199-7205: Add a regression snapshot for IR-populated RESUME depth.

These emitters now intentionally leave the DEPTH1 bit to ir.rs. A small disassembly test that covers yield, yield from, and await from an exception-depth-1 path would make this cross-file contract much harder to regress.

Also applies to: 7374-7378, 7596-7602

🤖 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 7199 - 7205, Add a small
regression snapshot test that verifies the IR-populated RESUME depth behavior
when the DEPTH1 bit is intentionally left to ir.rs: create a disassembly test
that exercises yield, yield from, and await paths coming from an
exception-depth-1 path and assert the emitted Instruction::Resume uses
oparg::ResumeContext/ResumeLocation as expected (cover the same cases as the
emit! sites that use Instruction::Resume and
oparg::ResumeContext::from(oparg::ResumeLocation::AfterAwait/AfterYieldFrom));
place the test alongside existing compile/disassembly tests so it snapshots the
bytecode/disassembly output to prevent regressions across compile.rs and ir.rs.