Skip to content

Commit c7c2e27

Browse files
committed
cleanup unused args
1 parent a5291c5 commit c7c2e27

File tree

6 files changed

+73
-114
lines changed

6 files changed

+73
-114
lines changed

compiler/codegen/src/compile.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2097,10 +2097,7 @@ impl Compiler<'_> {
20972097
});
20982098

20992099
// Create function with no flags
2100-
emit!(
2101-
self,
2102-
Instruction::MakeFunction(bytecode::MakeFunctionFlags::empty())
2103-
);
2100+
emit!(self, Instruction::MakeFunction);
21042101

21052102
// Now set attributes one by one using SET_FUNCTION_ATTRIBUTE
21062103
// Note: The order matters! Values must be on stack before calling SET_FUNCTION_ATTRIBUTE

compiler/core/src/bytecode.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -528,7 +528,7 @@ pub enum Instruction {
528528
JumpIfFalseOrPop {
529529
target: Arg<Label>,
530530
},
531-
MakeFunction(Arg<MakeFunctionFlags>),
531+
MakeFunction,
532532
SetFunctionAttribute {
533533
attr: Arg<MakeFunctionFlags>,
534534
},
@@ -1299,7 +1299,7 @@ impl Instruction {
12991299
-1
13001300
}
13011301
}
1302-
MakeFunction(_flags) => {
1302+
MakeFunction => {
13031303
// CPython 3.13 style: MakeFunction only pops code object
13041304
-1 + 1 // pop code, push function
13051305
}
@@ -1500,7 +1500,7 @@ impl Instruction {
15001500
JumpIfFalse { target } => w!(JumpIfFalse, target),
15011501
JumpIfTrueOrPop { target } => w!(JumpIfTrueOrPop, target),
15021502
JumpIfFalseOrPop { target } => w!(JumpIfFalseOrPop, target),
1503-
MakeFunction(flags) => w!(MakeFunction, ?flags),
1503+
MakeFunction => w!(MakeFunction),
15041504
SetFunctionAttribute { attr } => w!(SetFunctionAttribute, ?attr),
15051505
CallFunctionPositional { nargs } => w!(CallFunctionPositional, nargs),
15061506
CallFunctionKeyword { nargs } => w!(CallFunctionKeyword, nargs),

jit/tests/common.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ impl StackMachine {
122122
}
123123
self.stack.push(StackValue::Map(map));
124124
}
125-
Instruction::MakeFunction(_flags) => {
125+
Instruction::MakeFunction => {
126126
// CPython 3.13 style: MakeFunction only takes code object
127127
let code = if let Some(StackValue::Code(code)) = self.stack.pop() {
128128
code

vm/src/builtins/function.rs

Lines changed: 62 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -54,18 +54,10 @@ unsafe impl Traverse for PyFunction {
5454
}
5555

5656
impl PyFunction {
57-
#[allow(clippy::too_many_arguments)]
5857
#[inline]
5958
pub(crate) fn new(
6059
code: PyRef<PyCode>,
6160
globals: PyDictRef,
62-
closure: Option<PyRef<PyTuple<PyCellRef>>>,
63-
defaults: Option<PyTupleRef>,
64-
kw_only_defaults: Option<PyDictRef>,
65-
qualname: PyStrRef,
66-
type_params: PyTupleRef,
67-
annotations: PyDictRef,
68-
doc: PyObjectRef,
6961
vm: &VirtualMachine,
7062
) -> PyResult<Self> {
7163
let name = PyMutex::new(code.obj_name.to_owned());
@@ -79,18 +71,21 @@ impl PyFunction {
7971
}
8072
});
8173

74+
// CPython 3.13 style: Use minimal attributes, others will be set via SET_FUNCTION_ATTRIBUTE
75+
let qualname = vm.ctx.new_str(code.qualname.as_str());
76+
8277
let func = Self {
83-
code,
78+
code: code.clone(),
8479
globals,
8580
builtins,
86-
closure,
87-
defaults_and_kwdefaults: PyMutex::new((defaults, kw_only_defaults)),
81+
closure: None, // will be set by SET_FUNCTION_ATTRIBUTE
82+
defaults_and_kwdefaults: PyMutex::new((None, None)), // will be set by SET_FUNCTION_ATTRIBUTE
8883
name,
8984
qualname: PyMutex::new(qualname),
90-
type_params: PyMutex::new(type_params),
91-
annotations: PyMutex::new(annotations),
85+
type_params: PyMutex::new(vm.ctx.empty_tuple.clone()), // will be set by SET_FUNCTION_ATTRIBUTE
86+
annotations: PyMutex::new(vm.ctx.new_dict()), // will be set by SET_FUNCTION_ATTRIBUTE
9287
module: PyMutex::new(module),
93-
doc: PyMutex::new(doc),
88+
doc: PyMutex::new(vm.ctx.none()), // will be set by SET_FUNCTION_ATTRIBUTE
9489
#[cfg(feature = "jit")]
9590
jitted_code: OnceCell::new(),
9691
};
@@ -326,31 +321,40 @@ impl PyFunction {
326321
vm: &VirtualMachine,
327322
) -> PyResult<()> {
328323
use crate::builtins::PyDict;
329-
if attr.contains(bytecode::MakeFunctionFlags::DEFAULTS) {
330-
let defaults = attr_value.clone().downcast::<PyTuple>().map_err(|_| {
331-
vm.new_type_error(format!(
332-
"__defaults__ must be a tuple, not {}",
333-
attr_value.class().name()
334-
))
335-
})?;
324+
if attr == bytecode::MakeFunctionFlags::DEFAULTS {
325+
let defaults = match attr_value.downcast::<PyTuple>() {
326+
Ok(tuple) => tuple,
327+
Err(obj) => {
328+
return Err(vm.new_type_error(format!(
329+
"__defaults__ must be a tuple, not {}",
330+
obj.class().name()
331+
)));
332+
}
333+
};
336334
self.defaults_and_kwdefaults.lock().0 = Some(defaults);
337-
} else if attr.contains(bytecode::MakeFunctionFlags::KW_ONLY_DEFAULTS) {
338-
let kwdefaults = attr_value.clone().downcast::<PyDict>().map_err(|_| {
339-
vm.new_type_error(format!(
340-
"__kwdefaults__ must be a dict, not {}",
341-
attr_value.class().name()
342-
))
343-
})?;
335+
} else if attr == bytecode::MakeFunctionFlags::KW_ONLY_DEFAULTS {
336+
let kwdefaults = match attr_value.downcast::<PyDict>() {
337+
Ok(dict) => dict,
338+
Err(obj) => {
339+
return Err(vm.new_type_error(format!(
340+
"__kwdefaults__ must be a dict, not {}",
341+
obj.class().name()
342+
)));
343+
}
344+
};
344345
self.defaults_and_kwdefaults.lock().1 = Some(kwdefaults);
345-
} else if attr.contains(bytecode::MakeFunctionFlags::ANNOTATIONS) {
346-
let annotations = attr_value.clone().downcast::<PyDict>().map_err(|_| {
347-
vm.new_type_error(format!(
348-
"__annotations__ must be a dict, not {}",
349-
attr_value.class().name()
350-
))
351-
})?;
346+
} else if attr == bytecode::MakeFunctionFlags::ANNOTATIONS {
347+
let annotations = match attr_value.downcast::<PyDict>() {
348+
Ok(dict) => dict,
349+
Err(obj) => {
350+
return Err(vm.new_type_error(format!(
351+
"__annotations__ must be a dict, not {}",
352+
obj.class().name()
353+
)));
354+
}
355+
};
352356
*self.annotations.lock() = annotations;
353-
} else if attr.contains(bytecode::MakeFunctionFlags::CLOSURE) {
357+
} else if attr == bytecode::MakeFunctionFlags::CLOSURE {
354358
// For closure, we need special handling
355359
// The closure tuple contains cell objects
356360
let closure_tuple = attr_value
@@ -365,14 +369,16 @@ impl PyFunction {
365369
.into_pyref();
366370

367371
self.closure = Some(closure_tuple.try_into_typed::<PyCell>(vm)?);
368-
} else if attr.contains(bytecode::MakeFunctionFlags::TYPE_PARAMS) {
372+
} else if attr == bytecode::MakeFunctionFlags::TYPE_PARAMS {
369373
let type_params = attr_value.clone().downcast::<PyTuple>().map_err(|_| {
370374
vm.new_type_error(format!(
371375
"__type_params__ must be a tuple, not {}",
372376
attr_value.class().name()
373377
))
374378
})?;
375379
*self.type_params.lock() = type_params;
380+
} else {
381+
unreachable!("This is a compiler bug");
376382
}
377383
Ok(())
378384
}
@@ -682,34 +688,25 @@ impl Constructor for PyFunction {
682688
None
683689
};
684690

685-
// Get function name - use provided name or default to code object name
686-
let name = args
687-
.name
688-
.into_option()
689-
.unwrap_or_else(|| PyStr::from(args.code.obj_name.as_str()).into_ref(&vm.ctx));
690-
691-
// Get qualname - for now just use the name
692-
let qualname = name.clone();
693-
694-
// Create empty type_params and annotations
695-
let type_params = vm.ctx.new_tuple(vec![]);
696-
let annotations = vm.ctx.new_dict();
697-
698-
// Get doc from code object - for now just use None
699-
let doc = vm.ctx.none();
700-
701-
let func = Self::new(
702-
args.code,
703-
args.globals,
704-
closure,
705-
args.defaults.into_option(),
706-
args.kwdefaults.into_option(),
707-
qualname,
708-
type_params,
709-
annotations,
710-
doc,
711-
vm,
712-
)?;
691+
// CPython 3.13 style: Create minimal function, set attributes separately
692+
let mut func = Self::new(args.code.clone(), args.globals.clone(), vm)?;
693+
694+
// Set function name if provided
695+
if let Some(name) = args.name.into_option() {
696+
*func.name.lock() = name.clone();
697+
// Also update qualname to match the name
698+
*func.qualname.lock() = name;
699+
}
700+
// Now set additional attributes directly
701+
if let Some(closure_tuple) = closure {
702+
func.closure = Some(closure_tuple);
703+
}
704+
if let Some(defaults) = args.defaults.into_option() {
705+
func.defaults_and_kwdefaults.lock().0 = Some(defaults);
706+
}
707+
if let Some(kwdefaults) = args.kwdefaults.into_option() {
708+
func.defaults_and_kwdefaults.lock().1 = Some(kwdefaults);
709+
}
713710

714711
func.into_ref_with_type(vm, cls).map(Into::into)
715712
}

vm/src/frame.rs

Lines changed: 5 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1159,9 +1159,7 @@ impl ExecutingFrame<'_> {
11591159
}
11601160
}
11611161
bytecode::Instruction::ForIter { target } => self.execute_for_iter(vm, target.get(arg)),
1162-
bytecode::Instruction::MakeFunction(flags) => {
1163-
self.execute_make_function(vm, flags.get(arg))
1164-
}
1162+
bytecode::Instruction::MakeFunction => self.execute_make_function(vm),
11651163
bytecode::Instruction::SetFunctionAttribute { attr } => {
11661164
self.execute_set_function_attribute(vm, attr.get(arg))
11671165
}
@@ -1830,34 +1828,15 @@ impl ExecutingFrame<'_> {
18301828
}
18311829
}
18321830
}
1833-
fn execute_make_function(
1834-
&mut self,
1835-
vm: &VirtualMachine,
1836-
_flags: bytecode::MakeFunctionFlags,
1837-
) -> FrameResult {
1831+
fn execute_make_function(&mut self, vm: &VirtualMachine) -> FrameResult {
18381832
// CPython 3.13 style: MakeFunction only takes code object, no flags
18391833
let code_obj: PyRef<PyCode> = self
18401834
.pop_value()
18411835
.downcast()
18421836
.expect("Stack value should be code object");
18431837

18441838
// Create function with minimal attributes
1845-
// qualname is taken from code object's co_qualname (not co_name)
1846-
let qualname = code_obj.qualname.to_owned();
1847-
1848-
let func_obj = PyFunction::new(
1849-
code_obj,
1850-
self.globals.clone(),
1851-
None, // closure - will be set by SET_FUNCTION_ATTRIBUTE
1852-
None, // defaults - will be set by SET_FUNCTION_ATTRIBUTE
1853-
None, // kw_only_defaults - will be set by SET_FUNCTION_ATTRIBUTE
1854-
qualname,
1855-
vm.ctx.empty_tuple.clone(), // type_params - will be set by SET_FUNCTION_ATTRIBUTE
1856-
vm.ctx.new_dict(), // annotations - will be set by SET_FUNCTION_ATTRIBUTE
1857-
vm.ctx.none(), // doc
1858-
vm,
1859-
)?
1860-
.into_pyobject(vm);
1839+
let func_obj = PyFunction::new(code_obj, self.globals.clone(), vm)?.into_pyobject(vm);
18611840

18621841
self.push_value(func_obj);
18631842
Ok(None)
@@ -1873,8 +1852,9 @@ impl ExecutingFrame<'_> {
18731852
// Stack order: func is at -1, attr_value is at -2
18741853

18751854
let func = self.pop_value();
1876-
let attr_value = self.pop_value();
1855+
let attr_value = self.replace_top(func);
18771856

1857+
let func = self.top_value();
18781858
// Verify that we have a function object
18791859
if !func.class().is(vm.ctx.types.function_type) {
18801860
return Err(vm.new_type_error(format!(
@@ -1896,9 +1876,6 @@ impl ExecutingFrame<'_> {
18961876
(*payload_ptr).set_function_attribute(attr, attr_value, vm)?;
18971877
};
18981878

1899-
// Push function back onto stack
1900-
self.push_value(func);
1901-
19021879
Ok(None)
19031880
}
19041881

vm/src/vm/mod.rs

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -448,19 +448,7 @@ impl VirtualMachine {
448448
use crate::builtins::PyFunction;
449449

450450
// Create a function object for module code, similar to CPython's PyEval_EvalCode
451-
let qualname = self.ctx.new_str(code.obj_name.as_str());
452-
let func = PyFunction::new(
453-
code.clone(),
454-
scope.globals.clone(),
455-
None, // closure
456-
None, // defaults
457-
None, // kwdefaults
458-
qualname,
459-
self.ctx.new_tuple(vec![]), // type_params
460-
self.ctx.new_dict(), // annotations
461-
self.ctx.none(), // doc
462-
self,
463-
)?;
451+
let func = PyFunction::new(code.clone(), scope.globals.clone(), self)?;
464452
let func_obj = func.into_ref(&self.ctx).into();
465453

466454
let frame = Frame::new(code, scope, self.builtins.dict(), &[], Some(func_obj), self)

0 commit comments

Comments
 (0)