GH-93516: Store offset of first traceable instruction in code object by markshannon · Pull Request #93769 · python/cpython
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
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
As an experiment, can you remove the "import opcode" from deepfreeze.py and hardcode the RESUME opcode? We can debate whether that's the right fix, but so far the deepfreeze.py script has been carefully designed not to depend on the details of the target version.
Did you consider putting the new field in the marshaled code object? Then deepfreeze.py wouldn't have to import opcode.
Did you consider putting the new field in the marshaled code object?
Yes, but I decided against it.
This PR needs backporting to 3.11, so I want to keep it as small as possible, and it is already quite large.
Thanks @markshannon for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖
markshannon
deleted the
cache-first-traceable-instruction
branch
| @@ -20,6 +20,9 @@ | |||
| verbose = False | |||
| identifiers, strings = get_identifiers_and_strings() | |||
|
|
|||
| # This must be kept in sync with opcode.py | |||
| RESUME = 151 | |||
|
|
|||
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't this be opcode.opmap["RESUME"]?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, that would introduce a dependency on the latest opcode.py which would mean you cannot run deepfreeze.py with e.g. Python 3.10. On Windows and for cross compilations we need to be able to do that. (Windows doesn't have a separate bootstrap Python because it would near-doubel build times.)
We started with this (see #93771).