]> granicus.if.org Git - vim/commitdiff
patch 8.2.2321: Vim9: cannot nest closures v8.2.2321
authorBram Moolenaar <Bram@vim.org>
Sun, 10 Jan 2021 13:02:28 +0000 (14:02 +0100)
committerBram Moolenaar <Bram@vim.org>
Sun, 10 Jan 2021 13:02:28 +0000 (14:02 +0100)
Problem:    Vim9: cannot nest closures.
Solution:   Add the nesting level to ISN_LOADOUTER and ISN_STOREOUTER.
            (closes #7150, closes #7635)

src/structs.h
src/testdir/test_vim9_disassemble.vim
src/testdir/test_vim9_func.vim
src/version.c
src/vim9.h
src/vim9compile.c
src/vim9execute.c

index 9d1f82181825ef11f1d6ed4c5b91ee3f5f8d141d..1ed185d5b41b0833815b6fa077f1f0509851c88e 100644 (file)
@@ -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
 
index f25b04ae3bb3deb5d57d74559136e106cd949cc3..ca9b90e8860d0d5cef8dde69c83e4aeabc7379ee 100644 (file)
@@ -567,17 +567,17 @@ def Test_disassemble_closure()
   var res = execute('disass g:Append')
   assert_match('<lambda>\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('<lambda>\d\_s*' ..
         'return local\_s*' ..
-        '\d LOADOUTER $0\_s*' ..
+        '\d LOADOUTER level 1 $0\_s*' ..
         '\d RETURN',
         res)
 
index 3d4f4c9b74112cd7205831dd026cee4cc68c532a..efbf3d52c17ca18cbc7878fdb2dd146eec998728 100644 (file)
@@ -1812,6 +1812,16 @@ def Test_list_lambda()
   assert_match('def <lambda>\d\+(_: any, ...): number\n1  return 0\n   enddef', body)
 enddef
 
+def DoFilterThis(a: string): list<string>
+  # 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
 
index 2ef443b239b3eaccec346b3d1a047bcc74673d66..891126f4d12f06faa60e06ee02ce9787cdf87195 100644 (file)
@@ -750,6 +750,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    2321,
 /**/
     2320,
 /**/
index 7462b29db0a180ef2b3a7cc71a884fc2d0c36b97..52247540af633b3d6afc121ef55c36897e353ff9 100644 (file)
@@ -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;
 };
 
index 77200205dffca7c1e85bd1a9bba0660e6da34031..a6bb56571efe599e3b193e92d84fbdeb9f4b6d8d 100644 (file)
@@ -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(dfunc_T *dfunc, int mark_deleted);
 
 /*
  * 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, lvar_T *lvar, cctx_T *cctx)
            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, lvar_T *lvar, cctx_T *cctx)
            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;
        }
     }
@@ -1175,6 +1175,23 @@ generate_STORE(cctx_T *cctx, isntype_T isn_type, int idx, char_u *name)
     return OK;
 }
 
+/*
+ * 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)
  */
@@ -1233,6 +1250,27 @@ generate_LOAD(
     return OK;
 }
 
+/*
+ * 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.
  */
@@ -1439,6 +1477,11 @@ generate_FUNCREF(cctx_T *cctx, ufunc_T *ufunc)
     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)
+       if (gen_load_outer > 0)
        {
-           res = generate_LOAD(cctx, ISN_LOADOUTER, idx, NULL, type);
+           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 *eap, cmdidx_T cmdidx, cctx_T *cctx)
 
                // 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 *eap, cmdidx_T cmdidx, cctx_T *cctx)
                    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);
            }
index 9a04b6b8181b49013826941fc44bf8620314e826..ca702f67d3ca6baa09a16a8886c17648d63eff6d 100644 (file)
@@ -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, ectx_T *ectx)
     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_T *ectx)
     {
        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 free_arguments)
                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, ectx_T *ectx)
 
 /*
  * 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_arg, ectx_T *ectx)
 
        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, ufunc_T *ufunc, ectx_T *ectx)
        // 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 = &current_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: