Skip to content

Commit 87b9fb5

Browse files
committed
Bytecode parity - for-loop cleanup, return duplication, late jump threading fix
- Use POP_TOP instead of POP_ITER for for-loop break/return cleanup - Expand duplicate_end_returns to clone final return for jump predecessors - Restrict late jump threading pass to unconditional jumps only - Skip exception blocks in inline/reorder passes - Simplify threaded_jump_instr NoInterrupt handling
1 parent e372959 commit 87b9fb5

7 files changed

+326
-124
lines changed

crates/codegen/src/compile.rs

Lines changed: 97 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1513,12 +1513,12 @@ impl Compiler {
15131513

15141514
FBlockType::ForLoop => {
15151515
// When returning from a for-loop, CPython swaps the preserved
1516-
// value with the iterator and uses POP_TOP for the iterator slot.
1516+
// value with the iterator and uses POP_TOP for loop cleanup.
15171517
if preserve_tos {
15181518
emit!(self, Instruction::Swap { i: 2 });
15191519
emit!(self, Instruction::PopTop);
15201520
} else {
1521-
emit!(self, Instruction::PopIter);
1521+
emit!(self, Instruction::PopTop);
15221522
}
15231523
}
15241524

@@ -5809,7 +5809,10 @@ impl Compiler {
58095809
emit!(self, Instruction::PopTop); // pop lasti
58105810
emit!(self, Instruction::PopTop); // pop self_exit
58115811
emit!(self, Instruction::PopTop); // pop exit_func
5812-
emit!(self, PseudoInstruction::Jump { delta: after_block });
5812+
emit!(
5813+
self,
5814+
PseudoInstruction::JumpNoInterrupt { delta: after_block }
5815+
);
58135816

58145817
// ===== Cleanup block (for nested exception during __exit__) =====
58155818
// Stack: [..., __exit__, lasti, prev_exc, lasti2, exc2]
@@ -9920,9 +9923,9 @@ impl Compiler {
99209923
}
99219924
}
99229925

9923-
// For break in a for loop, pop the iterator
9926+
// CPython unwinds a for-loop break with POP_TOP rather than POP_ITER.
99249927
if is_break && is_for_loop {
9925-
emit!(self, Instruction::PopIter);
9928+
emit!(self, Instruction::PopTop);
99269929
}
99279930

99289931
// Jump to target
@@ -11155,12 +11158,12 @@ def f(base, cls, state):
1115511158
fn test_loop_store_subscr_threads_direct_backedge() {
1115611159
let code = compile_exec(
1115711160
"\
11158-
def f(kwonlyargs, kwonlydefaults, arg2value):
11161+
def f(kwonlyargs, kw_only_defaults, arg2value):
1115911162
missing = 0
1116011163
for kwarg in kwonlyargs:
1116111164
if kwarg not in arg2value:
11162-
if kwonlydefaults and kwarg in kwonlydefaults:
11163-
arg2value[kwarg] = kwonlydefaults[kwarg]
11165+
if kw_only_defaults and kwarg in kw_only_defaults:
11166+
arg2value[kwarg] = kw_only_defaults[kwarg]
1116411167
else:
1116511168
missing += 1
1116611169
return missing
@@ -11244,6 +11247,91 @@ def f(obj):
1124411247
);
1124511248
}
1124611249

11250+
#[test]
11251+
fn test_shared_final_return_is_cloned_for_jump_target() {
11252+
let code = compile_exec(
11253+
"\
11254+
def f(node):
11255+
if not isinstance(
11256+
node, (AsyncFunctionDef, FunctionDef, ClassDef, Module)
11257+
) or len(node.body) < 1:
11258+
return None
11259+
node = node.body[0]
11260+
if not isinstance(node, Expr):
11261+
return None
11262+
node = node.value
11263+
if isinstance(node, Constant) and isinstance(node.value, str):
11264+
return node
11265+
",
11266+
);
11267+
let f = find_code(&code, "f").expect("missing function code");
11268+
let ops: Vec<_> = f
11269+
.instructions
11270+
.iter()
11271+
.map(|unit| unit.op)
11272+
.filter(|op| !matches!(op, Instruction::Cache))
11273+
.collect();
11274+
11275+
let return_count = ops
11276+
.iter()
11277+
.filter(|op| matches!(op, Instruction::ReturnValue))
11278+
.count();
11279+
assert!(
11280+
return_count >= 3,
11281+
"expected multiple explicit return sites for shared final return case, got ops={ops:?}"
11282+
);
11283+
}
11284+
11285+
#[test]
11286+
fn test_for_break_uses_poptop_cleanup() {
11287+
let code = compile_exec(
11288+
"\
11289+
def f(parts):
11290+
for value in parts:
11291+
if value:
11292+
break
11293+
",
11294+
);
11295+
let f = find_code(&code, "f").expect("missing function code");
11296+
let ops: Vec<_> = f
11297+
.instructions
11298+
.iter()
11299+
.map(|unit| unit.op)
11300+
.filter(|op| !matches!(op, Instruction::Cache))
11301+
.collect();
11302+
11303+
let pop_iter_count = ops
11304+
.iter()
11305+
.filter(|op| matches!(op, Instruction::PopIter))
11306+
.count();
11307+
assert_eq!(
11308+
pop_iter_count, 1,
11309+
"expected only the loop-exhaustion POP_ITER, got ops={ops:?}"
11310+
);
11311+
11312+
let break_cleanup_idx = ops
11313+
.windows(3)
11314+
.position(|window| {
11315+
matches!(
11316+
window,
11317+
[
11318+
Instruction::PopTop,
11319+
Instruction::LoadConst { .. },
11320+
Instruction::ReturnValue
11321+
]
11322+
)
11323+
})
11324+
.expect("missing POP_TOP/LOAD_CONST/RETURN_VALUE break cleanup");
11325+
let end_for_idx = ops
11326+
.iter()
11327+
.position(|op| matches!(op, Instruction::EndFor))
11328+
.expect("missing END_FOR");
11329+
assert!(
11330+
break_cleanup_idx < end_for_idx,
11331+
"expected break cleanup before END_FOR, got ops={ops:?}"
11332+
);
11333+
}
11334+
1124711335
#[test]
1124811336
fn test_assert_without_message_raises_class_directly() {
1124911337
let code = compile_exec(
@@ -11335,7 +11423,7 @@ def f(names, cls):
1133511423
.filter(|unit| matches!(unit.op, Instruction::ReturnValue))
1133611424
.count();
1133711425

11338-
assert_eq!(return_count, 2);
11426+
assert_eq!(return_count, 1);
1133911427
}
1134011428

1134111429
#[test]

0 commit comments

Comments
 (0)