To: vim_dev@googlegroups.com
Subject: Patch 8.2.1734
Fcc: outbox
From: Bram Moolenaar <Bram@moolenaar.net>
Mime-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
------------

Patch 8.2.1734
Problem:    Vim9: cannot use a funcref for a closure twice.
Solution:   Instead of putting the funcref on the stack use a growarray on the
            execution context.
Files:      src/vim9.h, src/vim9compile.c, src/vim9execute.c,
            src/testdir/test_vim9_func.vim,
            src/testdir/test_vim9_disassemble.vim


*** ../vim-8.2.1733/src/vim9.h	2020-09-16 15:21:56.354720354 +0200
--- src/vim9.h	2020-09-23 19:33:49.030375047 +0200
***************
*** 244,250 ****
  // arguments to ISN_FUNCREF
  typedef struct {
      int		fr_func;	// function index
-     int		fr_var_idx;	// variable to store partial
  } funcref_T;
  
  // arguments to ISN_NEWFUNC
--- 244,249 ----
***************
*** 323,329 ****
      int		df_instr_count;
  
      int		df_varcount;	    // number of local variables
!     int		df_closure_count;   // number of closures created
  };
  
  // Number of entries used by stack frame for a function call.
--- 322,328 ----
      int		df_instr_count;
  
      int		df_varcount;	    // number of local variables
!     int		df_has_closure;	    // one if a closure was created
  };
  
  // Number of entries used by stack frame for a function call.
*** ../vim-8.2.1733/src/vim9compile.c	2020-09-23 18:51:07.326493552 +0200
--- src/vim9compile.c	2020-09-23 20:45:12.860005546 +0200
***************
*** 126,133 ****
      garray_T	ctx_locals;	    // currently visible local variables
      int		ctx_locals_count;   // total number of local variables
  
!     int		ctx_closure_count;  // number of closures created in the
! 				    // function
  
      garray_T	ctx_imports;	    // imported items
  
--- 126,133 ----
      garray_T	ctx_locals;	    // currently visible local variables
      int		ctx_locals_count;   // total number of local variables
  
!     int		ctx_has_closure;    // set to one if a closures was created in
! 				    // the function
  
      garray_T	ctx_imports;	    // imported items
  
***************
*** 1273,1279 ****
      if ((isn = generate_instr(cctx, ISN_FUNCREF)) == NULL)
  	return FAIL;
      isn->isn_arg.funcref.fr_func = ufunc->uf_dfunc_idx;
!     isn->isn_arg.funcref.fr_var_idx = cctx->ctx_closure_count++;
  
      if (ga_grow(stack, 1) == FAIL)
  	return FAIL;
--- 1273,1279 ----
      if ((isn = generate_instr(cctx, ISN_FUNCREF)) == NULL)
  	return FAIL;
      isn->isn_arg.funcref.fr_func = ufunc->uf_dfunc_idx;
!     cctx->ctx_has_closure = 1;
  
      if (ga_grow(stack, 1) == FAIL)
  	return FAIL;
***************
*** 7138,7144 ****
  	dfunc->df_instr = instr->ga_data;
  	dfunc->df_instr_count = instr->ga_len;
  	dfunc->df_varcount = cctx.ctx_locals_count;
! 	dfunc->df_closure_count = cctx.ctx_closure_count;
  	if (cctx.ctx_outer_used)
  	    ufunc->uf_flags |= FC_CLOSURE;
  	ufunc->uf_def_status = UF_COMPILED;
--- 7138,7144 ----
  	dfunc->df_instr = instr->ga_data;
  	dfunc->df_instr_count = instr->ga_len;
  	dfunc->df_varcount = cctx.ctx_locals_count;
! 	dfunc->df_has_closure = cctx.ctx_has_closure;
  	if (cctx.ctx_outer_used)
  	    ufunc->uf_flags |= FC_CLOSURE;
  	ufunc->uf_def_status = UF_COMPILED;
***************
*** 7312,7318 ****
  		dfunc_T *dfunc = ((dfunc_T *)def_functions.ga_data)
  					       + isn->isn_arg.dfunc.cdf_idx;
  
! 		if (func_name_refcount(dfunc->df_ufunc->uf_name))
  		    func_ptr_unref(dfunc->df_ufunc);
  	    }
  	    break;
--- 7312,7319 ----
  		dfunc_T *dfunc = ((dfunc_T *)def_functions.ga_data)
  					       + isn->isn_arg.dfunc.cdf_idx;
  
! 		if (dfunc->df_ufunc != NULL
! 			       && func_name_refcount(dfunc->df_ufunc->uf_name))
  		    func_ptr_unref(dfunc->df_ufunc);
  	    }
  	    break;
*** ../vim-8.2.1733/src/vim9execute.c	2020-09-19 15:16:46.399622447 +0200
--- src/vim9execute.c	2020-09-23 21:50:24.034802536 +0200
***************
*** 67,72 ****
--- 67,74 ----
      int		ec_dfunc_idx;	// current function index
      isn_T	*ec_instr;	// array with instructions
      int		ec_iidx;	// index in ec_instr: instruction to execute
+ 
+     garray_T	ec_funcrefs;	// partials that might be a closure
  } ectx_T;
  
  // Get pointer to item relative to the bottom of the stack, -1 is the last one.
***************
*** 165,170 ****
--- 167,173 ----
      ufunc_T *ufunc = dfunc->df_ufunc;
      int	    arg_to_add;
      int	    vararg_count = 0;
+     int	    varcount;
      int	    idx;
      estack_T *entry;
  
***************
*** 212,219 ****
  	    semsg(_(e_nr_arguments_too_many), -arg_to_add);
  	return FAIL;
      }
!     if (ga_grow(&ectx->ec_stack, arg_to_add + 3
! 		       + dfunc->df_varcount + dfunc->df_closure_count) == FAIL)
  	return FAIL;
  
      // Move the vararg-list to below the missing optional arguments.
--- 215,230 ----
  	    semsg(_(e_nr_arguments_too_many), -arg_to_add);
  	return FAIL;
      }
! 
!     // Reserve space for:
!     // - missing arguments
!     // - stack frame
!     // - local variables
!     // - if needed: a counter for number of closures created in
!     //   ectx->ec_funcrefs.
!     varcount = dfunc->df_varcount + dfunc->df_has_closure;
!     if (ga_grow(&ectx->ec_stack, arg_to_add + STACK_FRAME_SIZE + varcount)
! 								       == FAIL)
  	return FAIL;
  
      // Move the vararg-list to below the missing optional arguments.
***************
*** 232,241 ****
      ectx->ec_frame_idx = ectx->ec_stack.ga_len;
  
      // Initialize local variables
!     for (idx = 0; idx < dfunc->df_varcount + dfunc->df_closure_count; ++idx)
  	STACK_TV_BOT(STACK_FRAME_SIZE + idx)->v_type = VAR_UNKNOWN;
!     ectx->ec_stack.ga_len += STACK_FRAME_SIZE
! 				+ dfunc->df_varcount + dfunc->df_closure_count;
  
      // Set execution state to the start of the called function.
      ectx->ec_dfunc_idx = cdf_idx;
--- 243,258 ----
      ectx->ec_frame_idx = ectx->ec_stack.ga_len;
  
      // Initialize local variables
!     for (idx = 0; idx < dfunc->df_varcount; ++idx)
  	STACK_TV_BOT(STACK_FRAME_SIZE + idx)->v_type = VAR_UNKNOWN;
!     if (dfunc->df_has_closure)
!     {
! 	typval_T *tv = STACK_TV_BOT(STACK_FRAME_SIZE + dfunc->df_varcount);
! 
! 	tv->v_type = VAR_NUMBER;
! 	tv->vval.v_number = 0;
!     }
!     ectx->ec_stack.ga_len += STACK_FRAME_SIZE + varcount;
  
      // Set execution state to the start of the called function.
      ectx->ec_dfunc_idx = cdf_idx;
***************
*** 275,296 ****
      int		idx;
      typval_T	*tv;
      int		closure_in_use = FALSE;
  
      if (dfunc->df_ufunc == NULL)
! 	// function was freed
! 	return OK;
      argcount = ufunc_argcount(dfunc->df_ufunc);
      top = ectx->ec_frame_idx - argcount;
  
      // Check if any created closure is still in use.
!     for (idx = 0; idx < dfunc->df_closure_count; ++idx)
      {
! 	tv = STACK_TV(ectx->ec_frame_idx + STACK_FRAME_SIZE
! 						   + dfunc->df_varcount + idx);
! 	if (tv->v_type == VAR_PARTIAL && tv->vval.v_partial != NULL
! 					&& tv->vval.v_partial->pt_refcount > 1)
  	{
! 	    int refcount = tv->vval.v_partial->pt_refcount;
  	    int i;
  
  	    // A Reference in a local variables doesn't count, it gets
--- 292,321 ----
      int		idx;
      typval_T	*tv;
      int		closure_in_use = FALSE;
+     garray_T	*gap = &ectx->ec_funcrefs;
+     varnumber_T	closure_count;
  
      if (dfunc->df_ufunc == NULL)
! 	return OK;  // function was freed
!     if (dfunc->df_has_closure == 0)
! 	return OK;  // no closures
!     tv = STACK_TV(ectx->ec_frame_idx + STACK_FRAME_SIZE + dfunc->df_varcount);
!     closure_count = tv->vval.v_number;
!     if (closure_count == 0)
! 	return OK;  // no funcrefs created
! 
      argcount = ufunc_argcount(dfunc->df_ufunc);
      top = ectx->ec_frame_idx - argcount;
  
      // Check if any created closure is still in use.
!     for (idx = 0; idx < closure_count; ++idx)
      {
! 	partial_T *pt = ((partial_T **)gap->ga_data)[gap->ga_len
! 							- closure_count + idx];
! 
! 	if (pt->pt_refcount > 1)
  	{
! 	    int refcount = pt->pt_refcount;
  	    int i;
  
  	    // A Reference in a local variables doesn't count, it gets
***************
*** 299,306 ****
  	    {
  		typval_T *stv = STACK_TV(ectx->ec_frame_idx
  						       + STACK_FRAME_SIZE + i);
! 		if (stv->v_type == VAR_PARTIAL
! 				  && tv->vval.v_partial == stv->vval.v_partial)
  		    --refcount;
  	    }
  	    if (refcount > 1)
--- 324,330 ----
  	    {
  		typval_T *stv = STACK_TV(ectx->ec_frame_idx
  						       + STACK_FRAME_SIZE + i);
! 		if (stv->v_type == VAR_PARTIAL && pt == stv->vval.v_partial)
  		    --refcount;
  	    }
  	    if (refcount > 1)
***************
*** 355,400 ****
  	    if (tv->v_type == VAR_PARTIAL && tv->vval.v_partial != NULL)
  	    {
  		int i;
- 		typval_T *ctv;
  
! 		for (i = 0; i < dfunc->df_closure_count; ++i)
  		{
! 		    ctv = STACK_TV(ectx->ec_frame_idx + STACK_FRAME_SIZE
! 						     + dfunc->df_varcount + i);
! 		    if (tv->vval.v_partial == ctv->vval.v_partial)
  			break;
  		}
! 		if (i < dfunc->df_closure_count)
! 		{
! 		    (stack + argcount + STACK_FRAME_SIZE + idx)->v_type =
! 								   VAR_UNKNOWN;
  		    continue;
- 		}
  	    }
  
  	    *(stack + argcount + STACK_FRAME_SIZE + idx) = *tv;
  	    tv->v_type = VAR_UNKNOWN;
  	}
  
! 	for (idx = 0; idx < dfunc->df_closure_count; ++idx)
  	{
! 	    tv = STACK_TV(ectx->ec_frame_idx + STACK_FRAME_SIZE
! 						   + dfunc->df_varcount + idx);
! 	    if (tv->v_type == VAR_PARTIAL)
  	    {
! 		partial_T *partial = tv->vval.v_partial;
! 
! 		if (partial->pt_refcount > 1)
! 		{
! 		    ++funcstack->fs_refcount;
! 		    partial->pt_funcstack = funcstack;
! 		    partial->pt_ectx_stack = &funcstack->fs_ga;
! 		    partial->pt_ectx_frame = ectx->ec_frame_idx - top;
! 		}
  	    }
  	}
      }
  
      return OK;
  }
  
--- 379,421 ----
  	    if (tv->v_type == VAR_PARTIAL && tv->vval.v_partial != NULL)
  	    {
  		int i;
  
! 		for (i = 0; i < closure_count; ++i)
  		{
! 		    partial_T *pt = ((partial_T **)gap->ga_data)[gap->ga_len
! 							  - closure_count + i];
! 		    if (tv->vval.v_partial == pt)
  			break;
  		}
! 		if (i < closure_count)
  		    continue;
  	    }
  
  	    *(stack + argcount + STACK_FRAME_SIZE + idx) = *tv;
  	    tv->v_type = VAR_UNKNOWN;
  	}
  
! 	for (idx = 0; idx < closure_count; ++idx)
  	{
! 	    partial_T *pt = ((partial_T **)gap->ga_data)[gap->ga_len
! 							- closure_count + idx];
! 	    if (pt->pt_refcount > 1)
  	    {
! 		++funcstack->fs_refcount;
! 		pt->pt_funcstack = funcstack;
! 		pt->pt_ectx_stack = &funcstack->fs_ga;
! 		pt->pt_ectx_frame = ectx->ec_frame_idx - top;
  	    }
  	}
      }
  
+     for (idx = 0; idx < closure_count; ++idx)
+ 	partial_unref(((partial_T **)gap->ga_data)[gap->ga_len
+ 						       - closure_count + idx]);
+     gap->ga_len -= closure_count;
+     if (gap->ga_len == 0)
+ 	ga_clear(gap);
+ 
      return OK;
  }
  
***************
*** 809,814 ****
--- 830,836 ----
      if (ga_grow(&ectx.ec_stack, 20) == FAIL)
  	return FAIL;
      ga_init2(&ectx.ec_trystack, sizeof(trycmd_T), 10);
+     ga_init2(&ectx.ec_funcrefs, sizeof(partial_T *), 10);
  
      // Put arguments on the stack.
      for (idx = 0; idx < argc; ++idx)
***************
*** 896,909 ****
      }
  
      {
! 	// Reserve space for local variables and closure references.
  	dfunc_T	*dfunc = ((dfunc_T *)def_functions.ga_data)
  							 + ufunc->uf_dfunc_idx;
- 	int	count = dfunc->df_varcount + dfunc->df_closure_count;
  
! 	for (idx = 0; idx < count; ++idx)
  	    STACK_TV_VAR(idx)->v_type = VAR_UNKNOWN;
! 	ectx.ec_stack.ga_len += count;
  
  	ectx.ec_instr = dfunc->df_instr;
      }
--- 918,936 ----
      }
  
      {
! 	// Reserve space for local variables and any closure reference count.
  	dfunc_T	*dfunc = ((dfunc_T *)def_functions.ga_data)
  							 + ufunc->uf_dfunc_idx;
  
! 	for (idx = 0; idx < dfunc->df_varcount; ++idx)
  	    STACK_TV_VAR(idx)->v_type = VAR_UNKNOWN;
! 	ectx.ec_stack.ga_len += dfunc->df_varcount;
! 	if (dfunc->df_has_closure)
! 	{
! 	    STACK_TV_VAR(idx)->v_type = VAR_NUMBER;
! 	    STACK_TV_VAR(idx)->vval.v_number = 0;
! 	    ++ectx.ec_stack.ga_len;
! 	}
  
  	ectx.ec_instr = dfunc->df_instr;
      }
***************
*** 1812,1818 ****
  					       + iptr->isn_arg.funcref.fr_func;
  		    pt->pt_func = pt_dfunc->df_ufunc;
  		    pt->pt_refcount = 1;
- 		    ++pt_dfunc->df_ufunc->uf_refcount;
  
  		    if (pt_dfunc->df_ufunc->uf_flags & FC_CLOSURE)
  		    {
--- 1839,1844 ----
***************
*** 1825,1847 ****
  			pt->pt_ectx_frame = ectx.ec_frame_idx;
  
  			// If this function returns and the closure is still
! 			// 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.
! 			++pt->pt_refcount;
! 			tv = STACK_TV_VAR(dfunc->df_varcount
! 					   + iptr->isn_arg.funcref.fr_var_idx);
! 			if (tv->v_type == VAR_PARTIAL)
  			{
- 			    // TODO: use a garray_T on ectx.
- 			    SOURCING_LNUM = iptr->isn_lnum;
- 			    emsg("Multiple closures not supported yet");
  			    vim_free(pt);
  			    goto failed;
  			}
! 			tv->v_type = VAR_PARTIAL;
! 			tv->vval.v_partial = pt;
  		    }
  
  		    tv = STACK_TV_BOT(0);
  		    ++ectx.ec_stack.ga_len;
--- 1851,1875 ----
  			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;
  
  		    tv = STACK_TV_BOT(0);
  		    ++ectx.ec_stack.ga_len;
***************
*** 3124,3131 ****
  		    dfunc_T	*df = ((dfunc_T *)def_functions.ga_data)
  							    + funcref->fr_func;
  
! 		    smsg("%4d FUNCREF %s $%d", current, df->df_ufunc->uf_name,
! 				     funcref->fr_var_idx + dfunc->df_varcount);
  		}
  		break;
  
--- 3152,3158 ----
  		    dfunc_T	*df = ((dfunc_T *)def_functions.ga_data)
  							    + funcref->fr_func;
  
! 		    smsg("%4d FUNCREF %s", current, df->df_ufunc->uf_name);
  		}
  		break;
  
*** ../vim-8.2.1733/src/testdir/test_vim9_func.vim	2020-09-21 22:02:44.276720646 +0200
--- src/testdir/test_vim9_func.vim	2020-09-23 21:39:30.064382547 +0200
***************
*** 1367,1373 ****
      enddef
      Func()
    END
!   CheckScriptFailure(lines, 'Multiple closures not supported yet')
  enddef
  
  def Test_sort_return_type()
--- 1367,1373 ----
      enddef
      Func()
    END
!   CheckScriptSuccess(lines)
  enddef
  
  def Test_sort_return_type()
*** ../vim-8.2.1733/src/testdir/test_vim9_disassemble.vim	2020-09-16 15:21:56.358720340 +0200
--- src/testdir/test_vim9_disassemble.vim	2020-09-23 21:52:51.798480953 +0200
***************
*** 708,714 ****
    let instr = execute('disassemble WithLambda')
    assert_match('WithLambda\_s*' ..
          'let F = {a -> "X" .. a .. "X"}\_s*' ..
!         '\d FUNCREF <lambda>\d\+ $1\_s*' ..
          '\d STORE $0\_s*' ..
          'return F("x")\_s*' ..
          '\d PUSHS "x"\_s*' ..
--- 708,714 ----
    let instr = execute('disassemble WithLambda')
    assert_match('WithLambda\_s*' ..
          'let F = {a -> "X" .. a .. "X"}\_s*' ..
!         '\d FUNCREF <lambda>\d\+\_s*' ..
          '\d STORE $0\_s*' ..
          'return F("x")\_s*' ..
          '\d PUSHS "x"\_s*' ..
*** ../vim-8.2.1733/src/version.c	2020-09-23 18:51:07.326493552 +0200
--- src/version.c	2020-09-23 19:21:58.007737376 +0200
***************
*** 752,753 ****
--- 752,755 ----
  {   /* Add new patch number below this line */
+ /**/
+     1734,
  /**/

-- 
Team-building exercises come in many forms but they all trace their roots back
to the prison system.  In your typical team-building exercise the employees
are subjected to a variety of unpleasant situations until they become either a
cohesive team or a ring of car jackers.
				(Scott Adams - The Dilbert principle)

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