From 502357064fcb346d34270c16096d3952d417805d Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Wed, 22 Apr 2026 22:06:05 +0900 Subject: [PATCH 1/7] typealias reviews --- crates/codegen/src/compile.rs | 117 ++++++++++++++++++++++++++++-- crates/codegen/src/symboltable.rs | 2 +- 2 files changed, 111 insertions(+), 8 deletions(-) diff --git a/crates/codegen/src/compile.rs b/crates/codegen/src/compile.rs index e990771e249..2d181d23068 100644 --- a/crates/codegen/src/compile.rs +++ b/crates/codegen/src/compile.rs @@ -2226,11 +2226,15 @@ impl Compiler { let is_function_like = self.ctx.in_func(); // Look up the symbol, handling ast::TypeParams and Annotation scopes specially - let (symbol_scope, can_see_class_scope) = { - let current_table = self.current_symbol_table(); + let (symbol_scope, can_see_class_scope, class_declared_global) = { + let current_idx = self.symbol_table_stack.len() - 1; + let current_table = &self.symbol_table_stack[current_idx]; let is_typeparams = current_table.typ == CompilerScope::TypeParams; let is_annotation = current_table.typ == CompilerScope::Annotation; let can_see_class = current_table.can_see_class_scope; + let parent_table = current_idx + .checked_sub(1) + .and_then(|idx| self.symbol_table_stack.get(idx)); // First try to find in current table let symbol = current_table.lookup(name.as_ref()); @@ -2244,8 +2248,17 @@ impl Compiler { } else { symbol }; + let class_declared_global = can_see_class + && parent_table.is_some_and(|table| table.typ == CompilerScope::Class) + && parent_table + .and_then(|table| table.lookup(name.as_ref())) + .is_some_and(|symbol| symbol.flags.contains(SymbolFlags::GLOBAL)); - (symbol.map(|s| s.scope), can_see_class) + ( + symbol.map(|s| s.scope), + can_see_class, + class_declared_global, + ) }; // Special handling for class scope implicit cell variables @@ -2318,7 +2331,9 @@ impl Compiler { SymbolScope::GlobalImplicit => { // PEP 649: In annotation scope with class visibility, use DictOrGlobals // to check classdict first before globals - if can_see_class_scope { + if class_declared_global { + NameOp::Global + } else if can_see_class_scope { NameOp::DictOrGlobals } else if is_function_like { NameOp::Global @@ -2327,7 +2342,10 @@ impl Compiler { } } SymbolScope::GlobalExplicit => { - if can_see_class_scope { + // A global declared in the owning class body must bypass the + // classdict, but an explicit global inherited from an outer + // function still participates in DictOrGlobals lookup. + if can_see_class_scope && !class_declared_global { NameOp::DictOrGlobals } else { NameOp::Global @@ -3011,7 +3029,7 @@ impl Compiler { self.current_code_info() .metadata .varnames - .insert("format".to_owned()); + .insert(".format".to_owned()); self.emit_format_validation()?; @@ -4586,7 +4604,7 @@ impl Compiler { self.current_code_info() .metadata .varnames - .insert(".format".to_owned()); + .insert("format".to_owned()); // Emit format validation: if format > VALUE_WITH_FAKE_GLOBALS: raise NotImplementedError self.emit_format_validation()?; @@ -14777,6 +14795,91 @@ values = [-1, not True, ~0, +True, 5] ); } + #[test] + fn test_annotation_closure_uses_format_varname() { + let code = compile_exec( + "\ +class C: + x: int +", + ); + let annotate = find_code(&code, "__annotate__").expect("missing __annotate__ code"); + let varnames = annotate + .varnames + .iter() + .map(|name| name.as_str()) + .collect::>(); + assert_eq!(varnames, vec!["format"]); + } + + #[test] + fn test_type_param_evaluator_uses_dot_format_varname() { + let code = compile_exec( + "\ +class C[T: int]: + pass +", + ); + let evaluator = find_code(&code, "T").expect("missing type parameter evaluator"); + let varnames = evaluator + .varnames + .iter() + .map(|name| name.as_str()) + .collect::>(); + assert_eq!(varnames, vec![".format"]); + } + + #[test] + fn test_class_annotation_global_resolution_matches_cpython() { + let class_global = compile_exec( + "\ +X = 'global' +class C: + locals()['X'] = 'class' + global X + y: X +", + ); + let annotate = + find_code(&class_global, "__annotate__").expect("missing class __annotate__ code"); + assert!( + annotate + .instructions + .iter() + .any(|unit| matches!(unit.op, Instruction::LoadGlobal { .. })), + "expected explicit class global to use LOAD_GLOBAL, got instructions={:?}", + annotate.instructions + ); + assert!( + !annotate + .instructions + .iter() + .any(|unit| matches!(unit.op, Instruction::LoadFromDictOrGlobals { .. })), + "did not expect class explicit global to use LOAD_FROM_DICT_OR_GLOBALS, got instructions={:?}", + annotate.instructions + ); + + let outer_global = compile_exec( + "\ +def f(): + global X + class C: + locals()['X'] = 'class' + y: X +", + ); + let annotate = find_code(&outer_global, "__annotate__") + .expect("missing nested class __annotate__ code"); + assert!( + annotate + .instructions + .iter() + .any(|unit| matches!(unit.op, Instruction::LoadFromDictOrGlobals { .. })), + "expected outer explicit global in class annotation to use LOAD_FROM_DICT_OR_GLOBALS, got instructions={:?}", + annotate.instructions + ); + } + #[test] fn test_constant_tuple_binops_fold_like_cpython() { let code = compile_exec("value = (1,) * 17 + ('spam',)\n"); diff --git a/crates/codegen/src/symboltable.rs b/crates/codegen/src/symboltable.rs index 19f5a0ca151..5cc24b4d4ce 100644 --- a/crates/codegen/src/symboltable.rs +++ b/crates/codegen/src/symboltable.rs @@ -1768,7 +1768,7 @@ impl SymbolTableBuilder { }) => { let Some(name_expr) = name.as_name_expr() else { return Err(SymbolTableError { - error: "type alias expect name".to_owned(), + error: "type alias expects name".to_owned(), location: Some( self.source_file .to_source_code() From 701b081f4cef6562e4e27fc29d251d9d7ee60cef Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Wed, 22 Apr 2026 23:18:26 +0900 Subject: [PATCH 2/7] Bytecode parity - try/except block order, CFG reorder Reorder try/except/else/finally to emit else+finally before except handlers matching CPython layout. Add set_no_location for cleanup blocks. Extend CFG reorder pass to handle true-path jump-back for generators, break/continue, and assert in loops. Add stop-iteration error handler awareness to block protection. --- crates/codegen/src/compile.rs | 363 ++++++++++++++++++++++++++-------- crates/codegen/src/ir.rs | 76 ++++++- 2 files changed, 354 insertions(+), 85 deletions(-) diff --git a/crates/codegen/src/compile.rs b/crates/codegen/src/compile.rs index 2d181d23068..9ecc54c965e 100644 --- a/crates/codegen/src/compile.rs +++ b/crates/codegen/src/compile.rs @@ -3376,15 +3376,43 @@ impl Compiler { emit!(self, PseudoInstruction::Jump { delta: else_block }); self.set_no_location(); + let cleanup_block = self.new_block(); + // We successfully ran the try block: + // else: + self.switch_to_block(else_block); + self.compile_statements(orelse)?; + + // Pop the FinallyTry fblock before jumping to finally + if !finalbody.is_empty() { + emit!(self, PseudoInstruction::PopBlock); + self.pop_fblock(FBlockType::FinallyTry); + } + + // Snapshot sub_tables before first finally compilation (for double compilation issue) + let sub_table_cursor = if !finalbody.is_empty() && finally_except_block.is_some() { + self.symbol_table_stack.last().map(|t| t.next_sub_table) + } else { + None + }; + + // finally (normal path): + self.switch_to_block(finally_block); + if !finalbody.is_empty() { + self.compile_statements(finalbody)?; + // Jump to end_block to skip exception path blocks + // This prevents fall-through to finally_except_block + emit!(self, PseudoInstruction::Jump { delta: end_block }); + } + // except handlers: self.switch_to_block(handler_block); + self.set_no_location(); // SETUP_CLEANUP(cleanup) for except block // This handles exceptions during exception matching // Exception table: L2 to L3 -> L5 [1] lasti // After PUSH_EXC_INFO, stack is [prev_exc, exc] // depth=1 means keep prev_exc on stack when routing to cleanup - let cleanup_block = self.new_block(); emit!( self, PseudoInstruction::SetupCleanup { @@ -3395,6 +3423,7 @@ impl Compiler { // Exception is on top of stack now, pushed by unwind_blocks // PUSH_EXC_INFO transforms [exc] -> [prev_exc, exc] for PopExcept + self.set_no_location(); emit!(self, Instruction::PushExcInfo); for handler in handlers { let ast::ExceptHandler::ExceptHandler(ast::ExceptHandlerExceptHandler { @@ -3407,39 +3436,26 @@ impl Compiler { self.set_source_range(*handler_range); let next_handler = self.new_block(); - // If we gave a typ, - // check if this handler can handle the exception: if let Some(exc_type) = type_ { - // Check exception type: - // Stack: [prev_exc, exc] self.compile_expression(exc_type)?; - // Stack: [prev_exc, exc, type] emit!(self, Instruction::CheckExcMatch); - // Stack: [prev_exc, exc, bool] emit!( self, Instruction::PopJumpIfFalse { delta: next_handler } ); - // Stack: [prev_exc, exc] - // We have a match, store in name (except x as y) if let Some(alias) = name { self.store_name(alias.as_str())? } else { - // Drop exception from top of stack: emit!(self, Instruction::PopTop); } } else { - // Catch all! - // Drop exception from top of stack: emit!(self, Instruction::PopTop); } - // If name is bound, we need a cleanup handler for RERAISE let handler_cleanup_block = if name.is_some() { - // SETUP_CLEANUP(cleanup_end) for named handler let cleanup_end = self.new_block(); emit!(self, PseudoInstruction::SetupCleanup { delta: cleanup_end }); self.push_fblock_full( @@ -3450,24 +3466,17 @@ impl Compiler { )?; Some(cleanup_end) } else { - // no SETUP_CLEANUP for unnamed handler self.push_fblock(FBlockType::HandlerCleanup, finally_block, finally_block)?; None }; - // Handler code: self.compile_statements(body)?; self.pop_fblock(FBlockType::HandlerCleanup); - // PopBlock for inner SETUP_CLEANUP (named handler only) if handler_cleanup_block.is_some() { emit!(self, PseudoInstruction::PopBlock); } - // cleanup_end block for named handler - // IMPORTANT: In CPython, cleanup_end is within outer SETUP_CLEANUP scope. - // so when RERAISE is executed, it goes to the cleanup block which does POP_EXCEPT. - // We MUST compile cleanup_end BEFORE popping ExceptionHandler so RERAISE routes to cleanup_block. if let Some(cleanup_end) = handler_cleanup_block { let handler_normal_exit = self.new_block(); emit!( @@ -3479,43 +3488,28 @@ impl Compiler { self.switch_to_block(cleanup_end); if let Some(alias) = name { - // name = None; del name; before RERAISE self.emit_load_const(ConstantData::None); self.store_name(alias.as_str())?; self.compile_name(alias.as_str(), NameUsage::Delete)?; } - // RERAISE 1 (with lasti) - exception is on stack from exception table routing - // Stack at entry: [prev_exc (at handler_depth), lasti, exc] - // This RERAISE is within ExceptionHandler scope, so it routes to cleanup_block - // which does COPY 3; POP_EXCEPT; RERAISE emit!(self, Instruction::Reraise { depth: 1 }); - - // Switch to normal exit block - this is where handler body success continues self.switch_to_block(handler_normal_exit); } - // PopBlock for outer SETUP_CLEANUP (ExceptionHandler) emit!(self, PseudoInstruction::PopBlock); - // Now pop ExceptionHandler - the normal path continues from here self.pop_fblock(FBlockType::ExceptionHandler); emit!(self, Instruction::PopExcept); - // Delete the exception variable if it was bound (normal path) if let Some(alias) = name { - // Set the variable to None before deleting self.emit_load_const(ConstantData::None); self.store_name(alias.as_str())?; self.compile_name(alias.as_str(), NameUsage::Delete)?; } - // Pop FinallyTry block before jumping to finally body. - // The else_block path also pops this; both paths must agree - // on the except stack when entering finally_block. if !finalbody.is_empty() { emit!(self, PseudoInstruction::PopBlock); } - // Jump to finally block emit!( self, PseudoInstruction::JumpNoInterrupt { @@ -3523,62 +3517,22 @@ impl Compiler { } ); - // Re-push ExceptionHandler for next handler in the loop - // This will be popped at the end of handlers loop or when matched self.push_fblock(FBlockType::ExceptionHandler, cleanup_block, cleanup_block)?; - - // Emit a new label for the next handler self.switch_to_block(next_handler); } - // If code flows here, we have an unhandled exception, - // raise the exception again! - // RERAISE 0 - // Stack: [prev_exc, exc] - exception is on stack from PUSH_EXC_INFO - // NOTE: We emit RERAISE 0 BEFORE popping fblock so it is within cleanup handler scope + self.set_no_location(); emit!(self, Instruction::Reraise { depth: 0 }); - - // Pop EXCEPTION_HANDLER fblock - // Pop after RERAISE so the instruction has the correct exception handler self.pop_fblock(FBlockType::ExceptionHandler); - // cleanup block (POP_EXCEPT_AND_RERAISE) - // Stack at entry: [prev_exc, lasti, exc] (depth=1 + lasti + exc pushed) - // COPY 3: copy prev_exc to top -> [prev_exc, lasti, exc, prev_exc] - // POP_EXCEPT: pop prev_exc from stack and restore -> [prev_exc, lasti, exc] - // RERAISE 1: reraise with lasti self.switch_to_block(cleanup_block); + self.set_no_location(); emit!(self, Instruction::Copy { i: 3 }); + self.set_no_location(); emit!(self, Instruction::PopExcept); + self.set_no_location(); emit!(self, Instruction::Reraise { depth: 1 }); - // We successfully ran the try block: - // else: - self.switch_to_block(else_block); - self.compile_statements(orelse)?; - - // Pop the FinallyTry fblock before jumping to finally - if !finalbody.is_empty() { - emit!(self, PseudoInstruction::PopBlock); - self.pop_fblock(FBlockType::FinallyTry); - } - - // Snapshot sub_tables before first finally compilation (for double compilation issue) - let sub_table_cursor = if !finalbody.is_empty() && finally_except_block.is_some() { - self.symbol_table_stack.last().map(|t| t.next_sub_table) - } else { - None - }; - - // finally (normal path): - self.switch_to_block(finally_block); - if !finalbody.is_empty() { - self.compile_statements(finalbody)?; - // Jump to end_block to skip exception path blocks - // This prevents fall-through to finally_except_block - emit!(self, PseudoInstruction::Jump { delta: end_block }); - } - // finally (exception path) // This is where exceptions go to run finally before reraise // Stack at entry: [lasti, exc] (from exception table with preserve_lasti=true) @@ -3595,8 +3549,10 @@ impl Compiler { // SETUP_CLEANUP for finally body // Exceptions during finally body need to go to cleanup block if let Some(cleanup) = finally_cleanup_block { + self.set_no_location(); emit!(self, PseudoInstruction::SetupCleanup { delta: cleanup }); } + self.set_no_location(); emit!(self, Instruction::PushExcInfo); if let Some(cleanup) = finally_cleanup_block { self.push_fblock(FBlockType::FinallyEnd, cleanup, cleanup)?; @@ -13268,6 +13224,82 @@ def f(it): ); } + #[test] + fn test_generator_filter_keeps_cpython_style_forward_yield_body_entry() { + let code = compile_exec( + "\ +def gen(it): + for f in it: + if f.name: + yield f.name +", + ); + let gen_code = find_code(&code, "gen").expect("missing gen code"); + let ops: Vec<_> = gen_code + .instructions + .iter() + .map(|unit| unit.op) + .filter(|op| !matches!(op, Instruction::Cache)) + .collect(); + + assert!( + ops.windows(7).any(|window| { + matches!( + window, + [ + Instruction::ToBool, + Instruction::PopJumpIfTrue { .. }, + Instruction::NotTaken, + Instruction::JumpBackward { .. } + | Instruction::JumpBackwardNoInterrupt { .. }, + Instruction::LoadFastBorrow { .. } | Instruction::LoadFast { .. }, + Instruction::LoadAttr { .. }, + Instruction::YieldValue { .. }, + ] + ) + }), + "expected CPython-style generator filter to jump on true into the yield body and fall through into the loop backedge on false, got ops={ops:?}" + ); + } + + #[test] + fn test_generator_negated_filter_keeps_cpython_style_false_edge_into_yield_body() { + let code = compile_exec( + "\ +def gen(fields): + for f in fields: + if f.init and not f.kw_only: + yield f +", + ); + let gen_code = find_code(&code, "gen").expect("missing gen code"); + let ops: Vec<_> = gen_code + .instructions + .iter() + .map(|unit| unit.op) + .filter(|op| !matches!(op, Instruction::Cache)) + .collect(); + + assert!( + ops.windows(7).any(|window| { + matches!( + window, + [ + Instruction::ToBool, + Instruction::PopJumpIfFalse { .. }, + Instruction::NotTaken, + Instruction::JumpBackward { .. } + | Instruction::JumpBackwardNoInterrupt { .. }, + Instruction::LoadFastBorrow { .. } | Instruction::LoadFast { .. }, + Instruction::YieldValue { .. }, + Instruction::Resume { .. }, + ] + ) + }), + "expected CPython-style negated generator filter to jump on false into the yield body and fall through into the loop backedge on true, got ops={ops:?}" + ); + } + #[test] fn test_multi_with_header_uses_store_fast_load_fast() { let code = compile_exec( @@ -13576,6 +13608,61 @@ def f(filters, text, category, module, lineno, defaultaction, _wm): } } + #[test] + fn test_try_except_else_with_finally_keeps_with_handler_before_outer_except() { + let code = compile_exec( + "\ +def jumpy(i): + for i in range(10): + print(i) + if i < 4: + continue + if i > 6: + break + else: + print('I can haz else clause?') + while i: + print(i) + i -= 1 + if i > 6: + continue + if i < 4: + break + else: + print('Who let lolcatz into this test suite?') + try: + 1 / 0 + except ZeroDivisionError: + print('Here we go, here we go, here we go...') + else: + with i as dodgy: + print('Never reach this') + finally: + print('OK, now we\\'re done') +", + ); + let jumpy = find_code(&code, "jumpy").expect("missing jumpy code"); + let ops: Vec<_> = jumpy + .instructions + .iter() + .map(|unit| unit.op) + .filter(|op| !matches!(op, Instruction::Cache)) + .collect(); + + let with_except_idx = ops + .iter() + .position(|op| matches!(op, Instruction::WithExceptStart)) + .expect("missing WITH_EXCEPT_START"); + let check_exc_idx = ops + .iter() + .position(|op| matches!(op, Instruction::CheckExcMatch)) + .expect("missing CHECK_EXC_MATCH"); + assert!( + with_except_idx < check_exc_idx, + "expected with-except cleanup to be emitted before outer except matching like CPython, got ops={ops:?}", + ); + } + #[test] fn test_nested_boolop_same_or_prefixes_compile_without_extra_boolop_block() { let code = compile_exec( @@ -15795,6 +15882,122 @@ def while_not_chained(a, b, c): ); } + #[test] + fn test_while_break_else_keeps_true_edge_into_forward_break_body() { + let code = compile_exec( + "\ +def f(i): + while i: + i -= 1 + if i < 4: + break + else: + print('x') + print('y') +", + ); + let f = find_code(&code, "f").expect("missing f code"); + let ops: Vec<_> = f + .instructions + .iter() + .map(|unit| unit.op) + .filter(|op| !matches!(op, Instruction::Cache)) + .collect(); + + assert!( + ops.windows(4).any(|window| { + matches!( + window, + [ + Instruction::PopJumpIfTrue { .. }, + Instruction::NotTaken, + Instruction::JumpBackward { .. } + | Instruction::JumpBackwardNoInterrupt { .. }, + Instruction::JumpForward { .. }, + ] + ) + }), + "expected CPython-style true edge into forward break body with false path falling into the loop backedge, got ops={ops:?}" + ); + } + + #[test] + fn test_nested_if_continue_reorders_false_path_to_loop_backedge() { + let code = compile_exec( + "\ +def f(items, changes): + for x in items: + if not x: + if x in changes: + raise TypeError + continue +", + ); + let f = find_code(&code, "f").expect("missing f code"); + let ops: Vec<_> = f + .instructions + .iter() + .map(|unit| unit.op) + .filter(|op| !matches!(op, Instruction::Cache)) + .collect(); + + assert!( + ops.windows(7).any(|window| { + matches!( + window, + [ + Instruction::ToBool, + Instruction::PopJumpIfFalse { .. }, + Instruction::NotTaken, + Instruction::JumpBackward { .. } + | Instruction::JumpBackwardNoInterrupt { .. }, + Instruction::LoadFastBorrowLoadFastBorrow { .. } + | Instruction::LoadFastLoadFast { .. }, + Instruction::ContainsOp { .. }, + Instruction::PopJumpIfFalse { .. }, + ] + ) + }), + "expected nested if/continue to keep CPython-style false-edge jump-back tails, got ops={ops:?}" + ); + } + + #[test] + fn test_loop_assert_keeps_false_edge_into_raise_body() { + let code = compile_exec( + "\ +def f(bytecode): + for instr, positions in zip(bytecode, bytecode.codeobj.co_positions()): + assert instr.positions == positions +", + ); + let f = find_code(&code, "f").expect("missing f code"); + let ops: Vec<_> = f + .instructions + .iter() + .map(|unit| unit.op) + .filter(|op| !matches!(op, Instruction::Cache)) + .collect(); + + assert!( + ops.windows(6).any(|window| { + matches!( + window, + [ + Instruction::CompareOp { .. }, + Instruction::PopJumpIfFalse { .. }, + Instruction::NotTaken, + Instruction::JumpBackward { .. } + | Instruction::JumpBackwardNoInterrupt { .. }, + Instruction::LoadCommonConstant { .. }, + Instruction::RaiseVarargs { .. }, + ] + ) + }), + "expected loop assert to keep CPython-style false-edge into the raise body, got ops={ops:?}" + ); + } + #[test] fn test_and_is_not_none_loop_guard_uses_direct_jump_back_false_path() { let code = compile_exec( diff --git a/crates/codegen/src/ir.rs b/crates/codegen/src/ir.rs index dc455c7d1a2..caeff49377d 100644 --- a/crates/codegen/src/ir.rs +++ b/crates/codegen/src/ir.rs @@ -5874,6 +5874,39 @@ fn block_contains_suspension_point(block: &Block) -> bool { }) } +fn is_stop_iteration_error_handler_block(block: &Block) -> bool { + matches!( + block.instructions.as_slice(), + [ + InstructionInfo { + instr: AnyInstruction::Real(Instruction::CallIntrinsic1 { func }), + arg, + .. + }, + InstructionInfo { + instr: AnyInstruction::Real(Instruction::Reraise { .. }), + .. + } + ] if matches!(func.get(*arg), oparg::IntrinsicFunction1::StopIterationError) + ) +} + +fn block_has_only_stop_iteration_error_handlers(block: &Block, blocks: &[Block]) -> bool { + let mut saw_handler = false; + for info in &block.instructions { + let Some(handler) = info.except_handler else { + continue; + }; + saw_handler = true; + let target = next_nonempty_block(blocks, handler.handler_block); + if target == BlockIdx::NULL || !is_stop_iteration_error_handler_block(&blocks[target.idx()]) + { + return false; + } + } + saw_handler +} + fn block_is_exceptional(block: &Block) -> bool { block.except_handler || block.preserve_lasti || is_exception_cleanup_block(block) } @@ -5976,6 +6009,13 @@ fn reorder_conditional_exit_and_jump_blocks(blocks: &mut [Block]) { current = next; continue; } + if !matches!( + blocks[jump_block.idx()].instructions[0].instr.real(), + Some(Instruction::JumpForward { .. }) + ) { + current = next; + continue; + } let after_jump = blocks[jump_end.idx()].next; blocks[idx].next = jump_start; @@ -6118,10 +6158,6 @@ fn reorder_conditional_chain_and_jump_back_blocks(blocks: &mut Vec) { current = next; continue; }; - if !is_false_path_conditional_jump(&last.instr) { - current = next; - continue; - } let chain_start = next; let jump_start = last.target; @@ -6132,6 +6168,34 @@ fn reorder_conditional_chain_and_jump_back_blocks(blocks: &mut Vec) { current = next; continue; } + let mut chain_has_suspension_point = false; + let mut scan = chain_start; + while scan != BlockIdx::NULL && scan != jump_start { + if block_contains_suspension_point(&blocks[scan.idx()]) { + chain_has_suspension_point = true; + break; + } + scan = blocks[scan.idx()].next; + } + let chain_starts_with_false_path_jump = trailing_conditional_jump_index( + &blocks[chain_start.idx()], + ) + .is_some_and(|chain_cond_idx| { + is_false_path_conditional_jump( + &blocks[chain_start.idx()].instructions[chain_cond_idx].instr, + ) + }); + let chain_is_single_exit_block = is_scope_exit_block(&blocks[chain_start.idx()]) + && next_nonempty_block(blocks, blocks[chain_start.idx()].next) == jump_start; + let allow_true_path_jump_back_reorder = + matches!(last.instr.real(), Some(Instruction::PopJumpIfTrue { .. })) + && (chain_has_suspension_point + || chain_starts_with_false_path_jump + || chain_is_single_exit_block); + if !is_false_path_conditional_jump(&last.instr) && !allow_true_path_jump_back_reorder { + current = next; + continue; + } if block_is_protected(&blocks[idx]) && block_contains_suspension_point(&blocks[idx]) { current = next; continue; @@ -6162,8 +6226,9 @@ fn reorder_conditional_chain_and_jump_back_blocks(blocks: &mut Vec) { let mut chain_valid = true; while cursor != BlockIdx::NULL && cursor != jump_start { if block_is_exceptional(&blocks[cursor.idx()]) - || block_is_protected(&blocks[cursor.idx()]) + || (block_is_protected(&blocks[cursor.idx()]) && block_contains_suspension_point(&blocks[cursor.idx()]) + && !block_has_only_stop_iteration_error_handlers(&blocks[cursor.idx()], blocks)) { chain_valid = false; break; @@ -6220,6 +6285,7 @@ fn reorder_conditional_chain_and_jump_back_blocks(blocks: &mut Vec) { let after_jump = next_nonempty_block(blocks, blocks[jump_block.idx()].next); if nonempty_blocks == 1 + && !is_jump_only_block(&blocks[chain_start.idx()]) && after_jump != BlockIdx::NULL && !blocks[after_jump.idx()].cold && !block_is_exceptional(&blocks[after_jump.idx()]) From 5565f987de70b6ddc7c07ff088910f2872b54e32 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Wed, 29 Apr 2026 08:29:20 +0900 Subject: [PATCH 3/7] Align CFG cleanup bytecode with CPython --- .cspell.json | 1 + crates/codegen/src/compile.rs | 1806 ++++++++++++++++++++++++++------- crates/codegen/src/ir.rs | 1601 +++++++++++++++++++++++++---- 3 files changed, 2836 insertions(+), 572 deletions(-) diff --git a/.cspell.json b/.cspell.json index b7efb2e2311..a2f349fc060 100644 --- a/.cspell.json +++ b/.cspell.json @@ -67,6 +67,7 @@ "jitted", "jitting", "kwonly", + "lolcatz", "lossily", "mcache", "oparg", diff --git a/crates/codegen/src/compile.rs b/crates/codegen/src/compile.rs index 9ecc54c965e..8059d842615 100644 --- a/crates/codegen/src/compile.rs +++ b/crates/codegen/src/compile.rs @@ -57,7 +57,7 @@ impl ExprExt for ast::Expr { | ast::Expr::NoneLiteral(_) | ast::Expr::BooleanLiteral(_) | ast::Expr::EllipsisLiteral(_) - ) || matches!(self, ast::Expr::Tuple(ast::ExprTuple { elts, .. }) if elts.iter().all(ExprExt::is_constant)) + ) } fn is_constant_slice(&self) -> bool { @@ -566,6 +566,47 @@ impl Compiler { matches!(target, ast::Expr::List(_) | ast::Expr::Tuple(_)) } + fn statements_end_with_scope_exit(body: &[ast::Stmt]) -> bool { + body.last() + .is_some_and(Self::statement_ends_with_scope_exit) + } + + fn statement_ends_with_scope_exit(stmt: &ast::Stmt) -> bool { + match stmt { + ast::Stmt::Return(_) | ast::Stmt::Raise(_) => true, + ast::Stmt::If(ast::StmtIf { + body, + elif_else_clauses, + .. + }) => { + let has_else = elif_else_clauses + .last() + .is_some_and(|clause| clause.test.is_none()); + has_else + && Self::statements_end_with_scope_exit(body) + && elif_else_clauses + .iter() + .all(|clause| Self::statements_end_with_scope_exit(&clause.body)) + } + _ => false, + } + } + + fn preserves_finally_entry_nop(body: &[ast::Stmt]) -> bool { + body.last().is_some_and(|stmt| match stmt { + ast::Stmt::Try(ast::StmtTry { + body, + handlers, + finalbody, + .. + }) => { + !finalbody.is_empty() + || (!handlers.is_empty() && Self::statements_end_with_scope_exit(body)) + } + _ => false, + }) + } + fn compile_module_annotation_setup_sequence( &mut self, body: &[ast::Stmt], @@ -1423,6 +1464,9 @@ impl Compiler { folded_from_nonliteral_expr: false, lineno_override, cache_entries: 0, + preserve_redundant_jump_as_nop: false, + remove_no_location_nop: false, + preserve_block_start_no_location_nop: false, }); } @@ -3258,6 +3302,18 @@ impl Compiler { // End block - continuation point after try-finally // Normal path jumps here to skip exception path blocks let end_block = self.new_block(); + let has_bare_except = handlers.iter().any(|handler| { + matches!( + handler, + ast::ExceptHandler::ExceptHandler(ast::ExceptHandlerExceptHandler { + type_: None, + .. + }) + ) + }); + if has_bare_except { + self.disable_load_fast_borrow_for_block(end_block); + } // Emit NOP at the try: line so LINE events fire for it emit!(self, Instruction::Nop); @@ -3289,6 +3345,8 @@ impl Compiler { // if handlers is empty, compile body directly // without wrapping in TryExcept (only FinallyTry is needed) if handlers.is_empty() { + let preserve_finally_entry_nop = Self::preserves_finally_entry_nop(body); + // Just compile body with FinallyTry fblock active (if finalbody exists) self.compile_statements(body)?; @@ -3296,6 +3354,10 @@ impl Compiler { // This prevents exception table from covering the normal path if !finalbody.is_empty() { emit!(self, PseudoInstruction::PopBlock); + if !preserve_finally_entry_nop { + self.set_no_location(); + self.remove_last_no_location_nop(); + } self.pop_fblock(FBlockType::FinallyTry); } @@ -3316,7 +3378,12 @@ impl Compiler { } // Jump to end (skip exception path blocks) - emit!(self, PseudoInstruction::Jump { delta: end_block }); + emit!( + self, + PseudoInstruction::JumpNoInterrupt { delta: end_block } + ); + self.set_no_location(); + self.preserve_last_redundant_jump_as_nop(); if let Some(finally_except) = finally_except_block { // Restore sub_tables for exception path compilation @@ -3373,7 +3440,10 @@ impl Compiler { self.compile_statements(body)?; emit!(self, PseudoInstruction::PopBlock); self.pop_fblock(FBlockType::TryExcept); - emit!(self, PseudoInstruction::Jump { delta: else_block }); + emit!( + self, + PseudoInstruction::JumpNoInterrupt { delta: else_block } + ); self.set_no_location(); let cleanup_block = self.new_block(); @@ -3382,31 +3452,16 @@ impl Compiler { self.switch_to_block(else_block); self.compile_statements(orelse)?; - // Pop the FinallyTry fblock before jumping to finally - if !finalbody.is_empty() { - emit!(self, PseudoInstruction::PopBlock); - self.pop_fblock(FBlockType::FinallyTry); - } - - // Snapshot sub_tables before first finally compilation (for double compilation issue) - let sub_table_cursor = if !finalbody.is_empty() && finally_except_block.is_some() { - self.symbol_table_stack.last().map(|t| t.next_sub_table) - } else { - None - }; - - // finally (normal path): - self.switch_to_block(finally_block); - if !finalbody.is_empty() { - self.compile_statements(finalbody)?; - // Jump to end_block to skip exception path blocks - // This prevents fall-through to finally_except_block - emit!(self, PseudoInstruction::Jump { delta: end_block }); - } + emit!( + self, + PseudoInstruction::JumpNoInterrupt { + delta: finally_block, + } + ); + self.set_no_location(); // except handlers: self.switch_to_block(handler_block); - self.set_no_location(); // SETUP_CLEANUP(cleanup) for except block // This handles exceptions during exception matching @@ -3419,12 +3474,13 @@ impl Compiler { delta: cleanup_block } ); + self.set_no_location(); self.push_fblock(FBlockType::ExceptionHandler, cleanup_block, cleanup_block)?; // Exception is on top of stack now, pushed by unwind_blocks // PUSH_EXC_INFO transforms [exc] -> [prev_exc, exc] for PopExcept - self.set_no_location(); emit!(self, Instruction::PushExcInfo); + self.set_no_location(); for handler in handlers { let ast::ExceptHandler::ExceptHandler(ast::ExceptHandlerExceptHandler { type_, @@ -3506,37 +3562,62 @@ impl Compiler { self.compile_name(alias.as_str(), NameUsage::Delete)?; } - if !finalbody.is_empty() { - emit!(self, PseudoInstruction::PopBlock); - } - emit!( self, PseudoInstruction::JumpNoInterrupt { delta: finally_block, } ); + self.set_no_location(); self.push_fblock(FBlockType::ExceptionHandler, cleanup_block, cleanup_block)?; self.switch_to_block(next_handler); } - self.set_no_location(); emit!(self, Instruction::Reraise { depth: 0 }); + self.set_no_location(); self.pop_fblock(FBlockType::ExceptionHandler); self.switch_to_block(cleanup_block); - self.set_no_location(); emit!(self, Instruction::Copy { i: 3 }); self.set_no_location(); emit!(self, Instruction::PopExcept); self.set_no_location(); emit!(self, Instruction::Reraise { depth: 1 }); + self.set_no_location(); + + // finally (normal path): + // CPython's codegen_try_finally emits the wrapped try/except first and + // places the outer finally body at the inner try/except end label. Keep + // the FinallyTry fblock active through exception-handler normal exits so + // the CFG and exception-table ranges match that structure. + self.switch_to_block(finally_block); + if !finalbody.is_empty() { + emit!(self, PseudoInstruction::PopBlock); + self.set_no_location(); + self.pop_fblock(FBlockType::FinallyTry); + + // Snapshot sub_tables before first finally compilation (for double compilation issue) + let sub_table_cursor = if finally_except_block.is_some() { + self.symbol_table_stack.last().map(|t| t.next_sub_table) + } else { + None + }; + + self.compile_statements(finalbody)?; + // Jump to end_block to skip exception path blocks + // This prevents fall-through to finally_except_block + emit!( + self, + PseudoInstruction::JumpNoInterrupt { delta: end_block } + ); + self.set_no_location(); + self.preserve_last_redundant_jump_as_nop(); - // finally (exception path) - // This is where exceptions go to run finally before reraise - // Stack at entry: [lasti, exc] (from exception table with preserve_lasti=true) - if let Some(finally_except) = finally_except_block { + // finally (exception path) + // This is where exceptions go to run finally before reraise + // Stack at entry: [lasti, exc] (from exception table with preserve_lasti=true) + let finally_except = finally_except_block.expect("finally except block"); // Restore sub_tables for exception path compilation if let Some(cursor) = sub_table_cursor && let Some(current_table) = self.symbol_table_stack.last_mut() @@ -3549,11 +3630,11 @@ impl Compiler { // SETUP_CLEANUP for finally body // Exceptions during finally body need to go to cleanup block if let Some(cleanup) = finally_cleanup_block { - self.set_no_location(); emit!(self, PseudoInstruction::SetupCleanup { delta: cleanup }); + self.set_no_location(); } - self.set_no_location(); emit!(self, Instruction::PushExcInfo); + self.set_no_location(); if let Some(cleanup) = finally_cleanup_block { self.push_fblock(FBlockType::FinallyEnd, cleanup, cleanup)?; } @@ -3788,6 +3869,12 @@ impl Compiler { } else { self.new_block() }; + if !handlers.is_empty() { + self.disable_load_fast_borrow_for_block(end_block); + if !finalbody.is_empty() { + self.disable_load_fast_borrow_for_block(exit_block); + } + } // Emit NOP at the try: line so LINE events fire for it emit!(self, Instruction::Nop); @@ -3819,7 +3906,10 @@ impl Compiler { self.compile_statements(body)?; emit!(self, PseudoInstruction::PopBlock); self.pop_fblock(FBlockType::TryExcept); - emit!(self, PseudoInstruction::Jump { delta: else_block }); + emit!( + self, + PseudoInstruction::JumpNoInterrupt { delta: else_block } + ); // Exception handler entry self.switch_to_block(handler_block); @@ -3858,7 +3948,7 @@ impl Compiler { // Stack: [prev_exc, orig, list] emit!( self, - PseudoInstruction::Jump { + PseudoInstruction::JumpNoInterrupt { delta: reraise_star_block } ); @@ -3965,7 +4055,7 @@ impl Compiler { } emit!( self, - PseudoInstruction::Jump { + PseudoInstruction::JumpNoInterrupt { delta: next_handler_block } ); @@ -3999,14 +4089,14 @@ impl Compiler { emit!(self, Instruction::ListAppend { i: 1 }); emit!( self, - PseudoInstruction::Jump { + PseudoInstruction::JumpNoInterrupt { delta: reraise_star_block } ); } else { emit!( self, - PseudoInstruction::Jump { + PseudoInstruction::JumpNoInterrupt { delta: next_handler_block } ); @@ -4022,7 +4112,7 @@ impl Compiler { emit!(self, Instruction::ListAppend { i: 1 }); emit!( self, - PseudoInstruction::Jump { + PseudoInstruction::JumpNoInterrupt { delta: reraise_star_block } ); @@ -4076,14 +4166,9 @@ impl Compiler { emit!(self, Instruction::PopExcept); // Stack: [] - if !finalbody.is_empty() { - emit!(self, PseudoInstruction::PopBlock); - self.pop_fblock(FBlockType::FinallyTry); - } - emit!( self, - PseudoInstruction::Jump { + PseudoInstruction::JumpNoInterrupt { delta: continuation_block } ); @@ -4111,37 +4196,13 @@ impl Compiler { emit!(self, Instruction::Reraise { depth: 1 }); // try-else path - // NOTE: When we reach here in compilation, the nothing-to-reraise path above - // has already popped FinallyTry. But else_block is a different execution path - // that branches from try body success (where FinallyTry is still active). - // We need to re-push FinallyTry to reflect the correct fblock state for else path. - if !finalbody.is_empty() { - emit!( - self, - PseudoInstruction::SetupFinally { - delta: finally_block - } - ); - self.push_fblock_full( - FBlockType::FinallyTry, - finally_block, - finally_block, - FBlockDatum::FinallyBody(finalbody.to_vec()), - )?; - } if else_block != continuation_block { self.switch_to_block(else_block); self.compile_statements(orelse)?; - if !finalbody.is_empty() { - // Pop the FinallyTry fblock we just pushed for the else path - emit!(self, PseudoInstruction::PopBlock); - self.pop_fblock(FBlockType::FinallyTry); - } - emit!( self, - PseudoInstruction::Jump { + PseudoInstruction::JumpNoInterrupt { delta: continuation_block } ); @@ -4149,12 +4210,20 @@ impl Compiler { if !finalbody.is_empty() { self.switch_to_block(end_block); + emit!(self, PseudoInstruction::PopBlock); + self.set_no_location(); + self.remove_last_no_location_nop(); + self.pop_fblock(FBlockType::FinallyTry); + // Snapshot sub_tables before first finally compilation let sub_table_cursor = self.symbol_table_stack.last().map(|t| t.next_sub_table); // Compile finally body inline for normal path self.compile_statements(finalbody)?; - emit!(self, PseudoInstruction::Jump { delta: exit_block }); + emit!( + self, + PseudoInstruction::JumpNoInterrupt { delta: exit_block } + ); // Restore sub_tables for exception path compilation if let Some(cursor) = sub_table_cursor @@ -5012,133 +5081,22 @@ impl Compiler { Ok(()) } - /// Collect attribute names assigned via `self.xxx = ...` in methods. - /// These are stored as __static_attributes__ in the class dict. - fn collect_static_attributes(body: &[ast::Stmt], attrs: Option<&mut IndexSet>) { - let Some(attrs) = attrs else { return }; - for stmt in body { - let ast::Stmt::FunctionDef(f) = stmt else { - continue; - }; - Self::scan_function_store_attrs(f, attrs); - } - } - - fn scan_function_store_attrs(f: &ast::StmtFunctionDef, attrs: &mut IndexSet) { - // Skip @staticmethod and @classmethod decorated functions - let has_special_decorator = f.decorator_list.iter().any(|d| { - matches!(&d.expression, ast::Expr::Name(n) - if n.id.as_str() == "staticmethod" || n.id.as_str() == "classmethod") - }); - if has_special_decorator { - return; - } - // Skip implicit classmethods (__init_subclass__, __class_getitem__) - let fname = f.name.as_str(); - if fname == "__init_subclass__" || fname == "__class_getitem__" { - return; - } - // For __new__, scan for "self" (not the first param "cls") - if fname == "__new__" { - Self::scan_store_attrs(&f.body, "self", attrs); - return; - } - let first_param = f - .parameters - .posonlyargs - .first() - .or(f.parameters.args.first()) - .map(|p| &p.parameter.name); - let Some(self_name) = first_param else { + // Python/compile.c _PyCompile_MaybeAddStaticAttributeToClass + fn maybe_add_static_attribute_to_class(&mut self, value: &ast::Expr, attr: &str) { + if !matches!(value, ast::Expr::Name(n) if n.id.as_str() == "self") { return; - }; - Self::scan_store_attrs(&f.body, self_name.as_str(), attrs); - } - - /// Extract self.attr patterns from an assignment target expression. - fn scan_target_for_attrs(target: &ast::Expr, name: &str, attrs: &mut IndexSet) { - match target { - ast::Expr::Attribute(ast::ExprAttribute { value, attr, .. }) => { - if let ast::Expr::Name(n) = value.as_ref() - && n.id.as_str() == name - { - attrs.insert(attr.to_string()); - } - } - ast::Expr::Tuple(t) => { - for elt in &t.elts { - Self::scan_target_for_attrs(elt, name, attrs); - } - } - ast::Expr::List(l) => { - for elt in &l.elts { - Self::scan_target_for_attrs(elt, name, attrs); - } - } - ast::Expr::Starred(s) => { - Self::scan_target_for_attrs(&s.value, name, attrs); - } - _ => {} } - } - - /// Recursively scan statements for `name.attr = value` patterns. - fn scan_store_attrs(stmts: &[ast::Stmt], name: &str, attrs: &mut IndexSet) { - for stmt in stmts { - match stmt { - ast::Stmt::Assign(a) => { - for target in &a.targets { - Self::scan_target_for_attrs(target, name, attrs); - } - } - ast::Stmt::AnnAssign(a) => { - Self::scan_target_for_attrs(&a.target, name, attrs); - } - ast::Stmt::AugAssign(a) => { - if let ast::Expr::Attribute(ast::ExprAttribute { value, attr, .. }) = - a.target.as_ref() - && let ast::Expr::Name(n) = value.as_ref() - && n.id.as_str() == name - { - attrs.insert(attr.to_string()); - } - } - ast::Stmt::If(s) => { - Self::scan_store_attrs(&s.body, name, attrs); - for clause in &s.elif_else_clauses { - Self::scan_store_attrs(&clause.body, name, attrs); - } - } - ast::Stmt::For(s) => { - Self::scan_store_attrs(&s.body, name, attrs); - Self::scan_store_attrs(&s.orelse, name, attrs); - } - ast::Stmt::While(s) => { - Self::scan_store_attrs(&s.body, name, attrs); - Self::scan_store_attrs(&s.orelse, name, attrs); - } - ast::Stmt::Try(s) => { - Self::scan_store_attrs(&s.body, name, attrs); - for handler in &s.handlers { - let ast::ExceptHandler::ExceptHandler(h) = handler; - Self::scan_store_attrs(&h.body, name, attrs); - } - Self::scan_store_attrs(&s.orelse, name, attrs); - Self::scan_store_attrs(&s.finalbody, name, attrs); - } - ast::Stmt::With(s) => { - Self::scan_store_attrs(&s.body, name, attrs); - } - ast::Stmt::Match(s) => { - for case in &s.cases { - Self::scan_store_attrs(&case.body, name, attrs); - } - } - ast::Stmt::FunctionDef(f) => { - Self::scan_function_store_attrs(f, attrs); - } - _ => {} - } + if let Some(class_unit) = self + .code_stack + .iter_mut() + .rev() + .find(|unit| unit.static_attributes.is_some()) + { + class_unit + .static_attributes + .as_mut() + .unwrap() + .insert(attr.to_owned()); } } @@ -5283,16 +5241,6 @@ impl Compiler { } } - // Collect __static_attributes__: scan methods for self.xxx = ... patterns - Self::collect_static_attributes( - body, - self.code_stack - .last_mut() - .unwrap() - .static_attributes - .as_mut(), - ); - // 3. Compile the class body self.compile_statements(body)?; @@ -5802,8 +5750,10 @@ impl Compiler { } // Pop fblock before normal exit - self.set_source_range(with_range); emit!(self, PseudoInstruction::PopBlock); + self.set_no_location(); + self.remove_last_no_location_nop(); + self.preserve_last_block_start_no_location_nop(); self.pop_fblock(if is_async { FBlockType::AsyncWith } else { @@ -7388,72 +7338,80 @@ impl Compiler { } fn compile_store(&mut self, target: &ast::Expr) -> CompileResult<()> { - match &target { - ast::Expr::Name(ast::ExprName { id, .. }) => self.store_name(id.as_str())?, - ast::Expr::Subscript(ast::ExprSubscript { - value, slice, ctx, .. - }) => { - self.compile_subscript(value, slice, *ctx)?; - } - ast::Expr::Attribute(ast::ExprAttribute { value, attr, .. }) => { - self.compile_expression(value)?; - let namei = self.name(attr.as_str()); - emit!(self, Instruction::StoreAttr { namei }); - } - ast::Expr::List(ast::ExprList { elts, .. }) - | ast::Expr::Tuple(ast::ExprTuple { elts, .. }) => { - let mut seen_star = false; - - // Scan for star args: - for (i, element) in elts.iter().enumerate() { - if let ast::Expr::Starred(_) = &element { - if seen_star { - return Err(self.error(CodegenErrorType::MultipleStarArgs)); - } else { - seen_star = true; - let before = i; - let after = elts.len() - i - 1; - let (before, after) = (|| Some((before.to_u8()?, after.to_u8()?)))() + let prev_source_range = self.current_source_range; + self.set_source_range(target.range()); + let result = (|| -> CompileResult<()> { + match &target { + ast::Expr::Name(ast::ExprName { id, .. }) => self.store_name(id.as_str())?, + ast::Expr::Subscript(ast::ExprSubscript { + value, slice, ctx, .. + }) => { + self.compile_subscript(value, slice, *ctx)?; + } + ast::Expr::Attribute(ast::ExprAttribute { value, attr, .. }) => { + self.maybe_add_static_attribute_to_class(value, attr.as_str()); + self.compile_expression(value)?; + let namei = self.name(attr.as_str()); + emit!(self, Instruction::StoreAttr { namei }); + } + ast::Expr::List(ast::ExprList { elts, .. }) + | ast::Expr::Tuple(ast::ExprTuple { elts, .. }) => { + let mut seen_star = false; + + // Scan for star args: + for (i, element) in elts.iter().enumerate() { + if let ast::Expr::Starred(_) = &element { + if seen_star { + return Err(self.error(CodegenErrorType::MultipleStarArgs)); + } else { + seen_star = true; + let before = i; + let after = elts.len() - i - 1; + let (before, after) = (|| Some((before.to_u8()?, after.to_u8()?)))( + ) .ok_or_else(|| { self.error_ranged( CodegenErrorType::TooManyStarUnpack, target.range(), ) })?; - let counts = bytecode::UnpackExArgs { before, after }; - emit!(self, Instruction::UnpackEx { counts }); + let counts = bytecode::UnpackExArgs { before, after }; + emit!(self, Instruction::UnpackEx { counts }); + } } } - } - if !seen_star { - emit!( - self, - Instruction::UnpackSequence { - count: elts.len().to_u32(), - } - ); - } + if !seen_star { + emit!( + self, + Instruction::UnpackSequence { + count: elts.len().to_u32(), + } + ); + } - for element in elts { - if let ast::Expr::Starred(ast::ExprStarred { value, .. }) = &element { - self.compile_store(value)?; - } else { - self.compile_store(element)?; + for element in elts { + if let ast::Expr::Starred(ast::ExprStarred { value, .. }) = &element { + self.compile_store(value)?; + } else { + self.compile_store(element)?; + } } } + _ => { + return Err(self.error(match target { + ast::Expr::Starred(_) => CodegenErrorType::SyntaxError( + "starred assignment target must be in a list or tuple".to_owned(), + ), + _ => CodegenErrorType::Assign(target.python_name()), + })); + } } - _ => { - return Err(self.error(match target { - ast::Expr::Starred(_) => CodegenErrorType::SyntaxError( - "starred assignment target must be in a list or tuple".to_owned(), - ), - _ => CodegenErrorType::Assign(target.python_name()), - })); - } - } + Ok(()) + })(); - Ok(()) + self.set_source_range(prev_source_range); + result } fn compile_augassign( @@ -8221,12 +8179,12 @@ impl Compiler { ast::Expr::Slice(ast::ExprSlice { lower, upper, step, .. }) => { - // Try constant slice folding first - if self.try_fold_constant_slice( + if let Some(folded_const) = self.try_fold_constant_slice( lower.as_deref(), upper.as_deref(), step.as_deref(), )? { + self.emit_load_const(folded_const); return Ok(()); } let mut compile_bound = |bound: Option<&ast::Expr>| match bound { @@ -9833,6 +9791,9 @@ impl Compiler { folded_from_nonliteral_expr: false, lineno_override: None, cache_entries: 0, + preserve_redundant_jump_as_nop: false, + remove_no_location_nop: false, + preserve_block_start_no_location_nop: false, }); } @@ -9842,9 +9803,27 @@ impl Compiler { } } - /// Mark the last emitted instruction as having no source location. - /// Prevents it from triggering LINE events in sys.monitoring. - fn set_no_location(&mut self) { + fn preserve_last_redundant_jump_as_nop(&mut self) { + if let Some(info) = self.current_block().instructions.last_mut() { + info.preserve_redundant_jump_as_nop = true; + } + } + + fn remove_last_no_location_nop(&mut self) { + if let Some(info) = self.current_block().instructions.last_mut() { + info.remove_no_location_nop = true; + } + } + + fn preserve_last_block_start_no_location_nop(&mut self) { + if let Some(info) = self.current_block().instructions.last_mut() { + info.preserve_block_start_no_location_nop = true; + } + } + + /// Mark the last emitted instruction as having no source location. + /// Prevents it from triggering LINE events in sys.monitoring. + fn set_no_location(&mut self) { if let Some(last) = self.current_block().instructions.last_mut() { last.lineno_override = Some(-1); } @@ -10038,6 +10017,13 @@ impl Compiler { ast::Expr::BooleanLiteral(b) => ConstantData::Boolean { value: b.value }, ast::Expr::NoneLiteral(_) => ConstantData::None, ast::Expr::EllipsisLiteral(_) => ConstantData::Ellipsis, + ast::Expr::Name(ast::ExprName { id, ctx, .. }) + if matches!(ctx, ast::ExprContext::Load) && id.as_str() == "__debug__" => + { + ConstantData::Boolean { + value: self.opts.optimize == 0, + } + } ast::Expr::Tuple(ast::ExprTuple { elts, .. }) => { let mut elements = Vec::with_capacity(elts.len()); for elt in elts { @@ -10186,32 +10172,70 @@ impl Compiler { self.emit_arg(idx, |consti| Instruction::LoadConst { consti }) } - /// Fold constant slice: if all parts are compile-time constants, emit LOAD_CONST(slice). + fn try_fold_slice_constant_part( + &mut self, + expr: &ast::Expr, + ) -> CompileResult> { + Ok(Some(match expr { + ast::Expr::NumberLiteral(num) => match &num.value { + ast::Number::Int(int) => ConstantData::Integer { + value: ruff_int_to_bigint(int).map_err(|e| self.error(e))?, + }, + ast::Number::Float(f) => ConstantData::Float { value: *f }, + ast::Number::Complex { real, imag } => ConstantData::Complex { + value: Complex::new(*real, *imag), + }, + }, + ast::Expr::StringLiteral(s) => ConstantData::Str { + value: self.compile_string_value(s), + }, + ast::Expr::BytesLiteral(b) => ConstantData::Bytes { + value: b.value.bytes().collect(), + }, + ast::Expr::BooleanLiteral(b) => ConstantData::Boolean { value: b.value }, + ast::Expr::NoneLiteral(_) => ConstantData::None, + ast::Expr::EllipsisLiteral(_) => ConstantData::Ellipsis, + _ => return Ok(None), + })) + } + fn try_fold_constant_slice( &mut self, lower: Option<&ast::Expr>, upper: Option<&ast::Expr>, step: Option<&ast::Expr>, - ) -> CompileResult { - let to_const = |expr: Option<&ast::Expr>, this: &mut Self| -> CompileResult<_> { - match expr { - None => Ok(Some(ConstantData::None)), - Some(expr) => this.try_fold_constant_expr(expr), + ) -> CompileResult> { + let start = match lower { + Some(expr) => { + let Some(constant) = self.try_fold_slice_constant_part(expr)? else { + return Ok(None); + }; + constant } + None => ConstantData::None, }; - - let (Some(start), Some(stop), Some(step_val)) = ( - to_const(lower, self)?, - to_const(upper, self)?, - to_const(step, self)?, - ) else { - return Ok(false); + let stop = match upper { + Some(expr) => { + let Some(constant) = self.try_fold_slice_constant_part(expr)? else { + return Ok(None); + }; + constant + } + None => ConstantData::None, + }; + let step = match step { + Some(expr) => { + let Some(constant) = self.try_fold_slice_constant_part(expr)? else { + return Ok(None); + }; + constant + } + None => ConstantData::None, }; - self.emit_load_const(ConstantData::Slice { - elements: Box::new([start, stop, step_val]), - }); - Ok(true) + Ok(Some(ConstantData::Slice { + elements: Box::new([start, stop, step]), + })) } fn emit_return_const(&mut self, constant: ConstantData) { @@ -11563,6 +11587,62 @@ mod tests { stack_top.debug_late_cfg_trace().unwrap() } + #[test] + #[ignore = "debug helper"] + fn debug_trace_make_dataclass_borrow_tail() { + let trace = compile_single_function_late_cfg_trace( + r#" +def f(module, cls, decorator, init, repr, eq, order, unsafe_hash, frozen, match_args, kw_only, slots, weakref_slot): + if module is None: + try: + module = sys._getframemodulename(1) or '__main__' + except AttributeError: + try: + module = sys._getframe(1).f_globals.get('__name__', '__main__') + except (AttributeError, ValueError): + pass + if module is not None: + cls.__module__ = module + cls = decorator(cls, init=init, repr=repr, eq=eq, order=order, + unsafe_hash=unsafe_hash, frozen=frozen, + match_args=match_args, kw_only=kw_only, slots=slots, + weakref_slot=weakref_slot) + return cls +"#, + "f", + ); + for (label, dump) in trace { + if label.starts_with("after_") { + eprintln!("=== {label} ===\n{dump}"); + } + } + } + + #[test] + #[ignore = "debug helper"] + fn debug_trace_protected_attr_subscript_tail() { + let trace = compile_single_function_late_cfg_trace( + r#" +def f(f, oldcls, newcls): + try: + idx = f.__code__.co_freevars.index("__class__") + except ValueError: + return False + closure = f.__closure__[idx] + if closure.cell_contents is oldcls: + closure.cell_contents = newcls + return True + return False +"#, + "f", + ); + for (label, dump) in trace { + if label.starts_with("after_") { + eprintln!("=== {label} ===\n{dump}"); + } + } + } + fn find_code<'a>(code: &'a CodeObject, name: &str) -> Option<&'a CodeObject> { if code.obj_name == name { return Some(code); @@ -13661,6 +13741,110 @@ def jumpy(i): with_except_idx < check_exc_idx, "expected with-except cleanup to be emitted before outer except matching like CPython, got ops={ops:?}", ); + + let with_cleanup_end = ops + .windows(5) + .position(|window| { + matches!( + window, + [ + Instruction::LoadConst { .. }, + Instruction::LoadConst { .. }, + Instruction::LoadConst { .. }, + Instruction::Call { .. }, + Instruction::PopTop, + ] + ) + }) + .expect("missing with success cleanup") + + 5; + assert!( + !matches!(ops.get(with_cleanup_end), Some(Instruction::Nop)), + "expected with success cleanup to fall straight into the surrounding continuation without a synthetic NOP target, got ops={ops:?}", + ); + } + + #[test] + fn test_nested_try_finally_keeps_inner_finally_cleanup_nop() { + let code = compile_exec( + "\ +def f(a, b, d): + try: + try: + a() + finally: + b() + finally: + d() +", + ); + let f = find_code(&code, "f").expect("missing f code"); + let ops_lines: Vec<_> = f + .instructions + .iter() + .zip(&f.locations) + .filter_map(|(unit, (location, _))| { + (!matches!(unit.op, Instruction::Cache)).then_some((unit.op, location.line.get())) + }) + .collect(); + + assert!( + ops_lines.windows(3).any(|window| { + matches!( + window, + [ + (Instruction::PopTop, 6), + (Instruction::Nop, 6), + ( + Instruction::LoadFastBorrow { .. } | Instruction::LoadFast { .. }, + 8 + ), + ] + ) + }), + "expected CPython-style inner finally cleanup NOP before outer finalbody, got ops_lines={ops_lines:?}", + ); + } + + #[test] + fn test_conditional_break_finally_does_not_keep_break_cleanup_nop() { + let code = compile_exec( + "\ +def f(tar1, x): + try: + while True: + if x: + break + x = 1 + finally: + tar1.close() +", + ); + let f = find_code(&code, "f").expect("missing f code"); + let ops_lines: Vec<_> = f + .instructions + .iter() + .zip(&f.locations) + .filter_map(|(unit, (location, _))| { + (!matches!(unit.op, Instruction::Cache)).then_some((unit.op, location.line.get())) + }) + .collect(); + + assert!( + !ops_lines.windows(2).any(|window| { + matches!( + window, + [ + (Instruction::Nop, 5), + ( + Instruction::LoadFastBorrow { .. } | Instruction::LoadFast { .. }, + 8 + ), + ] + ) + }), + "expected CPython-style break cleanup to jump directly into finally body, got ops_lines={ops_lines:?}", + ); } #[test] @@ -13731,103 +13915,406 @@ def f(msg): } #[test] - fn test_genexpr_true_filter_omits_bool_scaffolding() { + fn test_try_import_return_handler_deopts_common_tail_borrow() { let code = compile_exec( "\ -def f(it): - return (x for x in it if True) +def f(): + try: + import pwd, grp + except ImportError: + return False + if pwd.getpwuid(0)[0] != 'root': + return False + if grp.getgrgid(0)[0] != 'root': + return False + return True ", ); - let genexpr = find_code(&code, "").expect("missing code"); - assert!( - !genexpr.instructions.iter().any(|unit| { - matches!(unit.op, Instruction::LoadConst { .. }) - && matches!( - genexpr.constants.get(usize::from(u8::from(unit.arg))), - Some(ConstantData::Boolean { value: true }) - ) - }), - "constant-true filter should not load True, got ops={:?}", - genexpr - .instructions - .iter() - .map(|unit| unit.op) - .collect::>() - ); + let f = find_code(&code, "f").expect("missing f code"); + let ops: Vec<_> = f + .instructions + .iter() + .map(|unit| unit.op) + .filter(|op| !matches!(op, Instruction::Cache)) + .collect(); + assert!( - !genexpr - .instructions - .iter() - .any(|unit| matches!(unit.op, Instruction::PopJumpIfTrue { .. })), - "constant-true filter should not leave POP_JUMP_IF_TRUE scaffolding, got ops={:?}", - genexpr - .instructions - .iter() - .map(|unit| unit.op) - .collect::>() + !ops.iter() + .any(|op| matches!(op, Instruction::LoadFastBorrow { .. })), + "expected CPython-style LOAD_FAST after protected import common tail, got ops={ops:?}", ); } #[test] - fn test_classdictcell_uses_load_closure_path_and_borrows_after_optimize() { + fn test_protected_attr_direct_return_keeps_borrow() { let code = compile_exec( "\ -class C: - def method(self): - return 1 +def f(obj): + try: + x = 1 + except ValueError: + return False + return obj.values() ", ); - let class_code = find_code(&code, "C").expect("missing class code"); - let store_classdictcell = class_code + let f = find_code(&code, "f").expect("missing function code"); + let ops: Vec<_> = f .instructions .iter() - .position(|unit| { - matches!( - unit.op, - Instruction::StoreName { namei } - if class_code.names - [namei.get(OpArg::new(u32::from(u8::from(unit.arg)))) as usize] - .as_str() - == "__classdictcell__" - ) - }) - .expect("missing STORE_NAME __classdictcell__"); + .map(|unit| unit.op) + .filter(|op| !matches!(op, Instruction::Cache)) + .collect(); + let handler_start = ops + .iter() + .position(|op| matches!(op, Instruction::PushExcInfo)) + .expect("missing handler entry"); + let protected_tail = &ops[..handler_start]; assert!( - matches!( - class_code - .instructions - .get(store_classdictcell.saturating_sub(1)) - .map(|unit| unit.op), - Some(Instruction::LoadFastBorrow { .. }) - ), - "expected LOAD_FAST_BORROW before __classdictcell__ store, got ops={:?}", - class_code - .instructions - .iter() - .map(|unit| unit.op) - .collect::>() + protected_tail.windows(4).any(|window| { + matches!( + window, + [ + Instruction::LoadFastBorrow { .. }, + Instruction::LoadAttr { .. }, + Instruction::Call { .. }, + Instruction::ReturnValue, + ] + ) + }), + "expected protected direct attr-call return to keep LOAD_FAST_BORROW, got tail={protected_tail:?}" ); } #[test] - fn test_nested_function_static_attributes_are_collected() { + fn test_protected_store_normal_tail_uses_strong_loads() { let code = compile_exec( "\ -class C: - def f(self): - self.x = 1 - self.y = 2 - self.x = 3 - - def g(self, obj): - self.y = 4 - self.z = 5 - - def h(self, a): - self.u = 6 - self.v = 7 - +def f(tarfile, tarinfo, self): + try: + filtered = tarfile.tar_filter(tarinfo, '') + except UnicodeEncodeError: + return None + self.assertIs(filtered.name, tarinfo.name) + return filtered +", + ); + 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 filtered_store = ops + .iter() + .position(|op| matches!(op, Instruction::StoreFast { .. })) + .expect("missing filtered store"); + let handler_start = ops + .iter() + .position(|op| matches!(op, Instruction::PushExcInfo)) + .expect("missing handler entry"); + let normal_tail = &ops[filtered_store + 1..handler_start]; + + assert!( + !normal_tail.iter().any(|op| matches!( + op, + Instruction::LoadFastBorrow { .. } + | Instruction::LoadFastBorrowLoadFastBorrow { .. } + )), + "expected CPython-style strong LOAD_FAST in protected store normal tail, got tail={normal_tail:?}", + ); + } + + #[test] + fn test_protected_attr_subscript_tail_uses_strong_load_fast() { + let code = compile_exec( + "\ +def f(obj, idx): + try: + x = 1 + except ValueError: + return False + return obj.__closure__[idx] +", + ); + 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 handler_start = ops + .iter() + .position(|op| matches!(op, Instruction::PushExcInfo)) + .expect("missing handler entry"); + let protected_tail = &ops[..handler_start]; + + assert!( + !protected_tail.iter().any(|op| { + matches!( + op, + Instruction::LoadFastBorrow { .. } + | Instruction::LoadFastBorrowLoadFastBorrow { .. } + ) + }), + "expected protected attr-subscript tail to keep strong LOAD_FAST ops, got tail={protected_tail:?}" + ); + } + + #[test] + fn test_protected_attr_iter_chain_uses_strong_load_fast() { + let code = compile_exec( + "\ +def f(fields): + try: + x = 1 + except ValueError: + return False + return tuple(v for v in fields.values()) +", + ); + 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 handler_start = ops + .iter() + .position(|op| matches!(op, Instruction::PushExcInfo)) + .expect("missing handler entry"); + let protected_tail = &ops[..handler_start]; + + assert!( + !protected_tail.iter().any(|op| { + matches!( + op, + Instruction::LoadFastBorrow { .. } + | Instruction::LoadFastBorrowLoadFastBorrow { .. } + ) + }), + "expected protected attr-iter chain to keep strong LOAD_FAST ops, got tail={protected_tail:?}" + ); + } + + #[test] + fn test_protected_attr_subscript_store_tail_uses_strong_load_fast() { + let code = compile_exec( + "\ +def f(f, oldcls, newcls): + try: + idx = f.__code__.co_freevars.index('__class__') + except ValueError: + return False + closure = f.__closure__[idx] + if closure.cell_contents is oldcls: + closure.cell_contents = newcls + return True + return False +", + ); + let f = find_code(&code, "f").expect("missing f code"); + let ops: Vec<_> = f + .instructions + .iter() + .map(|unit| unit.op) + .filter(|op| !matches!(op, Instruction::Cache)) + .collect(); + let handler_start = ops + .iter() + .position(|op| matches!(op, Instruction::PushExcInfo)) + .expect("missing handler entry"); + let protected_tail = &ops[..handler_start]; + let store_closure_idx = protected_tail + .windows(2) + .position(|window| { + matches!( + window, + [Instruction::BinaryOp { .. }, Instruction::StoreFast { .. }] + ) + }) + .map(|idx| idx + 1) + .expect("missing STORE_FAST for closure"); + let post_store_tail = &protected_tail[store_closure_idx + 1..]; + + assert!( + !post_store_tail.iter().any(|op| { + matches!( + op, + Instruction::LoadFastBorrow { .. } + | Instruction::LoadFastBorrowLoadFastBorrow { .. } + ) + }), + "expected protected attr-subscript store tail to keep strong LOAD_FAST ops, got tail={post_store_tail:?}" + ); + } + + #[test] + fn test_plain_attr_subscript_tail_keeps_borrow() { + let code = compile_exec( + "\ +def f(self, name): + annotations = self.method_annotations[name] + return annotations +", + ); + let f = find_code(&code, "f").expect("missing f code"); + let ops: Vec<_> = f + .instructions + .iter() + .map(|unit| unit.op) + .filter(|op| !matches!(op, Instruction::Cache)) + .collect(); + + assert!( + ops.windows(4).any(|window| { + matches!( + window, + [ + Instruction::LoadFastBorrow { .. }, + Instruction::LoadAttr { .. }, + Instruction::LoadFastBorrow { .. }, + Instruction::BinaryOp { .. }, + ] + ) + }), + "expected plain attr-subscript tail to keep borrowed receiver/index loads, got ops={ops:?}" + ); + } + + #[test] + fn test_plain_attr_iter_chain_keeps_borrow() { + let code = compile_exec( + "\ +def f(fields): + return tuple(v for v in fields.values()) +", + ); + let f = find_code(&code, "f").expect("missing f code"); + let ops: Vec<_> = f + .instructions + .iter() + .map(|unit| unit.op) + .filter(|op| !matches!(op, Instruction::Cache)) + .collect(); + + assert!( + ops.windows(4).any(|window| { + matches!( + window, + [ + Instruction::LoadFastBorrow { .. }, + Instruction::LoadAttr { .. }, + Instruction::Call { .. }, + Instruction::GetIter, + ] + ) + }), + "expected plain attr-iter chain to keep borrowed receiver, got ops={ops:?}" + ); + } + + #[test] + fn test_genexpr_true_filter_omits_bool_scaffolding() { + let code = compile_exec( + "\ +def f(it): + return (x for x in it if True) +", + ); + let genexpr = find_code(&code, "").expect("missing code"); + assert!( + !genexpr.instructions.iter().any(|unit| { + matches!(unit.op, Instruction::LoadConst { .. }) + && matches!( + genexpr.constants.get(usize::from(u8::from(unit.arg))), + Some(ConstantData::Boolean { value: true }) + ) + }), + "constant-true filter should not load True, got ops={:?}", + genexpr + .instructions + .iter() + .map(|unit| unit.op) + .collect::>() + ); + assert!( + !genexpr + .instructions + .iter() + .any(|unit| matches!(unit.op, Instruction::PopJumpIfTrue { .. })), + "constant-true filter should not leave POP_JUMP_IF_TRUE scaffolding, got ops={:?}", + genexpr + .instructions + .iter() + .map(|unit| unit.op) + .collect::>() + ); + } + + #[test] + fn test_classdictcell_uses_load_closure_path_and_borrows_after_optimize() { + let code = compile_exec( + "\ +class C: + def method(self): + return 1 +", + ); + let class_code = find_code(&code, "C").expect("missing class code"); + let store_classdictcell = class_code + .instructions + .iter() + .position(|unit| { + matches!( + unit.op, + Instruction::StoreName { namei } + if class_code.names + [namei.get(OpArg::new(u32::from(u8::from(unit.arg)))) as usize] + .as_str() + == "__classdictcell__" + ) + }) + .expect("missing STORE_NAME __classdictcell__"); + + assert!( + matches!( + class_code + .instructions + .get(store_classdictcell.saturating_sub(1)) + .map(|unit| unit.op), + Some(Instruction::LoadFastBorrow { .. }) + ), + "expected LOAD_FAST_BORROW before __classdictcell__ store, got ops={:?}", + class_code + .instructions + .iter() + .map(|unit| unit.op) + .collect::>() + ); + } + + #[test] + fn test_nested_function_static_attributes_are_collected() { + let code = compile_exec( + "\ +class C: + def f(self): + self.x = 1 + self.y = 2 + self.x = 3 + + def g(self, obj): + self.y = 4 + self.z = 5 + + def h(self, a): + self.u = 6 + self.v = 7 + obj.self = 8 ", ); @@ -13850,6 +14337,47 @@ class C: ); } + #[test] + fn test_static_attributes_match_cpython_store_rule() { + let code = compile_exec( + "\ +class C: + @staticmethod + def f(): + self.x = 1 + + @classmethod + def g(cls): + self.y = 2 + + def h(obj): + obj.z = 3 + tarinfo.uid = 4 + + def i(self): + self.a: int + self.b: int = 1 + self.c += 1 + del self.d +", + ); + let class_code = find_code(&code, "C").expect("missing class code"); + + assert!( + class_code.constants.iter().any(|constant| matches!( + constant, + ConstantData::Tuple { elements } + if elements + == &[ + ConstantData::Str { value: "b".into() }, + ConstantData::Str { value: "x".into() }, + ConstantData::Str { value: "y".into() }, + ] + )), + "expected only CPython-collected static attributes in class consts" + ); + } + #[test] fn test_decorated_class_uses_first_decorator_for_firstlineno() { let code = compile_exec( @@ -14170,7 +14698,41 @@ def f(self): } #[test] - fn test_constant_slice_folding_handles_string_and_bigint_bounds() { + fn test_dunder_debug_constant_false_if_deopts_tail_borrow() { + let code = compile_exec( + "\ +def f(self): + if not __debug__: + self.skipTest('need asserts, run without -O') + self.do_disassembly_test() +", + ); + let f = find_code(&code, "f").expect("missing f code"); + let instructions: Vec<_> = f + .instructions + .iter() + .filter(|unit| !matches!(unit.op, Instruction::Cache)) + .collect(); + let attr_idx = instructions + .iter() + .position(|unit| match unit.op { + Instruction::LoadAttr { namei } => { + let load_attr = namei.get(OpArg::new(u32::from(u8::from(unit.arg)))); + f.names[usize::try_from(load_attr.name_idx()).unwrap()].as_str() + == "do_disassembly_test" + } + _ => false, + }) + .expect("missing LOAD_ATTR for do_disassembly_test"); + let ops: Vec<_> = instructions.iter().map(|unit| unit.op).collect(); + assert!( + matches!(ops.get(attr_idx - 1), Some(Instruction::LoadFast { .. })), + "constant-false __debug__ tail should deopt self to LOAD_FAST, got ops={ops:?}" + ); + } + + #[test] + fn test_constant_slice_folds_constant_bounds() { let code = compile_exec( "\ def f(obj): @@ -14178,7 +14740,13 @@ def f(obj): ", ); let f = find_code(&code, "f").expect("missing function code"); - let slice = f + let ops: Vec<_> = f + .instructions + .iter() + .map(|unit| unit.op) + .filter(|op| !matches!(op, Instruction::Cache)) + .collect(); + let folded_slice = f .constants .iter() .find_map(|constant| match constant { @@ -14186,10 +14754,62 @@ def f(obj): _ => None, }) .expect("missing folded slice constant"); + assert!( + matches!( + folded_slice.as_ref(), + [ + ConstantData::Str { .. }, + ConstantData::Integer { .. }, + ConstantData::None, + ] + ), + "expected folded slice('a', 123456789012345678901234567890, None), got {folded_slice:?}" + ); + assert!( + matches!( + ops.as_slice(), + [ + Instruction::Resume { .. }, + Instruction::LoadFastBorrow { .. }, + Instruction::LoadConst { .. }, + Instruction::BinaryOp { .. }, + Instruction::ReturnValue, + ] + ), + "expected CPython-style LOAD_CONST(slice(...)) path for constant bounds, got ops={ops:?}" + ); + } + + #[test] + fn test_tuple_bound_slice_uses_two_part_slice_path() { + let code = compile_exec( + "\ +def f(obj): + return obj[(1, 2):] +", + ); + 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(); - assert!(matches!(slice[0], ConstantData::Str { .. })); - assert!(matches!(slice[1], ConstantData::Integer { .. })); - assert!(matches!(slice[2], ConstantData::None)); + assert!( + matches!( + ops.as_slice(), + [ + Instruction::Resume { .. }, + Instruction::LoadFastBorrow { .. }, + Instruction::LoadConst { .. }, + Instruction::LoadConst { .. }, + Instruction::BinarySlice, + Instruction::ReturnValue, + ] + ), + "expected CPython-style BINARY_SLICE path for tuple lower bound, got ops={ops:?}" + ); } #[test] @@ -14217,6 +14837,118 @@ def f(names, cls): ); } + #[test] + fn test_nested_with_bare_except_keeps_handler_cleanup_before_following_code() { + let code = compile_exec( + "\ +def f(cm, self): + try: + with cm: + raise Exception + except: + pass + self.g() +", + ); + 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 outer_handler = ops + .iter() + .enumerate() + .filter_map(|(idx, op)| matches!(op, Instruction::PushExcInfo).then_some(idx)) + .next_back() + .expect("missing outer handler"); + assert!( + ops[outer_handler..].windows(6).any(|window| { + matches!( + window, + [ + Instruction::PopExcept, + Instruction::JumpForward { .. }, + Instruction::Copy { .. }, + Instruction::PopExcept, + Instruction::Reraise { .. }, + Instruction::LoadFast { .. }, + ] + ) + }), + "expected CPython-style handler cleanup before following code, got ops={ops:?}" + ); + } + + #[test] + fn test_try_else_for_cleanup_drops_redundant_jump_nop() { + let code = compile_exec( + "\ +def f(self, xs, ys, cm1, cm2): + for x in xs: + with self.subTest(x=x): + try: + with cm1: + self.a() + except Exception: + if x: + pass + else: + raise + else: + for y in ys: + with self.subTest(y=y): + with cm2: + self.b() +", + ); + 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(); + + assert!( + ops.windows(7).any(|window| { + matches!( + window, + [ + Instruction::EndFor, + Instruction::PopIter, + Instruction::LoadConst { .. }, + Instruction::LoadConst { .. }, + Instruction::LoadConst { .. }, + Instruction::Call { .. }, + Instruction::PopTop, + ] + ) + }), + "expected inner for cleanup to fall directly into surrounding with cleanup, got ops={ops:?}", + ); + assert!( + !ops.windows(8).any(|window| { + matches!( + window, + [ + Instruction::EndFor, + Instruction::PopIter, + Instruction::Nop, + Instruction::LoadConst { .. }, + Instruction::LoadConst { .. }, + Instruction::LoadConst { .. }, + Instruction::Call { .. }, + Instruction::PopTop, + ] + ) + }), + "expected CPython-style removal of the redundant jump NOP after for cleanup, got ops={ops:?}", + ); + } + #[test] fn test_non_none_final_return_is_not_duplicated() { let code = compile_exec( @@ -14392,6 +15124,43 @@ def f(escaped_string, quote_types): ); } + #[test] + fn test_dictcomp_cleanup_tail_keeps_split_store_fast_pair() { + let code = compile_exec( + "\ +def f(obj, g): + return {g(k): g(v) for k, v in obj.items()} +", + ); + 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 pop_iter_idx = ops + .iter() + .position(|op| matches!(op, Instruction::PopIter)) + .expect("missing POP_ITER"); + let tail = &ops[pop_iter_idx + 1..]; + + assert!( + matches!( + tail, + [ + Instruction::Swap { .. }, + Instruction::StoreFast { .. }, + Instruction::StoreFast { .. }, + Instruction::ReturnValue, + .. + ] + ), + "expected split STORE_FAST pair after dictcomp cleanup, got ops={ops:?}" + ); + } + #[test] fn test_static_swap_triple_assign_keeps_store_fast_store_fast() { let code = compile_exec( @@ -15486,6 +16255,85 @@ def f(): ); } + #[test] + fn test_folded_multiline_bytes_binop_does_not_leave_operand_nops() { + let code = compile_exec( + "\ +def f(self, out): + self.assertIn( + b'gnu' + (b'/123' * 125) + b'/longlink' + (b'/123' * 125) + b'/longname', + out) +", + ); + let f = find_code(&code, "f").expect("missing f code"); + + assert!( + !f.instructions + .iter() + .any(|unit| matches!(unit.op, Instruction::Nop)), + "expected CPython nop_out-style folded operands to have no surviving NOPs, got instructions={:?}", + f.instructions + ); + } + + #[test] + fn test_folded_binop_at_branch_body_start_does_not_leave_nop() { + let code = compile_exec( + "\ +def f(sys): + if sys.platform == 'win32': + component = 'd' * 25 + return component +", + ); + let f = find_code(&code, "f").expect("missing f code"); + let ops: Vec<_> = f + .instructions + .iter() + .map(|unit| unit.op) + .filter(|op| !matches!(op, Instruction::Cache)) + .collect(); + + assert!( + !ops.windows(3).any(|window| { + matches!( + window, + [ + Instruction::NotTaken, + Instruction::Nop, + Instruction::LoadConst { .. } + ] + ) + }), + "expected CPython nop_out-style folded branch body to drop operand NOP, got ops={ops:?}", + ); + } + + #[test] + fn test_multiline_unpack_target_uses_element_locations() { + let code = compile_exec( + "\ +def f(cm): + with cm as (_, + filename_2): + return filename_2 +", + ); + let f = find_code(&code, "f").expect("missing f code"); + let ops: Vec<_> = f + .instructions + .iter() + .map(|unit| unit.op) + .filter(|op| !matches!(op, Instruction::Cache)) + .collect(); + + assert!( + !ops.iter() + .any(|op| matches!(op, Instruction::StoreFastStoreFast { .. })), + "expected multiline target elements to keep separate STORE_FAST instructions, got ops={ops:?}", + ); + } + #[test] fn test_or_condition_in_jump_context_uses_shared_true_fallthrough() { let code = compile_exec( @@ -16037,6 +16885,54 @@ def f(code): ); } + #[test] + fn test_large_is_not_none_loop_guard_uses_direct_jump_back_false_path() { + let code = compile_exec( + "\ +def f(cls, _FIELDS, _PARAMS): + all_frozen_bases = None + any_frozen_base = False + has_dataclass_bases = False + for b in cls.__mro__[-1:0:-1]: + base_fields = getattr(b, _FIELDS, None) + if base_fields is not None: + has_dataclass_bases = True + for field in base_fields.values(): + name = field.name + if all_frozen_bases is None: + all_frozen_bases = True + current_frozen = getattr(b, _PARAMS).frozen + all_frozen_bases = all_frozen_bases and current_frozen + any_frozen_base = any_frozen_base or current_frozen +", + ); + let f = find_code(&code, "f").expect("missing f code"); + let ops: Vec<_> = f + .instructions + .iter() + .map(|unit| unit.op) + .filter(|op| !matches!(op, Instruction::Cache)) + .collect(); + + assert!( + ops.windows(6).any(|window| { + matches!( + window, + [ + Instruction::LoadFastBorrow { .. } | Instruction::LoadFast { .. }, + Instruction::PopJumpIfNotNone { .. }, + Instruction::NotTaken, + Instruction::JumpBackward { .. } + | Instruction::JumpBackwardNoInterrupt { .. }, + Instruction::LoadConst { .. }, + Instruction::StoreFast { .. }, + ] + ) + }), + "expected CPython-style direct jump-back false path for large 'is not None' loop body, got ops={ops:?}" + ); + } + #[test] fn test_continue_inside_with_keeps_line_marker_nop_before_exit_cleanup() { let code = compile_exec( @@ -16582,7 +17478,7 @@ def f(s, size, pos, errors): } #[test] - fn test_handler_resume_join_keeps_strong_load_fast_common_tail() { + fn test_handler_resume_join_keeps_borrow_in_common_tail() { let code = compile_exec( "\ def f(p, errors, s, pos, look, final, escape_start, st): @@ -16623,20 +17519,79 @@ def f(p, errors, s, pos, look, final, escape_start, st): .collect(); assert!( - !tail.iter().any(|op| { + matches!( + tail.as_slice(), + [ + Instruction::LoadFastBorrow { .. }, + Instruction::LoadAttr { .. }, + Instruction::LoadFastBorrow { .. }, + .., + ] + ), + "expected handler resume common tail to start with borrowed append receiver/arg loads, got tail={tail:?}" + ); + assert!( + tail.iter().any(|op| { matches!( op, - Instruction::LoadFastBorrow { .. } - | Instruction::LoadFastBorrowLoadFastBorrow { .. } + Instruction::LoadFastBorrowLoadFastBorrow { .. } + | Instruction::LoadFastBorrow { .. } ) }), - "expected handler resume common tail to keep strong LOAD_FAST ops, got tail={tail:?}" + "expected handler resume common tail to keep borrowed LOAD_FAST ops, got tail={tail:?}" + ); + } + + #[test] + fn test_multi_handler_guarded_resume_tail_keeps_borrow() { + let code = compile_exec( + "\ +def f(a): + try: + g() + except ValueError: + pass + except TypeError: + pass + if a: + return a.x + return 0 +", ); + let f = find_code(&code, "f").expect("missing f code"); + let ops: Vec<_> = f + .instructions + .iter() + .map(|unit| unit.op) + .filter(|op| !matches!(op, Instruction::Cache)) + .collect(); assert!( - tail.iter() - .any(|op| matches!(op, Instruction::LoadFastLoadFast { .. })), - "expected handler resume common tail to keep LOAD_FAST_LOAD_FAST ops, got tail={tail:?}" + ops.windows(5).any(|window| { + matches!( + window, + [ + Instruction::LoadFastBorrow { .. }, + Instruction::ToBool, + Instruction::PopJumpIfFalse { .. }, + Instruction::NotTaken, + Instruction::LoadFastBorrow { .. }, + ] + ) + }), + "expected guarded resume tail to keep borrowed guard/body loads, got ops={ops:?}" + ); + assert!( + ops.windows(2).any(|window| { + matches!( + window, + [ + Instruction::LoadFastBorrow { .. }, + Instruction::LoadAttr { .. } + ] + ) + }), + "expected guarded resume tail attr access to keep borrowed receiver, got ops={ops:?}" ); } @@ -16683,6 +17638,111 @@ def f(args): ); } + #[test] + fn test_named_except_cleanup_simple_resume_tail_keeps_borrow() { + let code = compile_exec( + "\ +def f(self): + try: + 1 / 0 + except Exception as e: + tb = e.__traceback__ + self.get_disassemble_as_string(tb.tb_frame.f_code, tb.tb_lasti) +", + ); + let f = find_code(&code, "f").expect("missing f code"); + let instructions: Vec<_> = f + .instructions + .iter() + .filter(|unit| !matches!(unit.op, Instruction::Cache)) + .collect(); + let attr_idx = instructions + .iter() + .position(|unit| match unit.op { + Instruction::LoadAttr { namei } => { + let load_attr = namei.get(OpArg::new(u32::from(u8::from(unit.arg)))); + f.names[usize::try_from(load_attr.name_idx()).unwrap()].as_str() + == "get_disassemble_as_string" + } + _ => false, + }) + .expect("missing LOAD_ATTR for get_disassemble_as_string"); + let ops: Vec<_> = instructions.iter().map(|unit| unit.op).collect(); + assert!( + matches!( + ops.get(attr_idx - 1), + Some(Instruction::LoadFastBorrow { .. }) + ), + "expected named-except resume tail to keep borrowed self load, got ops={ops:?}" + ); + assert!( + matches!( + ops.get(attr_idx + 4), + Some(Instruction::LoadFastBorrow { .. }) + ), + "expected named-except resume tail to keep borrowed tb load, got ops={ops:?}" + ); + } + + #[test] + fn test_named_except_conditional_reraise_deopts_with_chain_tail() { + let code = compile_exec( + "\ +def f(self, arc, tmp_filename, new_mode): + try: + os.chmod(tmp_filename, new_mode) + except OSError as exc: + if exc.errno == ERR: + self.skipTest() + else: + raise + with self.check_context(arc.open(), 'fully_trusted'): + self.expect_file('a') + with self.check_context(arc.open(), 'tar'): + self.expect_file('b') +", + ); + let f = find_code(&code, "f").expect("missing f code"); + let instructions: Vec<_> = f + .instructions + .iter() + .filter(|unit| !matches!(unit.op, Instruction::Cache)) + .collect(); + let first_check_context = instructions + .iter() + .position(|unit| match unit.op { + Instruction::LoadAttr { namei } => { + let load_attr = namei.get(OpArg::new(u32::from(u8::from(unit.arg)))); + f.names[usize::try_from(load_attr.name_idx()).unwrap()].as_str() + == "check_context" + } + _ => false, + }) + .expect("missing check_context load"); + let first_handler = instructions + .iter() + .position(|unit| matches!(unit.op, Instruction::PushExcInfo)) + .unwrap_or(instructions.len()); + let warm_tail = &instructions[first_check_context.saturating_sub(1)..first_handler]; + + assert!( + warm_tail + .iter() + .any(|unit| matches!(unit.op, Instruction::LoadFast { .. })), + "expected conditional named-except reraise tail to use strong LOAD_FAST ops, got tail={warm_tail:?}" + ); + assert!( + warm_tail.iter().all(|unit| { + !matches!( + unit.op, + Instruction::LoadFastBorrow { .. } + | Instruction::LoadFastBorrowLoadFastBorrow { .. } + ) + }), + "expected all warm with-chain tail loads to stay strong after named-except reraise, got tail={warm_tail:?}" + ); + } + #[test] fn test_named_except_cleanup_deopts_same_guard_fallbacks_not_outer_tail() { let code = compile_exec( diff --git a/crates/codegen/src/ir.rs b/crates/codegen/src/ir.rs index caeff49377d..39c970ba029 100644 --- a/crates/codegen/src/ir.rs +++ b/crates/codegen/src/ir.rs @@ -119,6 +119,13 @@ pub struct InstructionInfo { pub lineno_override: Option, /// Number of CACHE code units emitted after this instruction pub cache_entries: u32, + /// Preserve a redundant jump until final emission so a zero-width jump + /// materializes as a line-marker NOP, matching CPython's late CFG shape. + pub preserve_redundant_jump_as_nop: bool, + /// Drop this NOP before line propagation if it still has no location. + pub remove_no_location_nop: bool, + /// Keep this no-location NOP until line propagation when it starts a block. + pub preserve_block_start_no_location_nop: bool, } /// Exception handler information for an instruction. @@ -138,6 +145,15 @@ fn set_to_nop(info: &mut InstructionInfo) { info.target = BlockIdx::NULL; info.folded_from_nonliteral_expr = false; info.cache_entries = 0; + info.preserve_redundant_jump_as_nop = false; + info.remove_no_location_nop = false; + info.preserve_block_start_no_location_nop = false; +} + +fn nop_out_no_location(info: &mut InstructionInfo) { + set_to_nop(info); + info.lineno_override = Some(-1); + info.remove_no_location_nop = true; } fn is_named_except_cleanup_normal_exit_block(block: &Block) -> bool { @@ -343,9 +359,11 @@ impl CodeInfo { self.deoptimize_borrow_after_named_except_cleanup_join(); self.deoptimize_borrow_in_protected_conditional_tail(); self.deoptimize_borrow_after_protected_import(); + self.deoptimize_borrow_after_protected_store_tail(); self.deoptimize_borrow_after_push_exc_info(); self.deoptimize_borrow_for_handler_return_paths(); self.deoptimize_borrow_for_match_keys_attr(); + self.deoptimize_borrow_in_protected_attr_chain_tail(); self.deoptimize_store_fast_store_fast_after_cleanup(); self.apply_static_swaps(); self.deoptimize_store_fast_store_fast_after_cleanup(); @@ -534,46 +552,60 @@ impl CodeInfo { if target != BlockIdx::NULL { let target_offset = block_to_offset[target.idx()].as_u32(); - // Direction must be based on concrete instruction offsets. - // Empty blocks can share offsets, so block-order-based resolution - // may classify some jumps incorrectly. - op = match op.into() { - Opcode::JumpForward if target_offset <= current_offset => { - Opcode::JumpBackward.into() - } - Opcode::JumpBackward if target_offset > current_offset => { - Opcode::JumpForward.into() - } - Opcode::JumpBackwardNoInterrupt if target_offset > current_offset => { - Opcode::JumpForward.into() - } - _ => op, - }; - info.instr = op.into(); - let updated_cache = op.cache_entries() as u32; - recompile |= updated_cache != old_cache_entries; - info.cache_entries = updated_cache; - let new_arg = if matches!(op, Instruction::EndAsyncFor) { - let arg = offset_after - .checked_sub(target_offset + END_SEND_OFFSET) - .expect("END_ASYNC_FOR target must be before instruction"); - OpArg::new(arg) - } else if matches!( - op.into(), - Opcode::JumpBackward | Opcode::JumpBackwardNoInterrupt - ) { - let arg = offset_after - .checked_sub(target_offset) - .expect("backward jump target must be before instruction"); - OpArg::new(arg) + if info.instr.is_unconditional_jump() && target_offset == offset_after { + op = Opcode::Nop.into(); + info.instr = op.into(); + info.target = BlockIdx::NULL; + let updated_cache = op.cache_entries() as u32; + recompile |= updated_cache != old_cache_entries; + info.cache_entries = updated_cache; + let new_arg = OpArg::NULL; + recompile |= new_arg.instr_size() != old_arg_size; + info.arg = new_arg; } else { - let arg = target_offset - .checked_sub(offset_after) - .expect("forward jump target must be after instruction"); - OpArg::new(arg) - }; - recompile |= new_arg.instr_size() != old_arg_size; - info.arg = new_arg; + // Direction must be based on concrete instruction offsets. + // Empty blocks can share offsets, so block-order-based resolution + // may classify some jumps incorrectly. + op = match op.into() { + Opcode::JumpForward if target_offset <= current_offset => { + Opcode::JumpBackward.into() + } + Opcode::JumpBackward if target_offset > current_offset => { + Opcode::JumpForward.into() + } + Opcode::JumpBackwardNoInterrupt + if target_offset > current_offset => + { + Opcode::JumpForward.into() + } + _ => op, + }; + info.instr = op.into(); + let updated_cache = op.cache_entries() as u32; + recompile |= updated_cache != old_cache_entries; + info.cache_entries = updated_cache; + let new_arg = if matches!(op, Instruction::EndAsyncFor) { + let arg = offset_after + .checked_sub(target_offset + END_SEND_OFFSET) + .expect("END_ASYNC_FOR target must be before instruction"); + OpArg::new(arg) + } else if matches!( + op.into(), + Opcode::JumpBackward | Opcode::JumpBackwardNoInterrupt + ) { + let arg = offset_after + .checked_sub(target_offset) + .expect("backward jump target must be before instruction"); + OpArg::new(arg) + } else { + let arg = target_offset + .checked_sub(offset_after) + .expect("forward jump target must be after instruction"); + OpArg::new(arg) + }; + recompile |= new_arg.instr_size() != old_arg_size; + info.arg = new_arg; + } } let cache_count = info.cache_entries as usize; @@ -913,10 +945,7 @@ impl CodeInfo { && let Some(folded_const) = Self::eval_unary_constant(&operand, op, intrinsic) { let (const_idx, _) = self.metadata.consts.insert_full(folded_const); - set_to_nop(&mut block.instructions[operand_index]); - block.instructions[operand_index].location = block.instructions[i].location; - block.instructions[operand_index].end_location = - block.instructions[i].end_location; + nop_out_no_location(&mut block.instructions[operand_index]); let mut prev = operand_index; while let Some(idx) = prev.checked_sub(1) { if !matches!(block.instructions[idx].instr.real(), Some(Instruction::Nop)) { @@ -1055,9 +1084,7 @@ impl CodeInfo { .iter() .any(|&idx| block.instructions[idx].folded_from_nonliteral_expr); for &idx in &operand_indices { - set_to_nop(&mut block.instructions[idx]); - block.instructions[idx].location = block.instructions[i].location; - block.instructions[idx].end_location = block.instructions[i].end_location; + nop_out_no_location(&mut block.instructions[idx]); } block.instructions[i].instr = Instruction::LoadConst { consti: Arg::marker(), @@ -2520,17 +2547,56 @@ impl CodeInfo { /// Remove NOP instructions from all blocks, but keep NOPs that introduce /// a new source line (they serve as line markers for monitoring LINE events). fn remove_nops(&mut self) { - for block in &mut self.blocks { + fn ends_with_for_cleanup(block: &Block) -> bool { + let mut reals = block + .instructions + .iter() + .rev() + .filter_map(|info| info.instr.real()); + matches!( + (reals.next(), reals.next()), + (Some(Instruction::PopIter), Some(Instruction::EndFor)) + ) + } + + let jump_targets: Vec<_> = (0..self.blocks.len()) + .map(|target_idx| has_jump_predecessor(&self.blocks, BlockIdx(target_idx as u32))) + .collect(); + let mut fallthrough_predecessors = vec![None; self.blocks.len()]; + for (pred_idx, block) in self.blocks.iter().enumerate() { + if block.next != BlockIdx::NULL { + fallthrough_predecessors[block.next.idx()] = Some(pred_idx); + } + } + let starts_after_for_cleanup: Vec<_> = fallthrough_predecessors + .iter() + .map(|pred_idx| pred_idx.is_some_and(|idx| ends_with_for_cleanup(&self.blocks[idx]))) + .collect(); + for (block_idx, block) in self.blocks.iter_mut().enumerate() { let mut prev_line = None; + let mut src = 0usize; block.instructions.retain(|ins| { - if matches!(ins.instr.real(), Some(Instruction::Nop)) { - let line = ins.location.line.get() as i32; - if prev_line == Some(line) { - return false; + let keep = 'keep: { + if matches!(ins.instr.real(), Some(Instruction::Nop)) { + if ins.remove_no_location_nop + && instruction_lineno(ins) < 0 + && !(src == 0 + && (jump_targets[block_idx] + || (ins.preserve_block_start_no_location_nop + && !starts_after_for_cleanup[block_idx]))) + { + break 'keep false; + } + let line = ins.location.line.get() as i32; + if prev_line == Some(line) { + break 'keep false; + } } - } - prev_line = Some(instruction_lineno(ins)); - true + prev_line = Some(instruction_lineno(ins)); + true + }; + src += 1; + keep }); } } @@ -3039,14 +3105,17 @@ impl CodeInfo { } fn deoptimize_borrow_after_multi_handler_resume_join(&mut self) { - fn second_last_real_instr(block: &Block) -> Option { - let mut reals = block + fn is_handler_resume_jump_block(block: &Block) -> bool { + let Some(last_info) = block.instructions.last() else { + return false; + }; + if last_info.target == BlockIdx::NULL || !last_info.instr.is_unconditional_jump() { + return false; + } + block .instructions .iter() - .rev() - .filter_map(|info| info.instr.real()); - let _last = reals.next()?; - reals.next() + .any(|info| matches!(info.instr.real(), Some(Instruction::PopExcept))) } fn deoptimize_block_borrows(block: &mut Block) { @@ -3069,21 +3138,57 @@ impl CodeInfo { } } + fn starts_with_conditional_guard(block: &Block) -> bool { + let infos: Vec<_> = block + .instructions + .iter() + .filter(|info| info.instr.real().is_some()) + .take(3) + .collect(); + if infos.len() < 2 { + return false; + } + let starts_with_load_fast = matches!( + infos[0].instr.real(), + Some(Instruction::LoadFast { .. } | Instruction::LoadFastBorrow { .. }) + ); + if !starts_with_load_fast { + return false; + } + matches!( + infos.get(1).and_then(|info| info.instr.real()), + Some( + Instruction::PopJumpIfFalse { .. } + | Instruction::PopJumpIfTrue { .. } + | Instruction::PopJumpIfNone { .. } + | Instruction::PopJumpIfNotNone { .. } + ) + ) || (matches!(infos[1].instr.real(), Some(Instruction::ToBool)) + && matches!( + infos.get(2).and_then(|info| info.instr.real()), + Some( + Instruction::PopJumpIfFalse { .. } + | Instruction::PopJumpIfTrue { .. } + | Instruction::PopJumpIfNone { .. } + | Instruction::PopJumpIfNotNone { .. } + ) + )) + } + let mut handler_resume_predecessors = vec![0usize; self.blocks.len()]; let mut is_handler_resume_block = vec![false; self.blocks.len()]; let mut predecessors = vec![Vec::new(); self.blocks.len()]; for (block_idx, block) in self.blocks.iter().enumerate() { - let Some(last_info) = block.instructions.last() else { - continue; - }; - if last_info.target == BlockIdx::NULL || !last_info.instr.is_unconditional_jump() { - continue; - } - if !matches!(second_last_real_instr(block), Some(Instruction::PopExcept)) { + if !is_handler_resume_jump_block(block) { continue; } is_handler_resume_block[block_idx] = true; - handler_resume_predecessors[last_info.target.idx()] += 1; + let target = block + .instructions + .last() + .expect("resume jump block has a last instruction") + .target; + handler_resume_predecessors[target.idx()] += 1; } for (pred_idx, block) in self.blocks.iter().enumerate() { if block.next != BlockIdx::NULL { @@ -3111,6 +3216,31 @@ impl CodeInfo { segment.push(cursor); cursor = self.blocks[cursor.idx()].next; } + let has_complex_tail = segment.iter().any(|block_idx| { + self.blocks[block_idx.idx()] + .instructions + .iter() + .any(|info| { + matches!( + info.instr.real(), + Some( + Instruction::ForIter { .. } + | Instruction::JumpBackward { .. } + | Instruction::JumpBackwardNoInterrupt { .. } + | Instruction::EndFor + | Instruction::PopIter + | Instruction::LoadFastAndClear { .. } + | Instruction::LoadFastCheck { .. } + | Instruction::ListAppend { .. } + | Instruction::MapAdd { .. } + | Instruction::SetAdd { .. } + ) + ) + }) + }); + if starts_with_conditional_guard(&self.blocks[seed.idx()]) && !has_complex_tail { + continue; + } let mut in_segment = vec![false; self.blocks.len()]; for block_idx in &segment { @@ -3193,8 +3323,141 @@ impl CodeInfo { } } + fn normal_successors(block: &Block) -> Vec { + let Some(last_info) = block.instructions.last() else { + return (block.next != BlockIdx::NULL) + .then_some(block.next) + .into_iter() + .collect(); + }; + if let Some(cond_idx) = trailing_conditional_jump_index(block) { + let mut successors = Vec::with_capacity(2); + let target = block.instructions[cond_idx].target; + if target != BlockIdx::NULL { + successors.push(target); + } + if block.next != BlockIdx::NULL { + successors.push(block.next); + } + return successors; + } + if last_info.instr.is_scope_exit() { + return Vec::new(); + } + if last_info.instr.is_unconditional_jump() { + return (last_info.target != BlockIdx::NULL) + .then_some(last_info.target) + .into_iter() + .collect(); + } + (block.next != BlockIdx::NULL) + .then_some(block.next) + .into_iter() + .collect() + } + + fn path_reaches_named_cleanup( + blocks: &[Block], + start: BlockIdx, + cleanup: BlockIdx, + resume_target: BlockIdx, + ) -> bool { + if start == BlockIdx::NULL || start == resume_target { + return false; + } + let mut visited = vec![false; blocks.len()]; + let mut stack = vec![start]; + while let Some(block_idx) = stack.pop() { + if block_idx == BlockIdx::NULL + || block_idx == resume_target + || visited[block_idx.idx()] + { + continue; + } + if block_idx == cleanup { + return true; + } + visited[block_idx.idx()] = true; + for successor in normal_successors(&blocks[block_idx.idx()]) { + stack.push(successor); + } + } + false + } + + fn path_reaches_explicit_raise( + blocks: &[Block], + start: BlockIdx, + cleanup: BlockIdx, + resume_target: BlockIdx, + ) -> bool { + if start == BlockIdx::NULL || start == cleanup || start == resume_target { + return false; + } + let mut visited = vec![false; blocks.len()]; + let mut stack = vec![start]; + while let Some(block_idx) = stack.pop() { + if block_idx == BlockIdx::NULL + || block_idx == cleanup + || block_idx == resume_target + || visited[block_idx.idx()] + { + continue; + } + let block = &blocks[block_idx.idx()]; + if block + .instructions + .iter() + .any(|info| matches!(info.instr.real(), Some(Instruction::RaiseVarargs { .. }))) + { + return true; + } + visited[block_idx.idx()] = true; + for successor in normal_successors(block) { + stack.push(successor); + } + } + false + } + + fn named_cleanup_has_conditional_raise_sibling( + blocks: &[Block], + cleanup: BlockIdx, + resume_target: BlockIdx, + ) -> bool { + for block in blocks { + let Some(cond_idx) = trailing_conditional_jump_index(block) else { + continue; + }; + let jump_target = block.instructions[cond_idx].target; + let fallthrough = block.next; + if jump_target == BlockIdx::NULL || fallthrough == BlockIdx::NULL { + continue; + } + + let jump_reaches_cleanup = + path_reaches_named_cleanup(blocks, jump_target, cleanup, resume_target); + let fallthrough_reaches_cleanup = + path_reaches_named_cleanup(blocks, fallthrough, cleanup, resume_target); + if jump_reaches_cleanup == fallthrough_reaches_cleanup { + continue; + } + + let sibling = if jump_reaches_cleanup { + fallthrough + } else { + jump_target + }; + if path_reaches_explicit_raise(blocks, sibling, cleanup, resume_target) { + return true; + } + } + false + } + let mut named_cleanup_predecessors = vec![0usize; self.blocks.len()]; - let mut is_named_cleanup_resume_block = vec![false; self.blocks.len()]; + let mut named_cleanup_requires_deopt = vec![false; self.blocks.len()]; + let mut is_allowed_cleanup_resume_block = vec![false; self.blocks.len()]; let mut predecessors = vec![Vec::new(); self.blocks.len()]; for (block_idx, block) in self.blocks.iter().enumerate() { @@ -3204,6 +3467,13 @@ impl CodeInfo { if last_info.target == BlockIdx::NULL || !last_info.instr.is_unconditional_jump() { continue; } + if block + .instructions + .iter() + .any(|info| matches!(info.instr.real(), Some(Instruction::PopExcept))) + { + is_allowed_cleanup_resume_block[block_idx] = true; + } if !is_named_except_cleanup_normal_exit_block(block) { continue; } @@ -3213,8 +3483,14 @@ impl CodeInfo { ) { continue; } - is_named_cleanup_resume_block[block_idx] = true; named_cleanup_predecessors[last_info.target.idx()] += 1; + if named_cleanup_has_conditional_raise_sibling( + &self.blocks, + BlockIdx::new(block_idx as u32), + last_info.target, + ) { + named_cleanup_requires_deopt[last_info.target.idx()] = true; + } } for (pred_idx, block) in self.blocks.iter().enumerate() { if block.next != BlockIdx::NULL { @@ -3253,6 +3529,9 @@ impl CodeInfo { segment.push(cursor); cursor = block.next; } + if fallback_guard_local.is_none() && !named_cleanup_requires_deopt[idx] { + continue; + } let mut in_segment = vec![false; self.blocks.len()]; for block_idx in &segment { @@ -3269,7 +3548,7 @@ impl CodeInfo { if block_idx != seed && !is_same_guard_fallback && predecessors[block_idx.idx()].iter().any(|pred| { - !in_segment[pred.idx()] && !is_named_cleanup_resume_block[pred.idx()] + !in_segment[pred.idx()] && !is_allowed_cleanup_resume_block[pred.idx()] }) { continue; @@ -3612,24 +3891,6 @@ impl CodeInfo { cursor = self.blocks[cursor.idx()].next; } - let has_backward_jump = segment.iter().any(|block_idx| { - self.blocks[block_idx.idx()] - .instructions - .iter() - .any(|info| { - matches!( - info.instr.real(), - Some( - Instruction::JumpBackward { .. } - | Instruction::JumpBackwardNoInterrupt { .. } - ) - ) - }) - }); - if !has_backward_jump { - continue; - } - for (i, block_idx) in segment.into_iter().enumerate() { if visited[block_idx.idx()] { continue; @@ -3640,87 +3901,784 @@ impl CodeInfo { } } - fn deoptimize_borrow_for_match_keys_attr(&mut self) { - let Some(key_name_idx) = self.metadata.names.get_index_of("KEY") else { - return; - }; - - let mut to_deopt = Vec::new(); - for block_idx in 0..self.blocks.len() { - let block = &self.blocks[block_idx]; - let len = block.instructions.len(); - for i in 0..len { - let Some(Instruction::LoadFastBorrow { .. }) = block.instructions[i].instr.real() - else { - continue; - }; - let Some(Instruction::LoadAttr { namei }) = block - .instructions - .get(i + 1) - .and_then(|info| info.instr.real()) - else { - continue; - }; - let load_attr = namei.get(block.instructions[i + 1].arg); - if load_attr.is_method() || load_attr.name_idx() as usize != key_name_idx { - continue; - } - - let mut saw_build_tuple = false; - let mut saw_match_keys = false; - let mut scan_block_idx = block_idx; - let mut scan_start = i + 2; - loop { - let scan_block = &self.blocks[scan_block_idx]; - for info in scan_block.instructions.iter().skip(scan_start) { - match info.instr.real() { - Some( - Instruction::LoadConst { .. } - | Instruction::LoadSmallInt { .. } - | Instruction::LoadFast { .. } - | Instruction::LoadFastBorrow { .. } - | Instruction::LoadAttr { .. } - | Instruction::Nop, - ) => {} - Some(Instruction::BuildTuple { .. }) => saw_build_tuple = true, - Some(Instruction::MatchKeys) => { - saw_match_keys = true; - break; - } - _ => { - saw_build_tuple = false; - break; - } + fn deoptimize_borrow_after_protected_store_tail(&mut self) { + fn deoptimize_block_borrows_from(block: &mut Block, start: usize) { + for info in block.instructions.iter_mut().skip(start) { + match info.instr.real() { + Some(Instruction::LoadFastBorrow { .. }) => { + info.instr = Instruction::LoadFast { + var_num: Arg::marker(), } + .into(); } - if saw_match_keys { - break; - } - let Some(last) = scan_block.instructions.last() else { - break; - }; - if scan_block.next == BlockIdx::NULL - || last.instr.is_scope_exit() - || last.instr.is_unconditional_jump() - || last.target != BlockIdx::NULL - { - break; + Some(Instruction::LoadFastBorrowLoadFastBorrow { .. }) => { + info.instr = Instruction::LoadFastLoadFast { + var_nums: Arg::marker(), + } + .into(); } - scan_block_idx = scan_block.next.idx(); - scan_start = 0; + _ => {} + } + } + } + + fn is_handler_resume_predecessor(block: &Block, target: BlockIdx) -> bool { + let has_pop_except = block + .instructions + .iter() + .any(|info| matches!(info.instr.real(), Some(Instruction::PopExcept))); + let jumps_to_target = block.instructions.iter().any(|info| { + info.target == target + && matches!( + info.instr.real(), + Some( + Instruction::JumpForward { .. } + | Instruction::JumpBackward { .. } + | Instruction::JumpBackwardNoInterrupt { .. } + ) + ) + }); + has_pop_except && jumps_to_target + } + + fn handler_chain_can_resume_to_segment( + blocks: &[Block], + block: &Block, + in_segment: &[bool], + ) -> bool { + let mut visited = vec![false; blocks.len()]; + let handler_blocks: Vec<_> = block + .instructions + .iter() + .filter_map(|info| info.except_handler.map(|handler| handler.handler_block)) + .collect(); + for handler_block in handler_blocks { + let mut cursor = handler_block; + while cursor != BlockIdx::NULL && !visited[cursor.idx()] { + visited[cursor.idx()] = true; + let handler = &blocks[cursor.idx()]; + let mut after_pop_except = false; + for info in &handler.instructions { + if matches!(info.instr.real(), Some(Instruction::PopExcept)) { + after_pop_except = true; + continue; + } + if after_pop_except + && info.target != BlockIdx::NULL + && in_segment[info.target.idx()] + && matches!( + info.instr.real(), + Some( + Instruction::JumpForward { .. } + | Instruction::JumpBackward { .. } + | Instruction::JumpBackwardNoInterrupt { .. } + ) + ) + { + return true; + } + } + cursor = handler.next; + } + } + false + } + + fn block_has_tail_deopt_trigger_from(block: &Block, start: usize) -> bool { + block.instructions.iter().skip(start).any(|info| { + matches!( + info.instr.real(), + Some( + Instruction::Call { .. } + | Instruction::CallKw { .. } + | Instruction::StoreAttr { .. } + ) + ) + }) + } + + fn block_has_protected_instructions(block: &Block) -> bool { + block + .instructions + .iter() + .any(|info| info.except_handler.is_some()) + } + + fn first_unprotected_suffix(block: &Block) -> Option { + let mut saw_protected = false; + for (idx, info) in block.instructions.iter().enumerate() { + if info.except_handler.is_some() { + saw_protected = true; + } else if saw_protected { + return Some(idx); + } + } + None + } + + fn collect_stored_fast_locals_until(block: &Block, end: usize) -> Vec { + let mut locals = Vec::new(); + for info in block.instructions.iter().take(end) { + match info.instr.real() { + Some(Instruction::StoreFast { var_num }) => { + locals.push(usize::from(var_num.get(info.arg))); + } + Some(Instruction::StoreFastLoadFast { var_nums }) => { + let (store_idx, _) = var_nums.get(info.arg).indexes(); + locals.push(usize::from(store_idx)); + } + Some(Instruction::StoreFastStoreFast { var_nums }) => { + let (idx1, idx2) = var_nums.get(info.arg).indexes(); + locals.push(usize::from(idx1)); + locals.push(usize::from(idx2)); + } + _ => {} + } + } + locals + } + + fn borrows_any_local_from(block: &Block, locals: &[usize], start: usize) -> bool { + block + .instructions + .iter() + .skip(start) + .any(|info| match info.instr.real() { + Some(Instruction::LoadFastBorrow { var_num }) => { + locals.contains(&usize::from(var_num.get(info.arg))) + } + Some(Instruction::LoadFastBorrowLoadFastBorrow { var_nums }) => { + let (idx1, idx2) = var_nums.get(info.arg).indexes(); + locals.contains(&usize::from(idx1)) || locals.contains(&usize::from(idx2)) + } + _ => false, + }) + } + + let mut predecessors = vec![Vec::new(); self.blocks.len()]; + for (pred_idx, block) in self.blocks.iter().enumerate() { + if block.next != BlockIdx::NULL { + predecessors[block.next.idx()].push(BlockIdx::new(pred_idx as u32)); + } + for info in &block.instructions { + if info.target != BlockIdx::NULL { + predecessors[info.target.idx()].push(BlockIdx::new(pred_idx as u32)); + } + } + } + + let mut to_deopt = Vec::new(); + for block in &self.blocks { + if block_is_exceptional(block) + || !block + .instructions + .iter() + .any(|info| info.except_handler.is_some()) + || !block.instructions.iter().any(|info| { + matches!( + info.instr.real(), + Some(Instruction::Call { .. } | Instruction::CallKw { .. }) + ) + }) + || !block_has_exception_match_handler(&self.blocks, block) + { + continue; + } + let same_block_tail_start = first_unprotected_suffix(block); + if same_block_tail_start.is_some() { + continue; + } + let stored_locals = collect_stored_fast_locals_until(block, block.instructions.len()); + if stored_locals.is_empty() { + continue; + } + let mut in_segment = vec![false; self.blocks.len()]; + let mut segment = Vec::new(); + let mut cursor = { + let tail = next_nonempty_block(&self.blocks, block.next); + if tail == BlockIdx::NULL + || block_is_exceptional(&self.blocks[tail.idx()]) + || block_has_protected_instructions(&self.blocks[tail.idx()]) + { + continue; + } + tail + }; + while cursor != BlockIdx::NULL { + let segment_block = &self.blocks[cursor.idx()]; + if block_is_exceptional(segment_block) + || block_has_protected_instructions(segment_block) + { + break; + } + segment.push((cursor, 0)); + in_segment[cursor.idx()] = true; + let last_real = segment_block + .instructions + .iter() + .rev() + .find_map(|info| info.instr.real()); + if last_real.is_some_and(|instr| { + instr.is_scope_exit() || AnyInstruction::Real(instr).is_unconditional_jump() + }) { + break; + } + cursor = next_nonempty_block(&self.blocks, segment_block.next); + } + if segment.is_empty() + || !segment.iter().any(|(block_idx, start)| { + block_has_tail_deopt_trigger_from(&self.blocks[block_idx.idx()], *start) + }) + || handler_chain_can_resume_to_segment(&self.blocks, block, &in_segment) + || !segment.iter().any(|(block_idx, start)| { + borrows_any_local_from(&self.blocks[block_idx.idx()], &stored_locals, *start) + }) + { + continue; + } + for (block_idx, start) in segment { + if predecessors[block_idx.idx()] + .iter() + .any(|pred| is_handler_resume_predecessor(&self.blocks[pred.idx()], block_idx)) + { + continue; + } + to_deopt.push((block_idx, start)); + } + } + + let mut continue_targets = Vec::new(); + for (handler_idx, block) in self.blocks.iter().enumerate() { + if !block.cold + || !block + .instructions + .iter() + .any(|info| matches!(info.instr.real(), Some(Instruction::CheckExcMatch))) + { + continue; + } + let mut visited = vec![false; self.blocks.len()]; + let mut cursor = BlockIdx::new(handler_idx as u32); + while cursor != BlockIdx::NULL && !visited[cursor.idx()] { + visited[cursor.idx()] = true; + let handler = &self.blocks[cursor.idx()]; + let has_pop_except = handler + .instructions + .iter() + .any(|info| matches!(info.instr.real(), Some(Instruction::PopExcept))); + if has_pop_except { + for info in &handler.instructions { + if info.target != BlockIdx::NULL + && matches!( + info.instr.real(), + Some( + Instruction::JumpBackward { .. } + | Instruction::JumpBackwardNoInterrupt { .. } + ) + ) + { + continue_targets.push(info.target); + } + } + } + cursor = handler.next; + } + } + + continue_targets.sort_by_key(|idx| idx.idx()); + continue_targets.dedup(); + for target in continue_targets { + let block = &self.blocks[target.idx()]; + if block.cold + || block_is_exceptional(block) + || !block_has_tail_deopt_trigger_from(block, 0) + { + continue; + } + let stored_locals = collect_stored_fast_locals_until(block, block.instructions.len()); + if stored_locals.is_empty() { + continue; + } + let tail = next_nonempty_block(&self.blocks, block.next); + if tail == BlockIdx::NULL + || block_is_exceptional(&self.blocks[tail.idx()]) + || !block_has_tail_deopt_trigger_from(&self.blocks[tail.idx()], 0) + || !borrows_any_local_from(&self.blocks[tail.idx()], &stored_locals, 0) + { + continue; + } + let tail_jumps_back_to_target = + self.blocks[tail.idx()].instructions.iter().any(|info| { + info.target == target + && matches!( + info.instr.real(), + Some( + Instruction::JumpBackward { .. } + | Instruction::JumpBackwardNoInterrupt { .. } + ) + ) + }); + if tail_jumps_back_to_target { + to_deopt.push((tail, 0)); + } + } + + to_deopt.sort_by_key(|(idx, start)| (idx.idx(), *start)); + let mut merged: Vec<(BlockIdx, usize)> = Vec::new(); + for (idx, start) in to_deopt { + match merged.last_mut() { + Some((last_idx, last_start)) if *last_idx == idx => { + *last_start = (*last_start).min(start); + } + _ => merged.push((idx, start)), + } + } + for (block_idx, start) in merged { + deoptimize_block_borrows_from(&mut self.blocks[block_idx.idx()], start); + } + } + + fn deoptimize_borrow_for_match_keys_attr(&mut self) { + let Some(key_name_idx) = self.metadata.names.get_index_of("KEY") else { + return; + }; + + let mut to_deopt = Vec::new(); + for block_idx in 0..self.blocks.len() { + let block = &self.blocks[block_idx]; + let len = block.instructions.len(); + for i in 0..len { + let Some(Instruction::LoadFastBorrow { .. }) = block.instructions[i].instr.real() + else { + continue; + }; + let Some(Instruction::LoadAttr { namei }) = block + .instructions + .get(i + 1) + .and_then(|info| info.instr.real()) + else { + continue; + }; + let load_attr = namei.get(block.instructions[i + 1].arg); + if load_attr.is_method() || load_attr.name_idx() as usize != key_name_idx { + continue; + } + + let mut saw_build_tuple = false; + let mut saw_match_keys = false; + let mut scan_block_idx = block_idx; + let mut scan_start = i + 2; + loop { + let scan_block = &self.blocks[scan_block_idx]; + for info in scan_block.instructions.iter().skip(scan_start) { + match info.instr.real() { + Some( + Instruction::LoadConst { .. } + | Instruction::LoadSmallInt { .. } + | Instruction::LoadFast { .. } + | Instruction::LoadFastBorrow { .. } + | Instruction::LoadAttr { .. } + | Instruction::Nop, + ) => {} + Some(Instruction::BuildTuple { .. }) => saw_build_tuple = true, + Some(Instruction::MatchKeys) => { + saw_match_keys = true; + break; + } + _ => { + saw_build_tuple = false; + break; + } + } + } + if saw_match_keys { + break; + } + let Some(last) = scan_block.instructions.last() else { + break; + }; + if scan_block.next == BlockIdx::NULL + || last.instr.is_scope_exit() + || last.instr.is_unconditional_jump() + || last.target != BlockIdx::NULL + { + break; + } + scan_block_idx = scan_block.next.idx(); + scan_start = 0; + } + + if saw_build_tuple && saw_match_keys { + to_deopt.push((block_idx, i)); + } + } + } + + for (block_idx, instr_idx) in to_deopt { + self.blocks[block_idx].instructions[instr_idx].instr = Instruction::LoadFast { + var_num: Arg::marker(), + } + .into(); + } + } + + fn deoptimize_borrow_in_protected_attr_chain_tail(&mut self) { + fn second_last_real_instr(block: &Block) -> Option { + let mut reals = block + .instructions + .iter() + .rev() + .filter_map(|info| info.instr.real()); + let _last = reals.next()?; + reals.next() + } + + fn deoptimize_borrow(info: &mut InstructionInfo) { + match info.instr.real() { + Some(Instruction::LoadFastBorrow { .. }) => { + info.instr = Instruction::LoadFast { + var_num: Arg::marker(), + } + .into(); + } + Some(Instruction::LoadFastBorrowLoadFastBorrow { .. }) => { + info.instr = Instruction::LoadFastLoadFast { + var_nums: Arg::marker(), + } + .into(); + } + _ => {} + } + } + + fn is_attr_load(instr: Instruction) -> bool { + matches!( + instr, + Instruction::LoadAttr { .. } | Instruction::LoadSuperAttr { .. } + ) + } + + fn attr_load_is_method(info: InstructionInfo) -> bool { + match info.instr.real() { + Some(Instruction::LoadAttr { namei }) => namei.get(info.arg).is_method(), + Some(Instruction::LoadSuperAttr { namei }) => namei.get(info.arg).is_load_method(), + _ => false, + } + } + + fn is_subscript_index_setup(instr: Instruction) -> bool { + matches!( + instr, + Instruction::LoadConst { .. } + | Instruction::LoadSmallInt { .. } + | Instruction::LoadFast { .. } + | Instruction::LoadFastBorrow { .. } + | Instruction::LoadFastCheck { .. } + | Instruction::Nop + ) + } + + enum DeoptKind { + ReturnIter { tail_start_idx: usize }, + Subscript { binary_op_idx: usize }, + } + + fn should_deopt_borrowed_attr_chain( + real_instrs: &[(usize, InstructionInfo)], + load_idx: usize, + ) -> Option { + let mut cursor = load_idx + 1; + if !real_instrs + .get(cursor) + .is_some_and(|(_, info)| info.instr.real().is_some_and(is_attr_load)) + { + return None; + } + let mut last_attr_is_method = false; + while let Some((_, info)) = real_instrs.get(cursor) { + if !info.instr.real().is_some_and(is_attr_load) { + break; + } + last_attr_is_method = attr_load_is_method(*info); + cursor += 1; + } + + let (_, next_info) = real_instrs.get(cursor)?; + + match next_info.instr.real() { + Some(Instruction::GetIter) => Some(DeoptKind::ReturnIter { + tail_start_idx: cursor + 1, + }), + Some(Instruction::Call { .. }) => real_instrs + .get(cursor + 1) + .and_then(|(_, info)| info.instr.real()) + .and_then(|instr| { + matches!(instr, Instruction::GetIter).then_some(DeoptKind::ReturnIter { + tail_start_idx: cursor + 2, + }) + }), + _ => { + if last_attr_is_method { + return None; + } + while real_instrs.get(cursor).is_some_and(|(_, info)| { + info.instr.real().is_some_and(is_subscript_index_setup) + }) { + cursor += 1; + } + real_instrs.get(cursor).and_then(|(_, info)| { + matches!( + info.instr.real(), + Some(Instruction::BinaryOp { op }) + if op.get(info.arg) == oparg::BinaryOperator::Subscr + ) + .then_some(DeoptKind::Subscript { + binary_op_idx: cursor, + }) + }) } + } + } - if saw_build_tuple && saw_match_keys { - to_deopt.push((block_idx, i)); + fn tail_returns_without_store( + blocks: &[Block], + is_pre_handler: &[bool], + start_block_idx: BlockIdx, + start_instr_idx: usize, + ) -> bool { + let mut block_idx = start_block_idx; + let mut current_start = start_instr_idx; + for _ in 0..blocks.len() { + if block_idx == BlockIdx::NULL || !is_pre_handler[block_idx.idx()] { + break; + } + let block = &blocks[block_idx.idx()]; + for info in block.instructions.iter().skip(current_start) { + match info.instr.real() { + Some(Instruction::ReturnValue) => return true, + Some( + Instruction::StoreFast { .. } + | Instruction::StoreFastLoadFast { .. } + | Instruction::StoreFastStoreFast { .. }, + ) + | Some(Instruction::DeleteFast { .. }) + | Some(Instruction::LoadFastAndClear { .. }) => return false, + _ => {} + } } + block_idx = block.next; + current_start = 0; } + false } - for (block_idx, instr_idx) in to_deopt { - self.blocks[block_idx].instructions[instr_idx].instr = Instruction::LoadFast { - var_num: Arg::marker(), + let mut order = Vec::new(); + let mut current = BlockIdx(0); + while current != BlockIdx::NULL { + order.push(current); + current = self.blocks[current.idx()].next; + } + + let mut has_handler_resume_predecessor = vec![false; self.blocks.len()]; + let mut predecessors = vec![Vec::new(); self.blocks.len()]; + for (pred_idx, block) in self.blocks.iter().enumerate() { + let Some(last_info) = block.instructions.last() else { + if block.next != BlockIdx::NULL { + predecessors[block.next.idx()].push(BlockIdx::new(pred_idx as u32)); + } + continue; + }; + if block.next != BlockIdx::NULL { + predecessors[block.next.idx()].push(BlockIdx::new(pred_idx as u32)); + } + if last_info.target == BlockIdx::NULL || !last_info.instr.is_unconditional_jump() { + for info in &block.instructions { + if info.target != BlockIdx::NULL { + predecessors[info.target.idx()].push(BlockIdx::new(pred_idx as u32)); + } + } + continue; + } + if !matches!(second_last_real_instr(block), Some(Instruction::PopExcept)) { + for info in &block.instructions { + if info.target != BlockIdx::NULL { + predecessors[info.target.idx()].push(BlockIdx::new(pred_idx as u32)); + } + } + continue; + } + has_handler_resume_predecessor[last_info.target.idx()] = true; + for info in &block.instructions { + if info.target != BlockIdx::NULL { + predecessors[info.target.idx()].push(BlockIdx::new(pred_idx as u32)); + } + } + } + + let Some(first_handler_pos) = order.iter().position(|block_idx| { + self.blocks[block_idx.idx()] + .instructions + .iter() + .any(|info| matches!(info.instr.real(), Some(Instruction::PushExcInfo))) + }) else { + return; + }; + let mut is_pre_handler = vec![false; self.blocks.len()]; + for &block_idx in &order[..first_handler_pos] { + is_pre_handler[block_idx.idx()] = true; + } + let mut reachable_from_protected = vec![false; self.blocks.len()]; + for &block_idx in &order[..first_handler_pos] { + let idx = block_idx.idx(); + let block = &self.blocks[idx]; + let is_protected_source = block_has_exception_match_handler(&self.blocks, block); + reachable_from_protected[idx] = predecessors[idx].iter().any(|pred| { + self.blocks[pred.idx()] + .instructions + .iter() + .any(|info| info.except_handler.is_some()) + || reachable_from_protected[pred.idx()] + }) || is_protected_source; + } + + let mut cross_block_deopts = Vec::new(); + for &block_idx in &order[..first_handler_pos] { + if has_handler_resume_predecessor[block_idx.idx()] { + continue; + } + let block_instr_len = self.blocks[block_idx.idx()].instructions.len(); + let real_instrs: Vec<_> = self.blocks[block_idx.idx()] + .instructions + .iter() + .copied() + .enumerate() + .filter(|(_, info)| info.instr.real().is_some()) + .collect(); + let mut to_deopt = Vec::new(); + for (real_idx, (instr_idx, info)) in real_instrs.iter().enumerate() { + let is_attr_chain_root = matches!( + info.instr.real(), + Some(Instruction::LoadFast { .. } | Instruction::LoadFastBorrow { .. }) + ); + if info.except_handler.is_some() || !is_attr_chain_root { + continue; + } + let Some(deopt_kind) = should_deopt_borrowed_attr_chain(&real_instrs, real_idx) + else { + continue; + }; + if let DeoptKind::ReturnIter { tail_start_idx } = deopt_kind { + let tail_instr_idx = real_instrs + .get(tail_start_idx) + .map(|(instr_idx, _)| *instr_idx) + .unwrap_or(block_instr_len); + if !tail_returns_without_store( + &self.blocks, + &is_pre_handler, + block_idx, + tail_instr_idx, + ) { + continue; + } + } + if matches!(deopt_kind, DeoptKind::Subscript { .. }) + && !reachable_from_protected[block_idx.idx()] + { + continue; + } + if matches!(info.instr.real(), Some(Instruction::LoadFastBorrow { .. })) { + to_deopt.push(*instr_idx); + } + if let DeoptKind::Subscript { binary_op_idx } = deopt_kind { + for (extra_instr_idx, extra_info) in real_instrs + .iter() + .skip(real_idx + 1) + .take(binary_op_idx.saturating_sub(real_idx + 1)) + .map(|(idx, info)| (*idx, *info)) + { + if matches!( + extra_info.instr.real(), + Some(Instruction::LoadFastBorrow { .. }) + ) { + to_deopt.push(extra_instr_idx); + } + } + if matches!( + real_instrs + .get(binary_op_idx + 1) + .and_then(|(_, info)| info.instr.real()), + Some(Instruction::StoreFast { .. }) + ) { + for (extra_instr_idx, extra_info) in + real_instrs.iter().skip(binary_op_idx + 2) + { + if matches!( + extra_info.instr.real(), + Some(Instruction::LoadFastBorrow { .. }) + | Some(Instruction::LoadFastBorrowLoadFastBorrow { .. }) + ) { + to_deopt.push(*extra_instr_idx); + } + } + let mut linear_tail = vec![block_idx]; + let mut cursor = self.blocks[block_idx.idx()].next; + while cursor != BlockIdx::NULL + && is_pre_handler[cursor.idx()] + && !block_is_exceptional(&self.blocks[cursor.idx()]) + { + if predecessors[cursor.idx()].iter().any(|pred| { + !linear_tail.contains(pred) + && !has_handler_resume_predecessor[pred.idx()] + }) { + break; + } + linear_tail.push(cursor); + cursor = self.blocks[cursor.idx()].next; + } + for tail_block_idx in linear_tail.into_iter().skip(1) { + for (tail_instr_idx, tail_info) in self.blocks[tail_block_idx.idx()] + .instructions + .iter() + .enumerate() + { + if matches!( + tail_info.instr.real(), + Some(Instruction::LoadFastBorrow { .. }) + | Some(Instruction::LoadFastBorrowLoadFastBorrow { .. }) + ) { + cross_block_deopts.push((tail_block_idx, tail_instr_idx)); + } + } + } + } + } + } + let block = &mut self.blocks[block_idx.idx()]; + for instr_idx in to_deopt { + deoptimize_borrow(&mut block.instructions[instr_idx]); + } + } + for (block_idx, instr_idx) in cross_block_deopts { + match self.blocks[block_idx.idx()].instructions[instr_idx] + .instr + .real() + { + Some(Instruction::LoadFastBorrow { .. }) => { + self.blocks[block_idx.idx()].instructions[instr_idx].instr = + Instruction::LoadFast { + var_num: Arg::marker(), + } + .into(); + } + Some(Instruction::LoadFastBorrowLoadFastBorrow { .. }) => { + self.blocks[block_idx.idx()].instructions[instr_idx].instr = + Instruction::LoadFastLoadFast { + var_nums: Arg::marker(), + } + .into(); + } + _ => {} } - .into(); } } @@ -3733,6 +4691,19 @@ impl CodeInfo { .find_map(|info| info.instr.real()) } + fn is_cleanup_restore_prefix(instructions: &[InstructionInfo]) -> bool { + let mut saw_pop_iter = false; + for info in instructions { + match info.instr.real() { + Some(Instruction::EndFor) if !saw_pop_iter => {} + Some(Instruction::PopIter) if !saw_pop_iter => saw_pop_iter = true, + Some(Instruction::Swap { .. } | Instruction::PopTop) if saw_pop_iter => {} + _ => return false, + } + } + saw_pop_iter + } + let mut predecessors = vec![Vec::new(); self.blocks.len()]; for (pred_idx, block) in self.blocks.iter().enumerate() { if block.next != BlockIdx::NULL { @@ -3770,23 +4741,20 @@ impl CodeInfo { ) ) && !new_instructions.is_empty() - && new_instructions.iter().all(|prev: &InstructionInfo| { + && (new_instructions.iter().all(|prev: &InstructionInfo| { matches!( prev.instr.real(), Some(Instruction::Swap { .. }) | Some(Instruction::PopTop) ) - }) + }) || is_cleanup_restore_prefix(&new_instructions)) { in_restore_prefix = true; } let expand = matches!( info.instr.real(), Some(Instruction::StoreFastStoreFast { .. }) - ) && (new_instructions.last().is_some_and( - |prev: &InstructionInfo| { - matches!(prev.instr.real(), Some(Instruction::PopIter)) - }, - ) || (i == 0 && starts_after_cleanup[block_idx]) + ) && (is_cleanup_restore_prefix(&new_instructions) + || (i == 0 && starts_after_cleanup[block_idx]) || in_restore_prefix); if expand { @@ -4200,6 +5168,12 @@ impl CodeInfo { )); mark_except_handlers(&mut self.blocks); label_exception_targets(&mut self.blocks); + redirect_empty_unconditional_jump_targets(&mut self.blocks); + inline_small_or_no_lineno_blocks(&mut self.blocks); + trace.push(( + "after_inline_small_or_no_lineno_blocks".to_owned(), + self.debug_block_dump(), + )); jump_threading(&mut self.blocks); trace.push(("after_jump_threading".to_owned(), self.debug_block_dump())); self.eliminate_unreachable_blocks(); @@ -4208,11 +5182,6 @@ impl CodeInfo { "after_early_remove_nops".to_owned(), self.debug_block_dump(), )); - inline_small_or_no_lineno_blocks(&mut self.blocks); - trace.push(( - "after_inline_small_or_no_lineno_blocks".to_owned(), - self.debug_block_dump(), - )); self.add_checks_for_loads_of_uninitialized_variables(); self.insert_superinstructions(); resolve_line_numbers(&mut self.blocks); @@ -4326,11 +5295,32 @@ impl CodeInfo { self.debug_block_dump(), )); self.optimize_load_fast_borrow(); + trace.push(( + "after_raw_optimize_load_fast_borrow".to_owned(), + self.debug_block_dump(), + )); self.deoptimize_borrow_for_folded_nonliteral_exprs(); + trace.push(( + "after_deoptimize_borrow_for_folded_nonliteral_exprs".to_owned(), + self.debug_block_dump(), + )); self.deoptimize_borrow_after_multi_handler_resume_join(); + trace.push(( + "after_deoptimize_borrow_after_multi_handler_resume_join".to_owned(), + self.debug_block_dump(), + )); self.deoptimize_borrow_after_named_except_cleanup_join(); + trace.push(( + "after_deoptimize_borrow_after_named_except_cleanup_join".to_owned(), + self.debug_block_dump(), + )); self.deoptimize_borrow_in_protected_conditional_tail(); + trace.push(( + "after_deoptimize_borrow_in_protected_conditional_tail".to_owned(), + self.debug_block_dump(), + )); self.deoptimize_borrow_after_protected_import(); + self.deoptimize_borrow_after_protected_store_tail(); trace.push(( "after_optimize_load_fast_borrow".to_owned(), self.debug_block_dump(), @@ -4338,6 +5328,7 @@ impl CodeInfo { self.deoptimize_borrow_after_push_exc_info(); self.deoptimize_borrow_for_handler_return_paths(); self.deoptimize_borrow_for_match_keys_attr(); + self.deoptimize_borrow_in_protected_attr_chain_tail(); trace.push(("after_borrow_deopts".to_owned(), self.debug_block_dump())); Ok(trace) @@ -4790,6 +5781,9 @@ fn push_cold_blocks_to_end(blocks: &mut Vec) { folded_from_nonliteral_expr: false, lineno_override: Some(-1), cache_entries: 0, + preserve_redundant_jump_as_nop: false, + remove_no_location_nop: false, + preserve_block_start_no_location_nop: false, }); jump_block.next = blocks[cold_idx.idx()].next; blocks[cold_idx.idx()].next = jump_block_idx; @@ -5031,6 +6025,20 @@ fn jump_threading_impl(blocks: &mut [Block], include_conditional: bool) { // compare loop exits to avoid wraparound-style jumps. continue; } + if !include_conditional + && matches!( + jump_thread_kind(ins.instr), + Some(JumpThreadKind::NoInterrupt) + ) + && matches!( + jump_thread_kind(target_ins.instr), + Some(JumpThreadKind::Plain) + ) + { + // CPython does not late-thread WITH suppress exits through + // the line-anchored continue/break jump that follows. + continue; + } let conditional = is_conditional_jump(&ins.instr); if conditional && !matches!( @@ -5134,6 +6142,9 @@ fn normalize_jumps(blocks: &mut Vec) { folded_from_nonliteral_expr: false, lineno_override: None, cache_entries: 0, + preserve_redundant_jump_as_nop: false, + remove_no_location_nop: false, + preserve_block_start_no_location_nop: false, }; blocks[idx].instructions.push(not_taken); } else { @@ -5166,6 +6177,9 @@ fn normalize_jumps(blocks: &mut Vec) { folded_from_nonliteral_expr: false, lineno_override: None, cache_entries: 0, + preserve_redundant_jump_as_nop: false, + remove_no_location_nop: false, + preserve_block_start_no_location_nop: false, }); new_block.instructions.push(InstructionInfo { instr: PseudoOpcode::Jump.into(), @@ -5177,6 +6191,9 @@ fn normalize_jumps(blocks: &mut Vec) { folded_from_nonliteral_expr: false, lineno_override: None, cache_entries: 0, + preserve_redundant_jump_as_nop: false, + remove_no_location_nop: false, + preserve_block_start_no_location_nop: false, }); new_block.next = old_next; @@ -5400,14 +6417,82 @@ fn inline_single_predecessor_artificial_expr_exit_blocks(blocks: &mut [Block]) { } } +fn has_jump_predecessor(blocks: &[Block], target: BlockIdx) -> bool { + blocks.iter().any(|block| { + block.instructions.iter().any(|instr| { + is_jump_instruction(instr) + && instr.target != BlockIdx::NULL + && next_nonempty_block(blocks, instr.target) == target + }) + }) +} + +fn has_plain_jump_predecessor(blocks: &[Block], target: BlockIdx) -> bool { + blocks.iter().any(|block| { + block.instructions.iter().any(|instr| { + matches!(jump_thread_kind(instr.instr), Some(JumpThreadKind::Plain)) + && instr.target != BlockIdx::NULL + && next_nonempty_block(blocks, instr.target) == target + }) + }) +} + fn remove_redundant_nops_in_blocks(blocks: &mut [Block]) -> usize { + fn ends_with_for_cleanup(block: &Block) -> bool { + let mut reals = block + .instructions + .iter() + .rev() + .filter_map(|info| info.instr.real()); + matches!( + (reals.next(), reals.next()), + (Some(Instruction::PopIter), Some(Instruction::EndFor)) + ) + } + let mut changes = 0; + let targeted_blocks: Vec<_> = (0..blocks.len()) + .map(|target_idx| { + let target = BlockIdx(target_idx as u32); + blocks.iter().any(|block| { + block.instructions.iter().any(|instr| { + instr.target != BlockIdx::NULL + && next_nonempty_block(blocks, instr.target) == target + }) + }) + }) + .collect(); + let jump_targets: Vec<_> = (0..blocks.len()) + .map(|target_idx| has_jump_predecessor(blocks, BlockIdx(target_idx as u32))) + .collect(); + let plain_jump_targets: Vec<_> = (0..blocks.len()) + .map(|target_idx| has_plain_jump_predecessor(blocks, BlockIdx(target_idx as u32))) + .collect(); let mut block_order = Vec::new(); let mut current = BlockIdx(0); while current != BlockIdx::NULL { block_order.push(current); current = blocks[current.idx()].next; } + let mut fallthrough_prev_lineno = vec![None; blocks.len()]; + let mut prev_nonempty = BlockIdx::NULL; + for &block_idx in &block_order { + if blocks[block_idx.idx()].instructions.is_empty() { + continue; + } + if prev_nonempty != BlockIdx::NULL + && !targeted_blocks[block_idx.idx()] + && ends_with_for_cleanup(&blocks[prev_nonempty.idx()]) + && block_has_fallthrough(&blocks[prev_nonempty.idx()]) + && next_nonempty_block(blocks, blocks[prev_nonempty.idx()].next) == block_idx + { + fallthrough_prev_lineno[block_idx.idx()] = blocks[prev_nonempty.idx()] + .instructions + .last() + .map(instruction_lineno); + } + prev_nonempty = block_idx; + } for block_idx in block_order { let bi = block_idx.idx(); @@ -5421,7 +6506,31 @@ fn remove_redundant_nops_in_blocks(blocks: &mut [Block]) -> usize { let mut remove = false; if matches!(instr.instr.real(), Some(Instruction::Nop)) { - if lineno < 0 || prev_lineno == lineno { + if lineno < 0 { + remove = !(instr.remove_no_location_nop + && src == 0 + && jump_targets[block_idx.idx()]); + } else if src == 0 + && fallthrough_prev_lineno[block_idx.idx()] + .is_some_and(|prev_lineno| prev_lineno == lineno) + { + remove = true; + } else if instr.remove_no_location_nop + && src == 0 + && plain_jump_targets[block_idx.idx()] + { + let next_lineno = src_instructions[src + 1..].iter().find_map(|next_instr| { + let line = instruction_lineno(next_instr); + if matches!(next_instr.instr.real(), Some(Instruction::Nop)) && line < 0 { + None + } else { + Some(line) + } + }); + if next_lineno.is_some_and(|next_lineno| lineno < next_lineno) { + remove = true; + } + } else if prev_lineno == lineno { remove = true; } else if src < src_instructions.len() - 1 { if src_instructions[src + 1].instr.is_block_push() { @@ -5488,18 +6597,40 @@ fn remove_redundant_jumps_in_blocks(blocks: &mut [Block]) -> usize { while current != BlockIdx::NULL { let idx = current.idx(); let next = next_nonempty_block(blocks, blocks[idx].next); - let jump_target = blocks[idx] - .instructions - .last() - .filter(|ins| ins.instr.is_unconditional_jump() && ins.target != BlockIdx::NULL) - .map(|ins| ins.target); - if next != BlockIdx::NULL - && let Some(target) = jump_target - && next_nonempty_block(blocks, target) == next - && let Some(last_instr) = blocks[idx].instructions.last_mut() - { - set_to_nop(last_instr); - changes += 1; + if next != BlockIdx::NULL { + let Some(last_instr) = blocks[idx].instructions.last().copied() else { + current = blocks[idx].next; + continue; + }; + if last_instr.instr.is_unconditional_jump() + && last_instr.target != BlockIdx::NULL + && next_nonempty_block(blocks, last_instr.target) == next + { + let preserve_as_nop = if last_instr.preserve_redundant_jump_as_nop { + let line = instruction_lineno(&last_instr); + let next_line = blocks[next.idx()].instructions.iter().find_map(|instr| { + let line = instruction_lineno(instr); + (!matches!(instr.instr.real(), Some(Instruction::Nop)) || line >= 0) + .then_some(line) + }); + line > 0 && next_line.is_some_and(|next_line| next_line < line) + } else { + false + }; + if preserve_as_nop { + current = blocks[idx].next; + continue; + } + if last_instr.preserve_redundant_jump_as_nop { + let last_instr = blocks[idx].instructions.last_mut().unwrap(); + last_instr.preserve_redundant_jump_as_nop = false; + } + let last_instr = blocks[idx].instructions.last_mut().unwrap(); + set_to_nop(last_instr); + changes += 1; + current = blocks[idx].next; + continue; + } } current = blocks[idx].next; } @@ -5632,6 +6763,9 @@ fn materialize_empty_conditional_exit_targets(blocks: &mut [Block]) { folded_from_nonliteral_expr: false, lineno_override: None, cache_entries: 0, + preserve_redundant_jump_as_nop: false, + remove_no_location_nop: false, + preserve_block_start_no_location_nop: false, }); } } @@ -5851,6 +6985,25 @@ fn is_exception_cleanup_block(block: &Block) -> bool { .is_some_and(|instr| matches!(instr.instr.real(), Some(Instruction::Reraise { .. }))) } +fn is_with_suppress_exit_block(block: &Block) -> bool { + let real_instrs: Vec<_> = block + .instructions + .iter() + .filter_map(|info| info.instr.real()) + .collect(); + matches!( + real_instrs.as_slice(), + [ + Instruction::PopTop, + Instruction::PopExcept, + Instruction::PopTop, + Instruction::PopTop, + Instruction::PopTop, + last, + ] if last.is_unconditional_jump() + ) +} + fn block_is_protected(block: &Block) -> bool { block .instructions @@ -5907,6 +7060,30 @@ fn block_has_only_stop_iteration_error_handlers(block: &Block, blocks: &[Block]) saw_handler } +fn block_has_exception_match_handler(blocks: &[Block], block: &Block) -> bool { + let mut visited = vec![false; blocks.len()]; + let handler_blocks: Vec<_> = block + .instructions + .iter() + .filter_map(|info| info.except_handler.map(|handler| handler.handler_block)) + .collect(); + for handler_block in handler_blocks { + let mut cursor = handler_block; + while cursor != BlockIdx::NULL && !visited[cursor.idx()] { + visited[cursor.idx()] = true; + if blocks[cursor.idx()] + .instructions + .iter() + .any(|info| matches!(info.instr.real(), Some(Instruction::CheckExcMatch))) + { + return true; + } + cursor = blocks[cursor.idx()].next; + } + } + false +} + fn block_is_exceptional(block: &Block) -> bool { block.except_handler || block.preserve_lasti || is_exception_cleanup_block(block) } @@ -6187,11 +7364,19 @@ fn reorder_conditional_chain_and_jump_back_blocks(blocks: &mut Vec) { }); let chain_is_single_exit_block = is_scope_exit_block(&blocks[chain_start.idx()]) && next_nonempty_block(blocks, blocks[chain_start.idx()].next) == jump_start; + let chain_is_jump_only_exit_block = is_jump_only_block(&blocks[chain_start.idx()]) + && !target_comes_before( + blocks[chain_start.idx()].instructions[0].target, + chain_start, + blocks, + ); let allow_true_path_jump_back_reorder = matches!(last.instr.real(), Some(Instruction::PopJumpIfTrue { .. })) && (chain_has_suspension_point || chain_starts_with_false_path_jump - || chain_is_single_exit_block); + || chain_is_single_exit_block + || chain_is_jump_only_exit_block); + let is_generic_false_path_reorder = !allow_true_path_jump_back_reorder; if !is_false_path_conditional_jump(&last.instr) && !allow_true_path_jump_back_reorder { current = next; continue; @@ -6245,13 +7430,11 @@ fn reorder_conditional_chain_and_jump_back_blocks(blocks: &mut Vec) { chain_end = cursor; cursor = blocks[cursor.idx()].next; } - if !chain_valid - || !saw_nonempty - || chain_end == BlockIdx::NULL - || cursor != jump_start - || nonempty_blocks > 8 - || real_instr_count > 80 - { + if !chain_valid || !saw_nonempty || chain_end == BlockIdx::NULL || cursor != jump_start { + current = next; + continue; + } + if !is_generic_false_path_reorder && (nonempty_blocks > 8 || real_instr_count > 80) { current = next; continue; } @@ -6316,6 +7499,10 @@ fn reorder_jump_over_exception_cleanup_blocks(blocks: &mut [Block]) { while current != BlockIdx::NULL { let idx = current.idx(); let next = blocks[idx].next; + if blocks[idx].cold && is_with_suppress_exit_block(&blocks[idx]) { + current = next; + continue; + } let Some(last) = blocks[idx].instructions.last().copied() else { current = next; continue; @@ -6382,11 +7569,14 @@ fn reorder_jump_over_exception_cleanup_blocks(blocks: &mut [Block]) { cursor = blocks[cursor.idx()].next; } + const MAX_REORDERED_EXIT_BLOCK_SIZE: usize = 4; + if target_end == BlockIdx::NULL || target_exit == BlockIdx::NULL || nonempty_target_blocks != 1 || target_exit != target_end || !is_scope_exit_block(&blocks[target_exit.idx()]) + || blocks[target_exit.idx()].instructions.len() > MAX_REORDERED_EXIT_BLOCK_SIZE { current = next; continue; @@ -6613,14 +7803,13 @@ fn propagate_line_numbers(blocks: &mut [Block], predecessors: &[u32]) { let incoming_origins = compute_incoming_origins(blocks, &reachable); let mut current = BlockIdx(0); while current != BlockIdx::NULL { - let last = blocks[current.idx()].instructions.last().copied(); - if let Some(last) = last { + if !blocks[current.idx()].instructions.is_empty() { let (next_block, has_fallthrough) = { let block = &blocks[current.idx()]; (block.next, block_has_fallthrough(block)) }; - { + let prev_location = { let block = &mut blocks[current.idx()]; let mut prev_location = None; for instr in &mut block.instructions { @@ -6629,7 +7818,9 @@ fn propagate_line_numbers(blocks: &mut [Block], predecessors: &[u32]) { } prev_location = propagation_location(instr); } - } + prev_location + }; + let last = blocks[current.idx()].instructions.last().copied().unwrap(); if has_fallthrough { let target = next_nonempty_block(blocks, next_block); @@ -6642,7 +7833,7 @@ fn propagate_line_numbers(blocks: &mut [Block], predecessors: &[u32]) { current, target, )) - && let Some((location, end_location)) = propagation_location(&last) + && let Some((location, end_location)) = prev_location && let Some(first) = blocks[target.idx()].instructions.first_mut() { maybe_propagate_location(first, location, end_location); @@ -6659,7 +7850,7 @@ fn propagate_line_numbers(blocks: &mut [Block], predecessors: &[u32]) { } if target != BlockIdx::NULL && predecessors[target.idx()] == 1 - && let Some((location, end_location)) = propagation_location(&last) + && let Some((location, end_location)) = prev_location && let Some(first) = blocks[target.idx()].instructions.first_mut() { maybe_propagate_location(first, location, end_location); @@ -7192,7 +8383,13 @@ pub(crate) fn label_exception_targets(blocks: &mut [Block]) { ); stack.pop(); // POP_BLOCK → NOP + let remove_no_location_nop = blocks[bi].instructions[i].remove_no_location_nop; + let preserve_block_start_no_location_nop = + blocks[bi].instructions[i].preserve_block_start_no_location_nop; set_to_nop(&mut blocks[bi].instructions[i]); + blocks[bi].instructions[i].remove_no_location_nop = remove_no_location_nop; + blocks[bi].instructions[i].preserve_block_start_no_location_nop = + preserve_block_start_no_location_nop; } else { // Set except_handler for this instruction from except stack top // stack_depth placeholder: filled by fixup_handler_depths @@ -7276,7 +8473,13 @@ pub(crate) fn convert_pseudo_ops(blocks: &mut [Block], cellfixedoffsets: &[u32]) // PopBlock in reachable blocks is converted to NOP by // label_exception_targets. Dead blocks may still have them. PseudoInstruction::PopBlock => { + let remove_no_location_nop = info.remove_no_location_nop; + let preserve_block_start_no_location_nop = + info.preserve_block_start_no_location_nop; set_to_nop(info); + info.remove_no_location_nop = remove_no_location_nop; + info.preserve_block_start_no_location_nop = + preserve_block_start_no_location_nop; } // LOAD_CLOSURE → LOAD_FAST (using cellfixedoffsets for merged layout) PseudoInstruction::LoadClosure { i } => { From b54cfd6dcad8f429216b57be8bf205c75d321bc2 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Thu, 30 Apr 2026 04:50:24 +0900 Subject: [PATCH 4/7] Unmark test_dis.test_findlabels expected failure --- Lib/test/test_dis.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/test/test_dis.py b/Lib/test/test_dis.py index 3e6ae9be458..fcd6a6b8be7 100644 --- a/Lib/test/test_dis.py +++ b/Lib/test/test_dis.py @@ -2410,7 +2410,6 @@ def test__find_store_names(self): res = tuple(dis._find_store_names(code)) self.assertEqual(res, expected) - @unittest.expectedFailure # TODO: RUSTPYTHON def test_findlabels(self): labels = dis.findlabels(jumpy.__code__.co_code) jumps = [ From 23c689d2510c6cf9203ba41581e16c1a02c1c64f Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Thu, 30 Apr 2026 08:56:56 +0900 Subject: [PATCH 5/7] Compute target predecessor flags in single pass remove_nops and remove_redundant_nops_in_blocks repeated has_jump_predecessor / has_plain_jump_predecessor / target lookups per block, scanning all blocks each time. With ~200,000 if blocks this became O(B^2 * I) and timed out test_compile.test_stack_overflow. Fold the three flags into one O(B * I) pass via compute_target_predecessor_flags. --- crates/codegen/src/ir.rs | 74 +++++++++++++++++++++------------------- 1 file changed, 38 insertions(+), 36 deletions(-) diff --git a/crates/codegen/src/ir.rs b/crates/codegen/src/ir.rs index 39c970ba029..0322c380e7d 100644 --- a/crates/codegen/src/ir.rs +++ b/crates/codegen/src/ir.rs @@ -2559,9 +2559,7 @@ impl CodeInfo { ) } - let jump_targets: Vec<_> = (0..self.blocks.len()) - .map(|target_idx| has_jump_predecessor(&self.blocks, BlockIdx(target_idx as u32))) - .collect(); + let jump_targets = compute_target_predecessor_flags(&self.blocks).jump; let mut fallthrough_predecessors = vec![None; self.blocks.len()]; for (pred_idx, block) in self.blocks.iter().enumerate() { if block.next != BlockIdx::NULL { @@ -6417,24 +6415,40 @@ fn inline_single_predecessor_artificial_expr_exit_blocks(blocks: &mut [Block]) { } } -fn has_jump_predecessor(blocks: &[Block], target: BlockIdx) -> bool { - blocks.iter().any(|block| { - block.instructions.iter().any(|instr| { - is_jump_instruction(instr) - && instr.target != BlockIdx::NULL - && next_nonempty_block(blocks, instr.target) == target - }) - }) +struct TargetPredecessorFlags { + targeted: Vec, + jump: Vec, + plain_jump: Vec, } -fn has_plain_jump_predecessor(blocks: &[Block], target: BlockIdx) -> bool { - blocks.iter().any(|block| { - block.instructions.iter().any(|instr| { - matches!(jump_thread_kind(instr.instr), Some(JumpThreadKind::Plain)) - && instr.target != BlockIdx::NULL - && next_nonempty_block(blocks, instr.target) == target - }) - }) +fn compute_target_predecessor_flags(blocks: &[Block]) -> TargetPredecessorFlags { + let mut targeted = vec![false; blocks.len()]; + let mut jump = vec![false; blocks.len()]; + let mut plain_jump = vec![false; blocks.len()]; + for block in blocks { + for instr in &block.instructions { + if instr.target == BlockIdx::NULL { + continue; + } + let target = next_nonempty_block(blocks, instr.target); + if target == BlockIdx::NULL { + continue; + } + let idx = target.idx(); + targeted[idx] = true; + if is_jump_instruction(instr) { + jump[idx] = true; + } + if matches!(jump_thread_kind(instr.instr), Some(JumpThreadKind::Plain)) { + plain_jump[idx] = true; + } + } + } + TargetPredecessorFlags { + targeted, + jump, + plain_jump, + } } fn remove_redundant_nops_in_blocks(blocks: &mut [Block]) -> usize { @@ -6451,23 +6465,11 @@ fn remove_redundant_nops_in_blocks(blocks: &mut [Block]) -> usize { } let mut changes = 0; - let targeted_blocks: Vec<_> = (0..blocks.len()) - .map(|target_idx| { - let target = BlockIdx(target_idx as u32); - blocks.iter().any(|block| { - block.instructions.iter().any(|instr| { - instr.target != BlockIdx::NULL - && next_nonempty_block(blocks, instr.target) == target - }) - }) - }) - .collect(); - let jump_targets: Vec<_> = (0..blocks.len()) - .map(|target_idx| has_jump_predecessor(blocks, BlockIdx(target_idx as u32))) - .collect(); - let plain_jump_targets: Vec<_> = (0..blocks.len()) - .map(|target_idx| has_plain_jump_predecessor(blocks, BlockIdx(target_idx as u32))) - .collect(); + let TargetPredecessorFlags { + targeted: targeted_blocks, + jump: jump_targets, + plain_jump: plain_jump_targets, + } = compute_target_predecessor_flags(blocks); let mut block_order = Vec::new(); let mut current = BlockIdx(0); while current != BlockIdx::NULL { From 238d1e780cabea43ff40faf98afa951ef62dc68c Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Thu, 30 Apr 2026 19:33:32 +0900 Subject: [PATCH 6/7] Address review feedback on slice folding, fallthrough, attr-chain - try_fold_constant_slice now delegates to try_fold_constant_expr so slice bounds accept the same constants other folding paths do (unary-folded values, __debug__, etc.). - remove_nops resolves fallthrough predecessors through empty blocks via next_nonempty_block before checking ends_with_for_cleanup. - should_deopt_borrowed_attr_chain ReturnIter matcher now accepts Instruction::CallKw alongside Instruction::Call, matching the Call/CallKw treatment in the surrounding deopt trigger check. --- crates/codegen/src/compile.rs | 33 +++------------------------------ crates/codegen/src/ir.rs | 7 ++++--- 2 files changed, 7 insertions(+), 33 deletions(-) diff --git a/crates/codegen/src/compile.rs b/crates/codegen/src/compile.rs index 8059d842615..b7653e53865 100644 --- a/crates/codegen/src/compile.rs +++ b/crates/codegen/src/compile.rs @@ -10172,33 +10172,6 @@ impl Compiler { self.emit_arg(idx, |consti| Instruction::LoadConst { consti }) } - fn try_fold_slice_constant_part( - &mut self, - expr: &ast::Expr, - ) -> CompileResult> { - Ok(Some(match expr { - ast::Expr::NumberLiteral(num) => match &num.value { - ast::Number::Int(int) => ConstantData::Integer { - value: ruff_int_to_bigint(int).map_err(|e| self.error(e))?, - }, - ast::Number::Float(f) => ConstantData::Float { value: *f }, - ast::Number::Complex { real, imag } => ConstantData::Complex { - value: Complex::new(*real, *imag), - }, - }, - ast::Expr::StringLiteral(s) => ConstantData::Str { - value: self.compile_string_value(s), - }, - ast::Expr::BytesLiteral(b) => ConstantData::Bytes { - value: b.value.bytes().collect(), - }, - ast::Expr::BooleanLiteral(b) => ConstantData::Boolean { value: b.value }, - ast::Expr::NoneLiteral(_) => ConstantData::None, - ast::Expr::EllipsisLiteral(_) => ConstantData::Ellipsis, - _ => return Ok(None), - })) - } - fn try_fold_constant_slice( &mut self, lower: Option<&ast::Expr>, @@ -10207,7 +10180,7 @@ impl Compiler { ) -> CompileResult> { let start = match lower { Some(expr) => { - let Some(constant) = self.try_fold_slice_constant_part(expr)? else { + let Some(constant) = self.try_fold_constant_expr(expr)? else { return Ok(None); }; constant @@ -10216,7 +10189,7 @@ impl Compiler { }; let stop = match upper { Some(expr) => { - let Some(constant) = self.try_fold_slice_constant_part(expr)? else { + let Some(constant) = self.try_fold_constant_expr(expr)? else { return Ok(None); }; constant @@ -10225,7 +10198,7 @@ impl Compiler { }; let step = match step { Some(expr) => { - let Some(constant) = self.try_fold_slice_constant_part(expr)? else { + let Some(constant) = self.try_fold_constant_expr(expr)? else { return Ok(None); }; constant diff --git a/crates/codegen/src/ir.rs b/crates/codegen/src/ir.rs index 0322c380e7d..18d878c4901 100644 --- a/crates/codegen/src/ir.rs +++ b/crates/codegen/src/ir.rs @@ -2562,8 +2562,9 @@ impl CodeInfo { let jump_targets = compute_target_predecessor_flags(&self.blocks).jump; let mut fallthrough_predecessors = vec![None; self.blocks.len()]; for (pred_idx, block) in self.blocks.iter().enumerate() { - if block.next != BlockIdx::NULL { - fallthrough_predecessors[block.next.idx()] = Some(pred_idx); + let next = next_nonempty_block(&self.blocks, block.next); + if next != BlockIdx::NULL { + fallthrough_predecessors[next.idx()] = Some(pred_idx); } } let starts_after_for_cleanup: Vec<_> = fallthrough_predecessors @@ -4407,7 +4408,7 @@ impl CodeInfo { Some(Instruction::GetIter) => Some(DeoptKind::ReturnIter { tail_start_idx: cursor + 1, }), - Some(Instruction::Call { .. }) => real_instrs + Some(Instruction::Call { .. } | Instruction::CallKw { .. }) => real_instrs .get(cursor + 1) .and_then(|(_, info)| info.instr.real()) .and_then(|instr| { From c267535c61150df71b01db8bd153bdb15a6370ae Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Thu, 30 Apr 2026 22:46:36 +0900 Subject: [PATCH 7/7] remove test --- .cspell.json | 1 - crates/codegen/src/compile.rs | 27 +++++---------------------- 2 files changed, 5 insertions(+), 23 deletions(-) diff --git a/.cspell.json b/.cspell.json index a2f349fc060..b7efb2e2311 100644 --- a/.cspell.json +++ b/.cspell.json @@ -67,7 +67,6 @@ "jitted", "jitting", "kwonly", - "lolcatz", "lossily", "mcache", "oparg", diff --git a/crates/codegen/src/compile.rs b/crates/codegen/src/compile.rs index b7653e53865..e79226cf141 100644 --- a/crates/codegen/src/compile.rs +++ b/crates/codegen/src/compile.rs @@ -13665,36 +13665,19 @@ def f(filters, text, category, module, lineno, defaultaction, _wm): fn test_try_except_else_with_finally_keeps_with_handler_before_outer_except() { let code = compile_exec( "\ -def jumpy(i): - for i in range(10): - print(i) - if i < 4: - continue - if i > 6: - break - else: - print('I can haz else clause?') - while i: - print(i) - i -= 1 - if i > 6: - continue - if i < 4: - break - else: - print('Who let lolcatz into this test suite?') +def f(i): try: 1 / 0 except ZeroDivisionError: - print('Here we go, here we go, here we go...') + print('e') else: with i as dodgy: - print('Never reach this') + print('w') finally: - print('OK, now we\\'re done') + print('d') ", ); - let jumpy = find_code(&code, "jumpy").expect("missing jumpy code"); + let jumpy = find_code(&code, "f").expect("missing f code"); let ops: Vec<_> = jumpy .instructions .iter()