# HG changeset patch # User Bram Moolenaar # Date 1602697504 -7200 # Node ID 209c7aa56325558dd852a56888ba358eb508c2d9 # Parent a6030e19b08fbcd22f7c17f20e27a918367abf56 patch 8.2.1845: Vim9: function defined in a block can't use block variables Commit: https://github.com/vim/vim/commit/8d739de43b84ef7817b3b5b826d1cbfe7572a62a Author: Bram Moolenaar Date: Wed Oct 14 19:39:19 2020 +0200 patch 8.2.1845: Vim9: function defined in a block can't use block variables Problem: Vim9: function defined in a block can't use variables defined in that block. Solution: First step: Make a second hashtab that holds all script variables, also block-local ones, with more information. diff --git a/src/evalvars.c b/src/evalvars.c --- a/src/evalvars.c +++ b/src/evalvars.c @@ -2882,7 +2882,7 @@ vars_clear_ext(hashtab_T *ht, int free_v } } hash_clear(ht); - ht->ht_used = 0; + hash_init(ht); } /* @@ -3142,30 +3142,10 @@ set_var_const( if (flags & ASSIGN_CONST) di->di_flags |= DI_FLAGS_LOCK; + // A Vim9 script-local variable is also added to sn_all_vars and + // sn_var_vals. if (is_script_local && vim9script) - { - scriptitem_T *si = SCRIPT_ITEM(current_sctx.sc_sid); - - // Store a pointer to the typval_T, so that it can be found by - // index instead of using a hastab lookup. - if (ga_grow(&si->sn_var_vals, 1) == OK) - { - svar_T *sv = ((svar_T *)si->sn_var_vals.ga_data) - + si->sn_var_vals.ga_len; - sv->sv_name = di->di_key; - sv->sv_tv = &di->di_tv; - if (type == NULL) - sv->sv_type = typval2type(tv, &si->sn_type_list); - else - sv->sv_type = type; - sv->sv_const = (flags & ASSIGN_CONST); - sv->sv_export = is_export; - ++si->sn_var_vals.ga_len; - - // let ex_export() know the export worked. - is_export = FALSE; - } - } + add_vim9_script_var(di, tv, type); } if (copy || tv->v_type == VAR_NUMBER || tv->v_type == VAR_FLOAT) diff --git a/src/ex_eval.c b/src/ex_eval.c --- a/src/ex_eval.c +++ b/src/ex_eval.c @@ -914,41 +914,35 @@ enter_block(cstack_T *cstack) { ++cstack->cs_idx; if (in_vim9script()) - cstack->cs_script_var_len[cstack->cs_idx] = - SCRIPT_ITEM(current_sctx.sc_sid)->sn_var_vals.ga_len; + { + scriptitem_T *si = SCRIPT_ITEM(current_sctx.sc_sid); + + cstack->cs_script_var_len[cstack->cs_idx] = si->sn_var_vals.ga_len; + cstack->cs_block_id[cstack->cs_idx] = ++si->sn_current_block_id; + } } static void leave_block(cstack_T *cstack) { - int i; - - if (in_vim9script()) + if (in_vim9script() && SCRIPT_ID_VALID(current_sctx.sc_sid)) { scriptitem_T *si = SCRIPT_ITEM(current_sctx.sc_sid); - hashtab_T *ht = get_script_local_ht(); + int i; - if (ht != NULL) + for (i = cstack->cs_script_var_len[cstack->cs_idx]; + i < si->sn_var_vals.ga_len; ++i) { - 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; - hashitem_T *hi; + svar_T *sv = ((svar_T *)si->sn_var_vals.ga_data) + i; - if (sv->sv_name != NULL) - { - // Remove a variable declared inside the block, if it still - // exists. - hi = hash_find(ht, sv->sv_name); - if (!HASHITEM_EMPTY(hi)) - { - delete_var(ht, hi); - sv->sv_name = NULL; - } - } - } + 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, sv); } + + // TODO: is this needed? + cstack->cs_script_var_len[cstack->cs_idx] = si->sn_var_vals.ga_len; } --cstack->cs_idx; } diff --git a/src/proto/vim9script.pro b/src/proto/vim9script.pro --- a/src/proto/vim9script.pro +++ b/src/proto/vim9script.pro @@ -3,11 +3,14 @@ int in_vim9script(void); void ex_vim9script(exarg_T *eap); int not_in_vim9(exarg_T *eap); void ex_export(exarg_T *eap); -void free_imports(int sid); +void free_imports_and_script_vars(int sid); void ex_import(exarg_T *eap); int find_exported(int sid, char_u *name, ufunc_T **ufunc, type_T **type); 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, svar_T *sv); +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); /* vim: set ft=c : */ diff --git a/src/scriptfile.c b/src/scriptfile.c --- a/src/scriptfile.c +++ b/src/scriptfile.c @@ -1348,8 +1348,8 @@ do_source( } } - // old imports are no longer valid - free_imports(sid); + // old imports and script variables are no longer valid + free_imports_and_script_vars(sid); // in Vim9 script functions are marked deleted if (is_vim9) @@ -1375,6 +1375,7 @@ do_source( // Allocate the local script variables to use for this script. new_script_vars(script_items.ga_len); ga_init2(&si->sn_var_vals, sizeof(svar_T), 10); + hash_init(&si->sn_all_vars.dv_hashtab); ga_init2(&si->sn_imports, sizeof(imported_T), 10); ga_init2(&si->sn_type_list, sizeof(type_T), 10); # ifdef FEAT_PROFILE @@ -1592,7 +1593,7 @@ free_scriptnames(void) vim_free(si->sn_vars); vim_free(si->sn_name); - free_imports(i); + free_imports_and_script_vars(i); free_string_option(si->sn_save_cpo); # ifdef FEAT_PROFILE ga_clear(&si->sn_prl_ga); diff --git a/src/structs.h b/src/structs.h --- a/src/structs.h +++ b/src/structs.h @@ -889,6 +889,7 @@ typedef struct { } cs_pend; void *cs_forinfo[CSTACK_LEN]; // info used by ":for" int cs_line[CSTACK_LEN]; // line nr of ":while"/":for" line + int cs_block_id[CSTACK_LEN]; // block ID stack int cs_script_var_len[CSTACK_LEN]; // value of sn_var_vals.ga_len // when entering the block int cs_idx; // current entry, or -1 if none @@ -1701,18 +1702,47 @@ struct funccal_entry { * Holds the hashtab with variables local to each sourced script. * Each item holds a variable (nameless) that points to the dict_T. */ -typedef struct -{ +typedef struct { dictitem_T sv_var; dict_T sv_dict; } scriptvar_T; /* + * Entry for "sn_all_vars". Contains the s: variables from sn_vars plus the + * block-local ones. + */ +typedef struct sallvar_S sallvar_T; +struct sallvar_S { + sallvar_T *sav_next; // var with same name but different block + int sav_block_id; // block ID where declared + int sav_var_vals_idx; // index in sn_var_vals + + // So long as the variable is valid (block it was defined in is still + // active) "sav_di" is used. It is set to NULL when leaving the block, + // then sav_tv and sav_flags are used. + dictitem_T *sav_di; // dictitem with di_key and di_tv + typval_T sav_tv; // type and value of the variable + char_u sav_flags; // DI_FLAGS_ flags (only used for variable) + char_u sav_key[1]; // key (actually longer!) +}; + +/* + * In the sn_all_vars hashtab item "hi_key" points to "sav_key" in a sallvar_T. + * This makes it possible to store and find the sallvar_T. + * SAV2HIKEY() converts a sallvar_T pointer to a hashitem key pointer. + * HIKEY2SAV() converts a hashitem key pointer to a sallvar_T pointer. + * HI2SAV() converts a hashitem pointer to a sallvar_T pointer. + */ +#define SAV2HIKEY(sav) ((sav)->sav_key) +#define HIKEY2SAV(p) ((sallvar_T *)(p - offsetof(sallvar_T, sav_key))) +#define HI2SAV(hi) HIKEY2SAV((hi)->hi_key) + +/* * Entry for "sn_var_vals". Used for script-local variables. */ typedef struct { - char_u *sv_name; // points into "sn_vars" di_key - typval_T *sv_tv; // points into "sn_vars" di_tv + char_u *sv_name; // points into "sn_all_vars" di_key + typval_T *sv_tv; // points into "sn_vars" or "sn_all_vars" di_tv type_T *sv_type; int sv_const; int sv_export; // "export let var = val" @@ -1742,12 +1772,27 @@ typedef struct { char_u *sn_name; - scriptvar_T *sn_vars; // stores s: variables for this script - garray_T sn_var_vals; // same variables as a list of svar_T + // "sn_vars" stores the s: variables currently valid. When leaving a block + // variables local to that block are removed. + scriptvar_T *sn_vars; + + // Specific for a Vim9 script. + // "sn_all_vars" stores all script variables ever declared. So long as the + // variable is still valid the value is in "sn_vars->sv_dict...di_tv". + // When the block of a declaration is left the value is moved to + // "sn_all_vars..sav_tv". + // Variables with duplicate names are possible, the sav_block_id must be + // used to check that which variable is valid. + dict_T sn_all_vars; // all script variables, dict of sallvar_T + + // Stores the same variables as in "sn_all_vars" as a list of svar_T, so + // that they can be quickly found by index instead of a hash table lookup. + // Also stores the type. + garray_T sn_var_vals; garray_T sn_imports; // imported items, imported_T - garray_T sn_type_list; // keeps types used by variables + int sn_current_block_id; // Unique ID for each script block int sn_version; // :scriptversion int sn_had_command; // TRUE if any command was executed diff --git a/src/version.c b/src/version.c --- 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 */ /**/ + 1845, +/**/ 1844, /**/ 1843, diff --git a/src/vim9script.c b/src/vim9script.c --- a/src/vim9script.c +++ b/src/vim9script.c @@ -134,7 +134,7 @@ new_imported(garray_T *gap) * Free all imported items in script "sid". */ void -free_imports(int sid) +free_imports_and_script_vars(int sid) { scriptitem_T *si = SCRIPT_ITEM(sid); int idx; @@ -146,7 +146,9 @@ free_imports(int sid) vim_free(imp->imp_name); } ga_clear(&si->sn_imports); - ga_clear(&si->sn_var_vals); + + free_all_script_vars(si); + clear_type_list(&si->sn_type_list); } @@ -565,6 +567,127 @@ vim9_declare_scriptvar(exarg_T *eap, cha } /* + * Vim9 part of adding a script variable: add it to sn_all_vars and + * sn_var_vals. + * When "type" is NULL use "tv" for the type. + */ + void +add_vim9_script_var(dictitem_T *di, typval_T *tv, type_T *type) +{ + scriptitem_T *si = SCRIPT_ITEM(current_sctx.sc_sid); + + // Store a pointer to the typval_T, so that it can be found by + // index instead of using a hastab lookup. + if (ga_grow(&si->sn_var_vals, 1) == OK) + { + svar_T *sv = ((svar_T *)si->sn_var_vals.ga_data) + + si->sn_var_vals.ga_len; + hashitem_T *hi; + sallvar_T *newsav = (sallvar_T *)alloc_clear( + sizeof(sallvar_T) + STRLEN(di->di_key)); + + if (newsav == NULL) + return; + + sv->sv_tv = &di->di_tv; + if (type == NULL) + sv->sv_type = typval2type(tv, &si->sn_type_list); + else + sv->sv_type = type; + sv->sv_const = (di->di_flags & DI_FLAGS_LOCK) ? ASSIGN_CONST : 0; + sv->sv_export = is_export; + newsav->sav_var_vals_idx = si->sn_var_vals.ga_len; + ++si->sn_var_vals.ga_len; + + STRCPY(&newsav->sav_key, di->di_key); + sv->sv_name = newsav->sav_key; + newsav->sav_di = di; + newsav->sav_block_id = si->sn_current_block_id; + + hi = hash_find(&si->sn_all_vars.dv_hashtab, newsav->sav_key); + if (!HASHITEM_EMPTY(hi)) + { + sallvar_T *sav = HI2SAV(hi); + + // variable with this name exists in another block + while (sav->sav_next != NULL) + sav = sav->sav_next; + sav->sav_next = newsav; + } + else + // new variable name + hash_add(&si->sn_all_vars.dv_hashtab, newsav->sav_key); + + // let ex_export() know the export worked. + is_export = FALSE; + } +} + + void +hide_script_var(scriptitem_T *si, svar_T *sv) +{ + hashtab_T *script_ht = get_script_local_ht(); + hashtab_T *all_ht = &si->sn_all_vars.dv_hashtab; + hashitem_T *script_hi; + hashitem_T *all_hi; + + // Remove a variable declared inside the block, if it still exists. + // 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); + if (!HASHITEM_EMPTY(script_hi) && !HASHITEM_EMPTY(all_hi)) + { + dictitem_T *di = HI2DI(script_hi); + sallvar_T *sav = HI2SAV(all_hi); + + sav->sav_tv = di->di_tv; + di->di_tv.v_type = VAR_UNKNOWN; + sav->sav_flags = di->di_flags; + sav->sav_di = NULL; + delete_var(script_ht, script_hi); + } +} + +/* + * Free the script variables from "sn_all_vars". + */ + void +free_all_script_vars(scriptitem_T *si) +{ + int todo; + hashtab_T *ht = &si->sn_all_vars.dv_hashtab; + hashitem_T *hi; + sallvar_T *sav; + sallvar_T *sav_next; + + hash_lock(ht); + todo = (int)ht->ht_used; + for (hi = ht->ht_array; todo > 0; ++hi) + { + if (!HASHITEM_EMPTY(hi)) + { + --todo; + + // Free the variable. Don't remove it from the hashtab, ht_array + // might change then. hash_clear() takes care of it later. + sav = HI2SAV(hi); + while (sav != NULL) + { + sav_next = sav->sav_next; + if (sav->sav_di == NULL) + clear_tv(&sav->sav_tv); + vim_free(sav); + sav = sav_next; + } + } + } + hash_clear(ht); + hash_init(ht); + + ga_clear(&si->sn_var_vals); +} + +/* * Find the script-local variable that links to "dest". * Returns NULL if not found. */