Skip to content

Bytecode parity - direct loop backedges #7578

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
youknowone merged 1 commit into from
Apr 10, 2026
+427 −98
Merged

Bytecode parity - direct loop backedges #7578

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 98 additions & 2 deletions crates/codegen/src/compile.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1512,11 +1512,14 @@ impl Compiler {
}

FBlockType::ForLoop => {
// Pop the iterator
// When returning from a for-loop, CPython swaps the preserved
// value with the iterator and uses POP_TOP for the iterator slot.
if preserve_tos {
emit!(self, Instruction::Swap { i: 2 });
emit!(self, Instruction::PopTop);
} else {
emit!(self, Instruction::PopIter);
}
emit!(self, Instruction::PopIter);
}

FBlockType::TryExcept => {
Expand Down Expand Up @@ -11148,6 +11151,99 @@ def f(base, cls, state):
assert_eq!(return_count, 2);
}

#[test]
fn test_loop_store_subscr_threads_direct_backedge() {
let code = compile_exec(
"\
def f(kwonlyargs, kwonlydefaults, arg2value):
missing = 0
for kwarg in kwonlyargs:
if kwarg not in arg2value:
if kwonlydefaults and kwarg in kwonlydefaults:
arg2value[kwarg] = kwonlydefaults[kwarg]
Comment on lines +11158 to +11163
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fix cspell blocker in embedded test source

CI is failing because kwonlydefaults is flagged as an unknown word in Line 11158-Line 11163. This currently blocks merge.

💡 Proposed fix (rename identifier in test fixture) 
 def f(kwonlyargs, kwonlydefaults, arg2value):
+def f(kwonlyargs, kw_only_defaults, arg2value):
     missing = 0
     for kwarg in kwonlyargs:
         if kwarg not in arg2value:
-            if kwonlydefaults and kwarg in kwonlydefaults:
-                arg2value[kwarg] = kwonlydefaults[kwarg]
+            if kw_only_defaults and kwarg in kw_only_defaults:
+                arg2value[kwarg] = kw_only_defaults[kwarg]
             else:
                 missing += 1
🧰 Tools
🪛 GitHub Actions: CI

[error] 11158-11163: cspell hook failed (exit code 1): Unknown word (kwonlydefaults)

🤖 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 11158 - 11163, Rename the
misspelled identifier kwonlydefaults in the test fixture to a cspell-friendly
name (e.g., kw_only_defaults) by updating the parameter name in the function f
signature and every use inside the function body (references to kwonlydefaults
and checks like "if kwonlydefaults and kwarg in kwonlydefaults" should use the
new name); ensure all other test fixtures or assertions that reference this
parameter are updated consistently (function f, parameters kwonlyargs and
arg2value, and any access/assignment to the defaults mapping).

else:
missing += 1
return missing
",
);
let f = find_code(&code, "f").expect("missing function code");
let ops: Vec<_> = f
.instructions
.iter()
.map(|unit| unit.op)
.filter(|op| !matches!(op, Instruction::Cache))
.collect();

let store_subscr = ops
.iter()
.position(|op| matches!(op, Instruction::StoreSubscr))
.expect("missing STORE_SUBSCR");
let next_op = ops
.get(store_subscr + 1)
.expect("missing jump after STORE_SUBSCR");
let window_start = store_subscr.saturating_sub(3);
let window_end = (store_subscr + 5).min(ops.len());
let window = &ops[window_start..window_end];

assert!(
matches!(next_op, Instruction::JumpBackward { .. }),
"expected direct loop backedge after STORE_SUBSCR, got {next_op:?}; ops={window:?}"
);
}

#[test]
fn test_loop_return_reorders_backedge_before_exit_cleanup() {
let code = compile_exec(
"\
def f(obj):
for base in obj.__mro__:
if base is not object:
doc = base.__doc__
if doc is not None:
return doc
",
);
let f = find_code(&code, "f").expect("missing function code");
let ops: Vec<_> = f
.instructions
.iter()
.map(|unit| unit.op)
.filter(|op| !matches!(op, Instruction::Cache))
.collect();

let cond_idx = ops
.iter()
.position(|op| matches!(op, Instruction::PopJumpIfNotNone { .. }))
.expect("missing POP_JUMP_IF_NOT_NONE");
assert!(
matches!(ops.get(cond_idx + 1), Some(Instruction::NotTaken)),
"expected NOT_TAKEN after conditional jump, got {:?}; ops={ops:?}",
ops.get(cond_idx + 1)
);
assert!(
matches!(
ops.get(cond_idx + 2),
Some(Instruction::JumpBackward { .. })
),
"expected loop backedge immediately after NOT_TAKEN, got {:?}; ops={ops:?}",
ops.get(cond_idx + 2)
);

let end_for_idx = ops
.iter()
.position(|op| matches!(op, Instruction::EndFor))
.expect("missing END_FOR");
let return_before_end = ops[..end_for_idx]
.iter()
.rposition(|op| matches!(op, Instruction::ReturnValue))
.expect("missing loop-body RETURN_VALUE");
assert!(
matches!(ops.get(return_before_end - 1), Some(Instruction::PopTop)),
"expected POP_TOP before loop-body RETURN_VALUE, got {:?}; ops={ops:?}",
ops.get(return_before_end.saturating_sub(1))
);
}

#[test]
fn test_assert_without_message_raises_class_directly() {
let code = compile_exec(
Expand Down
Loading
Loading