patch 9.0.0502: a closure in a nested loop in a :def function does not workv9.0.0502

Problem:    A closure in a nested loop in a :def function does not work.
Solution:   Use an array of loopvars, one per loop level.

1 files changed, 133 insertions, 68 deletions

diff --git a/src/vim9execute.c b/src/vim9execute.c
index c2a331081..c1c2e124a 100644
--- a/src/vim9execute.c
+++ b/src/vim9execute.c
@@ -1825,11 +1825,10 @@ call_eval_func(
  */
     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 @@ fill_partial_and_closure(
 	    }
 	}
 
-	// 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 @@ execute_for(isn_T *iptr, ectx_T *ectx)
     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 @@ execute_for(isn_T *iptr, ectx_T *ectx)
 	// 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 @@ execute_endloop(isn_T *iptr, ectx_T *ectx)
     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 @@ execute_endloop(isn_T *iptr, ectx_T *ectx)
     {
 	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 @@ execute_endloop(isn_T *iptr, ectx_T *ectx)
  * 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;
+	typval_T	*stack = gap->ga_data;
 
     if (loopvars->lvs_refcount > loopvars->lvs_min_refcount)
-	return;
+	return FALSE;
     for (i = 0; i < gap->ga_len; ++i)
     {
-	typval_T *tv = ((typval_T *)gap->ga_data) + 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;
+	{
+	    int	    depth;
 
-	// 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);
+	    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 @@ exec_instructions(ectx_T *ectx)
 			    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 @@ exec_instructions(ectx_T *ectx)
 			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 @@ exec_instructions(ectx_T *ectx)
 		    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 @@ exec_instructions(ectx_T *ectx)
 			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 @@ call_def_function(
 	// 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 @@ call_def_function(
 	    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 @@ call_def_function(
 	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 @@ may_invoke_defer_funcs(ectx_T *ectx)
 }
 
 /*
+ * 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 @@ list_instructions(char *pfx, isn_T *instr, int instr_count, ufunc_T *ufunc)
 
 		    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 @@ list_instructions(char *pfx, isn_T *instr, int instr_count, ufunc_T *ufunc)
 		    }
 		    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 @@ list_instructions(char *pfx, isn_T *instr, int instr_count, ufunc_T *ufunc)
 		    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 @@ list_instructions(char *pfx, isn_T *instr, int instr_count, ufunc_T *ufunc)
 		{
 		    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;