patch 9.0.0502: a closure in a nested loop in a :def function does not work
Problem: A closure in a nested loop in a :def function does not work.
Solution: Use an array of loopvars, one per loop level.
diff --git a/src/vim9execute.c b/src/vim9execute.c
index c2a3310..c1c2e12 100644
--- a/src/vim9execute.c
+++ b/src/vim9execute.c
@@ -1825,11 +1825,10 @@
*/
int
fill_partial_and_closure(
- partial_T *pt,
- ufunc_T *ufunc,
- short loop_var_idx,
- short loop_var_count,
- ectx_T *ectx)
+ partial_T *pt,
+ ufunc_T *ufunc,
+ loopvarinfo_T *lvi,
+ ectx_T *ectx)
{
pt->pt_func = ufunc;
pt->pt_refcount = 1;
@@ -1854,13 +1853,22 @@
}
}
- // The closure may need to find variables defined inside a loop. A
- // new reference is made every time, ISN_ENDLOOP will check if they
- // are actually used.
- pt->pt_outer.out_loop_stack = &ectx->ec_stack;
- pt->pt_outer.out_loop_var_idx = ectx->ec_frame_idx + STACK_FRAME_SIZE
- + loop_var_idx;
- pt->pt_outer.out_loop_var_count = loop_var_count;
+ if (lvi != NULL)
+ {
+ int depth;
+
+ // The closure may need to find variables defined inside a loop,
+ // for every nested loop. A new reference is made every time,
+ // ISN_ENDLOOP will check if they are actually used.
+ for (depth = 0; depth < lvi->lvi_depth; ++depth)
+ {
+ pt->pt_outer.out_loop[depth].stack = &ectx->ec_stack;
+ pt->pt_outer.out_loop[depth].var_idx = ectx->ec_frame_idx
+ + STACK_FRAME_SIZE + lvi->lvi_loop[depth].var_idx;
+ pt->pt_outer.out_loop[depth].var_count =
+ lvi->lvi_loop[depth].var_count;
+ }
+ }
// If the function currently executing returns and the closure is still
// being referenced, we need to make a copy of the context (arguments
@@ -2507,7 +2515,7 @@
int jump = FALSE;
typval_T *ltv = STACK_TV_BOT(-1);
typval_T *idxtv =
- STACK_TV_VAR(iptr->isn_arg.forloop.for_idx);
+ STACK_TV_VAR(iptr->isn_arg.forloop.for_loop_idx);
if (GA_GROW_FAILS(&ectx->ec_stack, 1))
return FAIL;
@@ -2613,7 +2621,7 @@
// Store the current number of funcrefs, this may be used in
// ISN_LOOPEND. The variable index is always one more than the loop
// variable index.
- tv = STACK_TV_VAR(iptr->isn_arg.forloop.for_idx + 1);
+ tv = STACK_TV_VAR(iptr->isn_arg.forloop.for_loop_idx + 1);
tv->vval.v_number = ectx->ec_funcrefs.ga_len;
}
@@ -2661,18 +2669,20 @@
endloop_T *endloop = &iptr->isn_arg.endloop;
typval_T *tv_refcount = STACK_TV_VAR(endloop->end_funcref_idx);
int prev_closure_count = tv_refcount->vval.v_number;
+ int depth = endloop->end_depth;
garray_T *gap = &ectx->ec_funcrefs;
int closure_in_use = FALSE;
loopvars_T *loopvars;
typval_T *stack;
int idx;
- // Check if any created closure is still being referenced.
+ // Check if any created closure is still being referenced and loopvars have
+ // not been saved yet for the current depth.
for (idx = prev_closure_count; idx < gap->ga_len; ++idx)
{
partial_T *pt = ((partial_T **)gap->ga_data)[idx];
- if (pt->pt_refcount > 1 && pt->pt_loopvars == NULL)
+ if (pt->pt_refcount > 1 && pt->pt_loopvars[depth] == NULL)
{
int refcount = pt->pt_refcount;
int i;
@@ -2728,14 +2738,14 @@
{
partial_T *pt = ((partial_T **)gap->ga_data)[idx];
- if (pt->pt_refcount > 1 && pt->pt_loopvars == NULL)
+ if (pt->pt_refcount > 1 && pt->pt_loopvars[depth] == NULL)
{
++loopvars->lvs_refcount;
- pt->pt_loopvars = loopvars;
+ pt->pt_loopvars[depth] = loopvars;
- pt->pt_outer.out_loop_stack = &loopvars->lvs_ga;
- pt->pt_outer.out_loop_var_idx -= ectx->ec_frame_idx
- + STACK_FRAME_SIZE + endloop->end_var_idx;
+ pt->pt_outer.out_loop[depth].stack = &loopvars->lvs_ga;
+ pt->pt_outer.out_loop[depth].var_idx -=
+ ectx->ec_frame_idx + STACK_FRAME_SIZE + endloop->end_var_idx;
}
}
@@ -2747,37 +2757,44 @@
* loopvars may be the only reference to the partials in the local variables.
* Go over all of them, the funcref and can be freed if all partials
* referencing the loopvars have a reference count of one.
+ * Return TRUE if it was freed.
*/
- void
+ int
loopvars_check_refcount(loopvars_T *loopvars)
{
int i;
garray_T *gap = &loopvars->lvs_ga;
int done = 0;
-
- if (loopvars->lvs_refcount > loopvars->lvs_min_refcount)
- return;
- for (i = 0; i < gap->ga_len; ++i)
- {
- typval_T *tv = ((typval_T *)gap->ga_data) + i;
-
- if (tv->v_type == VAR_PARTIAL && tv->vval.v_partial != NULL
- && tv->vval.v_partial->pt_loopvars == loopvars
- && tv->vval.v_partial->pt_refcount == 1)
- ++done;
- }
- if (done == loopvars->lvs_min_refcount)
- {
typval_T *stack = gap->ga_data;
- // All partials referencing the loopvars have a reference count of
- // one, thus the loopvars is no longer of use.
- for (i = 0; i < gap->ga_len; ++i)
- clear_tv(stack + i);
- vim_free(stack);
- remove_loopvars_from_list(loopvars);
- vim_free(loopvars);
+ if (loopvars->lvs_refcount > loopvars->lvs_min_refcount)
+ return FALSE;
+ for (i = 0; i < gap->ga_len; ++i)
+ {
+ typval_T *tv = ((typval_T *)gap->ga_data) + i;
+
+ if (tv->v_type == VAR_PARTIAL && tv->vval.v_partial != NULL
+ && tv->vval.v_partial->pt_refcount == 1)
+ {
+ int depth;
+
+ for (depth = 0; depth < MAX_LOOP_DEPTH; ++depth)
+ if (tv->vval.v_partial->pt_loopvars[depth] == loopvars)
+ ++done;
+ }
}
+ if (done != loopvars->lvs_min_refcount)
+ return FALSE;
+
+ // All partials referencing the loopvars have a reference count of
+ // one, thus the loopvars is no longer of use.
+ stack = gap->ga_data;
+ for (i = 0; i < gap->ga_len; ++i)
+ clear_tv(stack + i);
+ vim_free(stack);
+ remove_loopvars_from_list(loopvars);
+ vim_free(loopvars);
+ return TRUE;
}
/*
@@ -3804,12 +3821,13 @@
iemsg("LOADOUTER depth more than scope levels");
goto theend;
}
- if (depth == OUTER_LOOP_DEPTH)
+ if (depth < 0)
// Variable declared in loop. May be copied if the
// loop block has already ended.
- tv = ((typval_T *)outer->out_loop_stack->ga_data)
- + outer->out_loop_var_idx
- + iptr->isn_arg.outer.outer_idx;
+ tv = ((typval_T *)outer->out_loop[-depth - 1]
+ .stack->ga_data)
+ + outer->out_loop[-depth - 1].var_idx
+ + iptr->isn_arg.outer.outer_idx;
else
// Variable declared in a function. May be copied if
// the function has already returned.
@@ -4142,8 +4160,7 @@
goto theend;
}
if (fill_partial_and_closure(pt, ufunc,
- extra == NULL ? 0 : extra->fre_loop_var_idx,
- extra == NULL ? 0 : extra->fre_loop_var_count,
+ extra == NULL ? NULL : &extra->fre_loopvar_info,
ectx) == FAIL)
goto theend;
tv = STACK_TV_BOT(0);
@@ -4160,8 +4177,8 @@
newfuncarg_T *arg = iptr->isn_arg.newfunc.nf_arg;
if (copy_lambda_to_global_func(arg->nfa_lambda,
- arg->nfa_global, arg->nfa_loop_var_idx,
- arg->nfa_loop_var_count, ectx) == FAIL)
+ arg->nfa_global, &arg->nfa_loopvar_info,
+ ectx) == FAIL)
goto theend;
}
break;
@@ -4236,7 +4253,7 @@
ectx->ec_iidx = iptr->isn_arg.whileloop.while_end;
// Store the current funccal count, may be used by
- // ISN_LOOPEND later
+ // ISN_ENDLOOP later
tv = STACK_TV_VAR(
iptr->isn_arg.whileloop.while_funcref_idx);
tv->vval.v_number = ectx->ec_funcrefs.ga_len;
@@ -5581,6 +5598,11 @@
// Check the function was really compiled.
dfunc_T *dfunc = ((dfunc_T *)def_functions.ga_data)
+ ufunc->uf_dfunc_idx;
+ if (dfunc->df_ufunc == NULL)
+ {
+ semsg(_(e_function_was_deleted_str), printable_func_name(ufunc));
+ return FAIL;
+ }
if (INSTRUCTIONS(dfunc) == NULL)
{
iemsg("using call_def_function() on not compiled function");
@@ -5717,8 +5739,13 @@
if (partial != NULL)
{
outer_T *outer = get_pt_outer(partial);
+ int depth;
+ void *ptr = outer->out_stack;
- if (outer->out_stack == NULL && outer->out_loop_stack == NULL)
+ // see if any stack was set
+ for (depth = 0; ptr == NULL && depth < MAX_LOOP_DEPTH; ++depth)
+ ptr = outer->out_loop[depth].stack;
+ if (ptr == NULL)
{
if (current_ectx != NULL)
{
@@ -5767,6 +5794,8 @@
ectx.ec_stack.ga_len += dfunc->df_varcount;
if (dfunc->df_has_closure)
{
+ // Initialize the variable that counts how many closures were
+ // created. This is used in handle_closure_in_use().
STACK_TV_VAR(idx)->v_type = VAR_NUMBER;
STACK_TV_VAR(idx)->vval.v_number = 0;
++ectx.ec_stack.ga_len;
@@ -5912,6 +5941,32 @@
}
/*
+ * Return loopvarinfo in a printable form in allocated memory.
+ */
+ static char_u *
+printable_loopvarinfo(loopvarinfo_T *lvi)
+{
+ garray_T ga;
+ int depth;
+
+ ga_init2(&ga, 1, 100);
+ for (depth = 0; depth < lvi->lvi_depth; ++depth)
+ {
+ if (ga_grow(&ga, 50) == FAIL)
+ break;
+ if (lvi->lvi_loop[depth].var_idx == 0)
+ STRCPY(ga.ga_data + ga.ga_len, " -");
+ else
+ vim_snprintf(ga.ga_data + ga.ga_len, 50, " $%d-$%d",
+ lvi->lvi_loop[depth].var_idx,
+ lvi->lvi_loop[depth].var_idx
+ + lvi->lvi_loop[depth].var_count - 1);
+ ga.ga_len = STRLEN(ga.ga_data);
+ }
+ return ga.ga_data;
+}
+
+/*
* List instructions "instr" up to "instr_count" or until ISN_FINISH.
* "ufunc" has the source lines, NULL for the instructions of ISN_SUBSTITUTE.
* "pfx" is prefixed to every line.
@@ -6072,12 +6127,13 @@
if (outer->outer_idx < 0)
smsg("%s%4d LOADOUTER level %d arg[%d]", pfx, current,
- outer->outer_depth,
- outer->outer_idx
- + STACK_FRAME_SIZE);
- else if (outer->outer_depth == OUTER_LOOP_DEPTH)
- smsg("%s%4d LOADOUTER level 1 $%d in loop",
- pfx, current, outer->outer_idx);
+ outer->outer_depth,
+ outer->outer_idx + STACK_FRAME_SIZE);
+ else if (outer->outer_depth < 0)
+ smsg("%s%4d LOADOUTER $%d in loop level %d",
+ pfx, current,
+ outer->outer_idx,
+ -outer->outer_depth);
else
smsg("%s%4d LOADOUTER level %d $%d", pfx, current,
outer->outer_depth,
@@ -6400,29 +6456,36 @@
}
else
name = extra->fre_func_name;
- if (extra == NULL || extra->fre_loop_var_count == 0)
+ if (extra == NULL || extra->fre_loopvar_info.lvi_depth == 0)
smsg("%s%4d FUNCREF %s", pfx, current, name);
else
- smsg("%s%4d FUNCREF %s var $%d - $%d", pfx, current,
- name,
- extra->fre_loop_var_idx,
- extra->fre_loop_var_idx
- + extra->fre_loop_var_count - 1);
+ {
+ char_u *info = printable_loopvarinfo(
+ &extra->fre_loopvar_info);
+
+ smsg("%s%4d FUNCREF %s vars %s", pfx, current,
+ name, info);
+ vim_free(info);
+ }
}
break;
case ISN_NEWFUNC:
{
- newfuncarg_T *arg = iptr->isn_arg.newfunc.nf_arg;
+ newfuncarg_T *arg = iptr->isn_arg.newfunc.nf_arg;
- if (arg->nfa_loop_var_count == 0)
+ if (arg->nfa_loopvar_info.lvi_depth == 0)
smsg("%s%4d NEWFUNC %s %s", pfx, current,
arg->nfa_lambda, arg->nfa_global);
else
- smsg("%s%4d NEWFUNC %s %s var $%d - $%d", pfx, current,
- arg->nfa_lambda, arg->nfa_global,
- arg->nfa_loop_var_idx,
- arg->nfa_loop_var_idx + arg->nfa_loop_var_count - 1);
+ {
+ char_u *info = printable_loopvarinfo(
+ &arg->nfa_loopvar_info);
+
+ smsg("%s%4d NEWFUNC %s %s vars %s", pfx, current,
+ arg->nfa_lambda, arg->nfa_global, info);
+ vim_free(info);
+ }
}
break;
@@ -6479,7 +6542,7 @@
forloop_T *forloop = &iptr->isn_arg.forloop;
smsg("%s%4d FOR $%d -> %d", pfx, current,
- forloop->for_idx, forloop->for_end);
+ forloop->for_loop_idx, forloop->for_end);
}
break;
@@ -6487,10 +6550,12 @@
{
endloop_T *endloop = &iptr->isn_arg.endloop;
- smsg("%s%4d ENDLOOP $%d save $%d - $%d", pfx, current,
+ smsg("%s%4d ENDLOOP ref $%d save $%d-$%d depth %d",
+ pfx, current,
endloop->end_funcref_idx,
endloop->end_var_idx,
- endloop->end_var_idx + endloop->end_var_count - 1);
+ endloop->end_var_idx + endloop->end_var_count - 1,
+ endloop->end_depth);
}
break;