To: vim_dev@googlegroups.com Subject: Patch 8.2.2321 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ------------ Patch 8.2.2321 Problem: Vim9: cannot nest closures. Solution: Add the nesting level to ISN_LOADOUTER and ISN_STOREOUTER. (closes #7150, closes #7635) Files: src/vim9.h, src/vim9compile.c, src/vim9execute.c, src/structs.h, src/testdir/test_vim9_disassemble.vim, src/testdir/test_vim9_func.vim *** ../vim-8.2.2320/src/vim9.h 2021-01-09 15:45:20.353451132 +0100 --- src/vim9.h 2021-01-09 18:42:27.619303000 +0100 *************** *** 33,39 **** 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_LOADSCRIPT, // push script-local variable isn_arg.script. ISN_LOADOPT, // push option isn_arg.string ISN_LOADENV, // push environment variable isn_arg.string --- 33,39 ---- 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.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,53 **** 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_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 --- 47,53 ---- 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.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,308 **** --- 303,314 ---- 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,347 **** --- 348,354 ---- put_T put; cmod_T cmdmod; unpack_T unpack; + outer_T outer; } isn_arg; }; *** ../vim-8.2.2320/src/vim9compile.c 2021-01-09 15:45:20.353451132 +0100 --- src/vim9compile.c 2021-01-10 13:58:31.034647994 +0100 *************** *** 108,114 **** 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_const; // when TRUE cannot be assigned to int lv_arg; // when TRUE this is an argument } lvar_T; --- 108,114 ---- char_u *lv_name; type_T *lv_type; int lv_idx; // index of the variable on the stack ! 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,155 **** /* * Lookup variable "name" in the local scope and return it in "lvar". ! * "lvar->lv_from_outer" is set accordingly. * If "lvar" is NULL only check if the variable can be found. * Return FAIL if not found. */ --- 149,155 ---- /* * Lookup variable "name" in the local scope and return it in "lvar". ! * "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,178 **** if (lvar != NULL) { *lvar = *lvp; ! lvar->lv_from_outer = FALSE; } return OK; } --- 172,178 ---- if (lvar != NULL) { *lvar = *lvp; ! lvar->lv_from_outer = 0; } return OK; } *************** *** 186,192 **** if (lvar != NULL) { cctx->ctx_outer_used = TRUE; ! lvar->lv_from_outer = TRUE; } return OK; } --- 186,192 ---- if (lvar != NULL) { cctx->ctx_outer_used = TRUE; ! ++lvar->lv_from_outer; } return OK; } *************** *** 258,264 **** if (arg_exists(name, len, idxp, type, gen_load_outer, cctx->ctx_outer) == OK) { ! *gen_load_outer = TRUE; return OK; } } --- 258,264 ---- if (arg_exists(name, len, idxp, type, gen_load_outer, cctx->ctx_outer) == OK) { ! ++*gen_load_outer; return OK; } } *************** *** 1176,1181 **** --- 1176,1198 ---- } /* + * 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,1239 **** --- 1251,1277 ---- } /* + * 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,1444 **** --- 1477,1487 ---- 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,2595 **** size_t len = end - *arg; int idx; int gen_load = FALSE; ! int gen_load_outer = FALSE; name = vim_strnsave(*arg, end - *arg); if (name == NULL) --- 2632,2638 ---- size_t len = end - *arg; int idx; int gen_load = FALSE; ! int gen_load_outer = 0; name = vim_strnsave(*arg, end - *arg); if (name == NULL) *************** *** 2597,2603 **** if (arg_exists(*arg, len, &idx, &type, &gen_load_outer, cctx) == OK) { ! if (!gen_load_outer) gen_load = TRUE; } else --- 2640,2646 ---- if (arg_exists(*arg, len, &idx, &type, &gen_load_outer, cctx) == OK) { ! if (gen_load_outer == 0) gen_load = TRUE; } else *************** *** 2608,2615 **** { type = lvar.lv_type; idx = lvar.lv_idx; ! if (lvar.lv_from_outer) ! gen_load_outer = TRUE; else gen_load = TRUE; } --- 2651,2658 ---- { type = lvar.lv_type; idx = lvar.lv_idx; ! if (lvar.lv_from_outer != 0) ! gen_load_outer = lvar.lv_from_outer; else gen_load = TRUE; } *************** *** 2631,2639 **** } 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); cctx->ctx_outer_used = TRUE; } } --- 2674,2682 ---- } if (gen_load) res = generate_LOAD(cctx, ISN_LOAD, idx, NULL, type); ! if (gen_load_outer > 0) { ! res = generate_LOADOUTER(cctx, idx, gen_load_outer, type); cctx->ctx_outer_used = TRUE; } } *************** *** 5120,5128 **** 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); else generate_LOAD(cctx, ISN_LOAD, lvar->lv_idx, NULL, type); break; --- 5163,5171 ---- generate_LOADV(cctx, name + 2, TRUE); break; case dest_local: ! 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,6184 **** // optimization: turn "var = 123" from ISN_PUSHNR + // ISN_STORE into ISN_STORENR ! if (!lhs.lhs_lvar->lv_from_outer && instr->ga_len == instr_count + 1 && isn->isn_type == ISN_PUSHNR) { --- 6221,6227 ---- // optimization: turn "var = 123" from ISN_PUSHNR + // ISN_STORE into ISN_STORENR ! if (lhs.lhs_lvar->lv_from_outer == 0 && instr->ga_len == instr_count + 1 && isn->isn_type == ISN_PUSHNR) { *************** *** 6190,6198 **** 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 generate_STORE(cctx, ISN_STORE, lhs.lhs_lvar->lv_idx, NULL); } --- 6233,6241 ---- if (stack->ga_len > 0) --stack->ga_len; } ! 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); } *** ../vim-8.2.2320/src/vim9execute.c 2021-01-09 15:45:20.353451132 +0100 --- src/vim9execute.c 2021-01-10 13:36:30.598677638 +0100 *************** *** 60,65 **** --- 60,67 ---- 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,154 **** --- 151,158 ---- /* * 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,253 **** --- 252,258 ---- 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,271 **** --- 271,285 ---- { 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,422 **** --- 431,438 ---- 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,603 **** --- 614,622 ---- /* * 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,753 **** if (pt->pt_func != NULL) { 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; return ret; } --- 762,779 ---- if (pt->pt_func != NULL) { + int frame_idx = ectx->ec_frame_idx; int ret = call_ufunc(pt->pt_func, argcount, ectx, NULL); ! 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,1046 **** --- 1067,1074 ---- // 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,1236 **** --- 1248,1270 ---- // 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,1524 **** // 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), STACK_TV_BOT(0)); ! ++ectx.ec_stack.ga_len; break; // load v: variable --- 1548,1577 ---- // load variable or argument from outer scope case ISN_LOADOUTER: ! { ! 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; ! } break; // load v: variable *************** *** 1719,1725 **** // store variable or argument in outer scope case ISN_STOREOUTER: --ectx.ec_stack.ga_len; ! tv = STACK_OUT_TV_VAR(iptr->isn_arg.number); clear_tv(tv); *tv = *STACK_TV_BOT(0); break; --- 1772,1779 ---- // store variable or argument in outer scope case ISN_STOREOUTER: --ectx.ec_stack.ga_len; ! // 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,3638 **** (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, (varnumber_T)(iptr->isn_arg.number + STACK_FRAME_SIZE)); else ! smsg("%4d LOAD%s $%lld", current, add, ! (varnumber_T)(iptr->isn_arg.number)); } break; case ISN_LOADV: --- 3676,3702 ---- (varnumber_T)(iptr->isn_arg.number)); break; case ISN_LOAD: { if (iptr->isn_arg.number < 0) ! smsg("%4d LOAD arg[%lld]", current, (varnumber_T)(iptr->isn_arg.number + STACK_FRAME_SIZE)); else ! 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,3714 **** break; case ISN_STORE: 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)); else ! smsg("%4d STORE%s $%lld", current, add, ! (varnumber_T)(iptr->isn_arg.number)); } break; case ISN_STOREV: --- 3763,3784 ---- 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: { if (iptr->isn_arg.number < 0) ! 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 STOREOUTER level %d $%d", current, ! iptr->isn_arg.outer.outer_depth, ! iptr->isn_arg.outer.outer_idx); } break; case ISN_STOREV: *** ../vim-8.2.2320/src/structs.h 2021-01-09 15:45:20.353451132 +0100 --- src/structs.h 2021-01-10 12:15:48.701453238 +0100 *************** *** 1978,1983 **** --- 1978,1985 ---- // 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 *** ../vim-8.2.2320/src/testdir/test_vim9_disassemble.vim 2021-01-08 20:53:05.736404852 +0100 --- src/testdir/test_vim9_disassemble.vim 2021-01-09 19:13:45.530512401 +0100 *************** *** 567,583 **** var res = execute('disass g:Append') assert_match('\d\_s*' .. 'local ..= arg\_s*' .. ! '\d LOADOUTER $0\_s*' .. '\d LOAD arg\[-1\]\_s*' .. '\d CONCAT\_s*' .. ! '\d STOREOUTER $0\_s*' .. '\d RETURN 0', res) res = execute('disass g:Get') assert_match('\d\_s*' .. 'return local\_s*' .. ! '\d LOADOUTER $0\_s*' .. '\d RETURN', res) --- 567,583 ---- var res = execute('disass g:Append') assert_match('\d\_s*' .. 'local ..= arg\_s*' .. ! '\d LOADOUTER level 1 $0\_s*' .. '\d LOAD arg\[-1\]\_s*' .. '\d CONCAT\_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 level 1 $0\_s*' .. '\d RETURN', res) *** ../vim-8.2.2320/src/testdir/test_vim9_func.vim 2021-01-08 22:24:16.467725889 +0100 --- src/testdir/test_vim9_func.vim 2021-01-10 13:52:43.599603841 +0100 *************** *** 1812,1817 **** --- 1812,1827 ---- 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 *** ../vim-8.2.2320/src/version.c 2021-01-09 16:21:34.000353394 +0100 --- src/version.c 2021-01-09 18:43:59.655077205 +0100 *************** *** 752,753 **** --- 752,755 ---- { /* Add new patch number below this line */ + /**/ + 2321, /**/ -- hundred-and-one symptoms of being an internet addict: 119. You are reading a book and look for the scroll bar to get to the next page. /// 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 ///