Skip to content
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

MAKE_FUNCTION target in ceval does not handle co_annotations tuple #12

Open
carljm opened this issue Jun 3, 2021 · 1 comment
Open

Comments

@carljm
Copy link

carljm commented Jun 3, 2021

Context: I'm aiming to test import performance of __future__.co_annotations (relative to __future__.annotations) on a large realistic annotation-heavy codebase, as I offered to do in https://mail.python.org/archives/list/[email protected]/message/VBG2LXU6OHROQ3NPF373L7W4W23B24DE/

I'm finding that the current build doesn't seem to be able to even import the added Lib/test/future_co_annotations.py file without a crash; I'm wondering if I'm doing something wrong, or if this is a known issue?

The issue I'm seeing is that the compiler puts a tuple of (<co_annotations code object>, locals()) on the stack in a case where the annotations make use of local names (e.g. the Nested.f case in future_co_annotations.py), but there doesn't seem to be any handling of such a tuple in MAKE_FUNCTION target in ceval; it just crashes on the assert(TOP()->ob_type == &PyCode_Type); because the stack top is a tuple, not a code object.

If this is known issue that needs fixing, I may be able to work on a fix, just wanted to check first whether this is the right rabbit hole to go down.

@carljm
Copy link
Author

carljm commented Apr 8, 2022

Ok, on revisiting this and looking at b42ce63 more closely, I realized the issue here is simply an assert that wasn't updated, so it only crashes on a debug build. Updating the assert so it accepts either a code object or a tuple is sufficient to make debug builds happy too.

Fixed by #14

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 a pull request may close this issue.

1 participant