Patch 8.2.2322

[Bram Moolenaar]

Patch 8.2.2322
Problem: Vim9: closure nested limiting to one level.
Solution: Add outer_T. Also make STOREOUTER work.
Files: src/vim9execute.c, src/vim9.h, src/structs.h,
src/testdir/test_vim9_func.vim


*** ../vim-8.2.2321/src/vim9execute.c 2021-01-10 14:02:24.662205148 +0100
--- src/vim9execute.c 2021-01-10 18:05:18.451829039 +0100
***************
*** 58,67 ****
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

garray_T ec_trystack; // stack of trycmd_T values
int ec_in_catch; // when TRUE in catch or finally block
--- 58,64 ----
garray_T ec_stack; // stack of typval_T values
int ec_frame_idx; // index in ec_stack: context of ec_dfunc_idx

! 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,158 ****
--- 150,156 ----
* 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,170 ****
* - reserved space for local variables
*/
static int
! call_dfunc(int cdf_idx, int argcount_arg, ectx_T *ectx)
{
int argcount = argcount_arg;
dfunc_T *dfunc = ((dfunc_T *)def_functions.ga_data) + cdf_idx;
--- 162,168 ----
* - reserved space for local variables
*/
static int
! 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,258 ****
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?
ectx->ec_frame_idx = ectx->ec_stack.ga_len;

// Initialize local variables
--- 245,256 ----
ectx->ec_stack.ga_len += arg_to_add;

// Store current execution state in stack frame for ISN_RETURN.
! 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,286 ****
}
ectx->ec_stack.ga_len += STACK_FRAME_SIZE + varcount;

! 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;
! }
! 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.
ectx->ec_dfunc_idx = cdf_idx;
--- 265,296 ----
}
ectx->ec_stack.ga_len += STACK_FRAME_SIZE + varcount;

! if (pt != NULL || ufunc->uf_partial != NULL || ufunc->uf_flags & FC_CLOSURE)
{
! 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,438 ****
{
++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;
}
}
}
--- 439,447 ----
{
++funcstack->fs_refcount;
pt->pt_funcstack = funcstack;
! 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,534 ****
// 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)
ret_idx = 0;

// 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;
// restoring ec_frame_idx must be last
! ectx->ec_frame_idx = STACK_TV(ectx->ec_frame_idx + 4)->vval.v_number;
dfunc = ((dfunc_T *)def_functions.ga_data) + ectx->ec_dfunc_idx;
ectx->ec_instr = dfunc->df_instr;

--- 527,551 ----
// 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 + 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
! + 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
! + 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,626 ****
* 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
! call_ufunc(ufunc_T *ufunc, int argcount, ectx_T *ectx, isn_T *iptr)
{
typval_T argvars[MAX_FUNC_ARGS];
funcexe_T funcexe;
--- 634,649 ----
* 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,
! partial_T *pt,
! int argcount,
! ectx_T *ectx,
! isn_T *iptr)
{
typval_T argvars[MAX_FUNC_ARGS];
funcexe_T funcexe;
***************
*** 653,659 ****
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);
}

if (call_prepare(argcount, argvars, ectx) == FAIL)
--- 676,682 ----
iptr->isn_arg.dfunc.cdf_idx = ufunc->uf_dfunc_idx;
iptr->isn_arg.dfunc.cdf_argcount = argcount;
}
! return call_dfunc(ufunc->uf_dfunc_idx, pt, argcount, ectx);
}

if (call_prepare(argcount, argvars, ectx) == FAIL)
***************
*** 726,732 ****
}

if (ufunc != NULL)
! return call_ufunc(ufunc, argcount, ectx, iptr);

return FAIL;
}
--- 749,755 ----
}

if (ufunc != NULL)
! return call_ufunc(ufunc, NULL, argcount, ectx, iptr);

return FAIL;
}
***************
*** 761,782 ****
}

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;
- }
name = pt->pt_name;
}
else if (tv->v_type == VAR_FUNC)
--- 784,791 ----
}

if (pt->pt_func != NULL)
! return call_ufunc(pt->pt_func, pt, argcount, ectx, NULL);

name = pt->pt_name;
}
else if (tv->v_type == VAR_FUNC)
***************
*** 1065,1074 ****

// 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;

// If this function returns and the closure is still
// being used, we need to make a copy of the context
--- 1074,1083 ----

// The closure needs to find arguments and local
// variables in the current stack.
! 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,1143 ****
// 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))
--- 1144,1149 ----
***************
*** 1241,1270 ****
ectx.ec_frame_idx = ectx.ec_stack.ga_len;
initial_frame_idx = ectx.ec_frame_idx;

! if (partial != NULL)
{
! if (partial->pt_ectx_stack == NULL && current_ectx != 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;
}
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
--- 1247,1270 ----
ectx.ec_frame_idx = ectx.ec_stack.ga_len;
initial_frame_idx = ectx.ec_frame_idx;

! if (partial != NULL || ufunc->uf_partial != NULL)
{
! ectx.ec_outer = ALLOC_CLEAR_ONE(outer_T);
! if (ectx.ec_outer == NULL)
! goto failed_early;
! if (partial != NULL)
{
! 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 = ufunc->uf_partial->pt_outer;
! ectx.ec_outer->out_up_is_copy = TRUE;
}

// dummy frame entries
***************
*** 1546,1579 ****
++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)
--- 1546,1551 ----
***************
*** 1769,1783 ****
*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:
{
--- 1741,1746 ----
***************
*** 2058,2063 ****
--- 2021,2063 ----
}
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,2302 ****
// call a :def function
case ISN_DCALL:
SOURCING_LNUM = iptr->isn_lnum;
! if (call_dfunc(iptr->isn_arg.dfunc.cdf_idx,
iptr->isn_arg.dfunc.cdf_argcount,
&ectx) == FAIL)
goto on_error;
--- 2296,2302 ----
// call a :def function
case ISN_DCALL:
SOURCING_LNUM = iptr->isn_lnum;
! if (call_dfunc(iptr->isn_arg.dfunc.cdf_idx, NULL,
iptr->isn_arg.dfunc.cdf_argcount,
&ectx) == FAIL)
goto on_error;
***************
*** 3555,3560 ****
--- 3555,3569 ----
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;

*** ../vim-8.2.2321/src/vim9.h 2021-01-10 14:02:24.658205157 +0100
--- src/vim9.h 2021-01-10 17:27:31.485515079 +0100
***************
*** 307,313 ****
typedef struct {
int outer_idx; // index
int outer_depth; // nesting level, stack frames to go up
! } outer_T;

/*
* Instruction
--- 307,313 ----
typedef struct {
int outer_idx; // index
int outer_depth; // nesting level, stack frames to go up
! } isn_outer_T;

/*
* Instruction
***************
*** 348,354 ****
put_T put;
cmod_T cmdmod;
unpack_T unpack;
! outer_T outer;
} isn_arg;
};

--- 348,354 ----
put_T put;
cmod_T cmdmod;
unpack_T unpack;
! isn_outer_T outer;
} isn_arg;
};

***************
*** 375,384 ****
// 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_frame_idx: previous frame index
! #define STACK_FRAME_SIZE 5


#ifdef DEFINE_VIM9_GLOBALS
--- 375,387 ----
// Number of entries used by stack frame for a function call.
// - ec_dfunc_idx: function index
// - ec_iidx: instruction index
! // - ec_outer: stack used for closures
// - ec_frame_idx: previous frame index
! #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
*** ../vim-8.2.2321/src/structs.h 2021-01-10 14:02:24.662205148 +0100
--- src/structs.h 2021-01-10 15:42:01.094755333 +0100
***************
*** 1965,1970 ****
--- 1965,1978 ----
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,1987 ****
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

int pt_argc; // number of arguments
typval_T *pt_argv; // arguments in allocated array
--- 1983,1993 ----
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 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
*** ../vim-8.2.2321/src/testdir/test_vim9_func.vim 2021-01-10 14:02:24.662205148 +0100
--- src/testdir/test_vim9_func.vim 2021-01-10 18:21:30.645562021 +0100
***************
*** 1822,1827 ****
--- 1822,1834 ----
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

*** ../vim-8.2.2321/src/version.c 2021-01-10 14:02:24.662205148 +0100
--- src/version.c 2021-01-10 18:32:21.279830338 +0100
***************
*** 752,753 ****
--- 752,755 ----
{ /* Add new patch number below this line */
+ /**/
+ 2322,
/**/

--
hundred-and-one symptoms of being an internet addict:
120. You ask a friend, "What's that big shiny thing?" He says, "It's the sun."

/// Bram Moolenaar -- Br...@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 ///