# HG changeset patch # User Bram Moolenaar # Date 1627215304 -7200 # Node ID f03271631eb5bb5564150342daa8673eece7da8c # Parent 2cf7cefa5bfad5a8b0594982dadc24c610b44c85 patch 8.2.3216: Vim9: crash when using variable in a loop at script level Commit: https://github.com/vim/vim/commit/2eb6fc3b52148f961e804ec2be361d531ff770d8 Author: Bram Moolenaar Date: Sun Jul 25 14:13:53 2021 +0200 patch 8.2.3216: Vim9: crash when using variable in a loop at script level 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) diff --git a/src/eval.c b/src/eval.c --- a/src/eval.c +++ b/src/eval.c @@ -146,10 +146,14 @@ fill_evalarg_from_eap(evalarg_T *evalarg { CLEAR_FIELD(*evalarg); evalarg->eval_flags = skip ? 0 : EVAL_EVALUATE; - if (eap != NULL && getline_equal(eap->getline, eap->cookie, getsourceline)) + if (eap != NULL) { - evalarg->eval_getline = eap->getline; - evalarg->eval_cookie = eap->cookie; + evalarg->eval_cstack = eap->cstack; + if (getline_equal(eap->getline, eap->cookie, getsourceline)) + { + evalarg->eval_getline = eap->getline; + evalarg->eval_cookie = eap->cookie; + } } } diff --git a/src/evalvars.c b/src/evalvars.c --- a/src/evalvars.c +++ b/src/evalvars.c @@ -880,13 +880,7 @@ ex_let(exarg_T *eap) 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; - } + fill_evalarg_from_eap(&evalarg, eap, eap->skip); expr = skipwhite_and_linebreak(expr, &evalarg); cur_lnum = SOURCING_LNUM; i = eval0(expr, &rettv, eap, &evalarg); diff --git a/src/ex_eval.c b/src/ex_eval.c --- a/src/ex_eval.c +++ b/src/ex_eval.c @@ -972,9 +972,6 @@ leave_block(cstack_T *cstack) 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 @@ -1178,6 +1175,8 @@ ex_while(exarg_T *eap) { 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,10 +1191,8 @@ ex_while(exarg_T *eap) 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); + hide_script_var(si, i, func_defined); } - cstack->cs_script_var_len[cstack->cs_idx] = - si->sn_var_vals.ga_len; } } cstack->cs_flags[cstack->cs_idx] = @@ -1222,14 +1219,7 @@ ex_while(exarg_T *eap) /* * ":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; - } - + fill_evalarg_from_eap(&evalarg, eap, skip); if ((cstack->cs_lflags & CSL_HAD_LOOP) != 0) { // Jumping here from a ":continue" or ":endfor": use the diff --git a/src/structs.h b/src/structs.h --- a/src/structs.h +++ b/src/structs.h @@ -1888,6 +1888,9 @@ typedef struct { // 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. diff --git a/src/testdir/test_vim9_script.vim b/src/testdir/test_vim9_script.vim --- a/src/testdir/test_vim9_script.vim +++ b/src/testdir/test_vim9_script.vim @@ -2592,6 +2592,34 @@ def Test_for_loop() 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:') diff --git a/src/userfunc.c b/src/userfunc.c --- a/src/userfunc.c +++ b/src/userfunc.c @@ -614,6 +614,35 @@ is_function_cmd(char_u **cmd) } /* + * 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,6 +1224,8 @@ lambda_function_body( 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,6 +1473,8 @@ get_lambda_tv( 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,6 +1492,7 @@ theend: vim_free(tofree2); if (types_optional) ga_clear_strings(&argtypes); + return OK; errret: @@ -4313,28 +4347,8 @@ define_function(exarg_T *eap, char_u *na // 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; - } + // The function may use script variables from the context. + function_using_block_scopes(fp, cstack); if (parse_argument_types(fp, &argtypes, varargs) == FAIL) { diff --git a/src/version.c b/src/version.c --- a/src/version.c +++ b/src/version.c @@ -756,6 +756,8 @@ static char *(features[]) = static int included_patches[] = { /* Add new patch number below this line */ /**/ + 3216, +/**/ 3215, /**/ 3214, diff --git a/src/vim9script.c b/src/vim9script.c --- a/src/vim9script.c +++ b/src/vim9script.c @@ -758,47 +758,72 @@ update_vim9_script_var( { scriptitem_T *si = SCRIPT_ITEM(current_sctx.sc_sid); hashitem_T *hi; - svar_T *sv; + svar_T *sv = NULL; if (create) { - sallvar_T *newsav; + 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; - 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); + hi = hash_find(&si->sn_all_vars.dv_hashtab, di->di_key); if (!HASHITEM_EMPTY(hi)) { - sallvar_T *sav = HI2SAV(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; + } + } - // variable with this name exists in another block - while (sav->sav_next != NULL) - sav = sav->sav_next; - sav->sav_next = newsav; + 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 - // new variable name - hash_add(&si->sn_all_vars.dv_hashtab, newsav->sav_key); } else { @@ -807,8 +832,7 @@ update_vim9_script_var( if (sv != NULL) { if (*type == NULL) - *type = typval2type(tv, get_copyID(), &si->sn_type_list, - do_member); + *type = typval2type(tv, get_copyID(), &si->sn_type_list, do_member); sv->sv_type = *type; }