# HG changeset patch # User Bram Moolenaar # Date 1610284504 -3600 # Node ID f50ee1ae4d9bfa1e45b9d0113be78d5c2ce2f75a # Parent 9f919f241f467b5d8f25f98af304aa51fd66f9da patch 8.2.2321: Vim9: cannot nest closures Commit: https://github.com/vim/vim/commit/ab360526ef653b139f4b007a0efbdb3410c8fb4b Author: Bram Moolenaar Date: Sun Jan 10 14:02:28 2021 +0100 patch 8.2.2321: Vim9: cannot nest closures Problem: Vim9: cannot nest closures. Solution: Add the nesting level to ISN_LOADOUTER and ISN_STOREOUTER. (closes #7150, closes #7635) diff --git a/src/structs.h b/src/structs.h --- a/src/structs.h +++ b/src/structs.h @@ -1978,6 +1978,8 @@ struct partial_S // For a compiled closure: the arguments and local variables. garray_T *pt_ectx_stack; // where to find local vars int pt_ectx_frame; // index of function frame in uf_ectx_stack + garray_T *pt_outer_stack; // pt_ectx_stack one level up + int pt_outer_frame; // pt_ectx_frame one level up. funcstack_T *pt_funcstack; // copy of stack, used after context // function returns diff --git a/src/testdir/test_vim9_disassemble.vim b/src/testdir/test_vim9_disassemble.vim --- a/src/testdir/test_vim9_disassemble.vim +++ b/src/testdir/test_vim9_disassemble.vim @@ -567,17 +567,17 @@ def Test_disassemble_closure() var res = execute('disass g:Append') assert_match('\d\_s*' .. 'local ..= arg\_s*' .. - '\d LOADOUTER $0\_s*' .. + '\d LOADOUTER level 1 $0\_s*' .. '\d LOAD arg\[-1\]\_s*' .. '\d CONCAT\_s*' .. - '\d STOREOUTER $0\_s*' .. + '\d STOREOUTER level 1 $0\_s*' .. '\d RETURN 0', res) res = execute('disass g:Get') assert_match('\d\_s*' .. 'return local\_s*' .. - '\d LOADOUTER $0\_s*' .. + '\d LOADOUTER level 1 $0\_s*' .. '\d RETURN', res) 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 @@ -1812,6 +1812,16 @@ def Test_list_lambda() assert_match('def \d\+(_: any, ...): number\n1 return 0\n enddef', body) enddef +def DoFilterThis(a: string): list + # closure nested inside another closure using argument + var Filter = (l) => filter(l, (_, v) => stridx(v, a) == 0) + return ['x', 'y', 'a', 'x2', 'c']->Filter() +enddef + +def Test_nested_closure_using_argument() + assert_equal(['x', 'x2'], DoFilterThis('x')) +enddef + func Test_silent_echo() CheckScreendump 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 */ /**/ + 2321, +/**/ 2320, /**/ 2319, diff --git a/src/vim9.h b/src/vim9.h --- a/src/vim9.h +++ b/src/vim9.h @@ -33,7 +33,7 @@ typedef enum { ISN_LOADWDICT, // push w: dict ISN_LOADTDICT, // push t: dict ISN_LOADS, // push s: variable isn_arg.loadstore - ISN_LOADOUTER, // push variable from outer scope isn_arg.number + ISN_LOADOUTER, // push variable from outer scope isn_arg.outer ISN_LOADSCRIPT, // push script-local variable isn_arg.script. ISN_LOADOPT, // push option isn_arg.string ISN_LOADENV, // push environment variable isn_arg.string @@ -47,7 +47,7 @@ typedef enum { ISN_STOREW, // pop into window-local variable isn_arg.string ISN_STORET, // pop into tab-local variable isn_arg.string ISN_STORES, // pop into script variable isn_arg.loadstore - ISN_STOREOUTER, // pop variable into outer scope isn_arg.number + ISN_STOREOUTER, // pop variable into outer scope isn_arg.outer ISN_STORESCRIPT, // pop into script variable isn_arg.script ISN_STOREOPT, // pop into option isn_arg.string ISN_STOREENV, // pop into environment variable isn_arg.string @@ -303,6 +303,12 @@ typedef struct { int unp_semicolon; // last item gets list of remainder } unpack_T; +// arguments to ISN_LOADOUTER and ISN_STOREOUTER +typedef struct { + int outer_idx; // index + int outer_depth; // nesting level, stack frames to go up +} outer_T; + /* * Instruction */ @@ -342,6 +348,7 @@ struct isn_S { put_T put; cmod_T cmdmod; unpack_T unpack; + outer_T outer; } isn_arg; }; diff --git a/src/vim9compile.c b/src/vim9compile.c --- a/src/vim9compile.c +++ b/src/vim9compile.c @@ -108,7 +108,7 @@ typedef struct { char_u *lv_name; type_T *lv_type; int lv_idx; // index of the variable on the stack - int lv_from_outer; // when TRUE using ctx_outer scope + int lv_from_outer; // nesting level, using ctx_outer scope int lv_const; // when TRUE cannot be assigned to int lv_arg; // when TRUE this is an argument } lvar_T; @@ -149,7 +149,7 @@ static void delete_def_function_contents /* * Lookup variable "name" in the local scope and return it in "lvar". - * "lvar->lv_from_outer" is set accordingly. + * "lvar->lv_from_outer" is incremented accordingly. * If "lvar" is NULL only check if the variable can be found. * Return FAIL if not found. */ @@ -172,7 +172,7 @@ lookup_local(char_u *name, size_t len, l if (lvar != NULL) { *lvar = *lvp; - lvar->lv_from_outer = FALSE; + lvar->lv_from_outer = 0; } return OK; } @@ -186,7 +186,7 @@ lookup_local(char_u *name, size_t len, l if (lvar != NULL) { cctx->ctx_outer_used = TRUE; - lvar->lv_from_outer = TRUE; + ++lvar->lv_from_outer; } return OK; } @@ -258,7 +258,7 @@ arg_exists( if (arg_exists(name, len, idxp, type, gen_load_outer, cctx->ctx_outer) == OK) { - *gen_load_outer = TRUE; + ++*gen_load_outer; return OK; } } @@ -1176,6 +1176,23 @@ generate_STORE(cctx_T *cctx, isntype_T i } /* + * Generate an ISN_STOREOUTER instruction. + */ + static int +generate_STOREOUTER(cctx_T *cctx, int idx, int level) +{ + isn_T *isn; + + RETURN_OK_IF_SKIP(cctx); + if ((isn = generate_instr_drop(cctx, ISN_STOREOUTER, 1)) == NULL) + return FAIL; + isn->isn_arg.outer.outer_idx = idx; + isn->isn_arg.outer.outer_depth = level; + + return OK; +} + +/* * Generate an ISN_STORENR instruction (short for ISN_PUSHNR + ISN_STORE) */ static int @@ -1234,6 +1251,27 @@ generate_LOAD( } /* + * Generate an ISN_LOADOUTER instruction + */ + static int +generate_LOADOUTER( + cctx_T *cctx, + int idx, + int nesting, + type_T *type) +{ + isn_T *isn; + + RETURN_OK_IF_SKIP(cctx); + if ((isn = generate_instr_type(cctx, ISN_LOADOUTER, type)) == NULL) + return FAIL; + isn->isn_arg.outer.outer_idx = idx; + isn->isn_arg.outer.outer_depth = nesting; + + return OK; +} + +/* * Generate an ISN_LOADV instruction for v:var. */ static int @@ -1439,6 +1477,11 @@ generate_FUNCREF(cctx_T *cctx, ufunc_T * isn->isn_arg.funcref.fr_func = ufunc->uf_dfunc_idx; cctx->ctx_has_closure = 1; + // if the referenced function is a closure, it may use items further up in + // the nested context, including this one. + if (ufunc->uf_flags & FC_CLOSURE) + cctx->ctx_ufunc->uf_flags |= FC_CLOSURE; + if (ga_grow(stack, 1) == FAIL) return FAIL; ((type_T **)stack->ga_data)[stack->ga_len] = @@ -2589,7 +2632,7 @@ compile_load( size_t len = end - *arg; int idx; int gen_load = FALSE; - int gen_load_outer = FALSE; + int gen_load_outer = 0; name = vim_strnsave(*arg, end - *arg); if (name == NULL) @@ -2597,7 +2640,7 @@ compile_load( if (arg_exists(*arg, len, &idx, &type, &gen_load_outer, cctx) == OK) { - if (!gen_load_outer) + if (gen_load_outer == 0) gen_load = TRUE; } else @@ -2608,8 +2651,8 @@ compile_load( { type = lvar.lv_type; idx = lvar.lv_idx; - if (lvar.lv_from_outer) - gen_load_outer = TRUE; + if (lvar.lv_from_outer != 0) + gen_load_outer = lvar.lv_from_outer; else gen_load = TRUE; } @@ -2631,9 +2674,9 @@ compile_load( } if (gen_load) res = generate_LOAD(cctx, ISN_LOAD, idx, NULL, type); - if (gen_load_outer) - { - res = generate_LOAD(cctx, ISN_LOADOUTER, idx, NULL, type); + if (gen_load_outer > 0) + { + res = generate_LOADOUTER(cctx, idx, gen_load_outer, type); cctx->ctx_outer_used = TRUE; } } @@ -5120,9 +5163,9 @@ generate_loadvar( generate_LOADV(cctx, name + 2, TRUE); break; case dest_local: - if (lvar->lv_from_outer) - generate_LOAD(cctx, ISN_LOADOUTER, lvar->lv_idx, - NULL, type); + if (lvar->lv_from_outer > 0) + generate_LOADOUTER(cctx, lvar->lv_idx, lvar->lv_from_outer, + type); else generate_LOAD(cctx, ISN_LOAD, lvar->lv_idx, NULL, type); break; @@ -6178,7 +6221,7 @@ compile_assignment(char_u *arg, exarg_T // optimization: turn "var = 123" from ISN_PUSHNR + // ISN_STORE into ISN_STORENR - if (!lhs.lhs_lvar->lv_from_outer + if (lhs.lhs_lvar->lv_from_outer == 0 && instr->ga_len == instr_count + 1 && isn->isn_type == ISN_PUSHNR) { @@ -6190,9 +6233,9 @@ compile_assignment(char_u *arg, exarg_T if (stack->ga_len > 0) --stack->ga_len; } - else if (lhs.lhs_lvar->lv_from_outer) - generate_STORE(cctx, ISN_STOREOUTER, - lhs.lhs_lvar->lv_idx, NULL); + else if (lhs.lhs_lvar->lv_from_outer > 0) + generate_STOREOUTER(cctx, lhs.lhs_lvar->lv_idx, + lhs.lhs_lvar->lv_from_outer); else generate_STORE(cctx, ISN_STORE, lhs.lhs_lvar->lv_idx, NULL); } diff --git a/src/vim9execute.c b/src/vim9execute.c --- a/src/vim9execute.c +++ b/src/vim9execute.c @@ -60,6 +60,8 @@ struct ectx_S { garray_T *ec_outer_stack; // stack used for closures int ec_outer_frame; // stack frame in ec_outer_stack + garray_T *ec_outer_up_stack; // ec_outer_stack one level up + int ec_outer_up_frame; // ec_outer_frame one level up garray_T ec_trystack; // stack of trycmd_T values int ec_in_catch; // when TRUE in catch or finally block @@ -149,6 +151,8 @@ exe_newlist(int count, ectx_T *ectx) /* * Call compiled function "cdf_idx" from compiled code. + * This adds a stack frame and sets the instruction pointer to the start of the + * called function. * * Stack has: * - current arguments (already there) @@ -248,6 +252,7 @@ call_dfunc(int cdf_idx, int argcount_arg STACK_TV_BOT(2)->vval.v_string = (void *)ectx->ec_outer_stack; STACK_TV_BOT(3)->vval.v_number = ectx->ec_outer_frame; STACK_TV_BOT(4)->vval.v_number = ectx->ec_frame_idx; + // TODO: save ec_outer_up_stack as well? ectx->ec_frame_idx = ectx->ec_stack.ga_len; // Initialize local variables @@ -266,6 +271,15 @@ call_dfunc(int cdf_idx, int argcount_arg { ectx->ec_outer_stack = ufunc->uf_partial->pt_ectx_stack; ectx->ec_outer_frame = ufunc->uf_partial->pt_ectx_frame; + ectx->ec_outer_up_stack = ufunc->uf_partial->pt_outer_stack; + ectx->ec_outer_up_frame = ufunc->uf_partial->pt_outer_frame; + } + else if (ufunc->uf_flags & FC_CLOSURE) + { + ectx->ec_outer_stack = &ectx->ec_stack; + ectx->ec_outer_frame = ectx->ec_frame_idx; + ectx->ec_outer_up_stack = ectx->ec_outer_stack; + ectx->ec_outer_up_frame = ectx->ec_outer_frame; } // Set execution state to the start of the called function. @@ -417,6 +431,8 @@ handle_closure_in_use(ectx_T *ectx, int pt->pt_funcstack = funcstack; pt->pt_ectx_stack = &funcstack->fs_ga; pt->pt_ectx_frame = ectx->ec_frame_idx - top; + pt->pt_outer_stack = ectx->ec_outer_stack; + pt->pt_outer_frame = ectx->ec_outer_frame; } } } @@ -598,6 +614,9 @@ call_bfunc(int func_idx, int argcount, e /* * Execute a user defined function. + * If the function is compiled this will add a stack frame and set the + * instruction pointer at the start of the function. + * Otherwise the function is called here. * "iptr" can be used to replace the instruction with a more efficient one. */ static int @@ -743,11 +762,18 @@ call_partial(typval_T *tv, int argcount_ if (pt->pt_func != NULL) { + int frame_idx = ectx->ec_frame_idx; int ret = call_ufunc(pt->pt_func, argcount, ectx, NULL); - // closure may need the function context where it was defined - ectx->ec_outer_stack = pt->pt_ectx_stack; - ectx->ec_outer_frame = pt->pt_ectx_frame; + if (ectx->ec_frame_idx != frame_idx) + { + // call_dfunc() added a stack frame, closure may need the + // function context where it was defined. + ectx->ec_outer_stack = pt->pt_ectx_stack; + ectx->ec_outer_frame = pt->pt_ectx_frame; + ectx->ec_outer_up_stack = pt->pt_outer_stack; + ectx->ec_outer_up_frame = pt->pt_outer_frame; + } return ret; } @@ -1041,6 +1067,8 @@ fill_partial_and_closure(partial_T *pt, // variables in the current stack. pt->pt_ectx_stack = &ectx->ec_stack; pt->pt_ectx_frame = ectx->ec_frame_idx; + pt->pt_outer_stack = ectx->ec_outer_stack; + pt->pt_outer_frame = ectx->ec_outer_frame; // If this function returns and the closure is still // being used, we need to make a copy of the context @@ -1220,17 +1248,23 @@ call_def_function( // TODO: is this always the right way? ectx.ec_outer_stack = ¤t_ectx->ec_stack; ectx.ec_outer_frame = current_ectx->ec_frame_idx; + ectx.ec_outer_up_stack = current_ectx->ec_outer_stack; + ectx.ec_outer_up_frame = current_ectx->ec_outer_frame; } else { ectx.ec_outer_stack = partial->pt_ectx_stack; ectx.ec_outer_frame = partial->pt_ectx_frame; + ectx.ec_outer_up_stack = partial->pt_outer_stack; + ectx.ec_outer_up_frame = partial->pt_outer_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; + ectx.ec_outer_up_stack = ufunc->uf_partial->pt_outer_stack; + ectx.ec_outer_up_frame = ufunc->uf_partial->pt_outer_frame; } // dummy frame entries @@ -1514,11 +1548,30 @@ call_def_function( // load variable or argument from outer scope case ISN_LOADOUTER: - if (GA_GROW(&ectx.ec_stack, 1) == FAIL) - goto failed; - copy_tv(STACK_OUT_TV_VAR(iptr->isn_arg.number), + { + typval_T *stack; + int depth = iptr->isn_arg.outer.outer_depth; + + if (GA_GROW(&ectx.ec_stack, 1) == FAIL) + goto failed; + if (depth <= 1) + stack = ((typval_T *)ectx.ec_outer_stack->ga_data) + + ectx.ec_outer_frame; + else if (depth == 2) + stack = ((typval_T *)ectx.ec_outer_up_stack->ga_data) + + ectx.ec_outer_up_frame; + else + { + SOURCING_LNUM = iptr->isn_lnum; + iemsg("LOADOUTER level > 2 not supported yet"); + goto failed; + } + + copy_tv(stack + STACK_FRAME_SIZE + + iptr->isn_arg.outer.outer_idx, STACK_TV_BOT(0)); - ++ectx.ec_stack.ga_len; + ++ectx.ec_stack.ga_len; + } break; // load v: variable @@ -1719,7 +1772,8 @@ call_def_function( // store variable or argument in outer scope case ISN_STOREOUTER: --ectx.ec_stack.ga_len; - tv = STACK_OUT_TV_VAR(iptr->isn_arg.number); + // TODO: use outer_depth + tv = STACK_OUT_TV_VAR(iptr->isn_arg.outer.outer_idx); clear_tv(tv); *tv = *STACK_TV_BOT(0); break; @@ -3622,17 +3676,27 @@ ex_disassemble(exarg_T *eap) (varnumber_T)(iptr->isn_arg.number)); break; case ISN_LOAD: - case ISN_LOADOUTER: { - char *add = iptr->isn_type == ISN_LOAD ? "" : "OUTER"; - if (iptr->isn_arg.number < 0) - smsg("%4d LOAD%s arg[%lld]", current, add, + smsg("%4d LOAD arg[%lld]", current, (varnumber_T)(iptr->isn_arg.number + STACK_FRAME_SIZE)); else - smsg("%4d LOAD%s $%lld", current, add, - (varnumber_T)(iptr->isn_arg.number)); + smsg("%4d LOAD $%lld", current, + (varnumber_T)(iptr->isn_arg.number)); + } + break; + case ISN_LOADOUTER: + { + if (iptr->isn_arg.number < 0) + smsg("%4d LOADOUTER level %d arg[%d]", current, + iptr->isn_arg.outer.outer_depth, + iptr->isn_arg.outer.outer_idx + + STACK_FRAME_SIZE); + else + smsg("%4d LOADOUTER level %d $%d", current, + iptr->isn_arg.outer.outer_depth, + iptr->isn_arg.outer.outer_idx); } break; case ISN_LOADV: @@ -3699,16 +3763,22 @@ ex_disassemble(exarg_T *eap) break; case ISN_STORE: + if (iptr->isn_arg.number < 0) + smsg("%4d STORE arg[%lld]", current, + iptr->isn_arg.number + STACK_FRAME_SIZE); + else + smsg("%4d STORE $%lld", current, iptr->isn_arg.number); + break; case ISN_STOREOUTER: { - char *add = iptr->isn_type == ISN_STORE ? "" : "OUTER"; - if (iptr->isn_arg.number < 0) - smsg("%4d STORE%s arg[%lld]", current, add, - (varnumber_T)(iptr->isn_arg.number + STACK_FRAME_SIZE)); + smsg("%4d STOREOUTEr level %d arg[%d]", current, + iptr->isn_arg.outer.outer_depth, + iptr->isn_arg.outer.outer_idx + STACK_FRAME_SIZE); else - smsg("%4d STORE%s $%lld", current, add, - (varnumber_T)(iptr->isn_arg.number)); + smsg("%4d STOREOUTER level %d $%d", current, + iptr->isn_arg.outer.outer_depth, + iptr->isn_arg.outer.outer_idx); } break; case ISN_STOREV: