To: vim_dev@googlegroups.com Subject: Patch 8.2.2170 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ------------ Patch 8.2.2170 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) Files: src/userfunc.c, src/proto/userfunc.pro, src/vim9execute.c, src/structs.h, src/testdir/test_vim9_func.vim *** ../vim-8.2.2169/src/userfunc.c 2020-12-19 21:23:38.797667072 +0100 --- src/userfunc.c 2020-12-20 17:22:24.359338305 +0100 *************** *** 1225,1230 **** --- 1225,1232 ---- 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,1316 **** /* * Copy already defined function "lambda" to a new function with name "global". * This is for when a compiled function defines a global function. */ ! void copy_func(char_u *lambda, char_u *global) { ufunc_T *ufunc = find_func_even_dead(lambda, TRUE, NULL); ! ufunc_T *fp; if (ufunc == NULL) semsg(_(e_lambda_function_not_found_str), lambda); --- 1307,1319 ---- /* * 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. */ ! ufunc_T * copy_func(char_u *lambda, char_u *global) { ufunc_T *ufunc = find_func_even_dead(lambda, TRUE, NULL); ! ufunc_T *fp = NULL; if (ufunc == NULL) semsg(_(e_lambda_function_not_found_str), lambda); *************** *** 1321,1332 **** if (fp != NULL) { semsg(_(e_funcexts), global); ! return; } fp = alloc_clear(offsetof(ufunc_T, uf_name) + STRLEN(global) + 1); if (fp == NULL) ! return; fp->uf_varargs = ufunc->uf_varargs; fp->uf_flags = (ufunc->uf_flags & ~FC_VIM9) | FC_COPY; --- 1324,1335 ---- if (fp != NULL) { semsg(_(e_funcexts), global); ! return NULL; } fp = alloc_clear(offsetof(ufunc_T, uf_name) + STRLEN(global) + 1); if (fp == NULL) ! return NULL; fp->uf_varargs = ufunc->uf_varargs; fp->uf_flags = (ufunc->uf_flags & ~FC_VIM9) | FC_COPY; *************** *** 1362,1376 **** if (fp->uf_va_name == NULL) goto failed; } fp->uf_refcount = 1; STRCPY(fp->uf_name, global); hash_add(&func_hashtab, UF2HIKEY(fp)); } ! return; failed: func_clear_free(fp, TRUE); } static int funcdepth = 0; --- 1365,1381 ---- 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 fp; failed: func_clear_free(fp, TRUE); + return NULL; } static int funcdepth = 0; *** ../vim-8.2.2169/src/proto/userfunc.pro 2020-11-22 18:15:40.171258382 +0100 --- src/proto/userfunc.pro 2020-12-20 16:53:12.854005764 +0100 *************** *** 13,19 **** 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); int funcdepth_increment(void); void funcdepth_decrement(void); int funcdepth_get(void); --- 13,19 ---- 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); ! ufunc_T *copy_func(char_u *lambda, char_u *global); int funcdepth_increment(void); void funcdepth_decrement(void); int funcdepth_get(void); *** ../vim-8.2.2169/src/vim9execute.c 2020-12-19 16:30:39.439810130 +0100 --- src/vim9execute.c 2020-12-20 17:34:40.152702327 +0100 *************** *** 845,850 **** --- 845,893 ---- } /* + * 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,1008 **** --- 1046,1056 ---- 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,1978 **** // 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); if (pt == NULL) goto failed; if (GA_GROW(&ectx.ec_stack, 1) == FAIL) --- 2017,2026 ---- // push a function reference to a compiled function case ISN_FUNCREF: { ! 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,2020 **** 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; tv = STACK_TV_BOT(0); ++ectx.ec_stack.ga_len; --- 2028,2036 ---- vim_free(pt); goto failed; } ! 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,2035 **** case ISN_NEWFUNC: { newfunc_T *newfunc = &iptr->isn_arg.newfunc; ! copy_func(newfunc->nf_lambda, newfunc->nf_global); } break; --- 2044,2068 ---- case ISN_NEWFUNC: { newfunc_T *newfunc = &iptr->isn_arg.newfunc; + 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,3120 **** --- 3147,3156 ---- // 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; *** ../vim-8.2.2169/src/structs.h 2020-12-01 21:47:55.156840720 +0100 --- src/structs.h 2020-12-20 16:50:09.438638017 +0100 *************** *** 1598,1603 **** --- 1598,1606 ---- 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 *** ../vim-8.2.2169/src/testdir/test_vim9_func.vim 2020-12-12 20:42:16.123207537 +0100 --- src/testdir/test_vim9_func.vim 2020-12-20 17:24:00.002991777 +0100 *************** *** 1523,1528 **** --- 1523,1551 ---- 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 *** ../vim-8.2.2169/src/version.c 2020-12-20 15:43:27.667305001 +0100 --- src/version.c 2020-12-20 17:40:10.835180879 +0100 *************** *** 752,753 **** --- 752,755 ---- { /* Add new patch number below this line */ + /**/ + 2170, /**/ -- At some point in the project somebody will start whining about the need to determine the project "requirements". This involves interviewing people who don't know what they want but, curiously, know exactly when they need it. (Scott Adams - The Dilbert principle) /// 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 ///