patch 8.2.2170: Vim9: a global function defined in a :def function failsv8.2.2170

Problem:    Vim9: a global function defined in a :def function fails if it
            uses the context.
Solution:   Create a partial to store the closure context. (see #7410)

6 files changed, 114 insertions, 45 deletions

diff --git a/src/proto/userfunc.pro b/src/proto/userfunc.pro
index 0029dab0d..787d01090 100644
--- a/src/proto/userfunc.pro
+++ b/src/proto/userfunc.pro
@@ -13,7 +13,7 @@ ufunc_T *find_func_even_dead(char_u *name, int is_global, cctx_T *cctx);
 ufunc_T *find_func(char_u *name, int is_global, cctx_T *cctx);
 int func_is_global(ufunc_T *ufunc);
 int func_name_refcount(char_u *name);
-void copy_func(char_u *lambda, char_u *global);
+ufunc_T *copy_func(char_u *lambda, char_u *global);
 int funcdepth_increment(void);
 void funcdepth_decrement(void);
 int funcdepth_get(void);
diff --git a/src/structs.h b/src/structs.h
index 631881f5b..809a2d156 100644
--- a/src/structs.h
+++ b/src/structs.h
@@ -1598,6 +1598,9 @@ typedef struct
     garray_T	uf_type_list;	// types used in arg and return types
     int		*uf_def_arg_idx; // instruction indexes for evaluating
 				// uf_def_args; length: uf_def_args.ga_len + 1
+    partial_T	*uf_partial;	// for closure created inside :def function:
+				// information about the context
+
     char_u	*uf_va_name;	// name from "...name" or NULL
     type_T	*uf_va_type;	// type from "...name: type" or NULL
     type_T	*uf_func_type;	// type of the function, &t_func_any if unknown
diff --git a/src/testdir/test_vim9_func.vim b/src/testdir/test_vim9_func.vim
index 0a07e8de8..61e909bfc 100644
--- a/src/testdir/test_vim9_func.vim
+++ b/src/testdir/test_vim9_func.vim
@@ -1523,6 +1523,29 @@ def Test_nested_closure_fails()
   CheckScriptFailure(lines, 'E1012:')
 enddef
 
+def Test_global_closure()
+  var lines =<< trim END
+      vim9script
+      def ReverseEveryNLines(n: number, line1: number, line2: number)
+        var mods = 'sil keepj keepp lockm '
+        var range = ':' .. line1 .. ',' .. line2
+        def g:Offset(): number
+            var offset = (line('.') - line1 + 1) % n
+            return offset != 0 ? offset : n
+        enddef
+        exe mods .. range .. 'g/^/exe "m .-" .. g:Offset()'
+      enddef
+
+      new
+      repeat(['aaa', 'bbb', 'ccc'], 3)->setline(1)
+      ReverseEveryNLines(3, 1, 9)
+  END
+  CheckScriptSuccess(lines)
+  var expected = repeat(['ccc', 'bbb', 'aaa'], 3)
+  assert_equal(expected, getline(1, 9))
+  bwipe!
+enddef
+
 def Test_failure_in_called_function()
   # this was using the frame index as the return value
   var lines =<< trim END
diff --git a/src/userfunc.c b/src/userfunc.c
index 31c7af3e7..b9fb644f1 100644
--- a/src/userfunc.c
+++ b/src/userfunc.c
@@ -1225,6 +1225,8 @@ func_clear_items(ufunc_T *fp)
     VIM_CLEAR(fp->uf_block_ids);
     VIM_CLEAR(fp->uf_va_name);
     clear_type_list(&fp->uf_type_list);
+    partial_unref(fp->uf_partial);
+    fp->uf_partial = NULL;
 
 #ifdef FEAT_LUA
     if (fp->uf_cb_free != NULL)
@@ -1305,12 +1307,13 @@ func_clear_free(ufunc_T *fp, int force)
 /*
  * Copy already defined function "lambda" to a new function with name "global".
  * This is for when a compiled function defines a global function.
+ * Caller should take care of adding a partial for a closure.
  */
-    void
+    ufunc_T *
 copy_func(char_u *lambda, char_u *global)
 {
     ufunc_T *ufunc = find_func_even_dead(lambda, TRUE, NULL);
-    ufunc_T *fp;
+    ufunc_T *fp = NULL;
 
     if (ufunc == NULL)
 	semsg(_(e_lambda_function_not_found_str), lambda);
@@ -1321,12 +1324,12 @@ copy_func(char_u *lambda, char_u *global)
 	if (fp != NULL)
 	{
 	    semsg(_(e_funcexts), global);
-	    return;
+	    return NULL;
 	}
 
 	fp = alloc_clear(offsetof(ufunc_T, uf_name) + STRLEN(global) + 1);
 	if (fp == NULL)
-	    return;
+	    return NULL;
 
 	fp->uf_varargs = ufunc->uf_varargs;
 	fp->uf_flags = (ufunc->uf_flags & ~FC_VIM9) | FC_COPY;
@@ -1362,15 +1365,17 @@ copy_func(char_u *lambda, char_u *global)
 	    if (fp->uf_va_name == NULL)
 		goto failed;
 	}
+	fp->uf_ret_type = ufunc->uf_ret_type;
 
 	fp->uf_refcount = 1;
 	STRCPY(fp->uf_name, global);
 	hash_add(&func_hashtab, UF2HIKEY(fp));
     }
-    return;
+    return fp;
 
 failed:
     func_clear_free(fp, TRUE);
+    return NULL;
 }
 
 static int	funcdepth = 0;
diff --git a/src/version.c b/src/version.c
index a95c485b8..6e2e6abc5 100644
--- a/src/version.c
+++ b/src/version.c
@@ -751,6 +751,8 @@ static char *(features[]) =
 static int included_patches[] =
 {   /* Add new patch number below this line */
 /**/
+    2170,
+/**/
     2169,
 /**/
     2168,
diff --git a/src/vim9execute.c b/src/vim9execute.c
index 7e5a1dea3..6409d88f1 100644
--- a/src/vim9execute.c
+++ b/src/vim9execute.c
@@ -845,6 +845,49 @@ call_eval_func(char_u *name, int argcount, ectx_T *ectx, isn_T *iptr)
 }
 
 /*
+ * When a function reference is used, fill a partial with the information
+ * needed, especially when it is used as a closure.
+ */
+    static int
+fill_partial_and_closure(partial_T *pt, ufunc_T *ufunc, ectx_T *ectx)
+{
+    pt->pt_func = ufunc;
+    pt->pt_refcount = 1;
+
+    if (pt->pt_func->uf_flags & FC_CLOSURE)
+    {
+	dfunc_T	*dfunc = ((dfunc_T *)def_functions.ga_data)
+							  + ectx->ec_dfunc_idx;
+
+	// 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;
+
+	// If this function returns and the closure is still
+	// being used, we need to make a copy of the context
+	// (arguments and local variables). Store a reference
+	// to the partial so we can handle that.
+	if (ga_grow(&ectx->ec_funcrefs, 1) == FAIL)
+	{
+	    vim_free(pt);
+	    return FAIL;
+	}
+	// Extra variable keeps the count of closures created
+	// in the current function call.
+	++(((typval_T *)ectx->ec_stack.ga_data) + ectx->ec_frame_idx
+		       + STACK_FRAME_SIZE + dfunc->df_varcount)->vval.v_number;
+
+	((partial_T **)ectx->ec_funcrefs.ga_data)
+			       [ectx->ec_funcrefs.ga_len] = pt;
+	++pt->pt_refcount;
+	++ectx->ec_funcrefs.ga_len;
+    }
+    ++pt->pt_func->uf_refcount;
+    return OK;
+}
+
+/*
  * Call a "def" function from old Vim script.
  * Return OK or FAIL.
  */
@@ -1003,6 +1046,11 @@ call_def_function(
 	    ectx.ec_outer_frame = partial->pt_ectx_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;
+    }
 
     // dummy frame entries
     for (idx = 0; idx < STACK_FRAME_SIZE; ++idx)
@@ -1969,10 +2017,10 @@ call_def_function(
 	    // push a function reference to a compiled function
 	    case ISN_FUNCREF:
 		{
-		    partial_T   *pt = NULL;
-		    dfunc_T	*pt_dfunc;
+		    partial_T   *pt = ALLOC_CLEAR_ONE(partial_T);
+		    dfunc_T	*pt_dfunc = ((dfunc_T *)def_functions.ga_data)
+					       + iptr->isn_arg.funcref.fr_func;
 
-		    pt = ALLOC_CLEAR_ONE(partial_T);
 		    if (pt == NULL)
 			goto failed;
 		    if (GA_GROW(&ectx.ec_stack, 1) == FAIL)
@@ -1980,41 +2028,9 @@ call_def_function(
 			vim_free(pt);
 			goto failed;
 		    }
-		    pt_dfunc = ((dfunc_T *)def_functions.ga_data)
-					       + iptr->isn_arg.funcref.fr_func;
-		    pt->pt_func = pt_dfunc->df_ufunc;
-		    pt->pt_refcount = 1;
-
-		    if (pt_dfunc->df_ufunc->uf_flags & FC_CLOSURE)
-		    {
-			dfunc_T	*dfunc = ((dfunc_T *)def_functions.ga_data)
-							   + ectx.ec_dfunc_idx;
-
-			// 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;
-
-			// If this function returns and the closure is still
-			// being used, we need to make a copy of the context
-			// (arguments and local variables). Store a reference
-			// to the partial so we can handle that.
-			if (ga_grow(&ectx.ec_funcrefs, 1) == FAIL)
-			{
-			    vim_free(pt);
-			    goto failed;
-			}
-			// Extra variable keeps the count of closures created
-			// in the current function call.
-			tv = STACK_TV_VAR(dfunc->df_varcount);
-			++tv->vval.v_number;
-
-			((partial_T **)ectx.ec_funcrefs.ga_data)
-					       [ectx.ec_funcrefs.ga_len] = pt;
-			++pt->pt_refcount;
-			++ectx.ec_funcrefs.ga_len;
-		    }
-		    ++pt_dfunc->df_ufunc->uf_refcount;
+		    if (fill_partial_and_closure(pt, pt_dfunc->df_ufunc,
+								&ectx) == FAIL)
+			goto failed;
 
 		    tv = STACK_TV_BOT(0);
 		    ++ectx.ec_stack.ga_len;
@@ -2028,8 +2044,25 @@ call_def_function(
 	    case ISN_NEWFUNC:
 		{
 		    newfunc_T	*newfunc = &iptr->isn_arg.newfunc;
+		    ufunc_T	*new_ufunc;
 
-		    copy_func(newfunc->nf_lambda, newfunc->nf_global);
+		    new_ufunc = copy_func(
+				       newfunc->nf_lambda, newfunc->nf_global);
+		    if (new_ufunc != NULL
+					 && (new_ufunc->uf_flags & FC_CLOSURE))
+		    {
+			partial_T   *pt = ALLOC_CLEAR_ONE(partial_T);
+
+			// Need to create a partial to store the context of the
+			// function.
+			if (pt == NULL)
+			    goto failed;
+			if (fill_partial_and_closure(pt, new_ufunc,
+								&ectx) == FAIL)
+			    goto failed;
+			new_ufunc->uf_partial = pt;
+			--pt->pt_refcount;  // not referenced here
+		    }
 		}
 		break;
 
@@ -3114,7 +3147,10 @@ failed:
 
     // Deal with any remaining closures, they may be in use somewhere.
     if (ectx.ec_funcrefs.ga_len > 0)
+    {
 	handle_closure_in_use(&ectx, FALSE);
+	ga_clear(&ectx.ec_funcrefs);  // TODO: should not be needed?
+    }
 
     estack_pop();
     current_sctx = save_current_sctx;