diff --git a/Lib/test/test_compile.py b/Lib/test/test_compile.py index e5bc65651e9..94a9ae899b0 100644 --- a/Lib/test/test_compile.py +++ b/Lib/test/test_compile.py @@ -2576,7 +2576,6 @@ def test_if_else(self): def test_binop(self): self.check_stack_size("x + " * self.N + "x") - @unittest.expectedFailure # TODO: RUSTPYTHON; AssertionError: 101 not less than or equal to 6 def test_list(self): self.check_stack_size("[" + "x, " * self.N + "x]") @@ -2584,7 +2583,6 @@ def test_list(self): def test_tuple(self): self.check_stack_size("(" + "x, " * self.N + "x)") - @unittest.expectedFailure # TODO: RUSTPYTHON; AssertionError: 101 not less than or equal to 6 def test_set(self): self.check_stack_size("{" + "x, " * self.N + "x}") diff --git a/Lib/test/test_peepholer.py b/Lib/test/test_peepholer.py index c02bd559f1c..53ff218c4e1 100644 --- a/Lib/test/test_peepholer.py +++ b/Lib/test/test_peepholer.py @@ -441,7 +441,6 @@ def test_constant_folding_binop(self): self.check_lnotab(code) - @unittest.expectedFailure # TODO: RUSTPYTHON def test_constant_folding_remove_nop_location(self): sources = [ """ diff --git a/Lib/test/test_tstring.py b/Lib/test/test_tstring.py index a1f686c8f56..e6984342403 100644 --- a/Lib/test/test_tstring.py +++ b/Lib/test/test_tstring.py @@ -111,7 +111,6 @@ def test_conversions(self): with self.assertRaises(SyntaxError): eval("t'{num!z}'") - @unittest.expectedFailure # TODO: RUSTPYTHON; ? ++++++ def test_debug_specifier(self): # Test debug specifier value = 42 diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py index 448d16f1f4a..3ab16478c6b 100644 --- a/Lib/test/test_typing.py +++ b/Lib/test/test_typing.py @@ -4697,7 +4697,6 @@ class D(Generic[T]): pass with self.assertRaises(TypeError): D[()] - @unittest.expectedFailure # TODO: RUSTPYTHON def test_generic_init_subclass_not_called_error(self): notes = ["Note: this exception may have been caused by " r"'GenericTests.test_generic_init_subclass_not_called_error..Base.__init_subclass__' " diff --git a/crates/codegen/src/compile.rs b/crates/codegen/src/compile.rs index 2f7510f0d44..0b82f400dfb 100644 --- a/crates/codegen/src/compile.rs +++ b/crates/codegen/src/compile.rs @@ -673,7 +673,10 @@ impl Compiler { let can_fold_const_collection = match collection_type { CollectionType::Tuple => n > 0, - CollectionType::List | CollectionType::Set => n >= 3, + // Match CPython's constant ordering for list/set literals by + // letting the late IR folding passes introduce their tuple-backed + // constants instead of inserting them during AST lowering. + CollectionType::List | CollectionType::Set => false, }; if !self.disable_const_collection_folding && !seen_star @@ -720,9 +723,37 @@ impl Compiler { } // Has stars or too big: use streaming approach + let stream_big_nonconst_collection = if !seen_star + && big + && matches!(collection_type, CollectionType::List | CollectionType::Set) + { + elts.iter().try_fold(false, |has_nonconst, elt| { + if has_nonconst { + return Ok(true); + } + Ok(self.try_fold_constant_expr(elt)?.is_none()) + })? + } else { + false + }; + let mut sequence_built = false; let mut i = 0u32; + if stream_big_nonconst_collection { + match collection_type { + CollectionType::List => { + emit!(self, Instruction::BuildList { count: pushed }); + sequence_built = true; + } + CollectionType::Set => { + emit!(self, Instruction::BuildSet { count: pushed }); + sequence_built = true; + } + CollectionType::Tuple => {} + } + } + for elt in elts.iter() { if let ast::Expr::Starred(ast::ExprStarred { value, .. }) = elt { // When we hit first star, build sequence with elements so far @@ -1220,10 +1251,8 @@ impl Compiler { cellvar_cache.insert("__classdict__".to_string()); } - // Handle implicit __conditional_annotations__ cell if needed - if ste.has_conditional_annotations - && matches!(scope_type, CompilerScope::Class | CompilerScope::Module) - { + // Handle implicit __conditional_annotations__ cell if needed. + if Self::scope_needs_conditional_annotations_cell(ste) { cellvar_cache.insert("__conditional_annotations__".to_string()); } @@ -1923,7 +1952,7 @@ impl Compiler { self.future_annotations = symbol_table.future_annotations; // Module-level __conditional_annotations__ cell - let has_module_cond_ann = symbol_table.has_conditional_annotations; + let has_module_cond_ann = Self::scope_needs_conditional_annotations_cell(&symbol_table); if has_module_cond_ann { self.current_code_info() .metadata @@ -1951,18 +1980,16 @@ impl Compiler { emit!(self, Instruction::StoreName { namei: doc }) } - // Handle annotations based on future_annotations flag + // Handle annotation bookkeeping in CPython order: initialize the + // conditional annotation set first, then materialize __annotations__. if Self::find_ann(statements) { + if Self::scope_needs_conditional_annotations_cell(self.current_symbol_table()) { + emit!(self, Instruction::BuildSet { count: 0 }); + self.store_name("__conditional_annotations__")?; + } + if self.future_annotations { - // PEP 563: Initialize __annotations__ dict emit!(self, Instruction::SetupAnnotations); - } else { - // PEP 649: Initialize __conditional_annotations__ before the body. - // CPython generates __annotate__ after the body in codegen_body(). - if self.current_symbol_table().has_conditional_annotations { - emit!(self, Instruction::BuildSet { count: 0 }); - self.store_name("__conditional_annotations__")?; - } } } @@ -2102,6 +2129,20 @@ impl Compiler { Ok(()) } + fn scope_needs_conditional_annotations_cell(symbol_table: &SymbolTable) -> bool { + match symbol_table.typ { + CompilerScope::Module => { + symbol_table.has_conditional_annotations + || (symbol_table.future_annotations && symbol_table.annotation_block.is_some()) + } + CompilerScope::Class => { + symbol_table.has_conditional_annotations + || symbol_table.lookup("__conditional_annotations__").is_some() + } + _ => false, + } + } + fn load_name(&mut self, name: &str) -> CompileResult<()> { self.compile_name(name, NameUsage::Load) } @@ -5189,52 +5230,37 @@ impl Compiler { } ); - // PEP 649: Initialize __classdict__ cell (before __doc__) + // Set __type_params__ from the enclosing type-params closure when + // compiling a generic class body. + if type_params.is_some() { + self.load_name(".type_params")?; + self.store_name("__type_params__")?; + } + + // PEP 649: Initialize __classdict__ after synthetic generic-class + // setup so nested generic classes match CPython's prologue order. if self.current_symbol_table().needs_classdict { emit!(self, Instruction::LoadLocals); let classdict_idx = self.get_cell_var_index("__classdict__")?; emit!(self, Instruction::StoreDeref { i: classdict_idx }); } - // Store __doc__ only if there's an explicit docstring + // Store __doc__ only if there's an explicit docstring. if let Some(doc) = doc_str { self.emit_load_const(ConstantData::Str { value: doc.into() }); let doc_name = self.name("__doc__"); emit!(self, Instruction::StoreName { namei: doc_name }); } - // Set __type_params__ if we have type parameters - if type_params.is_some() { - // Load .type_params from enclosing scope - let dot_type_params = self.name(".type_params"); - emit!( - self, - Instruction::LoadName { - namei: dot_type_params - } - ); - - // Store as __type_params__ - let dunder_type_params = self.name("__type_params__"); - emit!( - self, - Instruction::StoreName { - namei: dunder_type_params - } - ); - } - - // Handle class annotations based on future_annotations flag + // Handle class annotation bookkeeping in CPython order. if Self::find_ann(body) { + if Self::scope_needs_conditional_annotations_cell(self.current_symbol_table()) { + emit!(self, Instruction::BuildSet { count: 0 }); + self.store_name("__conditional_annotations__")?; + } + if self.future_annotations { - // PEP 563: Initialize __annotations__ dict for class emit!(self, Instruction::SetupAnnotations); - } else { - // PEP 649: Initialize __conditional_annotations__ set if needed for class - if self.current_symbol_table().has_conditional_annotations { - emit!(self, Instruction::BuildSet { count: 0 }); - self.store_name("__conditional_annotations__")?; - } } } @@ -5363,15 +5389,10 @@ impl Compiler { in_async_scope: false, }; - // Compile type parameters and store as .type_params + // Compile type parameters and store them in the synthetic cell that + // generic class bodies close over. self.compile_type_params(type_params.unwrap())?; - let dot_type_params = self.name(".type_params"); - emit!( - self, - Instruction::StoreName { - namei: dot_type_params - } - ); + self.store_name(".type_params")?; } // Step 2: Compile class body (always done, whether generic or not) @@ -5387,47 +5408,25 @@ impl Compiler { // Step 3: Generate the rest of the code for the call if is_generic { - // Still in type params scope - let dot_type_params = self.name(".type_params"); - let dot_generic_base = self.name(".generic_base"); - - // Create .generic_base - emit!( - self, - Instruction::LoadName { - namei: dot_type_params - } - ); - emit!( - self, - Instruction::CallIntrinsic1 { - func: bytecode::IntrinsicFunction1::SubscriptGeneric - } - ); - emit!( - self, - Instruction::StoreName { - namei: dot_generic_base - } - ); - // Generate class creation code emit!(self, Instruction::LoadBuildClass); emit!(self, Instruction::PushNull); - // Set up the class function with type params - let mut func_flags = bytecode::MakeFunctionFlags::new(); + // Create the class body function with the .type_params closure + // captured through the class code object's freevars. + self.make_closure(class_code, bytecode::MakeFunctionFlags::new())?; + self.emit_load_const(ConstantData::Str { value: name.into() }); + + // Create .generic_base after the class function and name are on the + // stack so the remaining call shape matches CPython's ordering. + self.load_name(".type_params")?; emit!( self, - Instruction::LoadName { - namei: dot_type_params + Instruction::CallIntrinsic1 { + func: bytecode::IntrinsicFunction1::SubscriptGeneric } ); - func_flags.insert(bytecode::MakeFunctionFlag::TypeParams); - - // Create class function with closure - self.make_closure(class_code, func_flags)?; - self.emit_load_const(ConstantData::Str { value: name.into() }); + self.store_name(".generic_base")?; // Compile bases and call __build_class__ // Check for starred bases or **kwargs @@ -5463,12 +5462,7 @@ impl Compiler { } // Add .generic_base as final element - emit!( - self, - Instruction::LoadName { - namei: dot_generic_base - } - ); + self.load_name(".generic_base")?; emit!(self, Instruction::ListAppend { i: 1 }); // Convert list to tuple @@ -5499,12 +5493,7 @@ impl Compiler { }; // Load .generic_base as the last base - emit!( - self, - Instruction::LoadName { - namei: dot_generic_base - } - ); + self.load_name(".generic_base")?; let nargs = 2 + u32::try_from(base_count).expect("too many base classes") + 1; @@ -5931,15 +5920,13 @@ impl Compiler { emit!(self, Instruction::ForIter { delta: else_block }); - // Match CPython codegen_for(): keep a line anchor on the target line - // so multiline/single-line `for ...: pass` bodies preserve tracing layout. + // Match CPython's line attribution by compiling the loop target on + // the target range directly instead of leaving a synthetic anchor + // NOP between FOR_ITER and the unpack/store sequence. let saved_range = self.current_source_range; self.set_source_range(target.range()); - emit!(self, Instruction::Nop); - self.set_source_range(saved_range); - - // Start of loop iteration, set targets: self.compile_store(target)?; + self.set_source_range(saved_range); }; let was_in_loop = self.ctx.loop_data.replace((for_block, after_block)); @@ -9787,6 +9774,19 @@ impl Compiler { } } + fn compile_tstring_literal_value( + &self, + string: &ast::InterpolatedStringLiteralElement, + flags: ast::TStringFlags, + ) -> Wtf8Buf { + if string.value.contains(char::REPLACEMENT_CHARACTER) { + let source = self.source_file.slice(string.range); + crate::string_parser::parse_fstring_literal_element(source.into(), flags.into()).into() + } else { + string.value.to_string().into() + } + } + fn compile_fstring_part_literal_value(&self, string: &ast::StringLiteral) -> Wtf8Buf { if string.value.contains(char::REPLACEMENT_CHARACTER) { let source = self.source_file.slice(string.range); @@ -9926,6 +9926,72 @@ impl Compiler { } ConstantData::Tuple { elements } } + ast::Expr::Subscript(ast::ExprSubscript { value, slice, .. }) => { + let Some(container) = self.try_fold_constant_expr(value)? else { + return Ok(None); + }; + let Some(index) = self.try_fold_constant_expr(slice)? else { + return Ok(None); + }; + let ConstantData::Integer { value: index } = index else { + return Ok(None); + }; + let Some(index): Option = index.try_into().ok() else { + return Ok(None); + }; + + match container { + ConstantData::Str { value } => { + let string = value.to_string(); + if string.contains(char::REPLACEMENT_CHARACTER) { + return Ok(None); + } + let chars: Vec<_> = string.chars().collect(); + let Some(len) = i64::try_from(chars.len()).ok() else { + return Ok(None); + }; + let idx: i64 = if index < 0 { len + index } else { index }; + let Some(idx) = usize::try_from(idx).ok() else { + return Ok(None); + }; + let Some(ch) = chars.get(idx) else { + return Ok(None); + }; + ConstantData::Str { + value: ch.to_string().into(), + } + } + ConstantData::Bytes { value } => { + let Some(len) = i64::try_from(value.len()).ok() else { + return Ok(None); + }; + let idx: i64 = if index < 0 { len + index } else { index }; + let Some(idx) = usize::try_from(idx).ok() else { + return Ok(None); + }; + let Some(byte) = value.get(idx) else { + return Ok(None); + }; + ConstantData::Integer { + value: BigInt::from(*byte), + } + } + ConstantData::Tuple { elements } => { + let Some(len) = i64::try_from(elements.len()).ok() else { + return Ok(None); + }; + let idx: i64 = if index < 0 { len + index } else { index }; + let Some(idx) = usize::try_from(idx).ok() else { + return Ok(None); + }; + let Some(element) = elements.get(idx) else { + return Ok(None); + }; + element.clone() + } + _ => return Ok(None), + } + } ast::Expr::UnaryOp(ast::ExprUnaryOp { op, operand, .. }) => { let Some(constant) = self.try_fold_constant_expr(operand)? else { return Ok(None); @@ -10734,39 +10800,26 @@ impl Compiler { } fn compile_expr_tstring(&mut self, expr_tstring: &ast::ExprTString) -> CompileResult<()> { - // ast::TStringValue can contain multiple ast::TString parts (implicit concatenation) - // Each ast::TString part should be compiled and the results merged into a single Template + // ast::TStringValue can contain multiple ast::TString parts (implicit + // concatenation). Match CPython's stack order by materializing the + // strings tuple first, then evaluating interpolations left-to-right. let tstring_value = &expr_tstring.value; - // Collect all strings and compile all interpolations let mut all_strings: Vec = Vec::new(); let mut current_string = Wtf8Buf::new(); let mut interp_count: u32 = 0; for tstring in tstring_value.iter() { - self.compile_tstring_into( + self.collect_tstring_strings( tstring, &mut all_strings, &mut current_string, &mut interp_count, - )?; + ); } - // Add trailing string all_strings.push(core::mem::take(&mut current_string)); - // Now build the Template: - // Stack currently has all interpolations from compile_tstring_into calls - - // 1. Build interpolations tuple from the interpolations on the stack - emit!( - self, - Instruction::BuildTuple { - count: interp_count - } - ); - - // 2. Load all string parts let string_count: u32 = all_strings .len() .try_into() @@ -10774,8 +10827,6 @@ impl Compiler { for s in &all_strings { self.emit_load_const(ConstantData::Str { value: s.clone() }); } - - // 3. Build strings tuple emit!( self, Instruction::BuildTuple { @@ -10783,79 +10834,95 @@ impl Compiler { } ); - // 4. Swap so strings is below interpolations: [interps, strings] -> [strings, interps] - emit!(self, Instruction::Swap { i: 2 }); + for tstring in tstring_value.iter() { + self.compile_tstring_interpolations(tstring)?; + } - // 5. Build the Template + emit!( + self, + Instruction::BuildTuple { + count: interp_count + } + ); emit!(self, Instruction::BuildTemplate); Ok(()) } - fn compile_tstring_into( - &mut self, + fn collect_tstring_strings( + &self, tstring: &ast::TString, strings: &mut Vec, current_string: &mut Wtf8Buf, interp_count: &mut u32, - ) -> CompileResult<()> { + ) { for element in &tstring.elements { match element { ast::InterpolatedStringElement::Literal(lit) => { - // Accumulate literal parts into current_string - current_string.push_str(&lit.value); + current_string + .push_wtf8(&self.compile_tstring_literal_value(lit, tstring.flags)); } ast::InterpolatedStringElement::Interpolation(interp) => { - // Finish current string segment + if let Some(ast::DebugText { leading, trailing }) = &interp.debug_text { + let range = interp.expression.range(); + let source = self.source_file.slice(range); + let text = [ + strip_fstring_debug_comments(leading).as_str(), + source, + strip_fstring_debug_comments(trailing).as_str(), + ] + .concat(); + current_string.push_str(&text); + } strings.push(core::mem::take(current_string)); + *interp_count += 1; + } + } + } + } - // Compile the interpolation value - self.compile_expression(&interp.expression)?; + fn compile_tstring_interpolations(&mut self, tstring: &ast::TString) -> CompileResult<()> { + for element in &tstring.elements { + let ast::InterpolatedStringElement::Interpolation(interp) = element else { + continue; + }; - // Load the expression source string, including any - // whitespace between '{' and the expression start - let expr_range = interp.expression.range(); - let expr_source = if interp.range.start() < expr_range.start() - && interp.range.end() >= expr_range.end() - { - let after_brace = interp.range.start() + TextSize::new(1); - self.source_file - .slice(TextRange::new(after_brace, expr_range.end())) - } else { - // Fallback for programmatically constructed ASTs with dummy ranges - self.source_file.slice(expr_range) - }; - self.emit_load_const(ConstantData::Str { - value: expr_source.to_string().into(), - }); + self.compile_expression(&interp.expression)?; - // Determine conversion code - let conversion: u32 = match interp.conversion { - ast::ConversionFlag::None => 0, - ast::ConversionFlag::Str => 1, - ast::ConversionFlag::Repr => 2, - ast::ConversionFlag::Ascii => 3, - }; + let expr_range = interp.expression.range(); + let expr_source = if interp.range.start() < expr_range.start() + && interp.range.end() >= expr_range.end() + { + let after_brace = interp.range.start() + TextSize::new(1); + self.source_file + .slice(TextRange::new(after_brace, expr_range.end())) + } else { + self.source_file.slice(expr_range) + }; + self.emit_load_const(ConstantData::Str { + value: expr_source.to_string().into(), + }); - // Handle format_spec - let has_format_spec = interp.format_spec.is_some(); - if let Some(format_spec) = &interp.format_spec { - // Compile format_spec as a string using fstring element compilation - // Use default ast::FStringFlags since format_spec syntax is independent of t-string flags - self.compile_fstring_elements( - ast::FStringFlags::empty(), - &format_spec.elements, - )?; - } + let mut conversion: u32 = match interp.conversion { + ast::ConversionFlag::None => 0, + ast::ConversionFlag::Str => 1, + ast::ConversionFlag::Repr => 2, + ast::ConversionFlag::Ascii => 3, + }; - // Emit BUILD_INTERPOLATION - // oparg encoding: (conversion << 2) | has_format_spec - let format = (conversion << 2) | u32::from(has_format_spec); - emit!(self, Instruction::BuildInterpolation { format }); + if interp.debug_text.is_some() && conversion == 0 && interp.format_spec.is_none() { + conversion = 2; + } - *interp_count += 1; - } + let has_format_spec = interp.format_spec.is_some(); + if let Some(format_spec) = &interp.format_spec { + self.compile_fstring_elements(ast::FStringFlags::empty(), &format_spec.elements)?; } + + // CPython keeps bit 1 set in BUILD_INTERPOLATION's oparg and uses + // bit 0 for the optional format spec. + let format = 2 | (conversion << 2) | u32::from(has_format_spec); + emit!(self, Instruction::BuildInterpolation { format }); } Ok(()) @@ -12190,9 +12257,9 @@ def f(node): .iter() .filter(|op| matches!(op, Instruction::ReturnValue)) .count(); - assert!( - return_count >= 3, - "expected multiple explicit return sites for shared final return case, got ops={ops:?}" + assert_eq!( + return_count, 5, + "expected cloned return sites for each shared return edge, got ops={ops:?}" ); } @@ -12298,6 +12365,145 @@ elif maxsize == 9223372036854775807: ); } + #[test] + fn test_for_tuple_target_does_not_leave_loop_header_nop() { + let code = compile_exec( + "\ +def f(pairs): + for left, right in pairs: + pass +", + ); + 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(2).any(|window| { + matches!( + window, + [ + Instruction::ForIter { .. }, + Instruction::UnpackSequence { .. } + ] + ) + }), + "expected FOR_ITER to flow directly into UNPACK_SEQUENCE, got ops={ops:?}" + ); + assert!( + !ops.windows(3).any(|window| { + matches!( + window, + [ + Instruction::ForIter { .. }, + Instruction::Nop, + Instruction::UnpackSequence { .. }, + ] + ) + }), + "unexpected loop-header NOP before tuple unpack, got ops={ops:?}" + ); + } + + #[test] + fn test_tstring_build_template_matches_cpython_stack_order() { + let code = compile_exec("t = t\"{0}\""); + let units: Vec<_> = code + .instructions + .iter() + .copied() + .filter(|unit| !matches!(unit.op, Instruction::Cache)) + .collect(); + + assert!( + units.windows(6).any(|window| { + matches!( + window, + [ + a, + b, + c, + d, + e, + f, + ] + if matches!(a.op, Instruction::LoadConst { .. }) + && matches!(b.op, Instruction::LoadSmallInt { .. }) + && matches!(c.op, Instruction::LoadConst { .. }) + && matches!(d.op, Instruction::BuildInterpolation { .. }) + && u8::from(d.arg) == 2 + && matches!(e.op, Instruction::BuildTuple { .. }) + && u8::from(e.arg) == 1 + && matches!(f.op, Instruction::BuildTemplate) + ) + }), + "expected CPython-style t-string lowering, got units={units:?}" + ); + assert!( + !units + .iter() + .any(|unit| matches!(unit.op, Instruction::Swap { .. })), + "unexpected SWAP in t-string lowering, got units={units:?}" + ); + } + + #[test] + fn test_tstring_debug_specifier_uses_debug_literal_and_repr_default() { + let code = compile_exec( + "\ +value = 42 +t = t\"Value: {value=}\" +", + ); + + let string_consts = code + .instructions + .iter() + .filter_map(|unit| match unit.op { + Instruction::LoadConst { consti } => { + Some(&code.constants[consti.get(OpArg::new(u32::from(u8::from(unit.arg))))]) + } + _ => None, + }) + .collect::>(); + + assert!( + string_consts.iter().any(|constant| matches!( + constant, + ConstantData::Tuple { elements } + if matches!( + &elements[..], + [ + ConstantData::Str { value: first }, + ConstantData::Str { value: second }, + ] if first.to_string() == "Value: value=" && second.is_empty() + ) + )), + "expected debug literal prefix in t-string constants, got {string_consts:?}" + ); + assert!( + code.instructions.iter().any(|unit| matches!( + unit.op, + Instruction::BuildInterpolation { .. } + ) && u8::from(unit.arg) == 10), + "expected default repr conversion for debug t-string" + ); + } + + #[test] + fn test_tstring_literal_preserves_surrogate_wtf8() { + let code = compile_exec("t = t\"\\ud800\""); + + assert!(code.constants.iter().any(|constant| matches!( + constant, + ConstantData::Str { value } if value.clone().into_bytes() == [0xED, 0xA0, 0x80] + ))); + } + #[test] fn test_break_in_finally_after_return_keeps_load_fast_check_for_loop_locals() { let code = compile_exec( @@ -12823,6 +13029,27 @@ class C: ); } + #[test] + fn test_future_annotations_class_keeps_conditional_annotations_cell() { + let code = compile_exec( + "\ +from __future__ import annotations +class C: + x: int +", + ); + let class_code = find_code(&code, "C").expect("missing class code"); + + assert!( + class_code + .cellvars + .iter() + .any(|name| name.as_str() == "__conditional_annotations__"), + "expected __conditional_annotations__ cellvar, got cellvars={:?}", + class_code.cellvars + ); + } + #[test] fn test_plain_super_call_keeps_class_freevar() { let code = compile_exec( @@ -13116,6 +13343,200 @@ def f(names, cls): assert_eq!(return_count, 1); } + #[test] + fn test_non_none_final_return_is_not_duplicated() { + let code = compile_exec( + "\ +def f(p, s): + if p == '': + if s == '': + return 0 + return -1 +", + ); + let f = find_code(&code, "f").expect("missing function code"); + let minus_one_loads = f + .instructions + .iter() + .filter(|unit| { + matches!( + unit.op, + Instruction::LoadConst { consti } + if matches!( + f.constants.get( + consti + .get(OpArg::new(u32::from(u8::from(unit.arg)))) + .as_usize() + ), + Some(ConstantData::Integer { value }) if value == &BigInt::from(-1) + ) + ) + }) + .count(); + + assert_eq!( + minus_one_loads, + 1, + "expected a single final return -1 epilogue, got ops={:?}", + f.instructions + .iter() + .map(|unit| unit.op) + .collect::>() + ); + } + + #[test] + fn test_named_except_conditional_branch_duplicates_cleanup_return() { + let code = compile_exec( + "\ +def f(self): + try: + raise TypeError('x') + except TypeError as e: + if '+' not in str(e): + self.fail('join() ate exception message') +", + ); + 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 cleanup_return_count = ops + .windows(6) + .filter(|window| { + matches!( + window, + [ + Instruction::PopExcept, + Instruction::LoadConst { .. }, + Instruction::StoreFast { .. } | Instruction::StoreName { .. }, + Instruction::DeleteFast { .. } | Instruction::DeleteName { .. }, + Instruction::LoadConst { .. }, + Instruction::ReturnValue, + ] + ) + }) + .count(); + + assert_eq!( + cleanup_return_count, 2, + "expected duplicated named-except cleanup return blocks, got ops={ops:?}" + ); + } + + #[test] + fn test_listcomp_cleanup_tail_keeps_split_store_fast_pair() { + let code = compile_exec( + "\ +def f(escaped_string, quote_types): + possible_quotes = [q for q in quote_types if q not in escaped_string] + return possible_quotes +", + ); + 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::StoreFast { .. }, + Instruction::StoreFast { .. }, + Instruction::LoadFastBorrow { .. }, + Instruction::ReturnValue, + .. + ] + ), + "expected split STORE_FAST pair after listcomp cleanup, got ops={ops:?}" + ); + } + + #[test] + fn test_with_suppress_tail_duplicates_final_return_none() { + let code = compile_exec( + "\ +def f(cm, cond): + if cond: + with cm(): + pass +", + ); + 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 return_count = ops + .iter() + .filter(|op| matches!(op, Instruction::ReturnValue)) + .count(); + + assert_eq!( + return_count, 3, + "expected duplicated return-none epilogues, got ops={ops:?}" + ); + assert!( + !ops.iter() + .any(|op| matches!(op, Instruction::JumpBackwardNoInterrupt { .. })), + "with suppress tail should not jump back to shared return block, got ops={ops:?}" + ); + } + + #[test] + fn test_genexpr_compare_header_keeps_split_store_then_borrow_load() { + let code = compile_exec( + "\ +def f(it): + return (offset == (4, 10) for offset in it) +", + ); + let genexpr = find_code(&code, "").expect("missing code"); + let ops: Vec<_> = genexpr + .instructions + .iter() + .map(|unit| unit.op) + .filter(|op| !matches!(op, Instruction::Cache)) + .collect(); + + assert!( + !ops.iter() + .any(|op| matches!(op, Instruction::StoreFastLoadFast { .. })), + "expected compare header to keep split STORE_FAST/LOAD_FAST_BORROW, got ops={ops:?}" + ); + assert!( + ops.windows(4).any(|window| { + matches!( + window, + [ + Instruction::StoreFast { .. }, + Instruction::LoadFastBorrow { .. }, + Instruction::LoadConst { .. }, + Instruction::CompareOp { .. }, + ] + ) + }), + "expected split compare header sequence, got ops={ops:?}" + ); + } + #[test] fn test_fstring_adjacent_literals_are_merged() { let code = compile_exec( @@ -13233,6 +13654,74 @@ def f(x): })); } + #[test] + fn test_string_and_bytes_binops_constant_fold_like_cpython() { + let code = compile_exec( + "\ +x = b'\\\\' + b'u1881'\n\ +y = 103 * 'a' + 'x'\n", + ); + + assert!( + !code + .instructions + .iter() + .any(|unit| matches!(unit.op, Instruction::BinaryOp { .. })), + "unexpected runtime BINARY_OP in folded string/bytes constants: {:?}", + code.instructions + ); + assert!(code.constants.iter().any(|constant| matches!( + constant, + ConstantData::Bytes { value } if value == b"\\u1881" + ))); + let expected = format!("{}x", "a".repeat(103)); + assert!(code.constants.iter().any(|constant| matches!( + constant, + ConstantData::Str { value } + if value.to_string() == expected + ))); + } + + #[test] + fn test_constant_string_subscript_folds_inside_collection() { + let code = compile_exec( + "\ +values = [item for item in [r\"\\\\'a\\\\'\", r\"\\t3\", r\"\\\\\"[0]]]\n", + ); + + assert!( + !code + .instructions + .iter() + .any(|unit| matches!(unit.op, Instruction::BinaryOp { .. })), + "unexpected runtime BINARY_OP after constant subscript folding: {:?}", + code.instructions + ); + assert!(code.constants.iter().any(|constant| matches!( + constant, + ConstantData::Tuple { elements } + if elements.len() == 3 + && matches!(&elements[2], ConstantData::Str { value } if value.to_string() == "\\") + ))); + } + + #[test] + fn test_constant_string_subscript_with_surrogate_skips_lossy_fold() { + let code = compile_exec("value = \"\\ud800\"[0]\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::Subscr + } + _ => false, + }), + "expected runtime subscript for surrogate literal, got instructions={:?}", + code.instructions + ); + } + #[test] fn test_list_of_constant_tuples_uses_list_extend() { let code = compile_exec( diff --git a/crates/codegen/src/ir.rs b/crates/codegen/src/ir.rs index 01e6971f65b..7d09c2cef83 100644 --- a/crates/codegen/src/ir.rs +++ b/crates/codegen/src/ir.rs @@ -257,7 +257,7 @@ impl CodeInfo { self.eliminate_unreachable_blocks(); resolve_line_numbers(&mut self.blocks); redirect_empty_block_targets(&mut self.blocks); - duplicate_end_returns(&mut self.blocks); + duplicate_end_returns(&mut self.blocks, &self.metadata); self.dce(); // truncate after terminal in blocks that got return duplicated self.eliminate_unreachable_blocks(); // remove now-unreachable last block self.remove_redundant_const_pop_top_pairs(); @@ -271,6 +271,10 @@ impl CodeInfo { reorder_jump_over_exception_cleanup_blocks(&mut self.blocks); self.eliminate_unreachable_blocks(); remove_redundant_nops_and_jumps(&mut self.blocks); + inline_with_suppress_return_blocks(&mut self.blocks); + inline_pop_except_return_blocks(&mut self.blocks); + duplicate_named_except_cleanup_returns(&mut self.blocks, &self.metadata); + self.eliminate_unreachable_blocks(); // Late CFG cleanup can create new same-line STORE_FAST/LOAD_FAST and // STORE_FAST/STORE_FAST adjacencies in match/capture code paths that // did not exist during the earlier flowgraph-like pass. @@ -301,6 +305,7 @@ impl CodeInfo { self.deoptimize_store_fast_store_fast_after_cleanup(); self.apply_static_swaps(); self.insert_superinstructions(); + self.deoptimize_store_fast_store_fast_after_cleanup(); self.optimize_load_global_push_null(); self.reorder_entry_prefix_cell_setup(); self.remove_unused_consts(); @@ -1193,6 +1198,43 @@ impl CodeInfo { value: result.into(), }) } + (ConstantData::Integer { value: n }, ConstantData::Str { value: s }) + if matches!(op, BinOp::Multiply) => + { + let n: usize = n.try_into().ok()?; + if n > 4096 { + return None; + } + let result = s.to_string().repeat(n); + Some(ConstantData::Str { + value: result.into(), + }) + } + (ConstantData::Bytes { value: l }, ConstantData::Bytes { value: r }) + if matches!(op, BinOp::Add) => + { + let mut result = l.clone(); + result.extend_from_slice(r); + Some(ConstantData::Bytes { value: result }) + } + (ConstantData::Bytes { value: b }, ConstantData::Integer { value: n }) + if matches!(op, BinOp::Multiply) => + { + let n: usize = n.try_into().ok()?; + if n > 4096 { + return None; + } + Some(ConstantData::Bytes { value: b.repeat(n) }) + } + (ConstantData::Integer { value: n }, ConstantData::Bytes { value: b }) + if matches!(op, BinOp::Multiply) => + { + let n: usize = n.try_into().ok()?; + if n > 4096 { + return None; + } + Some(ConstantData::Bytes { value: b.repeat(n) }) + } _ => None, } } @@ -1201,6 +1243,7 @@ impl CodeInfo { match c { ConstantData::Integer { value } => value.bits() > 4096 * 8, ConstantData::Str { value } => value.len() > 4096, + ConstantData::Bytes { value } => value.len() > 4096, _ => false, } } @@ -1886,14 +1929,12 @@ impl CodeInfo { /// Eliminate dead stores in STORE_FAST sequences (apply_static_swaps). /// /// In sequences of consecutive STORE_FAST instructions (from tuple unpacking), - /// if the same variable is stored to more than once, only the first store - /// (which gets TOS — the rightmost value) matters. Later stores to the - /// same variable are dead and replaced with POP_TOP. - /// Simplified apply_static_swaps (CPython flowgraph.c): - /// In STORE_FAST sequences that follow UNPACK_SEQUENCE / UNPACK_EX, - /// replace duplicate stores to the same variable with POP_TOP. - /// UNPACK pushes values so stores execute left-to-right; the LAST - /// store to a variable carries the final value, earlier ones are dead. + /// only collapse directly adjacent duplicate targets. + /// + /// CPython preserves non-adjacent duplicates such as `_, expr, _` so the + /// store layout still reflects the original unpack order. Replacing the + /// first `_` with POP_TOP there changes the emitted superinstructions and + /// bytecode shape even though the final value is the same. fn eliminate_dead_stores(&mut self) { for block in &mut self.blocks { let instructions = &mut block.instructions; @@ -1921,18 +1962,18 @@ impl CodeInfo { run_end += 1; } if run_end - run_start >= 2 { - // Pass 1: find the LAST occurrence of each variable - let mut last_occurrence = std::collections::HashMap::new(); - for (j, instr) in instructions[run_start..run_end].iter().enumerate() { - last_occurrence.insert(u32::from(instr.arg), j); - } - // Pass 2: non-last stores to the same variable are dead - for (j, instr) in instructions[run_start..run_end].iter_mut().enumerate() { - let idx = u32::from(instr.arg); - if last_occurrence[&idx] != j { + let mut j = run_start; + while j < run_end { + let arg = u32::from(instructions[j].arg); + let mut group_end = j + 1; + while group_end < run_end && u32::from(instructions[group_end].arg) == arg { + group_end += 1; + } + for instr in &mut instructions[j..group_end.saturating_sub(1)] { instr.instr = Opcode::PopTop.into(); instr.arg = OpArg::new(0); } + j = group_end; } } i = run_end.max(i + 1); @@ -2353,6 +2394,14 @@ impl CodeInfo { continue; } + let prev_real = block.instructions[..i] + .iter() + .rev() + .find_map(|info| info.instr.real()); + let next_real = block.instructions[(j + 1)..] + .iter() + .find_map(|info| info.instr.real()); + match (curr.instr.real(), next.instr.real()) { (Some(Instruction::LoadFast { .. }), Some(Instruction::LoadFast { .. })) => { let idx1 = u32::from(curr.arg); @@ -2373,6 +2422,13 @@ impl CodeInfo { Some(Instruction::StoreFast { .. }), Some(Instruction::LoadFast { .. } | Instruction::LoadFastBorrow { .. }), ) => { + if self.flags.contains(CodeFlags::GENERATOR) + && matches!(prev_real, Some(Instruction::ForIter { .. })) + && !matches!(next_real, Some(Instruction::ToBool)) + { + i += 1; + continue; + } let store_idx = u32::from(curr.arg); let load_idx = u32::from(next.arg); if store_idx >= 16 || load_idx >= 16 { @@ -3479,7 +3535,7 @@ impl CodeInfo { self.debug_block_dump(), )); - duplicate_end_returns(&mut self.blocks); + duplicate_end_returns(&mut self.blocks, &self.metadata); trace.push(( "after_duplicate_end_returns".to_owned(), self.debug_block_dump(), @@ -4169,8 +4225,14 @@ fn jump_threading_impl(blocks: &mut [Block], include_conditional: bool) { if target == BlockIdx::NULL { continue; } - // Check if target block's first instruction is an unconditional jump - let target_jump = blocks[target.idx()].instructions.first().copied(); + // Thread through blocks that are only leading NOPs followed by an + // unconditional jump so late line anchors do not leave + // JUMP_FORWARD -> NOP -> JUMP_BACKWARD chains behind. + let target_jump = blocks[target.idx()] + .instructions + .iter() + .find(|info| !matches!(info.instr.real(), Some(Instruction::Nop))) + .copied(); if let Some(target_ins) = target_jump && target_ins.instr.is_unconditional_jump() && target_ins.target != BlockIdx::NULL @@ -4495,7 +4557,10 @@ fn remove_redundant_nops_in_blocks(blocks: &mut [Block]) -> usize { if lineno < 0 || prev_lineno == lineno { remove = true; } else if src < src_instructions.len() - 1 { - if src_instructions[src + 1].folded_from_nonliteral_expr { + if src_instructions[src + 1].instr.is_unconditional_jump() { + src_instructions[src + 1].lineno_override = Some(lineno); + remove = true; + } else if src_instructions[src + 1].folded_from_nonliteral_expr { remove = true; } else { let next_lineno = instruction_lineno(&src_instructions[src + 1]); @@ -4671,6 +4736,14 @@ fn next_nonempty_block(blocks: &[Block], mut idx: BlockIdx) -> BlockIdx { idx } +fn is_load_const_none(instr: &InstructionInfo, metadata: &CodeUnitMetadata) -> bool { + matches!(instr.instr.real(), Some(Instruction::LoadConst { .. })) + && matches!( + metadata.consts.get_index(u32::from(instr.arg) as usize), + Some(ConstantData::None) + ) +} + fn instruction_lineno(instr: &InstructionInfo) -> i32 { instr .lineno_override @@ -5268,7 +5341,7 @@ fn find_layout_predecessor(blocks: &[Block], target: BlockIdx) -> BlockIdx { /// Duplicate `LOAD_CONST None + RETURN_VALUE` for blocks that fall through /// to the final return block. -fn duplicate_end_returns(blocks: &mut Vec) { +fn duplicate_end_returns(blocks: &mut Vec, metadata: &CodeUnitMetadata) { // Walk the block chain and keep the last non-cold non-empty block. // After cold exception handlers are pushed to the end, the mainline // return epilogue can sit before trailing cold blocks. @@ -5292,12 +5365,13 @@ fn duplicate_end_returns(blocks: &mut Vec) { } let last_insts = &blocks[last_block.idx()].instructions; - // Only apply when the last block is EXACTLY a return-None epilogue + // Only apply when the last block is EXACTLY a return-None epilogue. let is_return_block = last_insts.len() == 2 && matches!( last_insts[0].instr, AnyInstruction::Real(Instruction::LoadConst { .. }) ) + && is_load_const_none(&last_insts[0], metadata) && matches!( last_insts[1].instr, AnyInstruction::Real(Instruction::ReturnValue) @@ -5319,7 +5393,7 @@ fn duplicate_end_returns(blocks: &mut Vec) { while current != BlockIdx::NULL { let block = &blocks[current.idx()]; let next = next_nonempty_block(blocks, block.next); - if current != last_block && !block.cold && !block.except_handler { + if current != last_block && !block.cold { let last_ins = block.instructions.last(); let has_fallthrough = last_ins .map(|ins| !ins.instr.is_scope_exit() && !ins.instr.is_unconditional_jump()) @@ -5335,20 +5409,28 @@ fn duplicate_end_returns(blocks: &mut Vec) { AnyInstruction::Real(Instruction::ReturnValue) ) }; - if next == last_block + if !block.except_handler + && next == last_block && has_fallthrough && trailing_conditional_jump_index(block).is_none() && !already_has_return { fallthrough_blocks_to_fix.push(current); } - if predecessors[last_block.idx()] > 1 - && let Some(last) = block.instructions.last() - && last.instr.is_unconditional_jump() - && last.target != BlockIdx::NULL - && next_nonempty_block(blocks, last.target) == last_block - { - jump_targets_to_fix.push((current, block.instructions.len() - 1)); + let jump_idx = trailing_conditional_jump_index(block).or_else(|| { + block.instructions.last().and_then(|last| { + (last.instr.is_unconditional_jump() && last.target != BlockIdx::NULL) + .then_some(block.instructions.len() - 1) + }) + }); + if let Some(jump_idx) = jump_idx { + let jump = &block.instructions[jump_idx]; + if jump.target != BlockIdx::NULL + && next_nonempty_block(blocks, jump.target) == last_block + && (is_conditional_jump(&jump.instr) || predecessors[last_block.idx()] > 1) + { + jump_targets_to_fix.push((current, jump_idx)); + } } } current = blocks[current.idx()].next; @@ -5371,7 +5453,7 @@ fn duplicate_end_returns(blocks: &mut Vec) { // Clone the final return block for jump predecessors so their target layout // matches CPython's duplicated exit blocks. - for (block_idx, instr_idx) in jump_targets_to_fix { + for (block_idx, instr_idx) in jump_targets_to_fix.into_iter().rev() { let jump = blocks[block_idx.idx()].instructions[instr_idx]; let mut cloned_return = return_insts.clone(); if let Some(first) = cloned_return.first_mut() { @@ -5404,6 +5486,182 @@ fn duplicate_end_returns(blocks: &mut Vec) { } } +fn inline_with_suppress_return_blocks(blocks: &mut [Block]) { + fn has_with_suppress_prefix(block: &Block, jump_idx: usize) -> bool { + let tail: Vec<_> = block.instructions[..jump_idx] + .iter() + .filter_map(|info| info.instr.real()) + .rev() + .take(5) + .collect(); + matches!( + tail.as_slice(), + [ + Instruction::PopTop, + Instruction::PopTop, + Instruction::PopTop, + Instruction::PopExcept, + Instruction::PopTop, + ] + ) + } + + for block_idx in 0..blocks.len() { + let Some(jump_idx) = blocks[block_idx].instructions.len().checked_sub(1) else { + continue; + }; + let jump = blocks[block_idx].instructions[jump_idx]; + if !jump.instr.is_unconditional_jump() || jump.target == BlockIdx::NULL { + continue; + } + if !has_with_suppress_prefix(&blocks[block_idx], jump_idx) { + continue; + } + + let target = next_nonempty_block(blocks, jump.target); + if target == BlockIdx::NULL || !is_const_return_block(&blocks[target.idx()]) { + continue; + } + + let mut cloned_return = blocks[target.idx()].instructions.clone(); + for instr in &mut cloned_return { + overwrite_location(instr, jump.location, jump.end_location); + } + blocks[block_idx].instructions.pop(); + blocks[block_idx].instructions.extend(cloned_return); + } +} + +fn is_named_except_cleanup_return_block(block: &Block, metadata: &CodeUnitMetadata) -> bool { + matches!( + block.instructions.as_slice(), + [pop_except, load_none1, store, delete, load_none2, ret] + if matches!(pop_except.instr.real(), Some(Instruction::PopExcept)) + && is_load_const_none(load_none1, metadata) + && matches!( + store.instr.real(), + Some(Instruction::StoreFast { .. } | Instruction::StoreName { .. }) + ) + && matches!( + delete.instr.real(), + Some(Instruction::DeleteFast { .. } | Instruction::DeleteName { .. }) + ) + && is_load_const_none(load_none2, metadata) + && matches!(ret.instr.real(), Some(Instruction::ReturnValue)) + ) +} + +fn duplicate_named_except_cleanup_returns(blocks: &mut Vec, metadata: &CodeUnitMetadata) { + let predecessors = compute_predecessors(blocks); + let mut clones = Vec::new(); + + for target in 0..blocks.len() { + let target = BlockIdx(target as u32); + if !is_named_except_cleanup_return_block(&blocks[target.idx()], metadata) { + continue; + } + + let layout_pred = find_layout_predecessor(blocks, target); + if layout_pred == BlockIdx::NULL + || next_nonempty_block(blocks, blocks[layout_pred.idx()].next) != target + { + continue; + } + + let fallthroughs_into_target = blocks[layout_pred.idx()] + .instructions + .last() + .map(|ins| !ins.instr.is_scope_exit() && !ins.instr.is_unconditional_jump()) + .unwrap_or(true); + if !fallthroughs_into_target || predecessors[target.idx()] < 2 { + continue; + } + + for block_idx in 0..blocks.len() { + if block_idx == target.idx() { + continue; + } + let Some(instr_idx) = trailing_conditional_jump_index(&blocks[block_idx]) else { + continue; + }; + if next_nonempty_block(blocks, blocks[block_idx].instructions[instr_idx].target) + != target + { + continue; + } + clones.push((BlockIdx(block_idx as u32), instr_idx, target)); + } + } + + for (block_idx, instr_idx, target) in clones.into_iter().rev() { + let jump = blocks[block_idx.idx()].instructions[instr_idx]; + let mut cloned = blocks[target.idx()].instructions.clone(); + if let Some(first) = cloned.first_mut() { + overwrite_location(first, jump.location, jump.end_location); + } + + let new_idx = BlockIdx(blocks.len() as u32); + let next = blocks[target.idx()].next; + blocks.push(Block { + cold: blocks[target.idx()].cold, + except_handler: blocks[target.idx()].except_handler, + disable_load_fast_borrow: blocks[target.idx()].disable_load_fast_borrow, + instructions: cloned, + next, + ..Block::default() + }); + blocks[target.idx()].next = new_idx; + blocks[block_idx.idx()].instructions[instr_idx].target = new_idx; + } +} + +fn is_const_return_block(block: &Block) -> bool { + block.instructions.len() == 2 + && matches!( + block.instructions[0].instr.real(), + Some(Instruction::LoadConst { .. }) + ) + && matches!( + block.instructions[1].instr.real(), + Some(Instruction::ReturnValue) + ) +} + +fn inline_pop_except_return_blocks(blocks: &mut [Block]) { + for block_idx in 0..blocks.len() { + let Some(jump_idx) = blocks[block_idx].instructions.len().checked_sub(1) else { + continue; + }; + let jump = blocks[block_idx].instructions[jump_idx]; + if !jump.instr.is_unconditional_jump() || jump.target == BlockIdx::NULL { + continue; + } + + let Some(last_real_before_jump) = blocks[block_idx].instructions[..jump_idx] + .iter() + .rev() + .find_map(|info| info.instr.real()) + else { + continue; + }; + if !matches!(last_real_before_jump, Instruction::PopExcept) { + continue; + } + + let target = next_nonempty_block(blocks, jump.target); + if target == BlockIdx::NULL || !is_const_return_block(&blocks[target.idx()]) { + continue; + } + + let mut cloned_return = blocks[target.idx()].instructions.clone(); + for instr in &mut cloned_return { + overwrite_location(instr, jump.location, jump.end_location); + } + blocks[block_idx].instructions.pop(); + blocks[block_idx].instructions.extend(cloned_return); + } +} + /// Label exception targets: walk CFG with except stack, set per-instruction /// handler info and block preserve_lasti flag. Converts POP_BLOCK to NOP. /// flowgraph.c label_exception_targets + push_except_block diff --git a/crates/codegen/src/symboltable.rs b/crates/codegen/src/symboltable.rs index 1098c34fac2..e87aa3d4aea 100644 --- a/crates/codegen/src/symboltable.rs +++ b/crates/codegen/src/symboltable.rs @@ -1077,6 +1077,9 @@ impl SymbolTableBuilder { // Register .type_params as a SET symbol (it will be converted to cell variable later) self.register_name(".type_params", SymbolUsage::Assigned, TextRange::default())?; + if for_class { + self.register_name(".generic_base", SymbolUsage::Assigned, TextRange::default())?; + } Ok(()) } @@ -1263,27 +1266,38 @@ impl SymbolTableBuilder { is_ann_assign: bool, ) -> SymbolTableResult { let current_scope = self.tables.last().map(|t| t.typ); + let needs_future_annotation_bookkeeping = is_ann_assign + && self.future_annotations + && matches!( + current_scope, + Some(CompilerScope::Module | CompilerScope::Class) + ); + let needs_non_future_conditional_annotations = is_ann_assign + && !self.future_annotations + && (matches!(current_scope, Some(CompilerScope::Module)) + || (matches!(current_scope, Some(CompilerScope::Class)) + && self.in_conditional_block)); + let should_register_conditional_annotations = needs_future_annotation_bookkeeping + || (needs_non_future_conditional_annotations + && !self.tables.last().unwrap().has_conditional_annotations); // PEP 649: Only AnnAssign annotations can be conditional. // Function parameter/return annotations are never conditional. - if is_ann_assign && !self.future_annotations { - let is_conditional = matches!(current_scope, Some(CompilerScope::Module)) - || (matches!(current_scope, Some(CompilerScope::Class)) - && self.in_conditional_block); - - if is_conditional && !self.tables.last().unwrap().has_conditional_annotations { - self.tables.last_mut().unwrap().has_conditional_annotations = true; - self.register_name( - "__conditional_annotations__", - SymbolUsage::Assigned, - annotation.range(), - )?; - self.register_name( - "__conditional_annotations__", - SymbolUsage::Used, - annotation.range(), - )?; - } + if needs_non_future_conditional_annotations { + self.tables.last_mut().unwrap().has_conditional_annotations = true; + } + + if should_register_conditional_annotations { + self.register_name( + "__conditional_annotations__", + SymbolUsage::Assigned, + annotation.range(), + )?; + self.register_name( + "__conditional_annotations__", + SymbolUsage::Used, + annotation.range(), + )?; } // Create annotation scope for deferred evaluation @@ -1437,6 +1451,10 @@ impl SymbolTableBuilder { self.register_name("__qualname__", SymbolUsage::Assigned, *range)?; self.register_name("__doc__", SymbolUsage::Assigned, *range)?; self.register_name("__class__", SymbolUsage::Assigned, *range)?; + if type_params.is_some() { + self.register_name(".type_params", SymbolUsage::Used, *range)?; + self.register_name("__type_params__", SymbolUsage::Assigned, *range)?; + } self.scan_statements(body)?; self.leave_scope(); self.in_conditional_block = saved_in_conditional; diff --git a/crates/vm/src/builtins/dict.rs b/crates/vm/src/builtins/dict.rs index eae4ec7fd6b..9791435a23a 100644 --- a/crates/vm/src/builtins/dict.rs +++ b/crates/vm/src/builtins/dict.rs @@ -663,7 +663,8 @@ impl Py { ) -> PyResult> { if self.exact_dict(vm) { self.entries.get(vm, key) - // FIXME: check __missing__? + // Match CPython's exact-dict fast path: __missing__ only participates + // for dict subclasses through the generic mapping lookup path below. } else { match self.as_object().get_item(key, vm) { Ok(value) => Ok(Some(value)), diff --git a/crates/vm/src/frame.rs b/crates/vm/src/frame.rs index 49d0a18292c..180c4fad0ed 100644 --- a/crates/vm/src/frame.rs +++ b/crates/vm/src/frame.rs @@ -2725,16 +2725,7 @@ impl ExecutingFrame<'_> { let class_dict = self.pop_value(); let idx = i.get(arg).as_usize(); let name = self.localsplus_name(idx); - // Only treat KeyError as "not found", propagate other exceptions - let value = if let Some(dict_obj) = class_dict.downcast_ref::() { - dict_obj.get_item_opt(name, vm)? - } else { - match class_dict.get_item(name, vm) { - Ok(v) => Some(v), - Err(e) if e.fast_isinstance(vm.ctx.exceptions.key_error) => None, - Err(e) => return Err(e), - } - }; + let value = self.mapping_get_optional(&class_dict, name, vm)?; self.push_value(match value { Some(v) => v, None => self @@ -2748,18 +2739,7 @@ impl ExecutingFrame<'_> { // PEP 649: Pop dict from stack (classdict), check there first, then globals let dict = self.pop_value(); let name = self.code.names[idx.get(arg) as usize]; - - // Only treat KeyError as "not found", propagate other exceptions - let value = if let Some(dict_obj) = dict.downcast_ref::() { - dict_obj.get_item_opt(name, vm)? - } else { - // Not an exact dict, use mapping protocol - match dict.get_item(name, vm) { - Ok(v) => Some(v), - Err(e) if e.fast_isinstance(vm.ctx.exceptions.key_error) => None, - Err(e) => return Err(e), - } - }; + let value = self.mapping_get_optional(&dict, name, vm)?; self.push_value(match value { Some(v) => v, @@ -6118,6 +6098,27 @@ impl ExecutingFrame<'_> { } } + #[inline] + fn mapping_get_optional( + &self, + mapping: &PyObjectRef, + name: &Py, + vm: &VirtualMachine, + ) -> PyResult> { + if mapping.class().is(vm.ctx.types.dict_type) { + let dict = mapping + .downcast_ref::() + .expect("exact dict must have a PyDict payload"); + dict.get_item_opt(name, vm) + } else { + match mapping.get_item(name, vm) { + Ok(value) => Ok(Some(value)), + Err(err) if err.fast_isinstance(vm.ctx.exceptions.key_error) => Ok(None), + Err(err) => Err(err), + } + } + } + #[inline] fn load_global_or_builtin(&self, name: &Py, vm: &VirtualMachine) -> PyResult { if let Some(builtins_dict) = self.builtins_dict {