Skip to content

SetFunctionAttribute #5968

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
youknowone merged 6 commits into from
Jul 14, 2025
Merged

SetFunctionAttribute #5968

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5bfbb77
PyRef::into_non_null
youknowone Jul 14, 2025
53d353b
SetFunctionAttribute
youknowone Jul 14, 2025
7787c6f
set_function_attribute
youknowone Jul 14, 2025
ae9e663
frame helper in PyFuncion
youknowone Jul 14, 2025
a5291c5
remove closure lock
youknowone Jul 14, 2025
1dd0f57
cleanup unused args
youknowone Jul 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
cleanup unused args
  • Loading branch information
@youknowone
youknowone committed Jul 14, 2025
commit 1dd0f575560cd1d63b5c759b2c26342943704a2b
70 changes: 31 additions & 39 deletions compiler/codegen/src/compile.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1989,7 +1989,7 @@ impl Compiler<'_> {
}

/// Determines if a variable should be CELL or FREE type
/// Equivalent to CPython's get_ref_type
// = get_ref_type
fn get_ref_type(&self, name: &str) -> Result<SymbolScope, CodegenErrorType> {
// Special handling for __class__ and __classdict__ in class scope
if self.ctx.in_class && (name == "__class__" || name == "__classdict__") {
Expand Down Expand Up @@ -2024,7 +2024,7 @@ impl Compiler<'_> {
if has_freevars {
// Build closure tuple by loading free variables

for var in &*code.freevars {
for var in &code.freevars {
// Special case: If a class contains a method with a
// free variable that has the same name as a method,
// the name will be considered free *and* local in the
Expand All @@ -2040,38 +2040,33 @@ impl Compiler<'_> {

// Look up the variable index based on reference type
let idx = match ref_type {
SymbolScope::Cell => {
match parent_code.metadata.cellvars.get_index_of(var) {
Some(i) => i,
None => {
// Try in freevars as fallback
match parent_code.metadata.freevars.get_index_of(var) {
Some(i) => i + cellvars_len,
None => {
return Err(self.error(CodegenErrorType::SyntaxError(format!(
"compiler_make_closure: cannot find '{var}' in parent vars",
))));
}
}
}
}
}
SymbolScope::Free => {
match parent_code.metadata.freevars.get_index_of(var) {
Some(i) => i + cellvars_len,
None => {
// Try in cellvars as fallback
match parent_code.metadata.cellvars.get_index_of(var) {
Some(i) => i,
None => {
return Err(self.error(CodegenErrorType::SyntaxError(format!(
"compiler_make_closure: cannot find '{var}' in parent vars",
))));
}
}
}
}
}
SymbolScope::Cell => parent_code
.metadata
.cellvars
.get_index_of(var)
.or_else(|| {
parent_code
.metadata
.freevars
.get_index_of(var)
.map(|i| i + cellvars_len)
})
.ok_or_else(|| {
self.error(CodegenErrorType::SyntaxError(format!(
"compiler_make_closure: cannot find '{var}' in parent vars",
)))
})?,
SymbolScope::Free => parent_code
.metadata
.freevars
.get_index_of(var)
.map(|i| i + cellvars_len)
.or_else(|| parent_code.metadata.cellvars.get_index_of(var))
.ok_or_else(|| {
self.error(CodegenErrorType::SyntaxError(format!(
"compiler_make_closure: cannot find '{var}' in parent vars",
)))
})?,
_ => {
return Err(self.error(CodegenErrorType::SyntaxError(format!(
"compiler_make_closure: unexpected ref_type {ref_type:?} for '{var}'",
Expand All @@ -2091,16 +2086,13 @@ impl Compiler<'_> {
);
}

// CPython 3.13 style: load code object and create function
// load code object and create function
self.emit_load_const(ConstantData::Code {
code: Box::new(code),
});

// Create function with no flags
emit!(
self,
Instruction::MakeFunction(bytecode::MakeFunctionFlags::empty())
);
emit!(self, Instruction::MakeFunction);

// Now set attributes one by one using SET_FUNCTION_ATTRIBUTE
// Note: The order matters! Values must be on stack before calling SET_FUNCTION_ATTRIBUTE
Expand Down
6 changes: 3 additions & 3 deletions compiler/core/src/bytecode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ pub enum Instruction {
JumpIfFalseOrPop {
target: Arg<Label>,
},
MakeFunction(Arg<MakeFunctionFlags>),
MakeFunction,
SetFunctionAttribute {
attr: Arg<MakeFunctionFlags>,
},
Expand Down Expand Up @@ -1299,7 +1299,7 @@ impl Instruction {
-1
}
}
MakeFunction(_flags) => {
MakeFunction => {
// CPython 3.13 style: MakeFunction only pops code object
-1 + 1 // pop code, push function
}
Expand Down Expand Up @@ -1500,7 +1500,7 @@ impl Instruction {
JumpIfFalse { target } => w!(JumpIfFalse, target),
JumpIfTrueOrPop { target } => w!(JumpIfTrueOrPop, target),
JumpIfFalseOrPop { target } => w!(JumpIfFalseOrPop, target),
MakeFunction(flags) => w!(MakeFunction, ?flags),
MakeFunction => w!(MakeFunction),
SetFunctionAttribute { attr } => w!(SetFunctionAttribute, ?attr),
CallFunctionPositional { nargs } => w!(CallFunctionPositional, nargs),
CallFunctionKeyword { nargs } => w!(CallFunctionKeyword, nargs),
Expand Down
5 changes: 1 addition & 4 deletions jit/tests/common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,22 +122,19 @@ impl StackMachine {
}
self.stack.push(StackValue::Map(map));
}
Instruction::MakeFunction(_flags) => {
// CPython 3.13 style: MakeFunction only takes code object
Instruction::MakeFunction => {
let code = if let Some(StackValue::Code(code)) = self.stack.pop() {
code
} else {
panic!("Expected function code")
};
// Create function with minimal attributes
// Other attributes will be set by SET_FUNCTION_ATTRIBUTE
self.stack.push(StackValue::Function(Function {
code,
annotations: HashMap::new(), // empty annotations, will be set later if needed
}));
}
Instruction::SetFunctionAttribute { attr } => {
// CPython 3.13 style: SET_FUNCTION_ATTRIBUTE sets attributes on a function
// Stack: [..., attr_value, func] -> [..., func]
let func = if let Some(StackValue::Function(func)) = self.stack.pop() {
func
Expand Down
130 changes: 58 additions & 72 deletions vm/src/builtins/function.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,18 +54,10 @@ unsafe impl Traverse for PyFunction {
}

impl PyFunction {
#[allow(clippy::too_many_arguments)]
#[inline]
pub(crate) fn new(
code: PyRef<PyCode>,
globals: PyDictRef,
closure: Option<PyRef<PyTuple<PyCellRef>>>,
defaults: Option<PyTupleRef>,
kw_only_defaults: Option<PyDictRef>,
qualname: PyStrRef,
type_params: PyTupleRef,
annotations: PyDictRef,
doc: PyObjectRef,
vm: &VirtualMachine,
) -> PyResult<Self> {
let name = PyMutex::new(code.obj_name.to_owned());
Expand All @@ -79,27 +71,22 @@ impl PyFunction {
}
});

let qualname = vm.ctx.new_str(code.qualname.as_str());
let func = Self {
code,
code: code.clone(),
globals,
builtins,
closure,
defaults_and_kwdefaults: PyMutex::new((defaults, kw_only_defaults)),
closure: None,
defaults_and_kwdefaults: PyMutex::new((None, None)),
name,
qualname: PyMutex::new(qualname),
type_params: PyMutex::new(type_params),
annotations: PyMutex::new(annotations),
type_params: PyMutex::new(vm.ctx.empty_tuple.clone()),
annotations: PyMutex::new(vm.ctx.new_dict()),
module: PyMutex::new(module),
doc: PyMutex::new(doc),
doc: PyMutex::new(vm.ctx.none()),
#[cfg(feature = "jit")]
jitted_code: OnceCell::new(),
};

// let name = qualname.as_str().split('.').next_back().unwrap();
// func.set_attr(identifier!(vm, __name__), vm.new_pyobj(name), vm)?;
// func.set_attr(identifier!(vm, __qualname__), qualname, vm)?;
// func.set_attr(identifier!(vm, __doc__), doc, vm)?;

Ok(func)
}

Expand Down Expand Up @@ -318,39 +305,47 @@ impl PyFunction {
}

/// Set function attribute based on MakeFunctionFlags
/// This is used by SET_FUNCTION_ATTRIBUTE instruction in CPython 3.13 style
pub(crate) fn set_function_attribute(
&mut self,
attr: bytecode::MakeFunctionFlags,
attr_value: PyObjectRef,
vm: &VirtualMachine,
) -> PyResult<()> {
use crate::builtins::PyDict;
if attr.contains(bytecode::MakeFunctionFlags::DEFAULTS) {
let defaults = attr_value.clone().downcast::<PyTuple>().map_err(|_| {
vm.new_type_error(format!(
"__defaults__ must be a tuple, not {}",
attr_value.class().name()
))
})?;
if attr == bytecode::MakeFunctionFlags::DEFAULTS {
let defaults = match attr_value.downcast::<PyTuple>() {
Ok(tuple) => tuple,
Err(obj) => {
return Err(vm.new_type_error(format!(
"__defaults__ must be a tuple, not {}",
obj.class().name()
)));
}
};
self.defaults_and_kwdefaults.lock().0 = Some(defaults);
} else if attr.contains(bytecode::MakeFunctionFlags::KW_ONLY_DEFAULTS) {
let kwdefaults = attr_value.clone().downcast::<PyDict>().map_err(|_| {
vm.new_type_error(format!(
"__kwdefaults__ must be a dict, not {}",
attr_value.class().name()
))
})?;
} else if attr == bytecode::MakeFunctionFlags::KW_ONLY_DEFAULTS {
let kwdefaults = match attr_value.downcast::<PyDict>() {
Ok(dict) => dict,
Err(obj) => {
return Err(vm.new_type_error(format!(
"__kwdefaults__ must be a dict, not {}",
obj.class().name()
)));
}
};
self.defaults_and_kwdefaults.lock().1 = Some(kwdefaults);
} else if attr.contains(bytecode::MakeFunctionFlags::ANNOTATIONS) {
let annotations = attr_value.clone().downcast::<PyDict>().map_err(|_| {
vm.new_type_error(format!(
"__annotations__ must be a dict, not {}",
attr_value.class().name()
))
})?;
} else if attr == bytecode::MakeFunctionFlags::ANNOTATIONS {
let annotations = match attr_value.downcast::<PyDict>() {
Ok(dict) => dict,
Err(obj) => {
return Err(vm.new_type_error(format!(
"__annotations__ must be a dict, not {}",
obj.class().name()
)));
}
};
*self.annotations.lock() = annotations;
} else if attr.contains(bytecode::MakeFunctionFlags::CLOSURE) {
} else if attr == bytecode::MakeFunctionFlags::CLOSURE {
// For closure, we need special handling
// The closure tuple contains cell objects
let closure_tuple = attr_value
Expand All @@ -365,14 +360,16 @@ impl PyFunction {
.into_pyref();

self.closure = Some(closure_tuple.try_into_typed::<PyCell>(vm)?);
} else if attr.contains(bytecode::MakeFunctionFlags::TYPE_PARAMS) {
} else if attr == bytecode::MakeFunctionFlags::TYPE_PARAMS {
let type_params = attr_value.clone().downcast::<PyTuple>().map_err(|_| {
vm.new_type_error(format!(
"__type_params__ must be a tuple, not {}",
attr_value.class().name()
))
})?;
*self.type_params.lock() = type_params;
} else {
unreachable!("This is a compiler bug");
}
Ok(())
}
Expand Down Expand Up @@ -682,34 +679,23 @@ impl Constructor for PyFunction {
None
};

// Get function name - use provided name or default to code object name
let name = args
.name
.into_option()
.unwrap_or_else(|| PyStr::from(args.code.obj_name.as_str()).into_ref(&vm.ctx));

// Get qualname - for now just use the name
let qualname = name.clone();

// Create empty type_params and annotations
let type_params = vm.ctx.new_tuple(vec![]);
let annotations = vm.ctx.new_dict();

// Get doc from code object - for now just use None
let doc = vm.ctx.none();

let func = Self::new(
args.code,
args.globals,
closure,
args.defaults.into_option(),
args.kwdefaults.into_option(),
qualname,
type_params,
annotations,
doc,
vm,
)?;
let mut func = Self::new(args.code.clone(), args.globals.clone(), vm)?;
// Set function name if provided
if let Some(name) = args.name.into_option() {
*func.name.lock() = name.clone();
// Also update qualname to match the name
*func.qualname.lock() = name;
}
// Now set additional attributes directly
if let Some(closure_tuple) = closure {
func.closure = Some(closure_tuple);
}
if let Some(defaults) = args.defaults.into_option() {
func.defaults_and_kwdefaults.lock().0 = Some(defaults);
}
if let Some(kwdefaults) = args.kwdefaults.into_option() {
func.defaults_and_kwdefaults.lock().1 = Some(kwdefaults);
}

func.into_ref_with_type(vm, cls).map(Into::into)
}
Expand Down
Loading
Loading