Skip to content

Commit c8a4e19

Browse files
youknowoneyouknowone
committed
cleanup unused args
1 parent a5291c5 commit c8a4e19

File tree

6 files changed

+71
-134
lines changed

6 files changed

+71
-134
lines changed

‎compiler/codegen/src/compile.rs‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1989,7 +1989,7 @@ impl Compiler<'_> {
19891989
}
19901990

19911991
/// Determines if a variable should be CELL or FREE type
1992-
/// Equivalent to CPython's get_ref_type
1992+
// = get_ref_type
19931993
fn get_ref_type(&self, name: &str) -> Result<SymbolScope, CodegenErrorType> {
19941994
// Special handling for __class__ and __classdict__ in class scope
19951995
if self.ctx.in_class && (name == "__class__" || name == "__classdict__") {
@@ -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 & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -122,22 +122,19 @@ impl StackMachine {
122122
}
123123
self.stack.push(StackValue::Map(map));
124124
}
125-
Instruction::MakeFunction(_flags) => {
126-
// CPython 3.13 style: MakeFunction only takes code object
125+
Instruction::MakeFunction => {
127126
let code = if let Some(StackValue::Code(code)) = self.stack.pop() {
128127
code
129128
} else {
130129
panic!("Expected function code")
131130
};
132-
// Create function with minimal attributes
133131
// Other attributes will be set by SET_FUNCTION_ATTRIBUTE
134132
self.stack.push(StackValue::Function(Function {
135133
code,
136134
annotations: HashMap::new(), // empty annotations, will be set later if needed
137135
}));
138136
}
139137
Instruction::SetFunctionAttribute { attr } => {
140-
// CPython 3.13 style: SET_FUNCTION_ATTRIBUTE sets attributes on a function
141138
// Stack: [..., attr_value, func] -> [..., func]
142139
let func = if let Some(StackValue::Function(func)) = self.stack.pop() {
143140
func

‎vm/src/builtins/function.rs‎

Lines changed: 58 additions & 72 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,27 +71,22 @@ impl PyFunction {
7971
}
8072
});
8173

74+
let qualname = vm.ctx.new_str(code.qualname.as_str());
8275
let func = Self {
83-
code,
76+
code: code.clone(),
8477
globals,
8578
builtins,
86-
closure,
87-
defaults_and_kwdefaults: PyMutex::new((defaults, kw_only_defaults)),
79+
closure: None,
80+
defaults_and_kwdefaults: PyMutex::new((None, None)),
8881
name,
8982
qualname: PyMutex::new(qualname),
90-
type_params: PyMutex::new(type_params),
91-
annotations: PyMutex::new(annotations),
83+
type_params: PyMutex::new(vm.ctx.empty_tuple.clone()),
84+
annotations: PyMutex::new(vm.ctx.new_dict()),
9285
module: PyMutex::new(module),
93-
doc: PyMutex::new(doc),
86+
doc: PyMutex::new(vm.ctx.none()),
9487
#[cfg(feature = "jit")]
9588
jitted_code: OnceCell::new(),
9689
};
97-
98-
// let name = qualname.as_str().split('.').next_back().unwrap();
99-
// func.set_attr(identifier!(vm, __name__), vm.new_pyobj(name), vm)?;
100-
// func.set_attr(identifier!(vm, __qualname__), qualname, vm)?;
101-
// func.set_attr(identifier!(vm, __doc__), doc, vm)?;
102-
10390
Ok(func)
10491
}
10592

@@ -318,39 +305,47 @@ impl PyFunction {
318305
}
319306

320307
/// Set function attribute based on MakeFunctionFlags
321-
/// This is used by SET_FUNCTION_ATTRIBUTE instruction in CPython 3.13 style
322308
pub(crate) fn set_function_attribute(
323309
&mut self,
324310
attr: bytecode::MakeFunctionFlags,
325311
attr_value: PyObjectRef,
326312
vm: &VirtualMachine,
327313
) -> PyResult<()> {
328314
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-
})?;
315+
if attr == bytecode::MakeFunctionFlags::DEFAULTS {
316+
let defaults = match attr_value.downcast::<PyTuple>() {
317+
Ok(tuple) => tuple,
318+
Err(obj) => {
319+
return Err(vm.new_type_error(format!(
320+
"__defaults__ must be a tuple, not {}",
321+
obj.class().name()
322+
)));
323+
}
324+
};
336325
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-
})?;
326+
} else if attr == bytecode::MakeFunctionFlags::KW_ONLY_DEFAULTS {
327+
let kwdefaults = match attr_value.downcast::<PyDict>() {
328+
Ok(dict) => dict,
329+
Err(obj) => {
330+
return Err(vm.new_type_error(format!(
331+
"__kwdefaults__ must be a dict, not {}",
332+
obj.class().name()
333+
)));
334+
}
335+
};
344336
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-
})?;
337+
} else if attr == bytecode::MakeFunctionFlags::ANNOTATIONS {
338+
let annotations = match attr_value.downcast::<PyDict>() {
339+
Ok(dict) => dict,
340+
Err(obj) => {
341+
return Err(vm.new_type_error(format!(
342+
"__annotations__ must be a dict, not {}",
343+
obj.class().name()
344+
)));
345+
}
346+
};
352347
*self.annotations.lock() = annotations;
353-
} else if attr.contains(bytecode::MakeFunctionFlags::CLOSURE) {
348+
} else if attr == bytecode::MakeFunctionFlags::CLOSURE {
354349
// For closure, we need special handling
355350
// The closure tuple contains cell objects
356351
let closure_tuple = attr_value
@@ -365,14 +360,16 @@ impl PyFunction {
365360
.into_pyref();
366361

367362
self.closure = Some(closure_tuple.try_into_typed::<PyCell>(vm)?);
368-
} else if attr.contains(bytecode::MakeFunctionFlags::TYPE_PARAMS) {
363+
} else if attr == bytecode::MakeFunctionFlags::TYPE_PARAMS {
369364
let type_params = attr_value.clone().downcast::<PyTuple>().map_err(|_| {
370365
vm.new_type_error(format!(
371366
"__type_params__ must be a tuple, not {}",
372367
attr_value.class().name()
373368
))
374369
})?;
375370
*self.type_params.lock() = type_params;
371+
} else {
372+
unreachable!("This is a compiler bug");
376373
}
377374
Ok(())
378375
}
@@ -682,34 +679,23 @@ impl Constructor for PyFunction {
682679
None
683680
};
684681

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-
)?;
682+
let mut func = Self::new(args.code.clone(), args.globals.clone(), vm)?;
683+
// Set function name if provided
684+
if let Some(name) = args.name.into_option() {
685+
*func.name.lock() = name.clone();
686+
// Also update qualname to match the name
687+
*func.qualname.lock() = name;
688+
}
689+
// Now set additional attributes directly
690+
if let Some(closure_tuple) = closure {
691+
func.closure = Some(closure_tuple);
692+
}
693+
if let Some(defaults) = args.defaults.into_option() {
694+
func.defaults_and_kwdefaults.lock().0 = Some(defaults);
695+
}
696+
if let Some(kwdefaults) = args.kwdefaults.into_option() {
697+
func.defaults_and_kwdefaults.lock().1 = Some(kwdefaults);
698+
}
713699

714700
func.into_ref_with_type(vm, cls).map(Into::into)
715701
}

‎vm/src/frame.rs‎

Lines changed: 6 additions & 37 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 {
1838-
// CPython 3.13 style: MakeFunction only takes code object, no flags
1831+
fn execute_make_function(&mut self, vm: &VirtualMachine) -> FrameResult {
1832+
// 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,16 +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();
1877-
1878-
// Verify that we have a function object
1879-
if !func.class().is(vm.ctx.types.function_type) {
1880-
return Err(vm.new_type_error(format!(
1881-
"SET_FUNCTION_ATTRIBUTE requires function, not {}",
1882-
func.class().name()
1883-
)));
1884-
}
1855+
let attr_value = self.replace_top(func);
18851856

1857+
let func = self.top_value();
18861858
// Get the function reference and call the new method
18871859
let func_ref = func
18881860
.downcast_ref::<PyFunction>()
@@ -1896,9 +1868,6 @@ impl ExecutingFrame<'_> {
18961868
(*payload_ptr).set_function_attribute(attr, attr_value, vm)?;
18971869
};
18981870

1899-
// Push function back onto stack
1900-
self.push_value(func);
1901-
19021871
Ok(None)
19031872
}
19041873

‎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)