# HG changeset patch # User Bram Moolenaar # Date 1608483606 -3600 # Node ID 43532077b5ff8cc8a3c8e97a4029012312a27fbf # Parent a0e114c6b39ee77e843ea423c896365be60cca2a patch 8.2.2170: Vim9: a global function defined in a :def function fails Commit: https://github.com/vim/vim/commit/f112f30a82f17114d8b08a0fb90928cd19440581 Author: Bram Moolenaar Date: Sun Dec 20 17:47:52 2020 +0100 patch 8.2.2170: Vim9: a global function defined in a :def function fails Problem: Vim9: a global function defined in a :def function fails if it uses the context. Solution: Create a partial to store the closure context. (see #7410) diff --git a/src/proto/userfunc.pro b/src/proto/userfunc.pro --- a/src/proto/userfunc.pro +++ b/src/proto/userfunc.pro @@ -13,7 +13,7 @@ ufunc_T *find_func_even_dead(char_u *nam ufunc_T *find_func(char_u *name, int is_global, cctx_T *cctx); int func_is_global(ufunc_T *ufunc); int func_name_refcount(char_u *name); -void copy_func(char_u *lambda, char_u *global); +ufunc_T *copy_func(char_u *lambda, char_u *global); int funcdepth_increment(void); void funcdepth_decrement(void); int funcdepth_get(void); diff --git a/src/structs.h b/src/structs.h --- a/src/structs.h +++ b/src/structs.h @@ -1598,6 +1598,9 @@ typedef struct garray_T uf_type_list; // types used in arg and return types int *uf_def_arg_idx; // instruction indexes for evaluating // uf_def_args; length: uf_def_args.ga_len + 1 + partial_T *uf_partial; // for closure created inside :def function: + // information about the context + char_u *uf_va_name; // name from "...name" or NULL type_T *uf_va_type; // type from "...name: type" or NULL type_T *uf_func_type; // type of the function, &t_func_any if unknown diff --git a/src/testdir/test_vim9_func.vim b/src/testdir/test_vim9_func.vim --- a/src/testdir/test_vim9_func.vim +++ b/src/testdir/test_vim9_func.vim @@ -1523,6 +1523,29 @@ def Test_nested_closure_fails() CheckScriptFailure(lines, 'E1012:') enddef +def Test_global_closure() + var lines =<< trim END + vim9script + def ReverseEveryNLines(n: number, line1: number, line2: number) + var mods = 'sil keepj keepp lockm ' + var range = ':' .. line1 .. ',' .. line2 + def g:Offset(): number + var offset = (line('.') - line1 + 1) % n + return offset != 0 ? offset : n + enddef + exe mods .. range .. 'g/^/exe "m .-" .. g:Offset()' + enddef + + new + repeat(['aaa', 'bbb', 'ccc'], 3)->setline(1) + ReverseEveryNLines(3, 1, 9) + END + CheckScriptSuccess(lines) + var expected = repeat(['ccc', 'bbb', 'aaa'], 3) + assert_equal(expected, getline(1, 9)) + bwipe! +enddef + def Test_failure_in_called_function() # this was using the frame index as the return value var lines =<< trim END diff --git a/src/userfunc.c b/src/userfunc.c --- a/src/userfunc.c +++ b/src/userfunc.c @@ -1225,6 +1225,8 @@ func_clear_items(ufunc_T *fp) VIM_CLEAR(fp->uf_block_ids); VIM_CLEAR(fp->uf_va_name); clear_type_list(&fp->uf_type_list); + partial_unref(fp->uf_partial); + fp->uf_partial = NULL; #ifdef FEAT_LUA if (fp->uf_cb_free != NULL) @@ -1305,12 +1307,13 @@ func_clear_free(ufunc_T *fp, int force) /* * Copy already defined function "lambda" to a new function with name "global". * This is for when a compiled function defines a global function. + * Caller should take care of adding a partial for a closure. */ - void + ufunc_T * copy_func(char_u *lambda, char_u *global) { ufunc_T *ufunc = find_func_even_dead(lambda, TRUE, NULL); - ufunc_T *fp; + ufunc_T *fp = NULL; if (ufunc == NULL) semsg(_(e_lambda_function_not_found_str), lambda); @@ -1321,12 +1324,12 @@ copy_func(char_u *lambda, char_u *global if (fp != NULL) { semsg(_(e_funcexts), global); - return; + return NULL; } fp = alloc_clear(offsetof(ufunc_T, uf_name) + STRLEN(global) + 1); if (fp == NULL) - return; + return NULL; fp->uf_varargs = ufunc->uf_varargs; fp->uf_flags = (ufunc->uf_flags & ~FC_VIM9) | FC_COPY; @@ -1362,15 +1365,17 @@ copy_func(char_u *lambda, char_u *global if (fp->uf_va_name == NULL) goto failed; } + fp->uf_ret_type = ufunc->uf_ret_type; fp->uf_refcount = 1; STRCPY(fp->uf_name, global); hash_add(&func_hashtab, UF2HIKEY(fp)); } - return; + return fp; failed: func_clear_free(fp, TRUE); + return NULL; } static int funcdepth = 0; diff --git a/src/version.c b/src/version.c --- a/src/version.c +++ b/src/version.c @@ -751,6 +751,8 @@ static char *(features[]) = static int included_patches[] = { /* Add new patch number below this line */ /**/ + 2170, +/**/ 2169, /**/ 2168, diff --git a/src/vim9execute.c b/src/vim9execute.c --- a/src/vim9execute.c +++ b/src/vim9execute.c @@ -845,6 +845,49 @@ call_eval_func(char_u *name, int argcoun } /* + * When a function reference is used, fill a partial with the information + * needed, especially when it is used as a closure. + */ + static int +fill_partial_and_closure(partial_T *pt, ufunc_T *ufunc, ectx_T *ectx) +{ + pt->pt_func = ufunc; + pt->pt_refcount = 1; + + if (pt->pt_func->uf_flags & FC_CLOSURE) + { + dfunc_T *dfunc = ((dfunc_T *)def_functions.ga_data) + + ectx->ec_dfunc_idx; + + // The closure needs to find arguments and local + // variables in the current stack. + pt->pt_ectx_stack = &ectx->ec_stack; + pt->pt_ectx_frame = ectx->ec_frame_idx; + + // If this function returns and the closure is still + // being used, we need to make a copy of the context + // (arguments and local variables). Store a reference + // to the partial so we can handle that. + if (ga_grow(&ectx->ec_funcrefs, 1) == FAIL) + { + vim_free(pt); + return FAIL; + } + // Extra variable keeps the count of closures created + // in the current function call. + ++(((typval_T *)ectx->ec_stack.ga_data) + ectx->ec_frame_idx + + STACK_FRAME_SIZE + dfunc->df_varcount)->vval.v_number; + + ((partial_T **)ectx->ec_funcrefs.ga_data) + [ectx->ec_funcrefs.ga_len] = pt; + ++pt->pt_refcount; + ++ectx->ec_funcrefs.ga_len; + } + ++pt->pt_func->uf_refcount; + return OK; +} + +/* * Call a "def" function from old Vim script. * Return OK or FAIL. */ @@ -1003,6 +1046,11 @@ call_def_function( ectx.ec_outer_frame = partial->pt_ectx_frame; } } + else if (ufunc->uf_partial != NULL) + { + ectx.ec_outer_stack = ufunc->uf_partial->pt_ectx_stack; + ectx.ec_outer_frame = ufunc->uf_partial->pt_ectx_frame; + } // dummy frame entries for (idx = 0; idx < STACK_FRAME_SIZE; ++idx) @@ -1969,10 +2017,10 @@ call_def_function( // push a function reference to a compiled function case ISN_FUNCREF: { - partial_T *pt = NULL; - dfunc_T *pt_dfunc; - - pt = ALLOC_CLEAR_ONE(partial_T); + partial_T *pt = ALLOC_CLEAR_ONE(partial_T); + dfunc_T *pt_dfunc = ((dfunc_T *)def_functions.ga_data) + + iptr->isn_arg.funcref.fr_func; + if (pt == NULL) goto failed; if (GA_GROW(&ectx.ec_stack, 1) == FAIL) @@ -1980,41 +2028,9 @@ call_def_function( vim_free(pt); goto failed; } - pt_dfunc = ((dfunc_T *)def_functions.ga_data) - + iptr->isn_arg.funcref.fr_func; - pt->pt_func = pt_dfunc->df_ufunc; - pt->pt_refcount = 1; - - if (pt_dfunc->df_ufunc->uf_flags & FC_CLOSURE) - { - dfunc_T *dfunc = ((dfunc_T *)def_functions.ga_data) - + ectx.ec_dfunc_idx; - - // The closure needs to find arguments and local - // variables in the current stack. - pt->pt_ectx_stack = &ectx.ec_stack; - pt->pt_ectx_frame = ectx.ec_frame_idx; - - // If this function returns and the closure is still - // being used, we need to make a copy of the context - // (arguments and local variables). Store a reference - // to the partial so we can handle that. - if (ga_grow(&ectx.ec_funcrefs, 1) == FAIL) - { - vim_free(pt); - goto failed; - } - // Extra variable keeps the count of closures created - // in the current function call. - tv = STACK_TV_VAR(dfunc->df_varcount); - ++tv->vval.v_number; - - ((partial_T **)ectx.ec_funcrefs.ga_data) - [ectx.ec_funcrefs.ga_len] = pt; - ++pt->pt_refcount; - ++ectx.ec_funcrefs.ga_len; - } - ++pt_dfunc->df_ufunc->uf_refcount; + if (fill_partial_and_closure(pt, pt_dfunc->df_ufunc, + &ectx) == FAIL) + goto failed; tv = STACK_TV_BOT(0); ++ectx.ec_stack.ga_len; @@ -2028,8 +2044,25 @@ call_def_function( case ISN_NEWFUNC: { newfunc_T *newfunc = &iptr->isn_arg.newfunc; - - copy_func(newfunc->nf_lambda, newfunc->nf_global); + ufunc_T *new_ufunc; + + new_ufunc = copy_func( + newfunc->nf_lambda, newfunc->nf_global); + if (new_ufunc != NULL + && (new_ufunc->uf_flags & FC_CLOSURE)) + { + partial_T *pt = ALLOC_CLEAR_ONE(partial_T); + + // Need to create a partial to store the context of the + // function. + if (pt == NULL) + goto failed; + if (fill_partial_and_closure(pt, new_ufunc, + &ectx) == FAIL) + goto failed; + new_ufunc->uf_partial = pt; + --pt->pt_refcount; // not referenced here + } } break; @@ -3114,7 +3147,10 @@ failed: // Deal with any remaining closures, they may be in use somewhere. if (ectx.ec_funcrefs.ga_len > 0) + { handle_closure_in_use(&ectx, FALSE); + ga_clear(&ectx.ec_funcrefs); // TODO: should not be needed? + } estack_pop(); current_sctx = save_current_sctx;