SetFunctionAttribute by youknowone · Pull Request #5968 · RustPython/RustPython
1485-1524: LGTM: Clean separation of concerns in function entry.
The refactored enter_function method now focuses solely on setting up the function scope and registering parameter names, which is a good separation of concerns. The removal of default argument compilation from this method aligns with the PR's goal of compiling defaults before entering the function scope.
1828-1878: Well-structured default argument compilation.
The new approach of compiling default arguments before entering the function scope is well-implemented. The code correctly handles both positional and keyword-only defaults, building appropriate tuples and maps for the bytecode.
1991-2013: Excellent helper method for symbol scope resolution.
The get_ref_type method provides a clean abstraction for determining whether a variable should be CELL or FREE type. The special handling for __class__ and __classdict__ in class scopes is correctly implemented, and the error handling is appropriate.
2017-2164: Excellent refactoring to match CPython 3.13 semantics.
The make_closure method has been thoroughly refactored to follow CPython 3.13's approach of creating the function first, then setting attributes individually. Key improvements:
- Proper handling of closure variables with
get_ref_type - Clear separation between function creation and attribute setting
- Correct order of operations for
SetFunctionAttributeinstructions - Good error handling for variable lookups
The logic for finding variables in parent cellvars/freevars with fallback lookups is particularly well-implemented.
4131-4182: Consistent lambda compilation with function compilation.
The lambda compilation changes mirror the function compilation approach, correctly compiling default arguments before entering the function scope. The consistency between lambda and regular function compilation is good for maintainability.
1974-1974: Proper removal of qualname parameter.
All calls to make_closure have been correctly updated to remove the qualname parameter, which aligns with the method signature changes. The qualified name handling is now properly managed within the make_closure method itself.
Also applies to: 2364-2364, 2413-2413, 2420-2420, 2748-2748, 4203-4203