patch 8.2.1870: Vim9: no need to keep all script variablesv8.2.1870

Problem:    Vim9: no need to keep all script variables.
Solution:   Only keep script variables when a function was defined that could
            use them.  Fix freeing static string on exit.

7 files changed, 76 insertions, 19 deletions

diff --git a/src/ex_eval.c b/src/ex_eval.c
index 22e83ae34c..3703690315 100644
--- a/src/ex_eval.c
+++ b/src/ex_eval.c
@@ -930,16 +930,22 @@ leave_block(cstack_T *cstack)
     {
 	scriptitem_T	*si = SCRIPT_ITEM(current_sctx.sc_sid);
 	int		i;
+	int		func_defined =
+			       cstack->cs_flags[cstack->cs_idx] & CSF_FUNC_DEF;
 
 	for (i = cstack->cs_script_var_len[cstack->cs_idx];
 					       i < si->sn_var_vals.ga_len; ++i)
 	{
 	    svar_T	*sv = ((svar_T *)si->sn_var_vals.ga_data) + i;
 
+	    // sv_name is set to NULL if it was already removed.  This happens
+	    // when it was defined in an inner block and no functions were
+	    // defined there.
 	    if (sv->sv_name != NULL)
 		// Remove a variable declared inside the block, if it still
-		// exists, from sn_vars and move the value into sn_all_vars.
-		hide_script_var(si, i);
+		// exists, from sn_vars and move the value into sn_all_vars
+		// if "func_defined" is non-zero.
+		hide_script_var(si, i, func_defined);
 	}
 
 	// TODO: is this needed?
diff --git a/src/proto/vim9script.pro b/src/proto/vim9script.pro
index 5b83f1219b..8f9cb6d3a0 100644
--- a/src/proto/vim9script.pro
+++ b/src/proto/vim9script.pro
@@ -9,7 +9,7 @@ int find_exported(int sid, char_u *name, ufunc_T **ufunc, type_T **type, cctx_T
 char_u *handle_import(char_u *arg_start, garray_T *gap, int import_sid, evalarg_T *evalarg, void *cctx);
 char_u *vim9_declare_scriptvar(exarg_T *eap, char_u *arg);
 void add_vim9_script_var(dictitem_T *di, typval_T *tv, type_T *type);
-void hide_script_var(scriptitem_T *si, int idx);
+void hide_script_var(scriptitem_T *si, int idx, int func_defined);
 void free_all_script_vars(scriptitem_T *si);
 svar_T *find_typval_in_script(typval_T *dest);
 int check_script_var_type(typval_T *dest, typval_T *value, char_u *name);
diff --git a/src/structs.h b/src/structs.h
index 4520e763c0..eb86e484ed 100644
--- a/src/structs.h
+++ b/src/structs.h
@@ -917,6 +917,8 @@ typedef struct {
 # define CSF_SILENT	0x1000	// "emsg_silent" reset by ":try"
 // Note that CSF_ELSE is only used when CSF_TRY and CSF_WHILE are unset
 // (an ":if"), and CSF_SILENT is only used when CSF_TRY is set.
+//
+#define CSF_FUNC_DEF	0x2000	// a function was defined in this block
 
 /*
  * What's pending for being reactivated at the ":endtry" of this try
diff --git a/src/testdir/test_vim9_script.vim b/src/testdir/test_vim9_script.vim
index 21de344829..9b526b3d9a 100644
--- a/src/testdir/test_vim9_script.vim
+++ b/src/testdir/test_vim9_script.vim
@@ -296,6 +296,25 @@ def Test_block_local_vars()
   delete('Xdidit')
 enddef
 
+def Test_block_local_vars_with_func()
+  var lines =<< trim END
+      vim9script
+      if true
+        var foo = 'foo'
+        if true
+          var bar = 'bar'
+          def Func(): list<string>
+            return [foo, bar]
+          enddef
+        endif
+      endif
+      # function is compiled here, after blocks have finished, can still access
+      # "foo" and "bar"
+      assert_equal(['foo', 'bar'], Func())
+  END
+  CheckScriptSuccess(lines)
+enddef
+
 func g:NoSuchFunc()
   echo 'none'
 endfunc
diff --git a/src/userfunc.c b/src/userfunc.c
index 2e33d604c3..5e1f199079 100644
--- a/src/userfunc.c
+++ b/src/userfunc.c
@@ -3471,27 +3471,35 @@ define_function(exarg_T *eap, char_u *name_arg)
 
     if (eap->cmdidx == CMD_def)
     {
-	int	lnum_save = SOURCING_LNUM;
+	int	    lnum_save = SOURCING_LNUM;
+	cstack_T    *cstack = eap->cstack;
 
 	fp->uf_def_status = UF_TO_BE_COMPILED;
 
 	// error messages are for the first function line
 	SOURCING_LNUM = sourcing_lnum_top;
 
-	if (eap->cstack != NULL && eap->cstack->cs_idx >= 0)
+	if (cstack != NULL && cstack->cs_idx >= 0)
 	{
-	    int count = eap->cstack->cs_idx + 1;
+	    int	    count = cstack->cs_idx + 1;
+	    int	    i;
 
 	    // The block context may be needed for script variables declared in
-	    // a block visible now but not when the function is compiled.
+	    // a block that is visible now but not when the function is called
+	    // later.
 	    fp->uf_block_ids = ALLOC_MULT(int, count);
 	    if (fp->uf_block_ids != NULL)
 	    {
-		mch_memmove(fp->uf_block_ids, eap->cstack->cs_block_id,
+		mch_memmove(fp->uf_block_ids, cstack->cs_block_id,
 							  sizeof(int) * count);
 		fp->uf_block_depth = count;
 	    }
-	    // TODO: set flag in each block to indicate a function was defined
+
+	    // Set flag in each block to indicate a function was defined.  This
+	    // is used to keep the variable when leaving the block, see
+	    // hide_script_var().
+	    for (i = 0; i <= cstack->cs_idx; ++i)
+		cstack->cs_flags[i] |= CSF_FUNC_DEF;
 	}
 
 	// parse the argument types
diff --git a/src/version.c b/src/version.c
index a0bc135466..835de6c5bf 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 */
 /**/
+    1870,
+/**/
     1869,
 /**/
     1868,
diff --git a/src/vim9script.c b/src/vim9script.c
index a6ed4ba79a..cb13f3c171 100644
--- a/src/vim9script.c
+++ b/src/vim9script.c
@@ -51,8 +51,8 @@ ex_vim9script(exarg_T *eap)
 
     if (STRCMP(p_cpo, CPO_VIM) != 0)
     {
-	si->sn_save_cpo = p_cpo;
-	p_cpo = vim_strsave((char_u *)CPO_VIM);
+	si->sn_save_cpo = vim_strsave(p_cpo);
+	set_option_value((char_u *)"cpo", 0L, (char_u *)CPO_VIM, 0);
     }
 }
 
@@ -569,8 +569,8 @@ vim9_declare_scriptvar(exarg_T *eap, char_u *arg)
 }
 
 /*
- * Vim9 part of adding a script variable: add it to sn_all_vars and
- * sn_var_vals.
+ * Vim9 part of adding a script variable: add it to sn_all_vars (lookup by name
+ * with a hashtable) and sn_var_vals (lookup by index).
  * When "type" is NULL use "tv" for the type.
  */
     void
@@ -628,9 +628,11 @@ add_vim9_script_var(dictitem_T *di, typval_T *tv, type_T *type)
 /*
  * Hide a script variable when leaving a block.
  * "idx" is de index in sn_var_vals.
+ * When "func_defined" is non-zero then a function was defined in this block,
+ * the variable may be accessed by it.  Otherwise the variable can be cleared.
  */
     void
-hide_script_var(scriptitem_T *si, int idx)
+hide_script_var(scriptitem_T *si, int idx, int func_defined)
 {
     svar_T	*sv = ((svar_T *)si->sn_var_vals.ga_data) + idx;
     hashtab_T	*script_ht = get_script_local_ht();
@@ -639,6 +641,7 @@ hide_script_var(scriptitem_T *si, int idx)
     hashitem_T	*all_hi;
 
     // Remove a variable declared inside the block, if it still exists.
+    // If it was added in a nested block it will already have been removed.
     // The typval is moved into the sallvar_T.
     script_hi = hash_find(script_ht, sv->sv_name);
     all_hi = hash_find(all_ht, sv->sv_name);
@@ -646,19 +649,36 @@ hide_script_var(scriptitem_T *si, int idx)
     {
 	dictitem_T	*di = HI2DI(script_hi);
 	sallvar_T	*sav = HI2SAV(all_hi);
+	sallvar_T	*sav_prev = NULL;
 
 	// There can be multiple entries with the same name in different
 	// blocks, find the right one.
 	while (sav != NULL && sav->sav_var_vals_idx != idx)
+	{
+	    sav_prev = sav;
 	    sav = sav->sav_next;
+	}
 	if (sav != NULL)
 	{
-	    sav->sav_tv = di->di_tv;
-	    di->di_tv.v_type = VAR_UNKNOWN;
-	    sav->sav_flags = di->di_flags;
-	    sav->sav_di = NULL;
+	    if (func_defined)
+	    {
+		// move the typval from the dictitem to the sallvar
+		sav->sav_tv = di->di_tv;
+		di->di_tv.v_type = VAR_UNKNOWN;
+		sav->sav_flags = di->di_flags;
+		sav->sav_di = NULL;
+		sv->sv_tv = &sav->sav_tv;
+	    }
+	    else
+	    {
+		if (sav_prev == NULL)
+		    hash_remove(all_ht, all_hi);
+		else
+		    sav_prev->sav_next = sav->sav_next;
+		sv->sv_name = NULL;
+		vim_free(sav);
+	    }
 	    delete_var(script_ht, script_hi);
-	    sv->sv_tv = &sav->sav_tv;
 	}
     }
 }