diff --git a/py/vm.c b/py/vm.c index 498ecb491d..bc596b0e86 100644 --- a/py/vm.c +++ b/py/vm.c @@ -1063,17 +1063,11 @@ unwind_jump:; ENTRY(MP_BC_RETURN_VALUE): MARK_EXC_IP_SELECTIVE(); - // These next 3 lines pop a try-finally exception handler, if one - // is there on the exception stack. Without this the finally block - // is executed a second time when the return is executed, because - // the try-finally exception handler is still on the stack. - // TODO Possibly find a better way to handle this case. - if (currently_in_except_block) { - POP_EXC_BLOCK(); - } unwind_return: + // Search for and execute finally handlers that aren't already active while (exc_sp >= exc_stack) { - if (MP_TAGPTR_TAG1(exc_sp->val_sp)) { + if (!currently_in_except_block && MP_TAGPTR_TAG1(exc_sp->val_sp)) { + // Found a finally handler that isn't active. // Getting here the stack looks like: // (..., X, [iter0, iter1, ...,] ret_val) // where X is pointed to by exc_sp->val_sp and in the case @@ -1092,10 +1086,10 @@ unwind_return: // done (when WITH_CLEANUP or END_FINALLY reached). PUSH(MP_OBJ_NEW_SMALL_INT(-1)); ip = exc_sp->handler; - exc_sp--; + POP_EXC_BLOCK(); goto dispatch_loop; } - exc_sp--; + POP_EXC_BLOCK(); } nlr_pop(); code_state->sp = sp; diff --git a/tests/basics/try_finally_return3.py b/tests/basics/try_finally_return3.py new file mode 100644 index 0000000000..a2a06ee975 --- /dev/null +++ b/tests/basics/try_finally_return3.py @@ -0,0 +1,103 @@ +# test 'return' within the finally block, with nested finally's +# only inactive finally's should be executed, and only once + +# basic nested finally's, the print should only be executed once +def f(): + try: + raise TypeError + finally: + print(1) + try: + raise ValueError + finally: + return 42 +print(f()) + +# similar to above but more nesting +def f(): + try: + raise ValueError + finally: + print(1) + try: + raise TypeError + finally: + print(2) + try: + pass + finally: + return 42 +print(f()) + +# similar to above but even more nesting +def f(): + try: + raise ValueError + finally: + print(1) + try: + raise TypeError + finally: + print(2) + try: + raise Exception + finally: + print(3) + return 42 +print(f()) + +# upon return some try's are active, some finally's are active, some inactive +def f(): + try: + try: + pass + finally: + print(2) + return 42 + finally: + print(1) +print(f()) + +# same as above but raise instead of pass +def f(): + try: + try: + raise ValueError + finally: + print(2) + return 42 + finally: + print(1) +print(f()) + +# upon return exception stack holds: active finally, inactive finally, active finally +def f(): + try: + raise Exception + finally: + print(1) + try: + try: + pass + finally: + print(3) + return 42 + finally: + print(2) +print(f()) + +# same as above but raise instead of pass in innermost try block +def f(): + try: + raise Exception + finally: + print(1) + try: + try: + raise Exception + finally: + print(3) + return 42 + finally: + print(2) +print(f()) diff --git a/tests/run-tests b/tests/run-tests index a3263fff8a..cfbd017774 100755 --- a/tests/run-tests +++ b/tests/run-tests @@ -368,6 +368,7 @@ def run_tests(pyb, tests, args, base_path="."): skip_tests.add('basics/try_finally_loops.py') # requires proper try finally code skip_tests.add('basics/try_finally_return.py') # requires proper try finally code skip_tests.add('basics/try_finally_return2.py') # requires proper try finally code + skip_tests.add('basics/try_finally_return3.py') # requires proper try finally code skip_tests.add('basics/unboundlocal.py') # requires checking for unbound local skip_tests.add('import/gen_context.py') # requires yield_value skip_tests.add('misc/features.py') # requires raise_varargs