From 29d2d88f1954a1a039eddc1efc5f38427d211c9e Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Thu, 8 Jan 2026 20:33:09 +0900 Subject: [PATCH 1/4] implementing call the correct way stack is [Option] --- Lib/test/test_xml_etree.py | 1 - crates/codegen/src/compile.rs | 127 ++++----- ...pile__tests__nested_double_async_with.snap | 240 +++++++++--------- crates/compiler-core/src/bytecode.rs | 23 +- crates/jit/src/instructions.rs | 19 +- crates/stdlib/src/opcode.rs | 2 +- crates/vm/src/frame.rs | 238 +++++++++-------- crates/vm/src/vm/method.rs | 15 +- 8 files changed, 361 insertions(+), 304 deletions(-) diff --git a/Lib/test/test_xml_etree.py b/Lib/test/test_xml_etree.py index 11c617b5f3..1489fb666e 100644 --- a/Lib/test/test_xml_etree.py +++ b/Lib/test/test_xml_etree.py @@ -2301,7 +2301,6 @@ def test_bug_xmltoolkit62(self): self.assertEqual(t.find('.//paragraph').text, 'A new cultivar of Begonia plant named \u2018BCT9801BEG\u2019.') - @unittest.expectedFailure # TODO: RUSTPYTHON @unittest.skipIf(sys.gettrace(), "Skips under coverage.") def test_bug_xmltoolkit63(self): # Check reference leak. diff --git a/crates/codegen/src/compile.rs b/crates/codegen/src/compile.rs index f61b01a43e..6c16d93428 100644 --- a/crates/codegen/src/compile.rs +++ b/crates/codegen/src/compile.rs @@ -98,12 +98,6 @@ enum NameUsage { Delete, } -enum CallType { - Positional { nargs: u32 }, - Keyword { nargs: u32 }, - Ex { has_kwargs: bool }, -} - fn is_forbidden_name(name: &str) -> bool { // See https://docs.python.org/3/library/constants.html#built-in-constants const BUILTIN_CONSTANTS: &[&str] = &["__debug__"]; @@ -1037,6 +1031,7 @@ impl Compiler { // Call __exit__(None, None, None) - compiler_call_exit_with_nones // Stack: [..., __exit__] or [..., return_value, __exit__] + emit!(self, Instruction::PushNull); self.emit_load_const(ConstantData::None); self.emit_load_const(ConstantData::None); self.emit_load_const(ConstantData::None); @@ -1875,6 +1870,7 @@ impl Compiler { let assertion_error = self.name("AssertionError"); emit!(self, Instruction::LoadGlobal(assertion_error)); + emit!(self, Instruction::PushNull); match msg { Some(e) => { self.compile_expression(e)?; @@ -2095,12 +2091,13 @@ impl Compiler { fn prepare_decorators(&mut self, decorator_list: &[Decorator]) -> CompileResult<()> { for decorator in decorator_list { self.compile_expression(&decorator.expression)?; + emit!(self, Instruction::PushNull); } Ok(()) } fn apply_decorators(&mut self, decorator_list: &[Decorator]) { - // Apply decorators: + // Apply decorators - each pops [decorator, NULL, arg] and pushes result for _ in decorator_list { emit!(self, Instruction::Call { nargs: 1 }); } @@ -2142,6 +2139,7 @@ impl Compiler { // Create type params function with closure self.make_closure(code, bytecode::MakeFunctionFlags::empty())?; + emit!(self, Instruction::PushNull); // Call the function immediately emit!(self, Instruction::Call { nargs: 0 }); @@ -3224,11 +3222,6 @@ impl Compiler { num_typeparam_args += 1; } - // SWAP if we have both - if num_typeparam_args == 2 { - emit!(self, Instruction::Swap { index: 2 }); - } - // Enter type params scope let type_params_name = format!(""); self.push_output( @@ -3308,13 +3301,20 @@ impl Compiler { self.make_closure(type_params_code, bytecode::MakeFunctionFlags::empty())?; // Call the closure + // Call expects stack: [callable, self_or_null, arg1, ..., argN] if num_typeparam_args > 0 { + // Stack: [arg1, ..., argN, closure] + // Need: [closure, NULL, arg1, ..., argN] + let reverse_amount = (num_typeparam_args + 1) as u32; emit!( self, - Instruction::Swap { - index: (num_typeparam_args + 1) as u32 + Instruction::Reverse { + amount: reverse_amount } ); + // Stack: [closure, argN, ..., arg1] + emit!(self, Instruction::PushNull); + // Stack: [closure, argN, ..., arg1, NULL] emit!( self, Instruction::Call { @@ -3322,7 +3322,10 @@ impl Compiler { } ); } else { - // No arguments, just call the closure + // Stack: [closure] + emit!(self, Instruction::PushNull); + // Stack: [closure, NULL] + // Call pops: args (0), then self_or_null (NULL), then callable (closure) emit!(self, Instruction::Call { nargs: 0 }); } } @@ -3691,6 +3694,7 @@ impl Compiler { // 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::empty(); @@ -3720,14 +3724,20 @@ impl Compiler { if let Some(arguments) = arguments && !arguments.keywords.is_empty() { + let mut kwarg_names = vec![]; for keyword in &arguments.keywords { - if let Some(name) = &keyword.arg { - self.emit_load_const(ConstantData::Str { - value: name.as_str().into(), - }); - } + let name = keyword + .arg + .as_ref() + .expect("keyword argument name must be set"); + kwarg_names.push(ConstantData::Str { + value: name.as_str().into(), + }); self.compile_expression(&keyword.value)?; } + self.emit_load_const(ConstantData::Tuple { + elements: kwarg_names, + }); emit!( self, Instruction::CallKw { @@ -3748,21 +3758,22 @@ impl Compiler { // Execute the type params function self.make_closure(type_params_code, bytecode::MakeFunctionFlags::empty())?; + emit!(self, Instruction::PushNull); emit!(self, Instruction::Call { nargs: 0 }); } else { // Non-generic class: standard path emit!(self, Instruction::LoadBuildClass); + emit!(self, Instruction::PushNull); // Create class function with closure self.make_closure(class_code, bytecode::MakeFunctionFlags::empty())?; self.emit_load_const(ConstantData::Str { value: name.into() }); - let call = if let Some(arguments) = arguments { - self.compile_call_inner(2, arguments)? + if let Some(arguments) = arguments { + self.compile_call_helper(2, arguments)?; } else { - CallType::Positional { nargs: 2 } - }; - self.compile_normal_call(call); + emit!(self, Instruction::Call { nargs: 2 }); + } } // Step 4: Apply decorators and store (common to both paths) @@ -3912,6 +3923,7 @@ impl Compiler { // Stack: [..., __exit__] // Call __exit__(None, None, None) self.set_source_range(with_range); + emit!(self, Instruction::PushNull); self.emit_load_const(ConstantData::None); self.emit_load_const(ConstantData::None); self.emit_load_const(ConstantData::None); @@ -6087,49 +6099,36 @@ impl Compiler { } fn compile_call(&mut self, func: &Expr, args: &Arguments) -> CompileResult<()> { - let method = if let Expr::Attribute(ExprAttribute { value, attr, .. }) = &func { + // Method call: obj → LOAD_ATTR_METHOD → [method, self_or_null] → args → CALL + // Regular call: func → PUSH_NULL → args → CALL + if let Expr::Attribute(ExprAttribute { value, attr, .. }) = &func { + // Method call: compile object, then LOAD_ATTR_METHOD + // LOAD_ATTR_METHOD pushes [method, self_or_null] on stack self.compile_expression(value)?; let idx = self.name(attr.as_str()); - emit!(self, Instruction::LoadMethod { idx }); - true + emit!(self, Instruction::LoadAttrMethod { idx }); + self.compile_call_helper(0, args)?; } else { + // Regular call: push func, then NULL for self_or_null slot + // Stack layout: [func, NULL, args...] - same as method call [func, self, args...] self.compile_expression(func)?; - false - }; - let call = self.compile_call_inner(0, args)?; - if method { - self.compile_method_call(call) - } else { - self.compile_normal_call(call) + emit!(self, Instruction::PushNull); + self.compile_call_helper(0, args)?; } Ok(()) } - fn compile_normal_call(&mut self, ty: CallType) { - match ty { - CallType::Positional { nargs } => { - emit!(self, Instruction::Call { nargs }) - } - CallType::Keyword { nargs } => emit!(self, Instruction::CallKw { nargs }), - CallType::Ex { has_kwargs } => emit!(self, Instruction::CallFunctionEx { has_kwargs }), - } - } - fn compile_method_call(&mut self, ty: CallType) { - match ty { - CallType::Positional { nargs } => { - emit!(self, Instruction::CallMethodPositional { nargs }) - } - CallType::Keyword { nargs } => emit!(self, Instruction::CallMethodKeyword { nargs }), - CallType::Ex { has_kwargs } => emit!(self, Instruction::CallMethodEx { has_kwargs }), - } - } - - fn compile_call_inner( + /// Compile call arguments and emit the appropriate CALL instruction. + /// This is shared between compiler_call and compiler_class. + fn compile_call_helper( &mut self, additional_positional: u32, arguments: &Arguments, - ) -> CompileResult { - let count = u32::try_from(arguments.len()).unwrap() + additional_positional; + ) -> CompileResult<()> { + let args_count = u32::try_from(arguments.len()).expect("too many arguments"); + let count = args_count + .checked_add(additional_positional) + .expect("too many arguments"); // Normal arguments: let (size, unpack) = self.gather_elements(additional_positional, &arguments.args)?; @@ -6141,7 +6140,7 @@ impl Compiler { } } - let call = if unpack || has_double_star { + if unpack || has_double_star { // Create a tuple with positional args: if unpack { emit!(self, Instruction::BuildTupleFromTuples { size }); @@ -6154,7 +6153,7 @@ impl Compiler { if has_kwargs { self.compile_keywords(&arguments.keywords)?; } - CallType::Ex { has_kwargs } + emit!(self, Instruction::CallFunctionEx { has_kwargs }); } else if !arguments.keywords.is_empty() { let mut kwarg_names = vec![]; for keyword in &arguments.keywords { @@ -6172,12 +6171,12 @@ impl Compiler { self.emit_load_const(ConstantData::Tuple { elements: kwarg_names, }); - CallType::Keyword { nargs: count } + emit!(self, Instruction::CallKw { nargs: count }); } else { - CallType::Positional { nargs: count } - }; + emit!(self, Instruction::Call { nargs: count }); + } - Ok(call) + Ok(()) } // Given a vector of expr / star expr generate code which gives either @@ -6409,6 +6408,7 @@ impl Compiler { // Create comprehension function with closure self.make_closure(code, bytecode::MakeFunctionFlags::empty())?; + emit!(self, Instruction::PushNull); // Evaluate iterated item: self.compile_expression(&generators[0].iter)?; @@ -6838,6 +6838,7 @@ impl Compiler { match action { UnwindAction::With { is_async } => { // compiler_call_exit_with_nones + emit!(self, Instruction::PushNull); self.emit_load_const(ConstantData::None); self.emit_load_const(ConstantData::None); self.emit_load_const(ConstantData::None); 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 679c5ebade..44eaca18e1 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 @@ -5,136 +5,142 @@ expression: "compile_exec(\"\\\nasync def test():\n for stop_exc in (StopIter 3 0 LOAD_CONST (): 1 0 RESUME (0) 2 1 LOAD_GLOBAL (0, StopIteration) - 2 LOAD_CONST ("spam") - 3 CALL (1) - 4 LOAD_GLOBAL (1, StopAsyncIteration) - 5 LOAD_CONST ("ham") - 6 CALL (1) - 7 BUILD_TUPLE (2) - 8 GET_ITER - >> 9 FOR_ITER (123) - 10 STORE_FAST (0, stop_exc) + 2 PUSH_NULL + 3 LOAD_CONST ("spam") + 4 CALL (1) + 5 LOAD_GLOBAL (1, StopAsyncIteration) + 6 PUSH_NULL + 7 LOAD_CONST ("ham") + 8 CALL (1) + 9 BUILD_TUPLE (2) + 10 GET_ITER + >> 11 FOR_ITER (129) + 12 STORE_FAST (0, stop_exc) - 3 11 LOAD_GLOBAL (2, self) - 12 LOAD_METHOD (3, subTest) - 13 LOAD_GLOBAL (4, type) - 14 LOAD_FAST (0, stop_exc) - 15 CALL (1) - 16 LOAD_CONST (("type")) - 17 CALL_METHOD_KW (1) - 18 BEFORE_WITH - 19 POP_TOP + 3 13 LOAD_GLOBAL (2, self) + 14 LOAD_ATTR_METHOD (3, subTest) + 15 LOAD_GLOBAL (4, type) + 16 PUSH_NULL + 17 LOAD_FAST (0, stop_exc) + 18 CALL (1) + 19 LOAD_CONST (("type")) + 20 CALL_KW (1) + 21 BEFORE_WITH + 22 POP_TOP - 5 20 LOAD_GLOBAL (5, egg) - 21 CALL (0) - 22 BEFORE_ASYNC_WITH - 23 GET_AWAITABLE - 24 LOAD_CONST (None) - >> 25 SEND (30) - 26 YIELD_VALUE (1) - 27 RESUME (3) - 28 JUMP (25) - 29 CLEANUP_THROW - >> 30 END_SEND - 31 POP_TOP + 5 23 LOAD_GLOBAL (5, egg) + 24 PUSH_NULL + 25 CALL (0) + 26 BEFORE_ASYNC_WITH + 27 GET_AWAITABLE + 28 LOAD_CONST (None) + >> 29 SEND (34) + 30 YIELD_VALUE (1) + 31 RESUME (3) + 32 JUMP (29) + 33 CLEANUP_THROW + >> 34 END_SEND + 35 POP_TOP - 6 32 LOAD_FAST (0, stop_exc) - 33 RAISE_VARARGS (Raise) + 6 36 LOAD_FAST (0, stop_exc) + 37 RAISE_VARARGS (Raise) - 5 34 LOAD_CONST (None) - 35 LOAD_CONST (None) - 36 LOAD_CONST (None) - 37 CALL (3) - 38 GET_AWAITABLE + 5 38 PUSH_NULL 39 LOAD_CONST (None) - >> 40 SEND (45) - 41 YIELD_VALUE (1) - 42 RESUME (3) - 43 JUMP (40) - 44 CLEANUP_THROW - >> 45 END_SEND - 46 POP_TOP - 47 JUMP (69) - 48 PUSH_EXC_INFO - 49 WITH_EXCEPT_START - 50 GET_AWAITABLE - 51 LOAD_CONST (None) - >> 52 SEND (57) - 53 YIELD_VALUE (1) - 54 RESUME (3) - 55 JUMP (52) - 56 CLEANUP_THROW - >> 57 END_SEND - 58 TO_BOOL - 59 POP_JUMP_IF_TRUE (61) - 60 RERAISE (2) - >> 61 POP_TOP - 62 POP_EXCEPT - 63 POP_TOP - 64 POP_TOP - 65 JUMP (69) - 66 COPY (3) + 40 LOAD_CONST (None) + 41 LOAD_CONST (None) + 42 CALL (3) + 43 GET_AWAITABLE + 44 LOAD_CONST (None) + >> 45 SEND (50) + 46 YIELD_VALUE (1) + 47 RESUME (3) + 48 JUMP (45) + 49 CLEANUP_THROW + >> 50 END_SEND + 51 POP_TOP + 52 JUMP (74) + 53 PUSH_EXC_INFO + 54 WITH_EXCEPT_START + 55 GET_AWAITABLE + 56 LOAD_CONST (None) + >> 57 SEND (62) + 58 YIELD_VALUE (1) + 59 RESUME (3) + 60 JUMP (57) + 61 CLEANUP_THROW + >> 62 END_SEND + 63 TO_BOOL + 64 POP_JUMP_IF_TRUE (66) + 65 RERAISE (2) + >> 66 POP_TOP 67 POP_EXCEPT - 68 RERAISE (1) - >> 69 JUMP (95) - 70 PUSH_EXC_INFO - 71 COPY (1) + 68 POP_TOP + 69 POP_TOP + 70 JUMP (74) + 71 COPY (3) + 72 POP_EXCEPT + 73 RERAISE (1) + >> 74 JUMP (100) + 75 PUSH_EXC_INFO + 76 COPY (1) - 7 72 LOAD_GLOBAL (6, Exception) - 73 JUMP_IF_NOT_EXC_MATCH(91) - 74 STORE_FAST (1, ex) + 7 77 LOAD_GLOBAL (6, Exception) + 78 JUMP_IF_NOT_EXC_MATCH(96) + 79 STORE_FAST (1, ex) - 8 75 LOAD_GLOBAL (2, self) - 76 LOAD_METHOD (7, assertIs) - 77 LOAD_FAST (1, ex) - 78 LOAD_FAST (0, stop_exc) - 79 CALL_METHOD (2) - 80 POP_TOP - 81 JUMP (86) - 82 LOAD_CONST (None) - 83 STORE_FAST (1, ex) - 84 DELETE_FAST (1, ex) - 85 RAISE_VARARGS (ReraiseFromStack) - >> 86 POP_EXCEPT + 8 80 LOAD_GLOBAL (2, self) + 81 LOAD_ATTR_METHOD (7, assertIs) + 82 LOAD_FAST (1, ex) + 83 LOAD_FAST (0, stop_exc) + 84 CALL (2) + 85 POP_TOP + 86 JUMP (91) 87 LOAD_CONST (None) 88 STORE_FAST (1, ex) 89 DELETE_FAST (1, ex) - 90 JUMP (103) - >> 91 RAISE_VARARGS (ReraiseFromStack) - 92 COPY (3) - 93 POP_EXCEPT - 94 RAISE_VARARGS (ReraiseFromStack) + 90 RAISE_VARARGS (ReraiseFromStack) + >> 91 POP_EXCEPT + 92 LOAD_CONST (None) + 93 STORE_FAST (1, ex) + 94 DELETE_FAST (1, ex) + 95 JUMP (108) + >> 96 RAISE_VARARGS (ReraiseFromStack) + 97 COPY (3) + 98 POP_EXCEPT + 99 RAISE_VARARGS (ReraiseFromStack) - 10 >> 95 LOAD_GLOBAL (2, self) - 96 LOAD_METHOD (8, fail) - 97 LOAD_FAST (0, stop_exc) - 98 FORMAT_SIMPLE - 99 LOAD_CONST (" was suppressed") - 100 BUILD_STRING (2) - 101 CALL_METHOD (1) - 102 POP_TOP - - 3 >> 103 LOAD_CONST (None) - 104 LOAD_CONST (None) - 105 LOAD_CONST (None) - 106 CALL (3) + 10 >> 100 LOAD_GLOBAL (2, self) + 101 LOAD_ATTR_METHOD (8, fail) + 102 LOAD_FAST (0, stop_exc) + 103 FORMAT_SIMPLE + 104 LOAD_CONST (" was suppressed") + 105 BUILD_STRING (2) + 106 CALL (1) 107 POP_TOP - 108 JUMP (122) - 109 PUSH_EXC_INFO - 110 WITH_EXCEPT_START - 111 TO_BOOL - 112 POP_JUMP_IF_TRUE (114) - 113 RERAISE (2) - >> 114 POP_TOP - 115 POP_EXCEPT - 116 POP_TOP - 117 POP_TOP - 118 JUMP (122) - 119 COPY (3) - 120 POP_EXCEPT - 121 RERAISE (1) - >> 122 JUMP (9) - >> 123 RETURN_CONST (None) + + 3 >> 108 PUSH_NULL + 109 LOAD_CONST (None) + 110 LOAD_CONST (None) + 111 LOAD_CONST (None) + 112 CALL (3) + 113 POP_TOP + 114 JUMP (128) + 115 PUSH_EXC_INFO + 116 WITH_EXCEPT_START + 117 TO_BOOL + 118 POP_JUMP_IF_TRUE (120) + 119 RERAISE (2) + >> 120 POP_TOP + 121 POP_EXCEPT + 122 POP_TOP + 123 POP_TOP + 124 JUMP (128) + 125 COPY (3) + 126 POP_EXCEPT + 127 RERAISE (1) + >> 128 JUMP (11) + >> 129 RETURN_CONST (None) 1 MAKE_FUNCTION 2 STORE_NAME (0, test) diff --git a/crates/compiler-core/src/bytecode.rs b/crates/compiler-core/src/bytecode.rs index 19380be471..265700841b 100644 --- a/crates/compiler-core/src/bytecode.rs +++ b/crates/compiler-core/src/bytecode.rs @@ -951,7 +951,7 @@ pub enum Instruction { target: Arg