-
Notifications
You must be signed in to change notification settings - Fork 88
Add support for goto into blocks. #23
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
Open
insignification
wants to merge
53
commits into
snoack:master
Choose a base branch
from
insignification:into
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains hidden or 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
(overrides previous fix, as merging them would be too complex and unneeded)
I was not able to get it to happen for pretty huge functions (much much larger than the ones in the test), though, so there's a good chance it's unreachable.
(Candidate for moving to 'fix' branch?)
Added 4 tests, 3 currently disabled (pre-existing issues, especially on pypy).
Add new test_jump_out_of_try_block_and_live
Uses a weak dictionary just in case code objects can be gc'd (can they? maybe on some implementations?)
add warnings for easier debugging (for peace of heart - they weren't currently triggered by any tests)
(later fix could go into fix branch as well)
Renamed goto.into to goto.params (more appropriate, maybe) Fixed two dumb dumb bugs that cancelled each other out (in most cases...)
Let me know what you want me to do with the long ifs and the like - the format I've chosen here may not be the one you'll want. (I haven't split things up into more functions than I already have since I'm not sure it'll ease readability. Let me know your preferences here as well.) I've also changed empty lines between if/elif to be consistent (either always 1, or always 0, depending on what's more readable.) Also remove unused variables I forgot about.
(This was originally in the second PR, but since you want to take this one first, I think it's a good commit to add here, since it's relevant to this change and would make diffing against future PRs easier)
Added 4 tests, 3 currently disabled (pre-existing issues, especially on pypy).
(The new test helps test functionality that needs special attention at least on py38)
(Candidate for moving to 'fix' branch?)
Add new test_jump_out_of_try_block_and_live
Uses a weak dictionary just in case code objects can be gc'd (can they? maybe on some implementations?)
add warnings for easier debugging (for peace of heart - they weren't currently triggered by any tests)
(later fix could go into fix branch as well)
(No testcase in this branch, as it'd be tricky to think of one. master branch has testcases becaues it does dis.findlabels)
Plus added extra test from master
# Conflicts: # goto.py # test_goto.py
Note that with this commit, we no longer look at POP_BLOCK at all, using the 'target offset' of the blocks instead (that always seemed a bit more reliable, and with the block_exits/block_stack simplification, there's no more reason to look at POP_BLOCK) (Turns out jumping into a 'with' loop previously travelled out of the bytecode and into a nearby function, and this wasn't caught by any of the tests in any of the pythons...)
(Change makes it so POP_BLOCK is no longer looked at (as it's a poor indicator in some cases), instead - we look at the target of each block. Also includes a simplification removing block_exits)
(Not possible in 3.8 and as comment says - it's not clear how pypy and py38 will reconcile their differences there)
While this isn't strictly required, making it happen requires fully handling the has_setup_except variation (replacing SETUP_FINALLY with SETUP_EXCEPT where appropriate) so it's a good thing to have in the repo for the future
# Conflicts: # goto.py # test_goto.py
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds support for goto-ing into blocks!
Based on pull requests #21 and #22
While/Try/Except/Finally blocks can be goto-ed into normally.
For/With blocks cannot be goto-ed into normally but require a new syntax:
goto.param .target = obj
Or:
goto.params .target = obj1, obj2, obj3[, ...]
Here, the objects passed to the right of the equals sign are:
This syntax was not chosen arbitrarily:
The "equals sign" syntax was chosen for implementation simplicity, since the stuff to the right of equals is evaluated before "goto.param(s) .target". (Function calls and the like would've been evaluated afterwards and would've made implementation considerably more complicated)
The ".param"/".params" suffix fulfills two purposes:
A) It allows distinguishing between the "enter one block" and the "enter several blocks" case (since the object "passed" to that one block may be a tuple), and the alternative of only supporting the "params" syntax is ugly and error-prone in python.
B) It pads out the size of the line in the bytecode to 6 bytes in python 3.x, which is also the size used for "goto .target". Doing "goto .target = whatever" would've taken 4 bytes (not including the evaluation of 'whatever', of course) and would not be enough to replace with an absolute jump in huge functions.
However, I'm still not too happy with this syntax. Do you have any ideas?
Another possibility (do you like it better?):
goto .target .param=obj
All locally tested on Python 2.6/2.7/3.4/3.8 & pypy2.7&3.6
(As usual, the pull request checks fail due to existing tooling issues unrelated to my changes)
By the way, async anything (and especially async with) has not been tested, and I imagine hasn't been in the past either.