Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.3k
bpo-43950: Initial base implementation for PEP 657#26955
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
isidentical commentedJun 29, 2021
A couple doctest failures FYI:https://travis-ci.com/github/python/cpython/jobs/520228793 |
b53b276 to0673116Comparebedevere-bot commentedJun 30, 2021
🤖 New build scheduled with the buildbot fleet by@pablogsal for commit 0673116a6fa30547c1138fd3f96059412d99a979 🤖 If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again. |
Uh oh!
There was an error while loading.Please reload this page.
markshannon commentedJun 30, 2021
Instead of adding three additional location fields to each instruction, and continually update the equivalent fields in the scope struct, why not change Doesn't PEP 657 require that the locations match those of the AST exactly? |
pablogsal commentedJun 30, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
That would be basically the same because you are modifiying these fields in the compilation and assembler steps. For example: that means that if we use a
No, the PEP just states what is the source of the information, but it doesn't explain how is propagated. So we have some freedom to select from which nodes do we pick the information or to "fix it" if we ever need to. |
pablogsal commentedJun 30, 2021
@isidentical By the way, it seems that there are more places where we are setting |
9ca2578 to2498168Compareammaraskar commentedJul 1, 2021
@terryjreedy Adding you as a reviewer as requested. You might not be too interested in this part, it's just exposing the lower level offset data but the tests might give you sort of an idea on how they can be used from Python. The follow up PRs will add the |
bedevere-bot commentedJul 1, 2021
🤖 New build scheduled with the buildbot fleet by@isidentical for commit ba64180d3f61ee75b2b624ae2055ec22a2fb27fc 🤖 If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again. |
terryjreedy left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
If I understand, the only officially visible change is the addition of code.co_positions, which returns an iterable of (start_line, end_line, start_column, end_column) tuples, where column numbers are 1-based. Suggested title after bpo tag: "Add code.co_positions (PEP 657)" or "AddPEP-657 code.co_positions".
I presume a doc change will come. I will review it for readability if pinged.
I read the tests and have two suggestions.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
This PR is part of PEP 657 and augments the compiler to emit endingline numbers as well as starting and ending columns from the ASTinto compiled code objects. This allows bytecodes to be correlatedto the exact source code ranges that generated them.This information is made available through the following public APIs:* The `co_positions` method on code objects.* The C API function `PyCode_Addr2Location`.Co-authored-by: Batuhan Taskaya <isidentical@gmail.com>Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>
ammaraskar commentedJul 2, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Amended the commit message as per Terry's recommendation and rebased#26958 on top of the latest changes, tests for the traceback changes are still passing. |
Uh oh!
There was an error while loading.Please reload this page.
All the internal structures, and data representation are subjected to change in future PRs as we add optimizations
This PR is part of PEP 657 and augments the compiler to emit ending
line numbers as well as starting and ending columns from the AST
into compiled code objects. This allows bytecodes to be correlated
to the exact source code ranges that generated them.
This information is made available through the following public APIs:
co_positionsmethod on code objects.PyCode_Addr2Location.Co-authored-by: Pablo GalindoPablogsal@gmail.com
Co-authored-by: Batuhan Taskayaisidentical@gmail.com
Co-authored-by: Ammar Askerammar@ammaraskar.com
https://bugs.python.org/issue43950