Skip to content

Commit 16d132c

Browse files
youknowoneyouknowone
committed
cleanup unused args
1 parent a5291c5 commit 16d132c

File tree

6 files changed

+85
-164
lines changed

6 files changed

+85
-164
lines changed

‎compiler/codegen/src/compile.rs‎

Lines changed: 16 additions & 35 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__") {
@@ -2024,7 +2024,7 @@ impl Compiler<'_> {
20242024
if has_freevars {
20252025
// Build closure tuple by loading free variables
20262026

2027-
for var in &*code.freevars {
2027+
for var in &code.freevars {
20282028
// Special case: If a class contains a method with a
20292029
// free variable that has the same name as a method,
20302030
// the name will be considered free *and* local in the
@@ -2041,36 +2041,20 @@ impl Compiler<'_> {
20412041
// Look up the variable index based on reference type
20422042
let idx = match ref_type {
20432043
SymbolScope::Cell => {
2044-
match parent_code.metadata.cellvars.get_index_of(var) {
2045-
Some(i) => i,
2046-
None => {
2047-
// Try in freevars as fallback
2048-
match parent_code.metadata.freevars.get_index_of(var) {
2049-
Some(i) => i + cellvars_len,
2050-
None => {
2051-
return Err(self.error(CodegenErrorType::SyntaxError(format!(
2052-
"compiler_make_closure: cannot find '{var}' in parent vars",
2053-
))));
2054-
}
2055-
}
2056-
}
2057-
}
2044+
parent_code.metadata.cellvars.get_index_of(var)
2045+
.or_else(|| parent_code.metadata.freevars.get_index_of(var)
2046+
.map(|i| i + cellvars_len))
2047+
.ok_or_else(|| self.error(CodegenErrorType::SyntaxError(format!(
2048+
"compiler_make_closure: cannot find '{var}' in parent vars",
2049+
))))?
20582050
}
20592051
SymbolScope::Free => {
2060-
match parent_code.metadata.freevars.get_index_of(var) {
2061-
Some(i) => i + cellvars_len,
2062-
None => {
2063-
// Try in cellvars as fallback
2064-
match parent_code.metadata.cellvars.get_index_of(var) {
2065-
Some(i) => i,
2066-
None => {
2067-
return Err(self.error(CodegenErrorType::SyntaxError(format!(
2068-
"compiler_make_closure: cannot find '{var}' in parent vars",
2069-
))));
2070-
}
2071-
}
2072-
}
2073-
}
2052+
parent_code.metadata.freevars.get_index_of(var)
2053+
.map(|i| i + cellvars_len)
2054+
.or_else(|| parent_code.metadata.cellvars.get_index_of(var))
2055+
.ok_or_else(|| self.error(CodegenErrorType::SyntaxError(format!(
2056+
"compiler_make_closure: cannot find '{var}' in parent vars",
2057+
))))?
20742058
}
20752059
_ => {
20762060
return Err(self.error(CodegenErrorType::SyntaxError(format!(
@@ -2091,16 +2075,13 @@ impl Compiler<'_> {
20912075
);
20922076
}
20932077

2094-
// CPython 3.13 style: load code object and create function
2078+
// load code object and create function
20952079
self.emit_load_const(ConstantData::Code {
20962080
code: Box::new(code),
20972081
});
20982082

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

21052086
// Now set attributes one by one using SET_FUNCTION_ATTRIBUTE
21062087
// 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
}

0 commit comments

Comments
 (0)