To: vim_dev@googlegroups.com Subject: Patch 8.2.2539 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ------------ Patch 8.2.2539 Problem: Vim9: return from finally block causes a hang. Solution: Store both the finally and endtry indexes. (closes #7885) Files: src/vim9execute.c, src/vim9compile.c, src/vim9.h, src/testdir/test_vim9_script.vim, src/testdir/test_vim9_disassemble.vim *** ../vim-8.2.2538/src/vim9execute.c 2021-02-20 17:03:57.984112613 +0100 --- src/vim9execute.c 2021-02-21 21:15:16.101960728 +0100 *************** *** 26,33 **** typedef struct { int tcd_frame_idx; // ec_frame_idx at ISN_TRY int tcd_stack_len; // size of ectx.ec_stack at ISN_TRY ! int tcd_catch_idx; // instruction of the first catch ! int tcd_finally_idx; // instruction of the finally block or :endtry int tcd_caught; // catch block entered int tcd_cont; // :continue encountered, jump here int tcd_return; // when TRUE return from end of :finally --- 26,34 ---- typedef struct { int tcd_frame_idx; // ec_frame_idx at ISN_TRY int tcd_stack_len; // size of ectx.ec_stack at ISN_TRY ! int tcd_catch_idx; // instruction of the first :catch or :finally ! int tcd_finally_idx; // instruction of the :finally block or zero ! int tcd_endtry_idx; // instruction of the :endtry int tcd_caught; // catch block entered int tcd_cont; // :continue encountered, jump here int tcd_return; // when TRUE return from end of :finally *************** *** 2517,2526 **** + trystack->ga_len - 1; if (trycmd != NULL && trycmd->tcd_frame_idx == ectx.ec_frame_idx ! && ectx.ec_instr[trycmd->tcd_finally_idx] ! .isn_type != ISN_ENDTRY) { ! // jump to ":finally" ectx.ec_iidx = trycmd->tcd_finally_idx; trycmd->tcd_return = TRUE; } --- 2518,2526 ---- + trystack->ga_len - 1; if (trycmd != NULL && trycmd->tcd_frame_idx == ectx.ec_frame_idx ! && trycmd->tcd_finally_idx != 0) { ! // jump to ":finally" once ectx.ec_iidx = trycmd->tcd_finally_idx; trycmd->tcd_return = TRUE; } *************** *** 2665,2672 **** CLEAR_POINTER(trycmd); trycmd->tcd_frame_idx = ectx.ec_frame_idx; trycmd->tcd_stack_len = ectx.ec_stack.ga_len; ! trycmd->tcd_catch_idx = iptr->isn_arg.try.try_catch; ! trycmd->tcd_finally_idx = iptr->isn_arg.try.try_finally; } break; --- 2665,2673 ---- CLEAR_POINTER(trycmd); trycmd->tcd_frame_idx = ectx.ec_frame_idx; trycmd->tcd_stack_len = ectx.ec_stack.ga_len; ! trycmd->tcd_catch_idx = iptr->isn_arg.try.try_ref->try_catch; ! trycmd->tcd_finally_idx = iptr->isn_arg.try.try_ref->try_finally; ! trycmd->tcd_endtry_idx = iptr->isn_arg.try.try_ref->try_endtry; } break; *************** *** 2731,2743 **** trycmd = ((trycmd_T *)trystack->ga_data) + trystack->ga_len - i; trycmd->tcd_cont = iidx; ! iidx = trycmd->tcd_finally_idx; } // jump to :finally or :endtry of current try statement ectx.ec_iidx = iidx; } break; // end of ":try" block case ISN_ENDTRY: { --- 2732,2757 ---- trycmd = ((trycmd_T *)trystack->ga_data) + trystack->ga_len - i; trycmd->tcd_cont = iidx; ! iidx = trycmd->tcd_finally_idx == 0 ! ? trycmd->tcd_endtry_idx : trycmd->tcd_finally_idx; } // jump to :finally or :endtry of current try statement ectx.ec_iidx = iidx; } break; + case ISN_FINALLY: + { + garray_T *trystack = &ectx.ec_trystack; + trycmd_T *trycmd = ((trycmd_T *)trystack->ga_data) + + trystack->ga_len - 1; + + // Reset the index to avoid a return statement jumps here + // again. + trycmd->tcd_finally_idx = 0; + break; + } + // end of ":try" block case ISN_ENDTRY: { *************** *** 4348,4358 **** { try_T *try = &iptr->isn_arg.try; ! smsg("%4d TRY catch -> %d, %s -> %d", current, ! try->try_catch, ! instr[try->try_finally].isn_type == ISN_ENDTRY ! ? "end" : "finally", ! try->try_finally); } break; case ISN_CATCH: --- 4362,4378 ---- { try_T *try = &iptr->isn_arg.try; ! if (try->try_ref->try_finally == 0) ! smsg("%4d TRY catch -> %d, endtry -> %d", ! current, ! try->try_ref->try_catch, ! try->try_ref->try_endtry); ! else ! smsg("%4d TRY catch -> %d, finally -> %d, endtry -> %d", ! current, ! try->try_ref->try_catch, ! try->try_ref->try_finally, ! try->try_ref->try_endtry); } break; case ISN_CATCH: *************** *** 4369,4374 **** --- 4389,4397 ---- trycont->tct_where); } break; + case ISN_FINALLY: + smsg("%4d FINALLY", current); + break; case ISN_ENDTRY: smsg("%4d ENDTRY", current); break; *** ../vim-8.2.2538/src/vim9compile.c 2021-02-20 17:03:57.984112613 +0100 --- src/vim9compile.c 2021-02-21 21:11:13.815292347 +0100 *************** *** 7518,7527 **** if (cctx->ctx_skip != SKIP_YES) { ! // "catch" is set when the first ":catch" is found. ! // "finally" is set when ":finally" or ":endtry" is found try_scope->se_u.se_try.ts_try_label = instr->ga_len; ! if (generate_instr(cctx, ISN_TRY) == NULL) return NULL; } --- 7518,7534 ---- if (cctx->ctx_skip != SKIP_YES) { ! isn_T *isn; ! ! // "try_catch" is set when the first ":catch" is found or when no catch ! // is found and ":finally" is found. ! // "try_finally" is set when ":finally" is found ! // "try_endtry" is set when ":endtry" is found try_scope->se_u.se_try.ts_try_label = instr->ga_len; ! if ((isn = generate_instr(cctx, ISN_TRY)) == NULL) ! return NULL; ! isn->isn_arg.try.try_ref = ALLOC_CLEAR_ONE(tryref_T); ! if (isn->isn_arg.try.try_ref == NULL) return NULL; } *************** *** 7577,7584 **** // End :try or :catch scope: set value in ISN_TRY instruction isn = ((isn_T *)instr->ga_data) + scope->se_u.se_try.ts_try_label; ! if (isn->isn_arg.try.try_catch == 0) ! isn->isn_arg.try.try_catch = instr->ga_len; if (scope->se_u.se_try.ts_catch_label != 0) { // Previous catch without match jumps here --- 7584,7591 ---- // End :try or :catch scope: set value in ISN_TRY instruction isn = ((isn_T *)instr->ga_data) + scope->se_u.se_try.ts_try_label; ! if (isn->isn_arg.try.try_ref->try_catch == 0) ! isn->isn_arg.try.try_ref->try_catch = instr->ga_len; if (scope->se_u.se_try.ts_catch_label != 0) { // Previous catch without match jumps here *************** *** 7670,7676 **** // End :catch or :finally scope: set value in ISN_TRY instruction isn = ((isn_T *)instr->ga_data) + scope->se_u.se_try.ts_try_label; ! if (isn->isn_arg.try.try_finally != 0) { emsg(_(e_finally_dup)); return NULL; --- 7677,7683 ---- // End :catch or :finally scope: set value in ISN_TRY instruction isn = ((isn_T *)instr->ga_data) + scope->se_u.se_try.ts_try_label; ! if (isn->isn_arg.try.try_ref->try_finally != 0) { emsg(_(e_finally_dup)); return NULL; *************** *** 7688,7694 **** compile_fill_jump_to_end(&scope->se_u.se_try.ts_end_label, this_instr, cctx); ! isn->isn_arg.try.try_finally = this_instr; if (scope->se_u.se_try.ts_catch_label != 0) { // Previous catch without match jumps here --- 7695,7704 ---- compile_fill_jump_to_end(&scope->se_u.se_try.ts_end_label, this_instr, cctx); ! // If there is no :catch then an exception jumps to :finally. ! if (isn->isn_arg.try.try_ref->try_catch == 0) ! isn->isn_arg.try.try_ref->try_catch = this_instr; ! isn->isn_arg.try.try_ref->try_finally = this_instr; if (scope->se_u.se_try.ts_catch_label != 0) { // Previous catch without match jumps here *************** *** 7696,7701 **** --- 7706,7713 ---- isn->isn_arg.jump.jump_where = this_instr; scope->se_u.se_try.ts_catch_label = 0; } + if (generate_instr(cctx, ISN_FINALLY) == NULL) + return NULL; // TODO: set index in ts_finally_label jumps *************** *** 7731,7738 **** try_isn = ((isn_T *)instr->ga_data) + scope->se_u.se_try.ts_try_label; if (cctx->ctx_skip != SKIP_YES) { ! if (try_isn->isn_arg.try.try_catch == 0 ! && try_isn->isn_arg.try.try_finally == 0) { emsg(_(e_missing_catch_or_finally)); return NULL; --- 7743,7750 ---- try_isn = ((isn_T *)instr->ga_data) + scope->se_u.se_try.ts_try_label; if (cctx->ctx_skip != SKIP_YES) { ! if (try_isn->isn_arg.try.try_ref->try_catch == 0 ! && try_isn->isn_arg.try.try_ref->try_finally == 0) { emsg(_(e_missing_catch_or_finally)); return NULL; *************** *** 7751,7762 **** compile_fill_jump_to_end(&scope->se_u.se_try.ts_end_label, instr->ga_len, cctx); - // End :catch or :finally scope: set value in ISN_TRY instruction - if (try_isn->isn_arg.try.try_catch == 0) - try_isn->isn_arg.try.try_catch = instr->ga_len; - if (try_isn->isn_arg.try.try_finally == 0) - try_isn->isn_arg.try.try_finally = instr->ga_len; - if (scope->se_u.se_try.ts_catch_label != 0) { // Last catch without match jumps here --- 7763,7768 ---- *************** *** 7770,7780 **** if (cctx->ctx_skip != SKIP_YES) { ! if (try_isn->isn_arg.try.try_finally == 0) ! // No :finally encountered, use the try_finaly field to point to ! // ENDTRY, so that TRYCONT can jump there. ! try_isn->isn_arg.try.try_finally = instr->ga_len; ! if (cctx->ctx_skip != SKIP_YES && generate_instr(cctx, ISN_ENDTRY) == NULL) return NULL; --- 7776,7784 ---- if (cctx->ctx_skip != SKIP_YES) { ! // End :catch or :finally scope: set instruction index in ISN_TRY ! // instruction ! try_isn->isn_arg.try.try_ref->try_endtry = instr->ga_len; if (cctx->ctx_skip != SKIP_YES && generate_instr(cctx, ISN_ENDTRY) == NULL) return NULL; *************** *** 8867,8872 **** --- 8871,8880 ---- vim_free(isn->isn_arg.script.scriptref); break; + case ISN_TRY: + vim_free(isn->isn_arg.try.try_ref); + break; + case ISN_2BOOL: case ISN_2STRING: case ISN_2STRING_ANY: *************** *** 8899,8904 **** --- 8907,8913 ---- case ISN_ENDTRY: case ISN_EXECCONCAT: case ISN_EXECUTE: + case ISN_FINALLY: case ISN_FOR: case ISN_GETITEM: case ISN_JUMP: *************** *** 8943,8949 **** case ISN_STRINDEX: case ISN_STRSLICE: case ISN_THROW: - case ISN_TRY: case ISN_TRYCONT: case ISN_UNLETINDEX: case ISN_UNLETRANGE: --- 8952,8957 ---- *** ../vim-8.2.2538/src/vim9.h 2021-02-20 17:03:57.984112613 +0100 --- src/vim9.h 2021-02-21 21:08:58.843799050 +0100 *************** *** 100,105 **** --- 100,106 ---- ISN_THROW, // pop value of stack, store in v:exception ISN_PUSHEXC, // push v:exception ISN_CATCH, // drop v:exception + ISN_FINALLY, // start of :finally block ISN_ENDTRY, // take entry off from ec_trystack ISN_TRYCONT, // handle :continue inside a :try statement *************** *** 208,217 **** int for_end; // position to jump to after done } forloop_T; ! // arguments to ISN_TRY typedef struct { int try_catch; // position to jump to on throw int try_finally; // :finally or :endtry position to jump to } try_T; // arguments to ISN_TRYCONT --- 209,224 ---- int for_end; // position to jump to after done } forloop_T; ! // indirect arguments to ISN_TRY typedef struct { int try_catch; // position to jump to on throw int try_finally; // :finally or :endtry position to jump to + int try_endtry; // :endtry position to jump to + } tryref_T; + + // arguments to ISN_TRY + typedef struct { + tryref_T *try_ref; } try_T; // arguments to ISN_TRYCONT *** ../vim-8.2.2538/src/testdir/test_vim9_script.vim 2021-02-20 08:16:33.823363221 +0100 --- src/testdir/test_vim9_script.vim 2021-02-21 21:20:29.772406658 +0100 *************** *** 577,582 **** --- 577,592 ---- counter += 1 endfor assert_equal(4, counter) + + # return in finally after empty catch + def ReturnInFinally(): number + try + finally + return 4 + endtry + return 2 + enddef + assert_equal(4, ReturnInFinally()) enddef def Test_cnext_works_in_catch() *** ../vim-8.2.2538/src/testdir/test_vim9_disassemble.vim 2021-02-13 15:02:43.063505534 +0100 --- src/testdir/test_vim9_disassemble.vim 2021-02-21 21:26:40.846730718 +0100 *************** *** 422,428 **** var res = execute('disass s:ScriptFuncTry') assert_match('\d*_ScriptFuncTry\_s*' .. 'try\_s*' .. ! '\d TRY catch -> \d\+, finally -> \d\+\_s*' .. 'echo "yes"\_s*' .. '\d PUSHS "yes"\_s*' .. '\d ECHO 1\_s*' .. --- 422,428 ---- var res = execute('disass s:ScriptFuncTry') assert_match('\d*_ScriptFuncTry\_s*' .. 'try\_s*' .. ! '\d TRY catch -> \d\+, finally -> \d\+, endtry -> \d\+\_s*' .. 'echo "yes"\_s*' .. '\d PUSHS "yes"\_s*' .. '\d ECHO 1\_s*' .. *************** *** 437,442 **** --- 437,443 ---- '\d\+ PUSHS "no"\_s*' .. '\d\+ ECHO 1\_s*' .. 'finally\_s*' .. + '\d\+ FINALLY\_s*' .. 'throw "end"\_s*' .. '\d\+ PUSHS "end"\_s*' .. '\d\+ THROW\_s*' .. *************** *** 1137,1148 **** '4 FOR $0 -> 22\_s*' .. '5 STORE $1\_s*' .. 'try\_s*' .. ! '6 TRY catch -> 17, end -> 20\_s*' .. 'echo "ok"\_s*' .. '7 PUSHS "ok"\_s*' .. '8 ECHO 1\_s*' .. 'try\_s*' .. ! '9 TRY catch -> 13, end -> 15\_s*' .. 'echo "deeper"\_s*' .. '10 PUSHS "deeper"\_s*' .. '11 ECHO 1\_s*' .. --- 1138,1149 ---- '4 FOR $0 -> 22\_s*' .. '5 STORE $1\_s*' .. 'try\_s*' .. ! '6 TRY catch -> 17, endtry -> 20\_s*' .. 'echo "ok"\_s*' .. '7 PUSHS "ok"\_s*' .. '8 ECHO 1\_s*' .. 'try\_s*' .. ! '9 TRY catch -> 13, endtry -> 15\_s*' .. 'echo "deeper"\_s*' .. '10 PUSHS "deeper"\_s*' .. '11 ECHO 1\_s*' .. *** ../vim-8.2.2538/src/version.c 2021-02-21 19:12:43.018019657 +0100 --- src/version.c 2021-02-21 21:02:30.801255811 +0100 *************** *** 752,753 **** --- 752,755 ---- { /* Add new patch number below this line */ + /**/ + 2539, /**/ -- ARTHUR: Go on, Bors, chop its head off. BORS: Right. Silly little bleeder. One rabbit stew coming up. "Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net \\\ /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\ \\\ an exciting new programming language -- http://www.Zimbu.org /// \\\ help me help AIDS victims -- http://ICCF-Holland.org ///