patch 8.2.2322: Vim9: closure nested limiting to one level

Problem:    Vim9: closure nested limiting to one level.
Solution:   Add outer_T.  Also make STOREOUTER work.

diff --git a/src/structs.h b/src/structs.h
index 1ed185d5b41b0833815b6fa077f1f0509851c88e..712382a6f6fa28d32c485489acec5fdf3719cab1 100644 (file)
--- a/src/structs.h
+++ b/src/structs.h
@@ -1965,6 +1965,14 @@ typedef struct funcstack_S
     int                fs_copyID;      // for garray_T collection
 } funcstack_T;
 
+typedef struct outer_S outer_T;
+struct outer_S {
+    garray_T   *out_stack;         // stack from outer scope
+    int                out_frame_idx;      // index of stack frame in out_stack
+    outer_T    *out_up;            // outer scope of outer scope or NULL
+    int                out_up_is_copy;     // don't free out_up
+};
+
 struct partial_S
 {
     int                pt_refcount;    // reference count
@@ -1975,13 +1983,11 @@ struct partial_S
     int                pt_auto;        // when TRUE the partial was created for using
                                // dict.member in handle_subscript()
 
-    // 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
+    // For a compiled closure: the arguments and local variables scope
+    outer_T    pt_outer;
+
+    funcstack_T        *pt_funcstack;  // copy of stack, used after context
+                               // function returns
 
     int                pt_argc;        // number of arguments
     typval_T   *pt_argv;       // arguments in allocated array

diff --git a/src/testdir/test_vim9_func.vim b/src/testdir/test_vim9_func.vim
index efbf3d52c17ca18cbc7878fdb2dd146eec998728..fdad359de00ed8c558134948fdf7bc168a4c54d7 100644 (file)
--- a/src/testdir/test_vim9_func.vim
+++ b/src/testdir/test_vim9_func.vim
@@ -1822,6 +1822,13 @@ def Test_nested_closure_using_argument()
   assert_equal(['x', 'x2'], DoFilterThis('x'))
 enddef
 
+def Test_triple_nested_closure()
+  var what = 'x'
+  var Match = (val: string, cmp: string): bool => stridx(val, cmp) == 0
+  var Filter = (l) => filter(l, (_, v) => Match(v, what))
+  assert_equal(['x', 'x2'], ['x', 'y', 'a', 'x2', 'c']->Filter())
+enddef
+
 func Test_silent_echo()
   CheckScreendump
 

diff --git a/src/version.c b/src/version.c
index 891126f4d12f06faa60e06ee02ce9787cdf87195..cdf27fac7bd07d6eb062fbe9efebee69307e3b4b 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -750,6 +750,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    2322,
 /**/
     2321,
 /**/

diff --git a/src/vim9.h b/src/vim9.h
index 52247540af633b3d6afc121ef55c36897e353ff9..b0c465deb2a168178af9cde4ae4a6d86ac0de73b 100644 (file)
--- a/src/vim9.h
+++ b/src/vim9.h
@@ -307,7 +307,7 @@ typedef struct {
 typedef struct {
     int                outer_idx;      // index
     int                outer_depth;    // nesting level, stack frames to go up
-} outer_T;
+} isn_outer_T;
 
 /*
  * Instruction
@@ -348,7 +348,7 @@ struct isn_S {
        put_T               put;
        cmod_T              cmdmod;
        unpack_T            unpack;
-       outer_T             outer;
+       isn_outer_T         outer;
     } isn_arg;
 };
 
@@ -375,10 +375,13 @@ struct dfunc_S {
 // Number of entries used by stack frame for a function call.
 // - ec_dfunc_idx:   function index
 // - ec_iidx:        instruction index
-// - ec_outer_stack: stack used for closures  TODO: can we avoid this?
-// - ec_outer_frame: stack frame for closures
+// - ec_outer:      stack used for closures
 // - ec_frame_idx:   previous frame index
-#define STACK_FRAME_SIZE 5
+#define STACK_FRAME_FUNC_OFF 0
+#define STACK_FRAME_IIDX_OFF 1
+#define STACK_FRAME_OUTER_OFF 2
+#define STACK_FRAME_IDX_OFF 3
+#define STACK_FRAME_SIZE 4
 
 
 #ifdef DEFINE_VIM9_GLOBALS

diff --git a/src/vim9execute.c b/src/vim9execute.c
index ca702f67d3ca6baa09a16a8886c17648d63eff6d..957988e65859af2f0304d3be560d2c1539743010 100644 (file)
--- a/src/vim9execute.c
+++ b/src/vim9execute.c
@@ -58,10 +58,7 @@ struct ectx_S {
     garray_T   ec_stack;       // stack of typval_T values
     int                ec_frame_idx;   // index in ec_stack: context of ec_dfunc_idx
 
-    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
+    outer_T    *ec_outer;      // outer scope used for closures, allocated
 
     garray_T   ec_trystack;    // stack of trycmd_T values
     int                ec_in_catch;    // when TRUE in catch or finally block
@@ -153,6 +150,7 @@ 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.
+ * If "pt" is not null use "pt->pt_outer" for ec_outer.
  *
  * Stack has:
  * - current arguments (already there)
@@ -164,7 +162,7 @@ exe_newlist(int count, ectx_T *ectx)
  * - reserved space for local variables
  */
     static int
-call_dfunc(int cdf_idx, int argcount_arg, ectx_T *ectx)
+call_dfunc(int cdf_idx, partial_T *pt, int argcount_arg, ectx_T *ectx)
 {
     int            argcount = argcount_arg;
     dfunc_T *dfunc = ((dfunc_T *)def_functions.ga_data) + cdf_idx;
@@ -247,12 +245,12 @@ call_dfunc(int cdf_idx, int argcount_arg, ectx_T *ectx)
     ectx->ec_stack.ga_len += arg_to_add;
 
     // Store current execution state in stack frame for ISN_RETURN.
-    STACK_TV_BOT(0)->vval.v_number = ectx->ec_dfunc_idx;
-    STACK_TV_BOT(1)->vval.v_number = ectx->ec_iidx;
-    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?
+    STACK_TV_BOT(STACK_FRAME_FUNC_OFF)->vval.v_number = ectx->ec_dfunc_idx;
+    STACK_TV_BOT(STACK_FRAME_IIDX_OFF)->vval.v_number = ectx->ec_iidx;
+    if (ectx->ec_outer != NULL)
+       printf("here");
+    STACK_TV_BOT(STACK_FRAME_OUTER_OFF)->vval.v_string = (void *)ectx->ec_outer;
+    STACK_TV_BOT(STACK_FRAME_IDX_OFF)->vval.v_number = ectx->ec_frame_idx;
     ectx->ec_frame_idx = ectx->ec_stack.ga_len;
 
     // Initialize local variables
@@ -267,20 +265,32 @@ call_dfunc(int cdf_idx, int argcount_arg, ectx_T *ectx)
     }
     ectx->ec_stack.ga_len += STACK_FRAME_SIZE + varcount;
 
-    if (ufunc->uf_partial != NULL)
+    if (pt != NULL || ufunc->uf_partial != NULL || ufunc->uf_flags & FC_CLOSURE)
     {
-       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;
+       outer_T *outer = ALLOC_CLEAR_ONE(outer_T);
+
+       if (outer == NULL)
+           return FAIL;
+       if (pt != NULL)
+       {
+           *outer = pt->pt_outer;
+           outer->out_up_is_copy = TRUE;
+       }
+       else if (ufunc->uf_partial != NULL)
+       {
+           *outer = ufunc->uf_partial->pt_outer;
+           outer->out_up_is_copy = TRUE;
+       }
+       else
+       {
+           outer->out_stack = &ectx->ec_stack;
+           outer->out_frame_idx = ectx->ec_frame_idx;
+           outer->out_up = ectx->ec_outer;
+       }
+       ectx->ec_outer = outer;
     }
+    else
+       ectx->ec_outer = NULL;
 
     // Set execution state to the start of the called function.
     ectx->ec_dfunc_idx = cdf_idx;
@@ -429,10 +439,9 @@ handle_closure_in_use(ectx_T *ectx, int free_arguments)
            {
                ++funcstack->fs_refcount;
                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;
+               pt->pt_outer.out_stack = &funcstack->fs_ga;
+               pt->pt_outer.out_frame_idx = ectx->ec_frame_idx - top;
+               pt->pt_outer.out_up = ectx->ec_outer;
            }
        }
     }
@@ -518,17 +527,25 @@ func_return(ectx_T *ectx)
     // The return value should be on top of the stack.  However, when aborting
     // it may not be there and ec_frame_idx is the top of the stack.
     ret_idx = ectx->ec_stack.ga_len - 1;
-    if (ret_idx == ectx->ec_frame_idx + 4)
+    if (ret_idx == ectx->ec_frame_idx + STACK_FRAME_IDX_OFF)
        ret_idx = 0;
 
+    if (ectx->ec_outer != NULL)
+       printf("here");
+    vim_free(ectx->ec_outer);
+
     // Restore the previous frame.
-    ectx->ec_dfunc_idx = STACK_TV(ectx->ec_frame_idx)->vval.v_number;
-    ectx->ec_iidx = STACK_TV(ectx->ec_frame_idx + 1)->vval.v_number;
-    ectx->ec_outer_stack =
-                      (void *)STACK_TV(ectx->ec_frame_idx + 2)->vval.v_string;
-    ectx->ec_outer_frame = STACK_TV(ectx->ec_frame_idx + 3)->vval.v_number;
+    ectx->ec_dfunc_idx = STACK_TV(ectx->ec_frame_idx
+                                       + STACK_FRAME_FUNC_OFF)->vval.v_number;
+    ectx->ec_iidx = STACK_TV(ectx->ec_frame_idx
+                                       + STACK_FRAME_IIDX_OFF)->vval.v_number;
+    ectx->ec_outer = (void *)STACK_TV(ectx->ec_frame_idx
+                                      + STACK_FRAME_OUTER_OFF)->vval.v_string;
     // restoring ec_frame_idx must be last
-    ectx->ec_frame_idx = STACK_TV(ectx->ec_frame_idx + 4)->vval.v_number;
+    ectx->ec_frame_idx = STACK_TV(ectx->ec_frame_idx
+                                      + STACK_FRAME_IDX_OFF)->vval.v_number;
+    if (ectx->ec_outer != NULL)
+       printf("here");
     dfunc = ((dfunc_T *)def_functions.ga_data) + ectx->ec_dfunc_idx;
     ectx->ec_instr = dfunc->df_instr;
 
@@ -617,10 +634,16 @@ call_bfunc(int func_idx, int argcount, ectx_T *ectx)
  * 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.
+ * If "pt" is not null use "pt->pt_outer" for ec_outer.
  * "iptr" can be used to replace the instruction with a more efficient one.
  */
     static int
-call_ufunc(ufunc_T *ufunc, int argcount, ectx_T *ectx, isn_T *iptr)
+call_ufunc(
+       ufunc_T     *ufunc,
+       partial_T   *pt,
+       int         argcount,
+       ectx_T      *ectx,
+       isn_T       *iptr)
 {
     typval_T   argvars[MAX_FUNC_ARGS];
     funcexe_T   funcexe;
@@ -653,7 +676,7 @@ call_ufunc(ufunc_T *ufunc, int argcount, ectx_T *ectx, isn_T *iptr)
            iptr->isn_arg.dfunc.cdf_idx = ufunc->uf_dfunc_idx;
            iptr->isn_arg.dfunc.cdf_argcount = argcount;
        }
-       return call_dfunc(ufunc->uf_dfunc_idx, argcount, ectx);
+       return call_dfunc(ufunc->uf_dfunc_idx, pt, argcount, ectx);
     }
 
     if (call_prepare(argcount, argvars, ectx) == FAIL)
@@ -726,7 +749,7 @@ call_by_name(char_u *name, int argcount, ectx_T *ectx, isn_T *iptr)
     }
 
     if (ufunc != NULL)
-       return call_ufunc(ufunc, argcount, ectx, iptr);
+       return call_ufunc(ufunc, NULL, argcount, ectx, iptr);
 
     return FAIL;
 }
@@ -761,22 +784,8 @@ 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);
-
-           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 call_ufunc(pt->pt_func, pt, argcount, ectx, NULL);
 
-           return ret;
-       }
        name = pt->pt_name;
     }
     else if (tv->v_type == VAR_FUNC)
@@ -1065,10 +1074,10 @@ fill_partial_and_closure(partial_T *pt, ufunc_T *ufunc, ectx_T *ectx)
 
        // 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;
-       pt->pt_outer_stack = ectx->ec_outer_stack;
-       pt->pt_outer_frame = ectx->ec_outer_frame;
+       pt->pt_outer.out_stack = &ectx->ec_stack;
+       pt->pt_outer.out_frame_idx = ectx->ec_frame_idx;
+       pt->pt_outer.out_up = ectx->ec_outer;
+       pt->pt_outer.out_up_is_copy = TRUE;
 
        // If this function returns and the closure is still
        // being used, we need to make a copy of the context
@@ -1135,9 +1144,6 @@ call_def_function(
 // Get pointer to a local variable on the stack.  Negative for arguments.
 #define STACK_TV_VAR(idx) (((typval_T *)ectx.ec_stack.ga_data) + ectx.ec_frame_idx + STACK_FRAME_SIZE + idx)
 
-// Like STACK_TV_VAR but use the outer scope
-#define STACK_OUT_TV_VAR(idx) (((typval_T *)ectx.ec_outer_stack->ga_data) + ectx.ec_outer_frame + STACK_FRAME_SIZE + idx)
-
     if (ufunc->uf_def_status == UF_NOT_COMPILED
            || (ufunc->uf_def_status == UF_TO_BE_COMPILED
                          && compile_def_function(ufunc, FALSE, NULL) == FAIL))
@@ -1241,30 +1247,24 @@ call_def_function(
     ectx.ec_frame_idx = ectx.ec_stack.ga_len;
     initial_frame_idx = ectx.ec_frame_idx;
 
-    if (partial != NULL)
+    if (partial != NULL || ufunc->uf_partial != NULL)
     {
-       if (partial->pt_ectx_stack == NULL && current_ectx != NULL)
+       ectx.ec_outer = ALLOC_CLEAR_ONE(outer_T);
+       if (ectx.ec_outer == NULL)
+           goto failed_early;
+       if (partial != NULL)
        {
-           // 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;
+           if (partial->pt_outer.out_stack == NULL && current_ectx != NULL)
+           {
+               if (current_ectx->ec_outer != NULL)
+                   *ectx.ec_outer = *current_ectx->ec_outer;
+           }
+           else
+               *ectx.ec_outer = partial->pt_outer;
        }
        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;
+           *ectx.ec_outer = ufunc->uf_partial->pt_outer;
+       ectx.ec_outer->out_up_is_copy = TRUE;
     }
 
     // dummy frame entries
@@ -1546,34 +1546,6 @@ call_def_function(
                ++ectx.ec_stack.ga_len;
                break;
 
-           // 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
            case ISN_LOADV:
                if (GA_GROW(&ectx.ec_stack, 1) == FAIL)
@@ -1769,15 +1741,6 @@ call_def_function(
                *tv = *STACK_TV_BOT(0);
                break;
 
-           // 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;
-
            // store s: variable in old script
            case ISN_STORES:
                {
@@ -2058,6 +2021,43 @@ call_def_function(
                }
                break;
 
+           // load or store variable or argument from outer scope
+           case ISN_LOADOUTER:
+           case ISN_STOREOUTER:
+               {
+                   int         depth = iptr->isn_arg.outer.outer_depth;
+                   outer_T     *outer = ectx.ec_outer;
+
+                   while (depth > 1 && outer != NULL)
+                   {
+                       outer = outer->out_up;
+                       --depth;
+                   }
+                   if (outer == NULL)
+                   {
+                       SOURCING_LNUM = iptr->isn_lnum;
+                       iemsg("LOADOUTER depth more than scope levels");
+                       goto failed;
+                   }
+                   tv = ((typval_T *)outer->out_stack->ga_data)
+                                   + outer->out_frame_idx + STACK_FRAME_SIZE
+                                   + iptr->isn_arg.outer.outer_idx;
+                   if (iptr->isn_type == ISN_LOADOUTER)
+                   {
+                       if (GA_GROW(&ectx.ec_stack, 1) == FAIL)
+                           goto failed;
+                       copy_tv(tv, STACK_TV_BOT(0));
+                       ++ectx.ec_stack.ga_len;
+                   }
+                   else
+                   {
+                       --ectx.ec_stack.ga_len;
+                       clear_tv(tv);
+                       *tv = *STACK_TV_BOT(0);
+                   }
+               }
+               break;
+
            // unlet item in list or dict variable
            case ISN_UNLETINDEX:
                {
@@ -2296,7 +2296,7 @@ call_def_function(
            // call a :def function
            case ISN_DCALL:
                SOURCING_LNUM = iptr->isn_lnum;
-               if (call_dfunc(iptr->isn_arg.dfunc.cdf_idx,
+               if (call_dfunc(iptr->isn_arg.dfunc.cdf_idx, NULL,
                              iptr->isn_arg.dfunc.cdf_argcount,
                              &ectx) == FAIL)
                    goto on_error;
@@ -3555,6 +3555,15 @@ failed_early:
     vim_free(ectx.ec_stack.ga_data);
     vim_free(ectx.ec_trystack.ga_data);
 
+    while (ectx.ec_outer != NULL)
+    {
+       outer_T     *up = ectx.ec_outer->out_up_is_copy
+                                               ? NULL : ectx.ec_outer->out_up;
+
+       vim_free(ectx.ec_outer);
+       ectx.ec_outer = up;
+    }
+
     // Not sure if this is necessary.
     suppress_errthrow = save_suppress_errthrow;