Skip to content

GH-105678: Split MAKE_FUNCTION into MAKE_FUNCTION and SET_FUNCTION_ATTRIBUTE #105680

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.

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
merged 1 commit into from
Jun 13, 2023

Conversation

markshannon
Copy link
Member

@markshannon markshannon commented Jun 12, 2023

The purpose of this PR is to simplify the stack effect of MAKE_FUNCTION to simplify our code generators and analysis.

This probably isn't the final version of MAKE_FUNCTION.
I expect that will take a variable number of cells, embedding the cells directly in the function object to avoid the indirection in accessing cells.

I've kept the same flags as before. SET_FUNCTION_ATTRIBUTE could be streamlined, but that's for another PR.

@@ -463,7 +465,7 @@ def _write_atomic(path, data, mode=0o666):
# Whenever MAGIC_NUMBER is changed, the ranges in the magic_values array
# in PC/launcher.c must also be updated.

MAGIC_NUMBER = (3551).to_bytes(2, 'little') + b'\r\n'
MAGIC_NUMBER = (3554).to_bytes(2, 'little') + b'\r\n'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll set this to the correct value before merging.
The actual number will depend on which PRs get merged first.

Copy link
Member

@carljm carljm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think _Py_set_function_type_params intrinsic should probably be rolled into SET_FUNCTION_ATTRIBUTE also? But that could be a separate change.

@markshannon
Copy link
Member Author

markshannon commented Jun 13, 2023

Once we handle closures in MAKE_FUNCTION we should either convert SET_FUNCTION_ATTRIBUTE into instrinsics or move the _Py_set_function_type_params into SET_FUNCTION_ATTRIBUTE. I suspect it will be the former, but it will depend on the numbers.

@markshannon markshannon merged commit 09ffa69 into python:main Jun 13, 2023
@markshannon markshannon deleted the simplify-make-function branch September 26, 2023 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants