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

bpo-17611. Move unwinding of stack for "pseudo exceptions" from interpreter to compiler. #5006

Merged
merged 85 commits into from
Feb 22, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
85 commits
Select commit Hold shift + click to select a range
2315076
bpo-17611: Move unwinding of stack from interpreter to compiler
pitrou Jul 23, 2017
b160900
Cleanup
pitrou Jul 23, 2017
9833631
Improve stack size calculation for SETUP_{EXCEPT,FINALLY}
pitrou Jul 23, 2017
9116fb8
Update frozen importlib
pitrou Jul 23, 2017
5b3796e
Remove useless code
pitrou Jul 23, 2017
77caab0
Don't predict FOR_ITER as it breaks tracing without computed gotos
pitrou Jul 23, 2017
9fffb61
Remove dead code and fix tracing tests
pitrou Jul 23, 2017
5fe5e1b
Fix test_dis
pitrou Jul 23, 2017
a93f2e2
Add JUMP_FINALLY to avoid finally block duplication
pitrou Jul 24, 2017
b007b1c
Remove finally block duplication
pitrou Jul 24, 2017
0bcf8a7
More precise computation of code object stack size
pitrou Jul 24, 2017
eecf02f
JUMP_FINALLY pushes 6 values to be consistent with the effect of an e…
pitrou Jul 24, 2017
64225ce
Exact stack size computation by popping stale exception state in RERAISE
pitrou Jul 24, 2017
04e055e
Remove the now pointless POP_MANY
pitrou Jul 24, 2017
a3e1b80
Add comment for RERAISE
pitrou Jul 24, 2017
a049635
Remove JUMP_FINALLY
pitrou Jul 24, 2017
0741d02
Remove last block duplication
pitrou Jul 24, 2017
5493616
Get rid of END_ITER
pitrou Jul 24, 2017
5c68acf
Fix comments in frameobject.c
pitrou Jul 24, 2017
2d41c17
Add stack size tests
pitrou Jul 29, 2017
afc2235
Fix test_dis.
serhiy-storchaka Aug 24, 2017
00a6681
Simplify handling of 'for' loops in frame_setlineno() and fix it for …
serhiy-storchaka Aug 24, 2017
f8cc274
Do not duplicate the finally block.
serhiy-storchaka Dec 3, 2017
160ce37
Refactor the frame unwinding code.
serhiy-storchaka Dec 4, 2017
b3b3250
Refactoring and optimizations in the compiler.
serhiy-storchaka Dec 5, 2017
e3f5099
Fix frame_setlineno().
serhiy-storchaka Dec 23, 2017
d8dd38d
Rename PUSH_NO_EXCEPT to BEGIN_FINALLY.
serhiy-storchaka Dec 23, 2017
f1d0bc9
Restore SETUP_WITH and SETUP_ASYNC_WITH.
serhiy-storchaka Dec 24, 2017
84c6400
Fix stack depth calculation.
serhiy-storchaka Dec 24, 2017
35bd61b
Remove SETUP_EXCEPT.
serhiy-storchaka Dec 24, 2017
191f2da
Fix rebase error.
serhiy-storchaka Dec 24, 2017
f1a478d
Change codes of new instructions.
serhiy-storchaka Dec 24, 2017
f44d8cd
Update the magic number.
serhiy-storchaka Dec 24, 2017
93ca80e
Remove debug print.
serhiy-storchaka Dec 24, 2017
c8b9129
Polishing.
serhiy-storchaka Dec 24, 2017
e7d03f3
Document changes.
serhiy-storchaka Dec 24, 2017
9273f10
Make CALL_FINALLY a jump instruction, but exclude it from stack depth…
serhiy-storchaka Dec 25, 2017
4ffb02f
Merge branch 'master' into unwind_stack
serhiy-storchaka Dec 26, 2017
b0ea014
Fix jump tests: jumping into a while block is possible now.
serhiy-storchaka Dec 26, 2017
bef9131
Polishing.
serhiy-storchaka Dec 28, 2017
7fc9111
Fix unrolling the finally block.
serhiy-storchaka Dec 28, 2017
800df5c
Optimize 'return' with constant value.
serhiy-storchaka Dec 28, 2017
cb806c5
Do not eliminate END_FINALLY in peepholer.
serhiy-storchaka Dec 29, 2017
c0623d6
Minimize changes in PyCompile_OpcodeStackEffect().
serhiy-storchaka Dec 29, 2017
69d6900
Document POP_FINALLY and add tests for 'break' and 'return' in 'final…
serhiy-storchaka Dec 29, 2017
c879d2e
Polishing.
serhiy-storchaka Dec 29, 2017
894a1f7
Minimize changes in WITH_CLEANUP_START and WITH_CLEANUP_START.
serhiy-storchaka Dec 29, 2017
dc6eae4
Address review comments.
serhiy-storchaka Dec 30, 2017
88a9050
Disable FOR_ITER jump optimization.
serhiy-storchaka Jan 1, 2018
ed175f3
Reorder tests.
serhiy-storchaka Jan 1, 2018
4485924
Merge branch 'master' into unwind_stack
serhiy-storchaka Jan 1, 2018
b49d46e
Merge branch 'master' into unwind_stack
serhiy-storchaka Jan 1, 2018
4ca9831
Clarify the use case of POP_FINALLY.
serhiy-storchaka Jan 1, 2018
e34670a
bpo-24340: Fix estimation of the code stack size.
serhiy-storchaka Jan 1, 2018
42b50d1
Add new tests for stack effect (ported from #5076).
serhiy-storchaka Jan 1, 2018
3d04b53
Add comments.
serhiy-storchaka Jan 1, 2018
4b1697d
Simplify the algorithm, add comments and asserts.
serhiy-storchaka Jan 3, 2018
dc25d14
Merge branch 'master' into stack-size
serhiy-storchaka Jan 3, 2018
8a327cb
Merge branch 'master' into unwind_stack
serhiy-storchaka Jan 3, 2018
4afb870
Merge branch 'stack-size' into unwind_stack
serhiy-storchaka Jan 3, 2018
4d81d0d
Fix stack effect computation.
serhiy-storchaka Jan 3, 2018
edde33a
Update comments and docs in responce to the review.
serhiy-storchaka Jan 3, 2018
44c7d39
Add more tests.
serhiy-storchaka Jan 3, 2018
8dcb17f
Merge branch 'stack-size' into unwind_stack_new
serhiy-storchaka Jan 3, 2018
0317c9d
Add more comments.
serhiy-storchaka Jan 3, 2018
7a33146
Define all stack effects in a single place.
serhiy-storchaka Jan 4, 2018
68e35bb
Make stack_effect() static.
serhiy-storchaka Jan 5, 2018
6056bc7
Add braces for PEP 7.
serhiy-storchaka Jan 7, 2018
44e1e1e
Merge branch 'master' into stack-size
serhiy-storchaka Jan 7, 2018
04647d6
Merge branch 'master' into unwind_stack
serhiy-storchaka Jan 7, 2018
6d5c333
Merge branch 'stack-size' into unwind_stack
serhiy-storchaka Jan 7, 2018
660a117
Add braces for PEP 7.
serhiy-storchaka Jan 7, 2018
a03d212
Merge branch 'master' into unwind_stack
serhiy-storchaka Jan 9, 2018
ff0448f
Merge branch 'master' into unwind_stack
serhiy-storchaka Jan 9, 2018
bd39e0a
Simplify WITH_CLEANUP_START. It is never called after CALL_FINALLY.
serhiy-storchaka Jan 9, 2018
156b455
Merge branch 'master' into unwind_stack
serhiy-storchaka Jan 11, 2018
cd56a99
Merge branch 'master' into unwind_stack
serhiy-storchaka Jan 16, 2018
0816c92
Merge branch 'master' into unwind_stack
serhiy-storchaka Feb 1, 2018
c161251
Merge branch 'master' into unwind_stack
serhiy-storchaka Feb 1, 2018
af9ddf0
Update the magic number.
serhiy-storchaka Feb 4, 2018
c566673
Merge branch 'master' into unwind_stack
serhiy-storchaka Feb 4, 2018
9aee1f0
Merge branch 'master' into unwind_stack
serhiy-storchaka Feb 4, 2018
24667a4
Merge branch 'master' into unwind_stack
serhiy-storchaka Feb 9, 2018
47c5067
Merge branch 'master' into unwind_stack
serhiy-storchaka Feb 22, 2018
4a63042
Merge branch 'master' into unwind_stack
serhiy-storchaka Feb 22, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Document POP_FINALLY and add tests for 'break' and 'return' in 'final…
…ly' clause.
  • Loading branch information
serhiy-storchaka committed Dec 29, 2017
commit 69d690032f5777ee9ae59b3f695d0c290e68e9c7
30 changes: 26 additions & 4 deletions Doc/library/dis.rst
Original file line number Diff line number Diff line change
Expand Up @@ -668,6 +668,22 @@ iterations of the loop.
popped values are used to restore the exception state.


.. opcode:: POP_FINALLY (preserve_tos)

Cleans up the value stack and the block stack. If *preserve_tos* is not
Copy link
Member

Choose a reason for hiding this comment

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

For the sake of static bytecode analyzers, perhaps it would be better to have two separate opcodes: one preserving TOS, the other not.

Copy link
Member Author

Choose a reason for hiding this comment

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

Wouldn't this just complicate the code and the documentation?

Copy link
Member

Choose a reason for hiding this comment

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

The code wouldn't be much more complicated (you can share the same implementation in the eval loop). OTOH I'm not sure it's necessary.

Other question: why are both POP_FINALLY and END_FINALLY needed? The documentation should be a bit clearer.

Copy link
Member Author

Choose a reason for hiding this comment

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

END_FINALLY re-raises an exception, POP_FINALLY 0 just drops it. END_FINALLY jumps to the return address, POP_FINALLY 0 ignores it. And POP_FINALLY 1 preserves TOS.

I don't know what to add to the documentation. Can you provide your changes?

If push 6 None instead of 1 NULL in BEGIN_FINALLY, and push an exception block in BEGIN_FINALLY, and update other instructions accordingly, and add additional hacks to stack effect calculation, POP_FINALLY 0 could be replaced with:

POP_TOP
POP_TOP
POP_TOP
POP_EXCEPT

and POP_FINALLY 1 could be replaced with:

ROT_FOUR
POP_TOP
POP_TOP
POP_TOP
ROT_FOUR
POP_EXCEPT

This was my initial solution for @nascheme's examples. But this would add too much unneeded overhead and complication. The single new opcode has lesser cost.

Copy link
Member

Choose a reason for hiding this comment

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

END_FINALLY re-raises an exception, POP_FINALLY 0 just drops it. END_FINALLY jumps to the return address, POP_FINALLY 0 ignores it.

Yes... So what I don't understand is why both are needed. In which case do you need to drop an exception?

Copy link
Member Author

Choose a reason for hiding this comment

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

'break' and 'return' in 'finally' clause.

'continue' is specially prohibited, but it can be allowed too after merging this PR.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you. Can you add some explanation of this in the docs?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

``0`` TOS first is popped from the stack and pushed on the stack after
perfoming other stack operations:

* If TOS is ``NULL`` or an integer (pushed by :opcode:`BEGIN_FINALLY`
or :opcode:`CALL_FINALLY`) it is popped from the stack.
* If TOS is an exception type (pushed when an exception has been raised)
6 values are popped from the stack, the last three popped values are
used to restore the exception state. An exception handler block is
removed from the block stack.

.. versionadded:: 3.7


.. opcode:: BEGIN_FINALLY

Pushes ``NULL`` onto the stack for using it in :opcode:`END_FINALLY`.
Expand All @@ -687,8 +703,10 @@ iterations of the loop.
* If TOS is an integer (pushed by :opcode:`CALL_FINALLY`), sets the
bytecode counter to TOS. TOS is popped.
* If TOS is an exception type (pushed when an exception has been raised)
re-raise the exception and restore ``sys.exc_info()``. 6 values are
popped from the stack.
6 values are popped from the stack, the first three popped values are
used to re-raise the exception and the last three popped values are used
to restore the exception state. An exception handler block is removed
from the block stack.


.. opcode:: LOAD_BUILD_CLASS
Expand Down Expand Up @@ -735,8 +753,12 @@ iterations of the loop.
* If SECOND is an integer (pushed by :opcode:`CALL_FINALLY`), sets the
bytecode counter to TOS. 4 values are popped from the stack.
* If SECOND is an exception type (pushed when an exception has been raised)
re-raise the exception and restore ``sys.exc_info()``. 9 values are
popped from the stack.
and TOS is true SIXTH, SEVENTH and EIGHTH are used for restoring the
exception state. 9 values are popped from the stack. An exception
handler block is removed from the block stack.
* If SECOND is an exception type and TOS is false THIRD, FOURTH and FIFTH
are used for re-raising the exception state and SIXTH, SEVENTH and EIGHTH
are used for restoring the exception state. 9 values are popped from the stack. An exception handler block is removed from the block stack.

.. versionchanged:: 3.7
WITH_CLEANUP_FINISH now includes the functionality of
Expand Down
6 changes: 3 additions & 3 deletions Doc/whatsnew/3.7.rst
Original file line number Diff line number Diff line change
Expand Up @@ -936,9 +936,9 @@ CPython bytecode changes

Removed opcodes :opcode:`BREAK_LOOP`, :opcode:`CONTINUE_LOOP`,
:opcode:`SETUP_LOOP` and :opcode:`SETUP_EXCEPT`. Added new opcodes
:opcode:`ROT_FOUR`, :opcode:`BEGIN_FINALLY` and :opcode:`CALL_FINALLY`.
Changed the behavior of :opcode:`END_FINALLY`. :opcode:`WITH_CLEANUP_FINISH`
now includes :opcode:`END_FINALLY`.
:opcode:`ROT_FOUR`, :opcode:`BEGIN_FINALLY`, :opcode:`CALL_FINALLY` and
:opcode:`CALL_FINALLY`. Changed the behavior of :opcode:`END_FINALLY`.
:opcode:`WITH_CLEANUP_FINISH` now includes :opcode:`END_FINALLY`.

(Contributed by Mark Shannon, Antoine Pitrou and Serhiy Storchaka in
:issue:`17611`.)
Expand Down
74 changes: 74 additions & 0 deletions Lib/test/test_grammar.py
Original file line number Diff line number Diff line change
Expand Up @@ -805,6 +805,80 @@ def g2(): return 1
x = g2()
check_syntax_error(self, "class foo:return 1")

def test_break_in_finally(self):
count = 0
while count < 2:
count += 1
try:
pass
finally:
break
self.assertEqual(count, 1)

count = 0
while count < 2:
count += 1
try:
continue
finally:
break
self.assertEqual(count, 1)

count = 0
while count < 2:
count += 1
try:
1/0
finally:
break
self.assertEqual(count, 1)

for count in [0, 1]:
self.assertEqual(count, 0)
try:
pass
finally:
break
self.assertEqual(count, 0)

for count in [0, 1]:
self.assertEqual(count, 0)
try:
continue
finally:
break
self.assertEqual(count, 0)

for count in [0, 1]:
self.assertEqual(count, 0)
try:
1/0
finally:
break
self.assertEqual(count, 0)

def test_return_in_finally(self):
def g1():
try:
pass
finally:
return 1
self.assertEqual(g1(), 1)

def g2():
try:
return 2
finally:
return 3
self.assertEqual(g2(), 3)

def g3():
try:
1/0
finally:
return 4
self.assertEqual(g3(), 4)

def test_yield(self):
# Allowed as standalone statement
def g(): yield 1
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
Simplified the interpreter loop by moving the logic of unrolling the stack
of blocks into the compiler. The compiler emits now explicit instructions
for adjusting the stack of values and calling the cleaning up code for
"break", "continue" and "return".
:keyword:`break`, :keyword:`continue` and :keyword:`return`.

Removed opcodes BREAK_LOOP, CONTINUE_LOOP, SETUP_LOOP and SETUP_EXCEPT.
Added new opcodes ROT_FOUR, BEGIN_FINALLY and CALL_FINALLY. Changed the
behavior of END_FINALLY. WITH_CLEANUP_FINISH now includes END_FINALLY.
Removed opcodes :opcode:`BREAK_LOOP`, :opcode:`CONTINUE_LOOP`,
:opcode:`SETUP_LOOP` and :opcode:`SETUP_EXCEPT`. Added new opcodes
:opcode:`ROT_FOUR`, :opcode:`BEGIN_FINALLY` and :opcode:`CALL_FINALLY` and
:opcode:`POP_FINALLY`. Changed the behavior of :opcode:`END_FINALLY`.
:opcode:`WITH_CLEANUP_FINISH` now includes the functionality of
:opcode:`END_FINALLY`.