Skip to content

Commit 29d2d88

Browse files
youknowoneyouknowone
committed
implementing call the correct way
stack is [Option<PyObjectRef>]
1 parent c2bfdf3 commit 29d2d88

File tree

8 files changed

+361
-304
lines changed

8 files changed

+361
-304
lines changed

‎Lib/test/test_xml_etree.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2301,7 +2301,6 @@ def test_bug_xmltoolkit62(self):
23012301
self.assertEqual(t.find('.//paragraph').text,
23022302
'A new cultivar of Begonia plant named \u2018BCT9801BEG\u2019.')
23032303

2304-
@unittest.expectedFailure # TODO: RUSTPYTHON
23052304
@unittest.skipIf(sys.gettrace(), "Skips under coverage.")
23062305
def test_bug_xmltoolkit63(self):
23072306
# Check reference leak.

‎crates/codegen/src/compile.rs‎

Lines changed: 64 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -98,12 +98,6 @@ enum NameUsage {
9898
Delete,
9999
}
100100

101-
enum CallType {
102-
Positional { nargs: u32 },
103-
Keyword { nargs: u32 },
104-
Ex { has_kwargs: bool },
105-
}
106-
107101
fn is_forbidden_name(name: &str) -> bool {
108102
// See https://docs.python.org/3/library/constants.html#built-in-constants
109103
const BUILTIN_CONSTANTS: &[&str] = &["__debug__"];
@@ -1037,6 +1031,7 @@ impl Compiler {
10371031

10381032
// Call __exit__(None, None, None) - compiler_call_exit_with_nones
10391033
// Stack: [..., __exit__] or [..., return_value, __exit__]
1034+
emit!(self, Instruction::PushNull);
10401035
self.emit_load_const(ConstantData::None);
10411036
self.emit_load_const(ConstantData::None);
10421037
self.emit_load_const(ConstantData::None);
@@ -1875,6 +1870,7 @@ impl Compiler {
18751870

18761871
let assertion_error = self.name("AssertionError");
18771872
emit!(self, Instruction::LoadGlobal(assertion_error));
1873+
emit!(self, Instruction::PushNull);
18781874
match msg {
18791875
Some(e) => {
18801876
self.compile_expression(e)?;
@@ -2095,12 +2091,13 @@ impl Compiler {
20952091
fn prepare_decorators(&mut self, decorator_list: &[Decorator]) -> CompileResult<()> {
20962092
for decorator in decorator_list {
20972093
self.compile_expression(&decorator.expression)?;
2094+
emit!(self, Instruction::PushNull);
20982095
}
20992096
Ok(())
21002097
}
21012098

21022099
fn apply_decorators(&mut self, decorator_list: &[Decorator]) {
2103-
// Apply decorators:
2100+
// Apply decorators - each pops [decorator, NULL, arg] and pushes result
21042101
for _ in decorator_list {
21052102
emit!(self, Instruction::Call { nargs: 1 });
21062103
}
@@ -2142,6 +2139,7 @@ impl Compiler {
21422139

21432140
// Create type params function with closure
21442141
self.make_closure(code, bytecode::MakeFunctionFlags::empty())?;
2142+
emit!(self, Instruction::PushNull);
21452143

21462144
// Call the function immediately
21472145
emit!(self, Instruction::Call { nargs: 0 });
@@ -3224,11 +3222,6 @@ impl Compiler {
32243222
num_typeparam_args += 1;
32253223
}
32263224

3227-
// SWAP if we have both
3228-
if num_typeparam_args == 2 {
3229-
emit!(self, Instruction::Swap { index: 2 });
3230-
}
3231-
32323225
// Enter type params scope
32333226
let type_params_name = format!("<generic parameters of {name}>");
32343227
self.push_output(
@@ -3308,21 +3301,31 @@ impl Compiler {
33083301
self.make_closure(type_params_code, bytecode::MakeFunctionFlags::empty())?;
33093302

33103303
// Call the closure
3304+
// Call expects stack: [callable, self_or_null, arg1, ..., argN]
33113305
if num_typeparam_args > 0 {
3306+
// Stack: [arg1, ..., argN, closure]
3307+
// Need: [closure, NULL, arg1, ..., argN]
3308+
let reverse_amount = (num_typeparam_args + 1) as u32;
33123309
emit!(
33133310
self,
3314-
Instruction::Swap {
3315-
index: (num_typeparam_args + 1) as u32
3311+
Instruction::Reverse {
3312+
amount: reverse_amount
33163313
}
33173314
);
3315+
// Stack: [closure, argN, ..., arg1]
3316+
emit!(self, Instruction::PushNull);
3317+
// Stack: [closure, argN, ..., arg1, NULL]
33183318
emit!(
33193319
self,
33203320
Instruction::Call {
33213321
nargs: num_typeparam_args as u32
33223322
}
33233323
);
33243324
} else {
3325-
// No arguments, just call the closure
3325+
// Stack: [closure]
3326+
emit!(self, Instruction::PushNull);
3327+
// Stack: [closure, NULL]
3328+
// Call pops: args (0), then self_or_null (NULL), then callable (closure)
33263329
emit!(self, Instruction::Call { nargs: 0 });
33273330
}
33283331
}
@@ -3691,6 +3694,7 @@ impl Compiler {
36913694

36923695
// Generate class creation code
36933696
emit!(self, Instruction::LoadBuildClass);
3697+
emit!(self, Instruction::PushNull);
36943698

36953699
// Set up the class function with type params
36963700
let mut func_flags = bytecode::MakeFunctionFlags::empty();
@@ -3720,14 +3724,20 @@ impl Compiler {
37203724
if let Some(arguments) = arguments
37213725
&& !arguments.keywords.is_empty()
37223726
{
3727+
let mut kwarg_names = vec![];
37233728
for keyword in &arguments.keywords {
3724-
if let Some(name) = &keyword.arg {
3725-
self.emit_load_const(ConstantData::Str {
3726-
value: name.as_str().into(),
3727-
});
3728-
}
3729+
let name = keyword
3730+
.arg
3731+
.as_ref()
3732+
.expect("keyword argument name must be set");
3733+
kwarg_names.push(ConstantData::Str {
3734+
value: name.as_str().into(),
3735+
});
37293736
self.compile_expression(&keyword.value)?;
37303737
}
3738+
self.emit_load_const(ConstantData::Tuple {
3739+
elements: kwarg_names,
3740+
});
37313741
emit!(
37323742
self,
37333743
Instruction::CallKw {
@@ -3748,21 +3758,22 @@ impl Compiler {
37483758

37493759
// Execute the type params function
37503760
self.make_closure(type_params_code, bytecode::MakeFunctionFlags::empty())?;
3761+
emit!(self, Instruction::PushNull);
37513762
emit!(self, Instruction::Call { nargs: 0 });
37523763
} else {
37533764
// Non-generic class: standard path
37543765
emit!(self, Instruction::LoadBuildClass);
3766+
emit!(self, Instruction::PushNull);
37553767

37563768
// Create class function with closure
37573769
self.make_closure(class_code, bytecode::MakeFunctionFlags::empty())?;
37583770
self.emit_load_const(ConstantData::Str { value: name.into() });
37593771

3760-
let call = if let Some(arguments) = arguments {
3761-
self.compile_call_inner(2, arguments)?
3772+
if let Some(arguments) = arguments {
3773+
self.compile_call_helper(2, arguments)?;
37623774
} else {
3763-
CallType::Positional { nargs: 2 }
3764-
};
3765-
self.compile_normal_call(call);
3775+
emit!(self, Instruction::Call { nargs: 2 });
3776+
}
37663777
}
37673778

37683779
// Step 4: Apply decorators and store (common to both paths)
@@ -3912,6 +3923,7 @@ impl Compiler {
39123923
// Stack: [..., __exit__]
39133924
// Call __exit__(None, None, None)
39143925
self.set_source_range(with_range);
3926+
emit!(self, Instruction::PushNull);
39153927
self.emit_load_const(ConstantData::None);
39163928
self.emit_load_const(ConstantData::None);
39173929
self.emit_load_const(ConstantData::None);
@@ -6087,49 +6099,36 @@ impl Compiler {
60876099
}
60886100

60896101
fn compile_call(&mut self, func: &Expr, args: &Arguments) -> CompileResult<()> {
6090-
let method = if let Expr::Attribute(ExprAttribute { value, attr, .. }) = &func {
6102+
// Method call: obj → LOAD_ATTR_METHOD → [method, self_or_null] → args → CALL
6103+
// Regular call: func → PUSH_NULL → args → CALL
6104+
if let Expr::Attribute(ExprAttribute { value, attr, .. }) = &func {
6105+
// Method call: compile object, then LOAD_ATTR_METHOD
6106+
// LOAD_ATTR_METHOD pushes [method, self_or_null] on stack
60916107
self.compile_expression(value)?;
60926108
let idx = self.name(attr.as_str());
6093-
emit!(self, Instruction::LoadMethod { idx });
6094-
true
6109+
emit!(self, Instruction::LoadAttrMethod { idx });
6110+
self.compile_call_helper(0, args)?;
60956111
} else {
6112+
// Regular call: push func, then NULL for self_or_null slot
6113+
// Stack layout: [func, NULL, args...] - same as method call [func, self, args...]
60966114
self.compile_expression(func)?;
6097-
false
6098-
};
6099-
let call = self.compile_call_inner(0, args)?;
6100-
if method {
6101-
self.compile_method_call(call)
6102-
} else {
6103-
self.compile_normal_call(call)
6115+
emit!(self, Instruction::PushNull);
6116+
self.compile_call_helper(0, args)?;
61046117
}
61056118
Ok(())
61066119
}
61076120

6108-
fn compile_normal_call(&mut self, ty: CallType) {
6109-
match ty {
6110-
CallType::Positional { nargs } => {
6111-
emit!(self, Instruction::Call { nargs })
6112-
}
6113-
CallType::Keyword { nargs } => emit!(self, Instruction::CallKw { nargs }),
6114-
CallType::Ex { has_kwargs } => emit!(self, Instruction::CallFunctionEx { has_kwargs }),
6115-
}
6116-
}
6117-
fn compile_method_call(&mut self, ty: CallType) {
6118-
match ty {
6119-
CallType::Positional { nargs } => {
6120-
emit!(self, Instruction::CallMethodPositional { nargs })
6121-
}
6122-
CallType::Keyword { nargs } => emit!(self, Instruction::CallMethodKeyword { nargs }),
6123-
CallType::Ex { has_kwargs } => emit!(self, Instruction::CallMethodEx { has_kwargs }),
6124-
}
6125-
}
6126-
6127-
fn compile_call_inner(
6121+
/// Compile call arguments and emit the appropriate CALL instruction.
6122+
/// This is shared between compiler_call and compiler_class.
6123+
fn compile_call_helper(
61286124
&mut self,
61296125
additional_positional: u32,
61306126
arguments: &Arguments,
6131-
) -> CompileResult<CallType> {
6132-
let count = u32::try_from(arguments.len()).unwrap() + additional_positional;
6127+
) -> CompileResult<()> {
6128+
let args_count = u32::try_from(arguments.len()).expect("too many arguments");
6129+
let count = args_count
6130+
.checked_add(additional_positional)
6131+
.expect("too many arguments");
61336132

61346133
// Normal arguments:
61356134
let (size, unpack) = self.gather_elements(additional_positional, &arguments.args)?;
@@ -6141,7 +6140,7 @@ impl Compiler {
61416140
}
61426141
}
61436142

6144-
let call = if unpack || has_double_star {
6143+
if unpack || has_double_star {
61456144
// Create a tuple with positional args:
61466145
if unpack {
61476146
emit!(self, Instruction::BuildTupleFromTuples { size });
@@ -6154,7 +6153,7 @@ impl Compiler {
61546153
if has_kwargs {
61556154
self.compile_keywords(&arguments.keywords)?;
61566155
}
6157-
CallType::Ex { has_kwargs }
6156+
emit!(self, Instruction::CallFunctionEx { has_kwargs });
61586157
} else if !arguments.keywords.is_empty() {
61596158
let mut kwarg_names = vec![];
61606159
for keyword in &arguments.keywords {
@@ -6172,12 +6171,12 @@ impl Compiler {
61726171
self.emit_load_const(ConstantData::Tuple {
61736172
elements: kwarg_names,
61746173
});
6175-
CallType::Keyword { nargs: count }
6174+
emit!(self, Instruction::CallKw { nargs: count });
61766175
} else {
6177-
CallType::Positional { nargs: count }
6178-
};
6176+
emit!(self, Instruction::Call { nargs: count });
6177+
}
61796178

6180-
Ok(call)
6179+
Ok(())
61816180
}
61826181

61836182
// Given a vector of expr / star expr generate code which gives either
@@ -6409,6 +6408,7 @@ impl Compiler {
64096408

64106409
// Create comprehension function with closure
64116410
self.make_closure(code, bytecode::MakeFunctionFlags::empty())?;
6411+
emit!(self, Instruction::PushNull);
64126412

64136413
// Evaluate iterated item:
64146414
self.compile_expression(&generators[0].iter)?;
@@ -6838,6 +6838,7 @@ impl Compiler {
68386838
match action {
68396839
UnwindAction::With { is_async } => {
68406840
// compiler_call_exit_with_nones
6841+
emit!(self, Instruction::PushNull);
68416842
self.emit_load_const(ConstantData::None);
68426843
self.emit_load_const(ConstantData::None);
68436844
self.emit_load_const(ConstantData::None);

0 commit comments

Comments
 (0)