To: vim_dev@googlegroups.com Subject: Patch 8.2.2404 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ------------ Patch 8.2.2404 Problem: Vim9: profiling try/catch not correct. Solution: Add profile instructions. Fix that "entry" did not rethrow an excpetion. Files: src/vim9compile.c, src/vim9execute.c, src/testdir/test_profile.vim *** ../vim-8.2.2403/src/vim9compile.c 2021-01-24 17:53:43.681840018 +0100 --- src/vim9compile.c 2021-01-24 20:10:03.758497746 +0100 *************** *** 6566,6572 **** } static void ! compile_fill_jump_to_end(endlabel_T **el, cctx_T *cctx) { garray_T *instr = &cctx->ctx_instr; --- 6566,6572 ---- } static void ! compile_fill_jump_to_end(endlabel_T **el, int jump_where, cctx_T *cctx) { garray_T *instr = &cctx->ctx_instr; *************** *** 6576,6582 **** isn_T *isn; isn = ((isn_T *)instr->ga_data) + cur->el_end_label; ! isn->isn_arg.jump.jump_where = instr->ga_len; *el = cur->el_next; vim_free(cur); } --- 6576,6582 ---- isn_T *isn; isn = ((isn_T *)instr->ga_data) + cur->el_end_label; ! isn->isn_arg.jump.jump_where = jump_where; *el = cur->el_next; vim_free(cur); } *************** *** 6939,6945 **** isn->isn_arg.jump.jump_where = instr->ga_len; } // Fill in the "end" label in jumps at the end of the blocks. ! compile_fill_jump_to_end(&ifscope->is_end_label, cctx); #ifdef FEAT_PROFILE // even when skipping we count the endif as executed, unless the block it's --- 6939,6945 ---- isn->isn_arg.jump.jump_where = instr->ga_len; } // Fill in the "end" label in jumps at the end of the blocks. ! compile_fill_jump_to_end(&ifscope->is_end_label, instr->ga_len, cctx); #ifdef FEAT_PROFILE // even when skipping we count the endif as executed, unless the block it's *************** *** 7182,7188 **** isn->isn_arg.forloop.for_end = instr->ga_len; // Fill in the "end" label any BREAK statements ! compile_fill_jump_to_end(&forscope->fs_end_label, cctx); // Below the ":for" scope drop the "expr" list from the stack. if (generate_instr_drop(cctx, ISN_DROP, 1) == NULL) --- 7182,7188 ---- isn->isn_arg.forloop.for_end = instr->ga_len; // Fill in the "end" label any BREAK statements ! compile_fill_jump_to_end(&forscope->fs_end_label, instr->ga_len, cctx); // Below the ":for" scope drop the "expr" list from the stack. if (generate_instr_drop(cctx, ISN_DROP, 1) == NULL) *************** *** 7245,7250 **** --- 7245,7251 ---- compile_endwhile(char_u *arg, cctx_T *cctx) { scope_T *scope = cctx->ctx_scope; + garray_T *instr = &cctx->ctx_instr; if (scope == NULL || scope->se_type != WHILE_SCOPE) { *************** *** 7264,7270 **** // Fill in the "end" label in the WHILE statement so it can jump here. // And in any jumps for ":break" ! compile_fill_jump_to_end(&scope->se_u.se_while.ws_end_label, cctx); vim_free(scope); --- 7265,7272 ---- // Fill in the "end" label in the WHILE statement so it can jump here. // And in any jumps for ":break" ! compile_fill_jump_to_end(&scope->se_u.se_while.ws_end_label, ! instr->ga_len, cctx); vim_free(scope); *************** *** 7446,7451 **** --- 7448,7459 ---- if (cctx->ctx_skip != SKIP_YES) { + #ifdef FEAT_PROFILE + // the profile-start should be after the jump + if (cctx->ctx_profiling && ((isn_T *)instr->ga_data)[instr->ga_len - 1] + .isn_type == ISN_PROF_START) + --instr->ga_len; + #endif // Jump from end of previous block to :finally or :endtry if (compile_jump_to_end(&scope->se_u.se_try.ts_end_label, JUMP_ALWAYS, cctx) == FAIL) *************** *** 7461,7466 **** --- 7469,7483 ---- isn = ((isn_T *)instr->ga_data) + scope->se_u.se_try.ts_catch_label; isn->isn_arg.jump.jump_where = instr->ga_len; } + #ifdef FEAT_PROFILE + if (cctx->ctx_profiling) + { + // a "throw" that jumps here needs to be counted + generate_instr(cctx, ISN_PROF_END); + // the "catch" is also counted + generate_instr(cctx, ISN_PROF_START); + } + #endif } p = skipwhite(arg); *************** *** 7521,7526 **** --- 7538,7544 ---- scope_T *scope = cctx->ctx_scope; garray_T *instr = &cctx->ctx_instr; isn_T *isn; + int this_instr; // end block scope from :try or :catch if (scope != NULL && scope->se_type == BLOCK_SCOPE) *************** *** 7542,7556 **** return NULL; } // Fill in the "end" label in jumps at the end of the blocks. ! compile_fill_jump_to_end(&scope->se_u.se_try.ts_end_label, cctx); ! isn->isn_arg.try.try_finally = instr->ga_len; if (scope->se_u.se_try.ts_catch_label != 0) { // Previous catch without match jumps here isn = ((isn_T *)instr->ga_data) + scope->se_u.se_try.ts_catch_label; ! isn->isn_arg.jump.jump_where = instr->ga_len; scope->se_u.se_try.ts_catch_label = 0; } --- 7560,7583 ---- return NULL; } + this_instr = instr->ga_len; + #ifdef FEAT_PROFILE + if (cctx->ctx_profiling && ((isn_T *)instr->ga_data)[instr->ga_len - 1] + .isn_type == ISN_PROF_START) + // jump to the profile start of the "finally" + --this_instr; + #endif + // Fill in the "end" label in jumps at the end of the blocks. ! 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 isn = ((isn_T *)instr->ga_data) + scope->se_u.se_try.ts_catch_label; ! isn->isn_arg.jump.jump_where = this_instr; scope->se_u.se_try.ts_catch_label = 0; } *************** *** 7595,7603 **** return NULL; } // Fill in the "end" label in jumps at the end of the blocks, if not // done by ":finally". ! compile_fill_jump_to_end(&scope->se_u.se_try.ts_end_label, cctx); // End :catch or :finally scope: set value in ISN_TRY instruction if (isn->isn_arg.try.try_catch == 0) --- 7622,7639 ---- return NULL; } + #ifdef FEAT_PROFILE + if (cctx->ctx_profiling && ((isn_T *)instr->ga_data)[instr->ga_len - 1] + .isn_type == ISN_PROF_START) + // move the profile start after "endtry" so that it's not counted when + // the exception is rethrown. + --instr->ga_len; + #endif + // Fill in the "end" label in jumps at the end of the blocks, if not // done by ":finally". ! 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 (isn->isn_arg.try.try_catch == 0) *************** *** 7617,7622 **** --- 7653,7662 ---- if (cctx->ctx_skip != SKIP_YES && generate_instr(cctx, ISN_ENDTRY) == NULL) return NULL; + #ifdef FEAT_PROFILE + if (cctx->ctx_profiling) + generate_instr(cctx, ISN_PROF_START); + #endif return arg; } *** ../vim-8.2.2403/src/vim9execute.c 2021-01-24 13:34:15.007739955 +0100 --- src/vim9execute.c 2021-01-24 20:38:24.450737466 +0100 *************** *** 2613,2619 **** if (trystack->ga_len > 0) { ! trycmd_T *trycmd = NULL; --trystack->ga_len; --trylevel; --- 2613,2619 ---- if (trystack->ga_len > 0) { ! trycmd_T *trycmd; --trystack->ga_len; --trylevel; *************** *** 2635,2668 **** break; case ISN_THROW: - if (ectx.ec_trystack.ga_len == 0 && trylevel == 0 - && emsg_silent) { ! // throwing an exception while using "silent!" causes the ! // function to abort but not display an error. ! tv = STACK_TV_BOT(-1); ! clear_tv(tv); ! tv->v_type = VAR_NUMBER; ! tv->vval.v_number = 0; ! goto done; ! } ! --ectx.ec_stack.ga_len; ! tv = STACK_TV_BOT(0); ! if (tv->vval.v_string == NULL || *skipwhite(tv->vval.v_string) == NUL) ! { ! vim_free(tv->vval.v_string); ! SOURCING_LNUM = iptr->isn_lnum; ! emsg(_(e_throw_with_empty_string)); ! goto failed; ! } ! if (throw_exception(tv->vval.v_string, ET_USER, NULL) == FAIL) ! { ! vim_free(tv->vval.v_string); ! goto failed; } - did_throw = TRUE; break; // compare with special values --- 2635,2688 ---- break; case ISN_THROW: { ! garray_T *trystack = &ectx.ec_trystack; ! ! if (trystack->ga_len == 0 && trylevel == 0 && emsg_silent) ! { ! // throwing an exception while using "silent!" causes ! // the function to abort but not display an error. ! tv = STACK_TV_BOT(-1); ! clear_tv(tv); ! tv->v_type = VAR_NUMBER; ! tv->vval.v_number = 0; ! goto done; ! } ! --ectx.ec_stack.ga_len; ! tv = STACK_TV_BOT(0); ! if (tv->vval.v_string == NULL || *skipwhite(tv->vval.v_string) == NUL) ! { ! vim_free(tv->vval.v_string); ! SOURCING_LNUM = iptr->isn_lnum; ! emsg(_(e_throw_with_empty_string)); ! goto failed; ! } ! // Inside a "catch" we need to first discard the caught ! // exception. ! if (trystack->ga_len > 0) ! { ! trycmd_T *trycmd = ((trycmd_T *)trystack->ga_data) ! + trystack->ga_len - 1; ! if (trycmd->tcd_caught && current_exception != NULL) ! { ! // discard the exception ! if (caught_stack == current_exception) ! caught_stack = caught_stack->caught; ! discard_current_exception(); ! trycmd->tcd_caught = FALSE; ! } ! } ! ! if (throw_exception(tv->vval.v_string, ET_USER, NULL) ! == FAIL) ! { ! vim_free(tv->vval.v_string); ! goto failed; ! } ! did_throw = TRUE; } break; // compare with special values *** ../vim-8.2.2403/src/testdir/test_profile.vim 2021-01-24 17:53:43.681840018 +0100 --- src/testdir/test_profile.vim 2021-01-24 20:15:03.369951390 +0100 *************** *** 100,110 **** endfunc func Test_profile_func_with_ifelse() ! call Run_profile_func_with_ifelse('func', 'let', 'let') ! call Run_profile_func_with_ifelse('def', 'var', '') endfunc ! func Run_profile_func_with_ifelse(command, declare, assign) let lines =<< trim [CODE] XXX Foo1() if 1 --- 100,110 ---- endfunc func Test_profile_func_with_ifelse() ! call Run_profile_func_with_ifelse('func', 'let') ! call Run_profile_func_with_ifelse('def', 'var') endfunc ! func Run_profile_func_with_ifelse(command, declare) let lines =<< trim [CODE] XXX Foo1() if 1 *************** *** 140,146 **** call map(lines, {k, v -> substitute(v, 'XXX', a:command, '') }) call map(lines, {k, v -> substitute(v, 'DDD', a:declare, '') }) - call map(lines, {k, v -> substitute(v, 'AAA', a:assign, '') }) call writefile(lines, 'Xprofile_func.vim') call system(GetVimCommand() --- 140,145 ---- *************** *** 219,260 **** endfunc func Test_profile_func_with_trycatch() let lines =<< trim [CODE] ! func! Foo1() try ! let x = 0 catch ! let x = 1 finally ! let x = 2 endtry ! endfunc ! func! Foo2() try throw 0 catch ! let x = 1 finally ! let x = 2 endtry ! endfunc ! func! Foo3() try throw 0 catch throw 1 finally ! let x = 2 endtry ! endfunc call Foo1() call Foo2() try call Foo3() catch endtry [CODE] call writefile(lines, 'Xprofile_func.vim') call system(GetVimCommand() \ . ' -es -i NONE --noplugin' --- 218,273 ---- endfunc func Test_profile_func_with_trycatch() + call Run_profile_func_with_trycatch('func', 'let') + call Run_profile_func_with_trycatch('def', 'var') + endfunc + + func Run_profile_func_with_trycatch(command, declare) let lines =<< trim [CODE] ! XXX Foo1() try ! DDD x = 0 catch ! DDD x = 1 finally ! DDD x = 2 endtry ! endXXX ! XXX Foo2() try throw 0 catch ! DDD x = 1 finally ! DDD x = 2 endtry ! endXXX ! XXX Foo3() try throw 0 catch throw 1 finally ! DDD x = 2 endtry ! endXXX call Foo1() call Foo2() + let rethrown = 0 try call Foo3() catch + let rethrown = 1 endtry + if rethrown != 1 + " call Foo1 again so that the test fails + call Foo1() + endif [CODE] + call map(lines, {k, v -> substitute(v, 'XXX', a:command, '') }) + call map(lines, {k, v -> substitute(v, 'DDD', a:declare, '') }) + call writefile(lines, 'Xprofile_func.vim') call system(GetVimCommand() \ . ' -es -i NONE --noplugin' *************** *** 279,289 **** call assert_equal('', lines[5]) call assert_equal('count total (s) self (s)', lines[6]) call assert_match('^\s*1\s\+.*\stry$', lines[7]) ! call assert_match('^\s*1\s\+.*\s let x = 0$', lines[8]) call assert_match( '^\s\+catch$', lines[9]) ! call assert_match( '^\s\+let x = 1$', lines[10]) call assert_match('^\s*1\s\+.*\sfinally$', lines[11]) ! call assert_match('^\s*1\s\+.*\s let x = 2$', lines[12]) call assert_match('^\s*1\s\+.*\sendtry$', lines[13]) call assert_equal('', lines[14]) call assert_equal('FUNCTION Foo2()', lines[15]) --- 292,302 ---- call assert_equal('', lines[5]) call assert_equal('count total (s) self (s)', lines[6]) call assert_match('^\s*1\s\+.*\stry$', lines[7]) ! call assert_match('^\s*1\s\+.*\s \(let\|var\) x = 0$', lines[8]) call assert_match( '^\s\+catch$', lines[9]) ! call assert_match( '^\s\+\(let\|var\) x = 1$', lines[10]) call assert_match('^\s*1\s\+.*\sfinally$', lines[11]) ! call assert_match('^\s*1\s\+.*\s \(let\|var\) x = 2$', lines[12]) call assert_match('^\s*1\s\+.*\sendtry$', lines[13]) call assert_equal('', lines[14]) call assert_equal('FUNCTION Foo2()', lines[15]) *************** *** 295,303 **** call assert_match('^\s*1\s\+.*\stry$', lines[22]) call assert_match('^\s*1\s\+.*\s throw 0$', lines[23]) call assert_match('^\s*1\s\+.*\scatch$', lines[24]) ! call assert_match('^\s*1\s\+.*\s let x = 1$', lines[25]) call assert_match('^\s*1\s\+.*\sfinally$', lines[26]) ! call assert_match('^\s*1\s\+.*\s let x = 2$', lines[27]) call assert_match('^\s*1\s\+.*\sendtry$', lines[28]) call assert_equal('', lines[29]) call assert_equal('FUNCTION Foo3()', lines[30]) --- 308,316 ---- call assert_match('^\s*1\s\+.*\stry$', lines[22]) call assert_match('^\s*1\s\+.*\s throw 0$', lines[23]) call assert_match('^\s*1\s\+.*\scatch$', lines[24]) ! call assert_match('^\s*1\s\+.*\s \(let\|var\) x = 1$', lines[25]) call assert_match('^\s*1\s\+.*\sfinally$', lines[26]) ! call assert_match('^\s*1\s\+.*\s \(let\|var\) x = 2$', lines[27]) call assert_match('^\s*1\s\+.*\sendtry$', lines[28]) call assert_equal('', lines[29]) call assert_equal('FUNCTION Foo3()', lines[30]) *************** *** 311,317 **** call assert_match('^\s*1\s\+.*\scatch$', lines[39]) call assert_match('^\s*1\s\+.*\s throw 1$', lines[40]) call assert_match('^\s*1\s\+.*\sfinally$', lines[41]) ! call assert_match('^\s*1\s\+.*\s let x = 2$', lines[42]) call assert_match( '^\s\+endtry$', lines[43]) call assert_equal('', lines[44]) call assert_equal('FUNCTIONS SORTED ON TOTAL TIME', lines[45]) --- 324,330 ---- call assert_match('^\s*1\s\+.*\scatch$', lines[39]) call assert_match('^\s*1\s\+.*\s throw 1$', lines[40]) call assert_match('^\s*1\s\+.*\sfinally$', lines[41]) ! call assert_match('^\s*1\s\+.*\s \(let\|var\) x = 2$', lines[42]) call assert_match( '^\s\+endtry$', lines[43]) call assert_equal('', lines[44]) call assert_equal('FUNCTIONS SORTED ON TOTAL TIME', lines[45]) *** ../vim-8.2.2403/src/version.c 2021-01-24 17:53:43.681840018 +0100 --- src/version.c 2021-01-24 20:16:34.329767024 +0100 *************** *** 752,753 **** --- 752,755 ---- { /* Add new patch number below this line */ + /**/ + 2404, /**/ -- A computer without Windows is like a fish without a bicycle. /// 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 ///