To: vim_dev@googlegroups.com Subject: Patch 8.2.3216 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ------------ Patch 8.2.3216 Problem: Vim9: crash when using variable in a loop at script level. Solution: Do not clear the variable if a function was defined. Do not create a new entry in sn_var_vals every time. (closes #8628) Files: src/eval.c, src/ex_eval.c, src/vim9script.c, src/userfunc.c, src/evalvars.c, src/structs.h *** ../vim-8.2.3215/src/eval.c 2021-07-22 15:14:21.723834160 +0200 --- src/eval.c 2021-07-25 13:22:04.324313325 +0200 *************** *** 146,155 **** { CLEAR_FIELD(*evalarg); evalarg->eval_flags = skip ? 0 : EVAL_EVALUATE; ! if (eap != NULL && getline_equal(eap->getline, eap->cookie, getsourceline)) { ! evalarg->eval_getline = eap->getline; ! evalarg->eval_cookie = eap->cookie; } } --- 146,159 ---- { CLEAR_FIELD(*evalarg); evalarg->eval_flags = skip ? 0 : EVAL_EVALUATE; ! if (eap != NULL) { ! evalarg->eval_cstack = eap->cstack; ! if (getline_equal(eap->getline, eap->cookie, getsourceline)) ! { ! evalarg->eval_getline = eap->getline; ! evalarg->eval_cookie = eap->cookie; ! } } } *** ../vim-8.2.3215/src/ex_eval.c 2021-07-20 21:07:32.964058857 +0200 --- src/ex_eval.c 2021-07-25 14:07:06.569602646 +0200 *************** *** 972,980 **** hide_script_var(si, i, func_defined); } - // TODO: is this needed? - cstack->cs_script_var_len[cstack->cs_idx] = si->sn_var_vals.ga_len; - if (cstack->cs_idx == 0) si->sn_current_block_id = 0; else --- 972,977 ---- *************** *** 1178,1183 **** --- 1175,1182 ---- { scriptitem_T *si = SCRIPT_ITEM(current_sctx.sc_sid); int i; + int func_defined = cstack->cs_flags[cstack->cs_idx] + & CSF_FUNC_DEF; // Any variables defined in the previous round are no longer // visible. *************** *** 1192,1201 **** if (sv->sv_name != NULL) // Remove a variable declared inside the block, if it // still exists, from sn_vars. ! hide_script_var(si, i, FALSE); } - cstack->cs_script_var_len[cstack->cs_idx] = - si->sn_var_vals.ga_len; } } cstack->cs_flags[cstack->cs_idx] = --- 1191,1198 ---- if (sv->sv_name != NULL) // Remove a variable declared inside the block, if it // still exists, from sn_vars. ! hide_script_var(si, i, func_defined); } } } cstack->cs_flags[cstack->cs_idx] = *************** *** 1222,1235 **** /* * ":for var in list-expr" */ ! CLEAR_FIELD(evalarg); ! evalarg.eval_flags = skip ? 0 : EVAL_EVALUATE; ! if (getline_equal(eap->getline, eap->cookie, getsourceline)) ! { ! evalarg.eval_getline = eap->getline; ! evalarg.eval_cookie = eap->cookie; ! } ! if ((cstack->cs_lflags & CSL_HAD_LOOP) != 0) { // Jumping here from a ":continue" or ":endfor": use the --- 1219,1225 ---- /* * ":for var in list-expr" */ ! fill_evalarg_from_eap(&evalarg, eap, skip); if ((cstack->cs_lflags & CSL_HAD_LOOP) != 0) { // Jumping here from a ":continue" or ":endfor": use the *** ../vim-8.2.3215/src/vim9script.c 2021-07-21 22:20:30.066401728 +0200 --- src/vim9script.c 2021-07-25 12:34:52.125269175 +0200 *************** *** 758,804 **** { scriptitem_T *si = SCRIPT_ITEM(current_sctx.sc_sid); hashitem_T *hi; ! svar_T *sv; if (create) { ! sallvar_T *newsav; // Store a pointer to the typval_T, so that it can be found by index // instead of using a hastab lookup. if (ga_grow(&si->sn_var_vals, 1) == FAIL) return; ! sv = ((svar_T *)si->sn_var_vals.ga_data) + si->sn_var_vals.ga_len; ! newsav = (sallvar_T *)alloc_clear( ! sizeof(sallvar_T) + STRLEN(di->di_key)); ! if (newsav == NULL) ! return; ! ! sv->sv_tv = &di->di_tv; ! sv->sv_const = (flags & ASSIGN_FINAL) ? ASSIGN_FINAL ! : (flags & ASSIGN_CONST) ? ASSIGN_CONST : 0; ! sv->sv_export = is_export; ! newsav->sav_var_vals_idx = si->sn_var_vals.ga_len; ! ++si->sn_var_vals.ga_len; ! STRCPY(&newsav->sav_key, di->di_key); ! sv->sv_name = newsav->sav_key; ! newsav->sav_di = di; ! newsav->sav_block_id = si->sn_current_block_id; ! ! hi = hash_find(&si->sn_all_vars.dv_hashtab, newsav->sav_key); if (!HASHITEM_EMPTY(hi)) { ! sallvar_T *sav = HI2SAV(hi); ! // variable with this name exists in another block ! while (sav->sav_next != NULL) ! sav = sav->sav_next; ! sav->sav_next = newsav; } - else - // new variable name - hash_add(&si->sn_all_vars.dv_hashtab, newsav->sav_key); } else { --- 758,829 ---- { scriptitem_T *si = SCRIPT_ITEM(current_sctx.sc_sid); hashitem_T *hi; ! svar_T *sv = NULL; if (create) { ! sallvar_T *newsav; ! sallvar_T *sav = NULL; // Store a pointer to the typval_T, so that it can be found by index // instead of using a hastab lookup. if (ga_grow(&si->sn_var_vals, 1) == FAIL) return; ! hi = hash_find(&si->sn_all_vars.dv_hashtab, di->di_key); if (!HASHITEM_EMPTY(hi)) { ! // Variable with this name exists, either in this block or in ! // another block. ! for (sav = HI2SAV(hi); ; sav = sav->sav_next) ! { ! if (sav->sav_block_id == si->sn_current_block_id) ! { ! // variable defined in a loop, re-use the entry ! sv = ((svar_T *)si->sn_var_vals.ga_data) ! + sav->sav_var_vals_idx; ! // unhide the variable ! if (sv->sv_tv == &sav->sav_tv) ! { ! clear_tv(&sav->sav_tv); ! sv->sv_tv = &di->di_tv; ! sav->sav_di = di; ! } ! break; ! } ! if (sav->sav_next == NULL) ! break; ! } ! } ! if (sv == NULL) ! { ! // Variable not defined or not defined in current block: Add a ! // svar_T and create a new sallvar_T. ! sv = ((svar_T *)si->sn_var_vals.ga_data) + si->sn_var_vals.ga_len; ! newsav = (sallvar_T *)alloc_clear( ! sizeof(sallvar_T) + STRLEN(di->di_key)); ! if (newsav == NULL) ! return; ! ! sv->sv_tv = &di->di_tv; ! sv->sv_const = (flags & ASSIGN_FINAL) ? ASSIGN_FINAL ! : (flags & ASSIGN_CONST) ? ASSIGN_CONST : 0; ! sv->sv_export = is_export; ! newsav->sav_var_vals_idx = si->sn_var_vals.ga_len; ! ++si->sn_var_vals.ga_len; ! STRCPY(&newsav->sav_key, di->di_key); ! sv->sv_name = newsav->sav_key; ! newsav->sav_di = di; ! newsav->sav_block_id = si->sn_current_block_id; ! ! if (HASHITEM_EMPTY(hi)) ! // new variable name ! hash_add(&si->sn_all_vars.dv_hashtab, newsav->sav_key); ! else if (sav != NULL) ! // existing name in a new block, append to the list ! sav->sav_next = newsav; } } else { *************** *** 807,814 **** if (sv != NULL) { if (*type == NULL) ! *type = typval2type(tv, get_copyID(), &si->sn_type_list, ! do_member); sv->sv_type = *type; } --- 832,838 ---- if (sv != NULL) { if (*type == NULL) ! *type = typval2type(tv, get_copyID(), &si->sn_type_list, do_member); sv->sv_type = *type; } *** ../vim-8.2.3215/src/userfunc.c 2021-07-20 21:07:32.972058844 +0200 --- src/userfunc.c 2021-07-25 13:29:55.963107877 +0200 *************** *** 614,619 **** --- 614,648 ---- } /* + * Called when defining a function: The context may be needed for script + * variables declared in a block that is visible now but not when the function + * is compiled or called later. + */ + static void + function_using_block_scopes(ufunc_T *fp, cstack_T *cstack) + { + if (cstack != NULL && cstack->cs_idx >= 0) + { + int count = cstack->cs_idx + 1; + int i; + + fp->uf_block_ids = ALLOC_MULT(int, count); + if (fp->uf_block_ids != NULL) + { + mch_memmove(fp->uf_block_ids, cstack->cs_block_id, + sizeof(int) * count); + fp->uf_block_depth = count; + } + + // Set flag in each block to indicate a function was defined. This + // is used to keep the variable when leaving the block, see + // hide_script_var(). + for (i = 0; i <= cstack->cs_idx; ++i) + cstack->cs_flags[i] |= CSF_FUNC_DEF; + } + } + + /* * Read the body of a function, put every line in "newlines". * This stops at "}", "endfunction" or "enddef". * "newlines" must already have been initialized. *************** *** 1195,1200 **** --- 1224,1231 ---- ufunc->uf_script_ctx.sc_lnum += sourcing_lnum_top; set_function_type(ufunc); + function_using_block_scopes(ufunc, evalarg->eval_cstack); + rettv->vval.v_partial = pt; rettv->v_type = VAR_PARTIAL; ufunc = NULL; *************** *** 1442,1447 **** --- 1473,1480 ---- fp->uf_script_ctx = current_sctx; fp->uf_script_ctx.sc_lnum += SOURCING_LNUM - newlines.ga_len; + function_using_block_scopes(fp, evalarg->eval_cstack); + pt->pt_func = fp; pt->pt_refcount = 1; rettv->vval.v_partial = pt; *************** *** 1459,1464 **** --- 1492,1498 ---- vim_free(tofree2); if (types_optional) ga_clear_strings(&argtypes); + return OK; errret: *************** *** 4313,4340 **** // error messages are for the first function line SOURCING_LNUM = sourcing_lnum_top; ! if (cstack != NULL && cstack->cs_idx >= 0) ! { ! int count = cstack->cs_idx + 1; ! int i; ! ! // The block context may be needed for script variables declared in ! // a block that is visible now but not when the function is called ! // later. ! fp->uf_block_ids = ALLOC_MULT(int, count); ! if (fp->uf_block_ids != NULL) ! { ! mch_memmove(fp->uf_block_ids, cstack->cs_block_id, ! sizeof(int) * count); ! fp->uf_block_depth = count; ! } ! ! // Set flag in each block to indicate a function was defined. This ! // is used to keep the variable when leaving the block, see ! // hide_script_var(). ! for (i = 0; i <= cstack->cs_idx; ++i) ! cstack->cs_flags[i] |= CSF_FUNC_DEF; ! } if (parse_argument_types(fp, &argtypes, varargs) == FAIL) { --- 4347,4354 ---- // error messages are for the first function line SOURCING_LNUM = sourcing_lnum_top; ! // The function may use script variables from the context. ! function_using_block_scopes(fp, cstack); if (parse_argument_types(fp, &argtypes, varargs) == FAIL) { *** ../vim-8.2.3215/src/evalvars.c 2021-07-24 16:16:11.542239515 +0200 --- src/evalvars.c 2021-07-25 14:03:48.594148955 +0200 *************** *** 880,892 **** if (eap->skip) ++emsg_skip; ! CLEAR_FIELD(evalarg); ! evalarg.eval_flags = eap->skip ? 0 : EVAL_EVALUATE; ! if (getline_equal(eap->getline, eap->cookie, getsourceline)) ! { ! evalarg.eval_getline = eap->getline; ! evalarg.eval_cookie = eap->cookie; ! } expr = skipwhite_and_linebreak(expr, &evalarg); cur_lnum = SOURCING_LNUM; i = eval0(expr, &rettv, eap, &evalarg); --- 880,886 ---- if (eap->skip) ++emsg_skip; ! fill_evalarg_from_eap(&evalarg, eap, eap->skip); expr = skipwhite_and_linebreak(expr, &evalarg); cur_lnum = SOURCING_LNUM; i = eval0(expr, &rettv, eap, &evalarg); *** ../vim-8.2.3215/src/structs.h 2021-07-22 14:58:43.473967313 +0200 --- src/structs.h 2021-07-25 13:17:02.956914996 +0200 *************** *** 1888,1893 **** --- 1888,1896 ---- // used when compiling a :def function, NULL otherwise cctx_T *eval_cctx; + // used when executing commands from a script, NULL otherwise + cstack_T *eval_cstack; + // Used to collect lines while parsing them, so that they can be // concatenated later. Used when "eval_ga.ga_itemsize" is not zero. // "eval_ga.ga_data" is a list of pointers to lines. *** ../vim-8.2.3215/src/testdir/test_vim9_script.vim 2021-07-20 19:18:40.387377742 +0200 --- src/testdir/test_vim9_script.vim 2021-07-25 14:10:16.761104276 +0200 *************** *** 2592,2597 **** --- 2592,2625 ---- CheckDefAndScriptSuccess(lines) enddef + def Test_for_loop_with_closure() + var lines =<< trim END + var flist: list + for i in range(5) + var inloop = i + flist[i] = () => inloop + endfor + for i in range(5) + assert_equal(4, flist[i]()) + endfor + END + CheckDefAndScriptSuccess(lines) + + lines =<< trim END + var flist: list + for i in range(5) + var inloop = i + flist[i] = () => { + return inloop + } + endfor + for i in range(5) + assert_equal(4, flist[i]()) + endfor + END + CheckDefAndScriptSuccess(lines) + enddef + def Test_for_loop_fails() CheckDefAndScriptFailure2(['for '], 'E1097:', 'E690:') CheckDefAndScriptFailure2(['for x'], 'E1097:', 'E690:') *** ../vim-8.2.3215/src/version.c 2021-07-24 21:33:22.395681940 +0200 --- src/version.c 2021-07-25 14:12:33.216758073 +0200 *************** *** 757,758 **** --- 757,760 ---- { /* Add new patch number below this line */ + /**/ + 3216, /**/ -- hundred-and-one symptoms of being an internet addict: 231. You sprinkle Carpet Fresh on the rugs and put your vacuum cleaner in the front doorway permanently so it always looks like you are actually attempting to do something about that mess that has amassed since you discovered the Internet. /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net \\\ /// \\\ \\\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ /// \\\ help me help AIDS victims -- http://ICCF-Holland.org ///