From ca6e56ae3a36b7362fd23586d1e6c7a59b918ef1 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Tue, 31 Mar 2026 13:11:13 +0900 Subject: [PATCH] Improve codegen bytecode parity - Add CFG block splitting, jump threading, backward jump normalization - Add genexpr StopIteration wrapper - Add ConstantData::Slice and constant slice folding - Add duplicate_exits_without_lineno and Block: Clone - Add builtin(genexpr) optimization for tuple/list/set/all/any - Add compile_try_except_no_finally for try-except without finally - Add module_name_declared_global_in_nested_scope - Add constant tuple folding in try_fold_constant_expr - Add fstring literal-only optimization and empty literal elision - Fix duplicate_exits_without_lineno: splice new blocks into linked list --- Lib/test/test_monitoring.py | 1 - Lib/test/test_scope.py | 1 - crates/codegen/src/compile.rs | 779 +++++++++++++++++- crates/codegen/src/ir.rs | 118 ++- ...pile__tests__nested_double_async_with.snap | 158 ++-- crates/compiler-core/src/bytecode/oparg.rs | 6 + crates/vm/src/frame.rs | 14 +- crates/vm/src/version.rs | 4 +- crates/vm/src/vm/mod.rs | 4 + 9 files changed, 933 insertions(+), 152 deletions(-) diff --git a/Lib/test/test_monitoring.py b/Lib/test/test_monitoring.py index ed28ae07f86..5125701202b 100644 --- a/Lib/test/test_monitoring.py +++ b/Lib/test/test_monitoring.py @@ -1261,7 +1261,6 @@ def func2(): ('instruction', 'func2', 46), ('line', 'get_events', 11)]) - @unittest.expectedFailure # TODO: RUSTPYTHON; - instruction offsets differ from CPython def test_try_except(self): def func3(): diff --git a/Lib/test/test_scope.py b/Lib/test/test_scope.py index 952afb7e0d3..520fbc1b662 100644 --- a/Lib/test/test_scope.py +++ b/Lib/test/test_scope.py @@ -692,7 +692,6 @@ def dec(self): self.assertEqual(c.dec(), 1) self.assertEqual(c.dec(), 0) - @unittest.expectedFailure # TODO: RUSTPYTHON; figure out how to communicate that `y = 9` should be stored as a global rather than a STORE_NAME, even when the `global y` is in a nested subscope def testGlobalInParallelNestedFunctions(self): # A symbol table bug leaked the global statement from one # function to other nested functions in the same block. diff --git a/crates/codegen/src/compile.rs b/crates/codegen/src/compile.rs index e0debb48cd2..69a89897061 100644 --- a/crates/codegen/src/compile.rs +++ b/crates/codegen/src/compile.rs @@ -56,7 +56,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 { @@ -121,6 +121,15 @@ enum SuperCallType<'a> { ZeroArg, } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum BuiltinGeneratorCallKind { + Tuple, + List, + Set, + All, + Any, +} + #[derive(Debug, Clone)] pub struct FBlockInfo { pub fb_type: FBlockType, @@ -2002,6 +2011,16 @@ impl Compiler { symboltable::maybe_mangle_name(private, mangled_names, name) } + fn module_name_declared_global_in_nested_scope(table: &SymbolTable, name: &str) -> bool { + table.sub_tables.iter().any(|subtable| { + (!subtable.comp_inlined + && subtable + .lookup(name) + .is_some_and(|symbol| symbol.scope == SymbolScope::GlobalExplicit)) + || Self::module_name_declared_global_in_nested_scope(subtable, name) + }) + } + // = compiler_nameop fn compile_name(&mut self, name: &str, usage: NameUsage) -> CompileResult<()> { enum NameOp { @@ -2088,12 +2107,20 @@ impl Compiler { } }; + let module_global_from_nested_scope = { + let current_table = self.current_symbol_table(); + current_table.typ == CompilerScope::Module + && Self::module_name_declared_global_in_nested_scope(current_table, name.as_ref()) + }; + // Determine operation type based on scope let op_type = match actual_scope { SymbolScope::Free => NameOp::Deref, SymbolScope::Cell => NameOp::Deref, SymbolScope::Local => { - if is_function_like { + if module_global_from_nested_scope { + NameOp::Global + } else if is_function_like { NameOp::Fast } else { NameOp::Name @@ -2111,7 +2138,13 @@ impl Compiler { } } SymbolScope::GlobalExplicit => NameOp::Global, - SymbolScope::Unknown => NameOp::Name, + SymbolScope::Unknown => { + if module_global_from_nested_scope { + NameOp::Global + } else { + NameOp::Name + } + } }; // Generate appropriate instructions based on operation type @@ -2538,9 +2571,15 @@ impl Compiler { statement.range(), )); } - self.compile_expression(v)?; - // Unwind fblock stack with preserve_tos=true (preserve return value) - self.unwind_fblock_stack(true, false)?; + let folded_constant = self.try_fold_constant_expr(v)?; + let preserve_tos = folded_constant.is_none(); + if preserve_tos { + self.compile_expression(v)?; + } + self.unwind_fblock_stack(preserve_tos, false)?; + if let Some(constant) = folded_constant { + self.emit_load_const(constant); + } self.emit_return_value(); } None => { @@ -2987,6 +3026,10 @@ impl Compiler { orelse: &[ast::Stmt], finalbody: &[ast::Stmt], ) -> CompileResult<()> { + if finalbody.is_empty() { + return self.compile_try_except_no_finally(body, handlers, orelse); + } + let handler_block = self.new_block(); let finally_block = self.new_block(); @@ -3397,6 +3440,175 @@ impl Compiler { Ok(()) } + fn compile_try_except_no_finally( + &mut self, + body: &[ast::Stmt], + handlers: &[ast::ExceptHandler], + orelse: &[ast::Stmt], + ) -> CompileResult<()> { + let handler_block = self.new_block(); + let cleanup_block = self.new_block(); + let orelse_block = self.new_block(); + let end_block = self.new_block(); + + emit!(self, Instruction::Nop); + emit!( + self, + PseudoInstruction::SetupFinally { + delta: handler_block + } + ); + + self.push_fblock(FBlockType::TryExcept, handler_block, handler_block)?; + self.compile_statements(body)?; + self.pop_fblock(FBlockType::TryExcept); + emit!(self, PseudoInstruction::PopBlock); + self.set_no_location(); + emit!( + self, + PseudoInstruction::JumpNoInterrupt { + delta: orelse_block + } + ); + self.set_no_location(); + + self.switch_to_block(handler_block); + emit!( + self, + PseudoInstruction::SetupCleanup { + delta: cleanup_block + } + ); + self.set_no_location(); + emit!(self, Instruction::PushExcInfo); + self.set_no_location(); + self.push_fblock(FBlockType::ExceptionHandler, cleanup_block, cleanup_block)?; + + for handler in handlers { + let ast::ExceptHandler::ExceptHandler(ast::ExceptHandlerExceptHandler { + type_, + name, + body, + range: handler_range, + .. + }) = handler; + self.set_source_range(*handler_range); + let next_handler = self.new_block(); + + if let Some(exc_type) = type_ { + self.compile_expression(exc_type)?; + emit!(self, Instruction::CheckExcMatch); + emit!( + self, + Instruction::PopJumpIfFalse { + delta: next_handler + } + ); + } + + if let Some(alias) = name { + self.store_name(alias.as_str())?; + + let cleanup_end = self.new_block(); + let handler_normal_exit = self.new_block(); + emit!(self, PseudoInstruction::SetupCleanup { delta: cleanup_end }); + self.push_fblock_full( + FBlockType::HandlerCleanup, + cleanup_end, + cleanup_end, + FBlockDatum::ExceptionName(alias.as_str().to_owned()), + )?; + + self.compile_statements(body)?; + + self.pop_fblock(FBlockType::HandlerCleanup); + emit!(self, PseudoInstruction::PopBlock); + self.set_no_location(); + emit!( + self, + PseudoInstruction::JumpNoInterrupt { + delta: handler_normal_exit + } + ); + self.set_no_location(); + + self.switch_to_block(cleanup_end); + self.emit_load_const(ConstantData::None); + self.set_no_location(); + self.store_name(alias.as_str())?; + self.set_no_location(); + self.compile_name(alias.as_str(), NameUsage::Delete)?; + self.set_no_location(); + emit!(self, Instruction::Reraise { depth: 1 }); + self.set_no_location(); + + self.switch_to_block(handler_normal_exit); + emit!(self, PseudoInstruction::PopBlock); + self.set_no_location(); + self.pop_fblock(FBlockType::ExceptionHandler); + emit!(self, Instruction::PopExcept); + self.set_no_location(); + + self.emit_load_const(ConstantData::None); + self.set_no_location(); + self.store_name(alias.as_str())?; + self.set_no_location(); + self.compile_name(alias.as_str(), NameUsage::Delete)?; + self.set_no_location(); + + emit!( + self, + PseudoInstruction::JumpNoInterrupt { delta: end_block } + ); + self.set_no_location(); + } else { + emit!(self, Instruction::PopTop); + self.push_fblock(FBlockType::HandlerCleanup, end_block, end_block)?; + + self.compile_statements(body)?; + + self.pop_fblock(FBlockType::HandlerCleanup); + emit!(self, PseudoInstruction::PopBlock); + self.set_no_location(); + self.pop_fblock(FBlockType::ExceptionHandler); + emit!(self, Instruction::PopExcept); + self.set_no_location(); + emit!( + self, + PseudoInstruction::JumpNoInterrupt { delta: end_block } + ); + self.set_no_location(); + } + + self.push_fblock(FBlockType::ExceptionHandler, cleanup_block, cleanup_block)?; + self.switch_to_block(next_handler); + } + + emit!(self, Instruction::Reraise { depth: 0 }); + self.set_no_location(); + self.pop_fblock(FBlockType::ExceptionHandler); + + self.switch_to_block(cleanup_block); + 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(); + + self.switch_to_block(orelse_block); + self.set_no_location(); + self.compile_statements(orelse)?; + emit!( + self, + PseudoInstruction::JumpNoInterrupt { delta: end_block } + ); + self.set_no_location(); + + self.switch_to_block(end_block); + Ok(()) + } + fn compile_try_star_except( &mut self, body: &[ast::Stmt], @@ -6801,21 +7013,7 @@ impl Compiler { self.compile_expression(last_comparator)?; self.set_source_range(compare_range); self.compile_addcompare(last_op); - if condition { - emit!( - self, - Instruction::PopJumpIfTrue { - delta: target_block - } - ); - } else { - emit!( - self, - Instruction::PopJumpIfFalse { - delta: target_block, - } - ); - } + self.emit_pop_jump_by_condition(condition, target_block); return Ok(()); } @@ -6834,21 +7032,7 @@ impl Compiler { self.compile_expression(last_comparator)?; self.set_source_range(compare_range); self.compile_addcompare(last_op); - if condition { - emit!( - self, - Instruction::PopJumpIfTrue { - delta: target_block - } - ); - } else { - emit!( - self, - Instruction::PopJumpIfFalse { - delta: target_block, - } - ); - } + self.emit_pop_jump_by_condition(condition, target_block); emit!(self, PseudoInstruction::Jump { delta: end }); self.switch_to_block(cleanup); @@ -6866,6 +7050,24 @@ impl Compiler { Ok(()) } + fn emit_pop_jump_by_condition(&mut self, condition: bool, target_block: BlockIdx) { + if condition { + emit!( + self, + Instruction::PopJumpIfTrue { + delta: target_block + } + ); + } else { + emit!( + self, + Instruction::PopJumpIfFalse { + delta: target_block, + } + ); + } + } + fn compile_annotation(&mut self, annotation: &ast::Expr) -> CompileResult<()> { if self.future_annotations { self.emit_load_const(ConstantData::Str { @@ -7554,7 +7756,26 @@ impl Compiler { | ast::Expr::BooleanLiteral(_) | ast::Expr::NoneLiteral(_) | ast::Expr::EllipsisLiteral(_) - ) + ) || matches!(expr, ast::Expr::FString(fstring) if Self::fstring_value_is_const(&fstring.value)) + } + + fn fstring_value_is_const(fstring: &ast::FStringValue) -> bool { + for part in fstring { + if !Self::fstring_part_is_const(part) { + return false; + } + } + true + } + + fn fstring_part_is_const(part: &ast::FStringPart) -> bool { + match part { + ast::FStringPart::Literal(_) => true, + ast::FStringPart::FString(fstring) => fstring + .elements + .iter() + .all(|element| matches!(element, ast::InterpolatedStringElement::Literal(_))), + } } fn compile_expression(&mut self, expression: &ast::Expr) -> CompileResult<()> { @@ -8081,6 +8302,149 @@ impl Compiler { Ok(()) } + fn detect_builtin_generator_call( + &self, + func: &ast::Expr, + args: &ast::Arguments, + ) -> Option { + let ast::Expr::Name(ast::ExprName { id, .. }) = func else { + return None; + }; + if args.args.len() != 1 + || !args.keywords.is_empty() + || !matches!(args.args[0], ast::Expr::Generator(_)) + { + return None; + } + match id.as_str() { + "tuple" => Some(BuiltinGeneratorCallKind::Tuple), + "list" => Some(BuiltinGeneratorCallKind::List), + "set" => Some(BuiltinGeneratorCallKind::Set), + "all" => Some(BuiltinGeneratorCallKind::All), + "any" => Some(BuiltinGeneratorCallKind::Any), + _ => None, + } + } + + /// Emit the optimized inline loop for builtin(genexpr) calls. + /// + /// Stack on entry: `[func, iter]` where `iter` is the already-compiled + /// generator iterator and `func` is the builtin candidate. + /// On return the compiler is positioned at the fallback block with + /// `[func, iter]` still on the stack (for the normal CALL path). + fn optimize_builtin_generator_call( + &mut self, + kind: BuiltinGeneratorCallKind, + end: BlockIdx, + ) -> CompileResult<()> { + let common_constant = match kind { + BuiltinGeneratorCallKind::Tuple => bytecode::CommonConstant::BuiltinTuple, + BuiltinGeneratorCallKind::List => bytecode::CommonConstant::BuiltinList, + BuiltinGeneratorCallKind::Set => bytecode::CommonConstant::BuiltinSet, + BuiltinGeneratorCallKind::All => bytecode::CommonConstant::BuiltinAll, + BuiltinGeneratorCallKind::Any => bytecode::CommonConstant::BuiltinAny, + }; + + let loop_block = self.new_block(); + let cleanup = self.new_block(); + let fallback = self.new_block(); + let result = matches!( + kind, + BuiltinGeneratorCallKind::All | BuiltinGeneratorCallKind::Any + ) + .then(|| self.new_block()); + + // Stack: [func, iter] — copy func (TOS1) for identity check + emit!(self, Instruction::Copy { i: 2 }); + emit!( + self, + Instruction::LoadCommonConstant { + idx: common_constant + } + ); + emit!(self, Instruction::IsOp { invert: Invert::No }); + emit!(self, Instruction::PopJumpIfFalse { delta: fallback }); + emit!(self, Instruction::NotTaken); + // Remove func from [func, iter] → [iter] + emit!(self, Instruction::Swap { i: 2 }); + emit!(self, Instruction::PopTop); + + if matches!( + kind, + BuiltinGeneratorCallKind::Tuple | BuiltinGeneratorCallKind::List + ) { + // [iter] → [iter, list] → [list, iter] + emit!(self, Instruction::BuildList { count: 0 }); + emit!(self, Instruction::Swap { i: 2 }); + } else if matches!(kind, BuiltinGeneratorCallKind::Set) { + // [iter] → [iter, set] → [set, iter] + emit!(self, Instruction::BuildSet { count: 0 }); + emit!(self, Instruction::Swap { i: 2 }); + } + + self.switch_to_block(loop_block); + emit!(self, Instruction::ForIter { delta: cleanup }); + + match kind { + BuiltinGeneratorCallKind::Tuple | BuiltinGeneratorCallKind::List => { + emit!(self, Instruction::ListAppend { i: 2 }); + emit!(self, PseudoInstruction::Jump { delta: loop_block }); + } + BuiltinGeneratorCallKind::Set => { + emit!(self, Instruction::SetAdd { i: 2 }); + emit!(self, PseudoInstruction::Jump { delta: loop_block }); + } + BuiltinGeneratorCallKind::All => { + let result = result.expect("all() optimization should have a result block"); + emit!(self, Instruction::ToBool); + emit!(self, Instruction::PopJumpIfFalse { delta: result }); + emit!(self, Instruction::NotTaken); + emit!(self, PseudoInstruction::Jump { delta: loop_block }); + } + BuiltinGeneratorCallKind::Any => { + let result = result.expect("any() optimization should have a result block"); + emit!(self, Instruction::ToBool); + emit!(self, Instruction::PopJumpIfTrue { delta: result }); + emit!(self, Instruction::NotTaken); + emit!(self, PseudoInstruction::Jump { delta: loop_block }); + } + } + + if let Some(result_block) = result { + self.switch_to_block(result_block); + emit!(self, Instruction::PopIter); + self.emit_load_const(ConstantData::Boolean { + value: matches!(kind, BuiltinGeneratorCallKind::Any), + }); + emit!(self, PseudoInstruction::Jump { delta: end }); + } + + self.switch_to_block(cleanup); + emit!(self, Instruction::EndFor); + emit!(self, Instruction::PopIter); + match kind { + BuiltinGeneratorCallKind::Tuple => { + emit!( + self, + Instruction::CallIntrinsic1 { + func: IntrinsicFunction1::ListToTuple + } + ); + } + BuiltinGeneratorCallKind::List | BuiltinGeneratorCallKind::Set => {} + BuiltinGeneratorCallKind::All => { + self.emit_load_const(ConstantData::Boolean { value: true }); + } + BuiltinGeneratorCallKind::Any => { + self.emit_load_const(ConstantData::Boolean { value: false }); + } + } + emit!(self, PseudoInstruction::Jump { delta: end }); + + self.switch_to_block(fallback); + Ok(()) + } + fn compile_call(&mut self, func: &ast::Expr, args: &ast::Arguments) -> CompileResult<()> { // Save the call expression's source range so CALL instructions use the // call start line, not the last argument's line. @@ -8131,6 +8495,23 @@ impl Compiler { } self.codegen_call_helper(0, args, call_range)?; } + } else if let Some(kind) = (!uses_ex_call) + .then(|| self.detect_builtin_generator_call(func, args)) + .flatten() + { + // Optimized builtin(genexpr) path: compile the genexpr only once + // so its code object appears exactly once in co_consts. + let end = self.new_block(); + self.compile_expression(func)?; + self.compile_expression(&args.args[0])?; + // Stack: [func, iter] + self.optimize_builtin_generator_call(kind, end)?; + // Fallback block: [func, iter] → [func, null, iter] → CALL + emit!(self, Instruction::PushNull); + emit!(self, Instruction::Swap { i: 2 }); + self.set_source_range(call_range); + emit!(self, Instruction::Call { argc: 1 }); + self.switch_to_block(end); } else { // Regular call: push func, then NULL for self_or_null slot // Stack layout: [func, NULL, args...] - same as method call [func, self, args...] @@ -9101,6 +9482,16 @@ impl Compiler { ast::Expr::BooleanLiteral(b) => ConstantData::Boolean { value: b.value }, ast::Expr::NoneLiteral(_) => ConstantData::None, ast::Expr::EllipsisLiteral(_) => ConstantData::Ellipsis, + ast::Expr::Tuple(ast::ExprTuple { elts, .. }) => { + let mut elements = Vec::with_capacity(elts.len()); + for elt in elts { + let Some(constant) = self.try_fold_constant_expr(elt)? else { + return Ok(None); + }; + elements.push(constant); + } + ConstantData::Tuple { elements } + } _ => return Ok(None), })) } @@ -9640,10 +10031,8 @@ impl Compiler { mut pending_literal: Option, mut element_count: u32, ) -> CompileResult<()> { - if let Some(value) = pending_literal.take() { - self.emit_load_const(ConstantData::Str { value }); - element_count += 1; - } + let keep_empty = element_count == 0; + self.emit_pending_fstring_literal(&mut pending_literal, &mut element_count, keep_empty); if element_count == 0 { self.emit_load_const(ConstantData::Str { @@ -9661,6 +10050,27 @@ impl Compiler { Ok(()) } + fn emit_pending_fstring_literal( + &mut self, + pending_literal: &mut Option, + element_count: &mut u32, + keep_empty: bool, + ) { + let Some(value) = pending_literal.take() else { + return; + }; + + // CPython drops empty literal fragments when they are adjacent to + // formatted values, but still emits an empty string for a fully-empty + // f-string. + if value.is_empty() && (!keep_empty || *element_count > 0) { + return; + } + + self.emit_load_const(ConstantData::Str { value }); + *element_count += 1; + } + /// Optimize `'format_str' % (args,)` into f-string bytecode. /// Returns true if optimization was applied, false to fall back to normal BINARY_OP %. /// Matches CPython's codegen.c `compiler_formatted_value` optimization. @@ -9846,10 +10256,7 @@ impl Compiler { } } - if let Some(value) = pending_literal.take() { - self.emit_load_const(ConstantData::Str { value }); - *element_count += 1; - } + self.emit_pending_fstring_literal(pending_literal, element_count, false); self.compile_expression(&fstring_expr.expression)?; @@ -10393,6 +10800,24 @@ mod tests { }) } + fn has_common_constant(code: &CodeObject, expected: bytecode::CommonConstant) -> bool { + code.instructions.iter().any(|unit| match unit.op { + Instruction::LoadCommonConstant { idx } => { + idx.get(OpArg::new(u32::from(u8::from(unit.arg)))) == expected + } + _ => false, + }) + } + + fn has_intrinsic_1(code: &CodeObject, expected: IntrinsicFunction1) -> bool { + code.instructions.iter().any(|unit| match unit.op { + Instruction::CallIntrinsic1 { func } => { + func.get(OpArg::new(u32::from(u8::from(unit.arg)))) == expected + } + _ => false, + }) + } + macro_rules! assert_dis_snapshot { ($value:expr) => { insta::assert_snapshot!( @@ -10550,6 +10975,116 @@ def f(obj, arg): ); } + #[test] + fn test_builtin_any_genexpr_call_is_optimized() { + let code = compile_exec( + "\ +def f(xs): + return any(x for x in xs) +", + ); + let f = find_code(&code, "f").expect("missing function code"); + + assert!(has_common_constant(f, bytecode::CommonConstant::BuiltinAny)); + assert!( + f.instructions + .iter() + .any(|unit| matches!(unit.op, Instruction::PopJumpIfTrue { .. })) + ); + assert!( + f.instructions + .iter() + .any(|unit| matches!(unit.op, Instruction::NotTaken)) + ); + assert_eq!( + f.instructions + .iter() + .filter(|unit| matches!(unit.op, Instruction::PushNull)) + .count(), + 1, + "fallback call path should remain for shadowed any()" + ); + } + + #[test] + fn test_builtin_tuple_list_set_genexpr_calls_are_optimized() { + let code = compile_exec( + "\ +def tuple_f(xs): + return tuple(x for x in xs) + +def list_f(xs): + return list(x for x in xs) + +def set_f(xs): + return set(x for x in xs) +", + ); + + let tuple_f = find_code(&code, "tuple_f").expect("missing tuple_f code"); + assert!(has_common_constant( + tuple_f, + bytecode::CommonConstant::BuiltinTuple + )); + assert!(has_intrinsic_1(tuple_f, IntrinsicFunction1::ListToTuple)); + let tuple_list_append = tuple_f + .instructions + .iter() + .find_map(|unit| match unit.op { + Instruction::ListAppend { .. } => Some(u32::from(u8::from(unit.arg))), + _ => None, + }) + .expect("tuple(genexpr) fast path should emit LIST_APPEND"); + assert_eq!(tuple_list_append, 2); + + let list_f = find_code(&code, "list_f").expect("missing list_f code"); + assert!(has_common_constant( + list_f, + bytecode::CommonConstant::BuiltinList + )); + assert!( + list_f + .instructions + .iter() + .any(|unit| matches!(unit.op, Instruction::ListAppend { .. })) + ); + + let set_f = find_code(&code, "set_f").expect("missing set_f code"); + assert!(has_common_constant( + set_f, + bytecode::CommonConstant::BuiltinSet + )); + assert!( + set_f + .instructions + .iter() + .any(|unit| matches!(unit.op, Instruction::SetAdd { .. })) + ); + } + + #[test] + fn test_module_store_uses_store_global_when_nested_scope_declares_global() { + let code = compile_exec( + "\ +_address_fmt_re = None + +class C: + def f(self): + global _address_fmt_re + if _address_fmt_re is None: + _address_fmt_re = 1 +", + ); + + assert!(code.instructions.iter().any(|unit| match unit.op { + Instruction::StoreGlobal { namei } => { + let idx = namei.get(OpArg::new(u32::from(u8::from(unit.arg)))); + code.names[usize::try_from(idx).unwrap()].as_str() == "_address_fmt_re" + } + _ => false, + })); + } + #[test] fn test_conditional_return_epilogue_is_duplicated() { let code = compile_exec( @@ -10708,6 +11243,160 @@ def f(cls, proto): ); } + #[test] + fn test_literal_only_fstring_statement_is_optimized_away() { + let code = compile_exec( + "\ +def f(): + f'''Not a docstring''' +", + ); + let f = find_code(&code, "f").expect("missing function code"); + + assert!( + !f.instructions + .iter() + .any(|unit| matches!(unit.op, Instruction::PopTop)), + "literal-only f-string statement should be removed" + ); + assert!( + !f.constants.iter().any(|constant| matches!( + constant, + ConstantData::Str { value } if value.to_string() == "Not a docstring" + )), + "literal-only f-string should not survive in constants" + ); + } + + #[test] + fn test_empty_fstring_literals_are_elided_around_interpolation() { + let code = compile_exec( + "\ +def f(x): + if '' f'{x}': + return 1 + return 2 +", + ); + let f = find_code(&code, "f").expect("missing function code"); + + let empty_string_loads = f + .instructions + .iter() + .filter_map(|unit| match unit.op { + Instruction::LoadConst { consti } => { + Some(&f.constants[consti.get(OpArg::new(u32::from(u8::from(unit.arg))))]) + } + _ => None, + }) + .filter(|constant| { + matches!( + constant, + ConstantData::Str { value } if value.is_empty() + ) + }) + .count(); + let build_string_count = f + .instructions + .iter() + .filter(|unit| matches!(unit.op, Instruction::BuildString { .. })) + .count(); + + assert_eq!(empty_string_loads, 0); + assert_eq!(build_string_count, 0); + } + + #[test] + fn test_large_power_is_not_constant_folded() { + let code = compile_exec("x = 2**100\n"); + + assert!(code.instructions.iter().any(|unit| match unit.op { + Instruction::BinaryOp { op } => { + op.get(OpArg::new(u32::from(u8::from(unit.arg)))) == oparg::BinaryOperator::Power + } + _ => false, + })); + } + + #[test] + fn test_list_of_constant_tuples_uses_list_extend() { + let code = compile_exec( + "\ +deprecated_cases = [('a', 'b'), ('c', 'd'), ('e', 'f'), ('g', 'h'), ('i', 'j')] +", + ); + + assert!( + code.instructions + .iter() + .any(|unit| matches!(unit.op, Instruction::ListExtend { .. })), + "expected constant tuple list folding" + ); + } + + #[test] + fn test_constant_list_iterable_uses_tuple() { + let code = compile_exec( + "\ +def f(): + return {x: y for x, y in [(1, 2), ]} +", + ); + let f = find_code(&code, "f").expect("missing function code"); + + assert!( + !f.instructions + .iter() + .any(|unit| matches!(unit.op, Instruction::BuildList { .. })), + "constant list iterable should avoid BUILD_LIST before GET_ITER" + ); + assert!(f.constants.iter().any(|constant| matches!( + constant, + ConstantData::Tuple { elements } + if matches!( + elements.as_slice(), + [ConstantData::Tuple { elements: inner }] + if matches!( + inner.as_slice(), + [ + ConstantData::Integer { .. }, + ConstantData::Integer { .. } + ] + ) + ) + ))); + } + + #[test] + fn test_constant_set_iterable_keeps_runtime_set_build() { + let code = compile_exec( + "\ +def f(): + return [x for x in {1, 2, 3}] +", + ); + let f = find_code(&code, "f").expect("missing function code"); + + assert!( + f.instructions + .iter() + .any(|unit| matches!(unit.op, Instruction::BuildSet { .. })), + "constant set iterable should keep BUILD_SET before GET_ITER" + ); + assert!(f.constants.iter().any(|constant| matches!( + constant, + ConstantData::Tuple { elements } + if matches!( + elements.as_slice(), + [ + ConstantData::Integer { .. }, + ConstantData::Integer { .. }, + ConstantData::Integer { .. } + ] + ) + ))); + } + #[test] fn test_optimized_assert_preserves_nested_scope_order() { compile_exec_optimized( diff --git a/crates/codegen/src/ir.rs b/crates/codegen/src/ir.rs index 12c1961e88f..bc709147479 100644 --- a/crates/codegen/src/ir.rs +++ b/crates/codegen/src/ir.rs @@ -25,6 +25,9 @@ struct LineTableLocation { end_col: i32, } +const MAX_INT_SIZE_BITS: u64 = 128; +const MIN_CONST_SEQUENCE_SIZE: usize = 3; + /// Metadata for a code unit // = _PyCompile_CodeUnitMetadata #[derive(Clone, Debug)] @@ -795,7 +798,12 @@ impl CodeInfo { let result = match op { BinOp::Add => l + r, BinOp::Subtract => l - r, - BinOp::Multiply => l * r, + BinOp::Multiply => { + if !l.is_zero() && !r.is_zero() && l.bits() + r.bits() > MAX_INT_SIZE_BITS { + return None; + } + l * r + } BinOp::FloorDivide => { if r.is_zero() { return None; @@ -821,18 +829,22 @@ impl CodeInfo { } } BinOp::Power => { - let exp: u32 = r.try_into().ok()?; - if exp > 128 { + let exp: u64 = r.try_into().ok()?; + let exp_usize = usize::try_from(exp).ok()?; + if !l.is_zero() && exp > 0 && l.bits() > MAX_INT_SIZE_BITS / exp { return None; - } // prevent huge results - num_traits::pow::pow(l.clone(), exp as usize) + } + num_traits::pow::pow(l.clone(), exp_usize) } BinOp::Lshift => { - let shift: u32 = r.try_into().ok()?; - if shift > 128 { + let shift: u64 = r.try_into().ok()?; + let shift_usize = usize::try_from(shift).ok()?; + if shift > MAX_INT_SIZE_BITS + || (!l.is_zero() && l.bits() > MAX_INT_SIZE_BITS - shift) + { return None; } - l << (shift as usize) + l << shift_usize } BinOp::Rshift => { let shift: u32 = r.try_into().ok()?; @@ -1070,7 +1082,7 @@ impl CodeInfo { } } - if !all_const || list_size < 3 { + if !all_const || list_size < MIN_CONST_SEQUENCE_SIZE { i += 1; continue; } @@ -1117,31 +1129,42 @@ impl CodeInfo { } } - /// Convert constant list/set construction before GET_ITER to just LOAD_CONST tuple. + /// Convert constant list construction before GET_ITER to just LOAD_CONST tuple. /// BUILD_LIST 0 + LOAD_CONST (tuple) + LIST_EXTEND 1 + GET_ITER /// → LOAD_CONST (tuple) + GET_ITER - /// Also handles BUILD_SET 0 + LOAD_CONST + SET_UPDATE 1 + GET_ITER. fn fold_const_iterable_for_iter(&mut self) { for block in &mut self.blocks { let mut i = 0; - while i + 3 < block.instructions.len() { + while i + 1 < block.instructions.len() { let is_build = matches!( block.instructions[i].instr.real(), - Some(Instruction::BuildList { .. } | Instruction::BuildSet { .. }) + Some(Instruction::BuildList { .. }) ) && u32::from(block.instructions[i].arg) == 0; let is_const = matches!( - block.instructions[i + 1].instr.real(), + block + .instructions + .get(i + 1) + .and_then(|instr| instr.instr.real()), Some(Instruction::LoadConst { .. }) ); let is_extend = matches!( - block.instructions[i + 2].instr.real(), - Some(Instruction::ListExtend { .. } | Instruction::SetUpdate { .. }) - ) && u32::from(block.instructions[i + 2].arg) == 1; + block + .instructions + .get(i + 2) + .and_then(|instr| instr.instr.real()), + Some(Instruction::ListExtend { .. }) + ) && block + .instructions + .get(i + 2) + .is_some_and(|instr| u32::from(instr.arg) == 1); let is_iter = matches!( - block.instructions[i + 3].instr.real(), + block + .instructions + .get(i + 3) + .and_then(|instr| instr.instr.real()), Some(Instruction::GetIter) ); @@ -1153,6 +1176,56 @@ impl CodeInfo { block.instructions[i + 2].instr = Instruction::Nop.into(); block.instructions[i + 2].location = loc; i += 4; + } else if matches!( + block.instructions[i].instr.real(), + Some(Instruction::BuildList { .. }) + ) && matches!( + block.instructions[i + 1].instr.real(), + Some(Instruction::GetIter) + ) { + let seq_size = u32::from(block.instructions[i].arg) as usize; + + if seq_size != 0 && i >= seq_size { + let start_idx = i - seq_size; + let mut elements = Vec::with_capacity(seq_size); + let mut all_const = true; + + for j in start_idx..i { + match Self::get_const_value_from(&self.metadata, &block.instructions[j]) + { + Some(constant) => elements.push(constant), + None => { + all_const = false; + break; + } + } + } + + if all_const { + let const_data = ConstantData::Tuple { elements }; + let (const_idx, _) = self.metadata.consts.insert_full(const_data); + let folded_loc = block.instructions[i].location; + + for j in start_idx..i { + block.instructions[j].instr = Instruction::Nop.into(); + block.instructions[j].location = folded_loc; + } + + block.instructions[i].instr = Instruction::LoadConst { + consti: Arg::marker(), + } + .into(); + block.instructions[i].arg = OpArg::new(const_idx as u32); + i += 2; + continue; + } + } + + block.instructions[i].instr = Instruction::BuildTuple { + count: Arg::marker(), + } + .into(); + i += 2; } else { i += 1; } @@ -2669,14 +2742,16 @@ fn duplicate_exits_without_lineno(blocks: &mut Vec, predecessors: &mut Ve continue; } - // Copy the exit block + // Copy the exit block and splice it into the linked list after current let new_idx = BlockIdx(blocks.len() as u32); let mut new_block = blocks[target.idx()].clone(); let jump_loc = last.location; let jump_end_loc = last.end_location; propagate_locations_in_block(&mut new_block, jump_loc, jump_end_loc); - new_block.next = blocks[target.idx()].next; + let old_next = blocks[current.idx()].next; + new_block.next = old_next; blocks.push(new_block); + blocks[current.idx()].next = new_idx; // Update the jump target let last_mut = blocks[current.idx()].instructions.last_mut().unwrap(); @@ -2684,7 +2759,8 @@ fn duplicate_exits_without_lineno(blocks: &mut Vec, predecessors: &mut Ve predecessors[target.idx()] -= 1; predecessors.push(1); - current = blocks[current.idx()].next; + // Skip past the newly inserted block + current = old_next; } current = BlockIdx(0); diff --git a/crates/codegen/src/snapshots/rustpython_codegen__compile__tests__nested_double_async_with.snap b/crates/codegen/src/snapshots/rustpython_codegen__compile__tests__nested_double_async_with.snap index 02af6cad00b..ba8be3589d3 100644 --- a/crates/codegen/src/snapshots/rustpython_codegen__compile__tests__nested_double_async_with.snap +++ b/crates/codegen/src/snapshots/rustpython_codegen__compile__tests__nested_double_async_with.snap @@ -33,12 +33,12 @@ expression: "compile_exec(\"\\\nasync def test():\n for stop_exc in (StopIter 26 CACHE 27 STORE_FAST (0, stop_exc) - 3 >> 28 LOAD_GLOBAL (4, self) + 3 28 LOAD_GLOBAL (4, self) 29 CACHE 30 CACHE 31 CACHE - 32 CACHE - >> 33 LOAD_ATTR (7, subTest, method=true) + >> 32 CACHE + 33 LOAD_ATTR (7, subTest, method=true) 34 CACHE 35 CACHE 36 CACHE @@ -51,9 +51,9 @@ expression: "compile_exec(\"\\\nasync def test():\n for stop_exc in (StopIter 43 LOAD_GLOBAL (9, NULL + type) 44 CACHE 45 CACHE - >> 46 CACHE + 46 CACHE 47 CACHE - 48 LOAD_FAST (0, stop_exc) + >> 48 LOAD_FAST (0, stop_exc) 49 CALL (1) 50 CACHE 51 CACHE @@ -138,30 +138,30 @@ expression: "compile_exec(\"\\\nasync def test():\n for stop_exc in (StopIter 125 POP_TOP 126 POP_TOP 127 POP_TOP - 128 JUMP_FORWARD (3) + 128 JUMP_FORWARD (48) 129 COPY (3) 130 POP_EXCEPT 131 RERAISE (1) - 132 JUMP_FORWARD (46) - 133 PUSH_EXC_INFO + 132 PUSH_EXC_INFO - 7 134 LOAD_GLOBAL (12, Exception) + 7 133 LOAD_GLOBAL (12, Exception) + 134 CACHE 135 CACHE 136 CACHE 137 CACHE - 138 CACHE - 139 CHECK_EXC_MATCH - 140 POP_JUMP_IF_FALSE (33) - 141 CACHE - 142 NOT_TAKEN - 143 STORE_FAST (1, ex) + 138 CHECK_EXC_MATCH + 139 POP_JUMP_IF_FALSE (32) + 140 CACHE + 141 NOT_TAKEN + 142 STORE_FAST (1, ex) - 8 144 LOAD_GLOBAL (4, self) + 8 143 LOAD_GLOBAL (4, self) + 144 CACHE 145 CACHE 146 CACHE 147 CACHE - 148 CACHE - 149 LOAD_ATTR (15, assertIs, method=true) + 148 LOAD_ATTR (15, assertIs, method=true) + 149 CACHE 150 CACHE 151 CACHE 152 CACHE @@ -170,87 +170,85 @@ expression: "compile_exec(\"\\\nasync def test():\n for stop_exc in (StopIter 155 CACHE 156 CACHE 157 CACHE - 158 CACHE - 159 LOAD_FAST_LOAD_FAST (ex, stop_exc) - 160 CALL (2) + 158 LOAD_FAST_LOAD_FAST (ex, stop_exc) + 159 CALL (2) + 160 CACHE 161 CACHE 162 CACHE - 163 CACHE - 164 POP_TOP - 165 JUMP_FORWARD (4) - 166 LOAD_CONST (None) - 167 STORE_FAST (1, ex) - 168 DELETE_FAST (1, ex) - 169 RERAISE (1) - 170 POP_EXCEPT - 171 LOAD_CONST (None) - 172 STORE_FAST (1, ex) - 173 DELETE_FAST (1, ex) - 174 JUMP_FORWARD (28) - 175 RERAISE (0) - 176 COPY (3) - 177 POP_EXCEPT - 178 RERAISE (1) + 163 POP_TOP + 164 POP_EXCEPT + 165 LOAD_CONST (None) + 166 STORE_FAST (1, ex) + 167 DELETE_FAST (1, ex) + 168 JUMP_FORWARD (32) + 169 LOAD_CONST (None) + 170 STORE_FAST (1, ex) + 171 DELETE_FAST (1, ex) + 172 RERAISE (1) + 173 RERAISE (0) + 174 COPY (3) + 175 POP_EXCEPT + 176 RERAISE (1) - 10 179 LOAD_GLOBAL (4, self) + 10 177 LOAD_GLOBAL (4, self) + 178 CACHE + 179 CACHE 180 CACHE 181 CACHE - 182 CACHE + 182 LOAD_ATTR (17, fail, method=true) 183 CACHE - 184 LOAD_ATTR (17, fail, method=true) + 184 CACHE 185 CACHE 186 CACHE - 187 CACHE + >> 187 CACHE 188 CACHE - >> 189 CACHE + 189 CACHE 190 CACHE 191 CACHE - 192 CACHE - 193 CACHE - 194 LOAD_FAST_BORROW (0, stop_exc) - 195 FORMAT_SIMPLE - 196 LOAD_CONST (" was suppressed") - 197 BUILD_STRING (2) - 198 CALL (1) + 192 LOAD_FAST_BORROW (0, stop_exc) + 193 FORMAT_SIMPLE + 194 LOAD_CONST (" was suppressed") + 195 BUILD_STRING (2) + 196 CALL (1) + 197 CACHE + 198 CACHE 199 CACHE - 200 CACHE - 201 CACHE - 202 POP_TOP - 203 NOP + 200 POP_TOP + 201 NOP - 3 204 LOAD_CONST (None) - 205 LOAD_CONST (None) - >> 206 LOAD_CONST (None) - 207 CALL (3) + 3 202 LOAD_CONST (None) + 203 LOAD_CONST (None) + >> 204 LOAD_CONST (None) + 205 CALL (3) + 206 CACHE + 207 CACHE 208 CACHE - 209 CACHE - 210 CACHE - 211 POP_TOP - 212 JUMP_BACKWARD (189) - 213 CACHE - 214 PUSH_EXC_INFO - 215 WITH_EXCEPT_START - 216 TO_BOOL + 209 POP_TOP + 210 JUMP_BACKWARD (187) + 211 CACHE + 212 PUSH_EXC_INFO + 213 WITH_EXCEPT_START + 214 TO_BOOL + 215 CACHE + 216 CACHE 217 CACHE - 218 CACHE + 218 POP_JUMP_IF_TRUE (2) 219 CACHE - 220 POP_JUMP_IF_TRUE (2) - 221 CACHE - 222 NOT_TAKEN - 223 RERAISE (2) + 220 NOT_TAKEN + 221 RERAISE (2) + 222 POP_TOP + 223 POP_EXCEPT 224 POP_TOP - 225 POP_EXCEPT + 225 POP_TOP 226 POP_TOP - 227 POP_TOP - 228 POP_TOP - 229 JUMP_BACKWARD (206) - 230 CACHE - 231 COPY (3) - 232 POP_EXCEPT - 233 RERAISE (1) + 227 JUMP_BACKWARD (204) + 228 CACHE + 229 COPY (3) + 230 POP_EXCEPT + 231 RERAISE (1) - 2 234 CALL_INTRINSIC_1 (StopIterationError) - 235 RERAISE (1) + 2 232 CALL_INTRINSIC_1 (StopIterationError) + 233 RERAISE (1) 2 MAKE_FUNCTION 3 STORE_NAME (0, test) diff --git a/crates/compiler-core/src/bytecode/oparg.rs b/crates/compiler-core/src/bytecode/oparg.rs index 068229f10fd..a4edbcab0c5 100644 --- a/crates/compiler-core/src/bytecode/oparg.rs +++ b/crates/compiler-core/src/bytecode/oparg.rs @@ -645,6 +645,10 @@ oparg_enum!( BuiltinAll = 3, /// Built-in `any` function BuiltinAny = 4, + /// Built-in `list` type + BuiltinList = 5, + /// Built-in `set` type + BuiltinSet = 6, } ); @@ -656,6 +660,8 @@ impl fmt::Display for CommonConstant { Self::BuiltinTuple => "tuple", Self::BuiltinAll => "all", Self::BuiltinAny => "any", + Self::BuiltinList => "list", + Self::BuiltinSet => "set", }; write!(f, "{name}") } diff --git a/crates/vm/src/frame.rs b/crates/vm/src/frame.rs index 5c2312015e9..7cb33ec95d9 100644 --- a/crates/vm/src/frame.rs +++ b/crates/vm/src/frame.rs @@ -2786,8 +2786,18 @@ impl ExecutingFrame<'_> { vm.ctx.exceptions.not_implemented_error.to_owned().into() } CommonConstant::BuiltinTuple => vm.ctx.types.tuple_type.to_owned().into(), - CommonConstant::BuiltinAll => vm.builtins.get_attr("all", vm)?, - CommonConstant::BuiltinAny => vm.builtins.get_attr("any", vm)?, + CommonConstant::BuiltinAll => vm + .callable_cache + .builtin_all + .clone() + .expect("builtin_all not initialized"), + CommonConstant::BuiltinAny => vm + .callable_cache + .builtin_any + .clone() + .expect("builtin_any not initialized"), + CommonConstant::BuiltinList => vm.ctx.types.list_type.to_owned().into(), + CommonConstant::BuiltinSet => vm.ctx.types.set_type.to_owned().into(), }; self.push_value(value); Ok(None) diff --git a/crates/vm/src/version.rs b/crates/vm/src/version.rs index 21efecd6c5a..5b80a33322b 100644 --- a/crates/vm/src/version.rs +++ b/crates/vm/src/version.rs @@ -129,8 +129,8 @@ pub fn get_git_datetime() -> String { } // Must be aligned to Lib/importlib/_bootstrap_external.py -// Bumped to 2997 for MAKE_CELL/COPY_FREE_VARS prolog and cell-local merging -pub const PYC_MAGIC_NUMBER: u16 = 2997; +// Bumped to 2994 for new CommonConstant discriminants (BuiltinList, BuiltinSet) +pub const PYC_MAGIC_NUMBER: u16 = 2994; // CPython format: magic_number | ('\r' << 16) | ('\n' << 24) // This protects against text-mode file reads diff --git a/crates/vm/src/vm/mod.rs b/crates/vm/src/vm/mod.rs index eee2055b751..da9149cc5fc 100644 --- a/crates/vm/src/vm/mod.rs +++ b/crates/vm/src/vm/mod.rs @@ -576,6 +576,8 @@ pub(crate) struct CallableCache { pub len: Option, pub isinstance: Option, pub list_append: Option, + pub builtin_all: Option, + pub builtin_any: Option, } pub struct PyGlobalState { @@ -641,6 +643,8 @@ impl VirtualMachine { .get_attr(self.ctx.intern_str("append")) .ok_or_else(|| self.new_runtime_error("failed to cache list.append".to_owned()))?; self.callable_cache.list_append = Some(list_append); + self.callable_cache.builtin_all = Some(self.builtins.get_attr("all", self)?); + self.callable_cache.builtin_any = Some(self.builtins.get_attr("any", self)?); Ok(()) }