# HG changeset patch # User Christian Brabandt # Date 1697053504 -7200 # Node ID f99f5a56ff27a775885e20ba092be0943c0e9782 # Parent 1a769647d6619316c03a43e66044e7f5fba4ad27 patch 9.0.2015: Vim9: does not handle islocked() from a method correctly Commit: https://github.com/vim/vim/commit/4c8da025ef8140168b7a09d9fe922ce4bb40f19d Author: Ernie Rael Date: Wed Oct 11 21:35:11 2023 +0200 patch 9.0.2015: Vim9: does not handle islocked() from a method correctly Problem: Vim9: does not handle islocked() from a method correctly Solution: Handle islocked() builtin from a method. - Setup `lval_root` from `f_islocked()`. - Add function `fill_exec_lval_root()` to get info about executing method. - `sync_root` added in get_lval to handle method member access. - Conservative approach to reference counting. closes: #13309 Signed-off-by: Christian Brabandt Co-authored-by: Ernie Rael diff --git a/src/eval.c b/src/eval.c --- a/src/eval.c +++ b/src/eval.c @@ -1050,11 +1050,14 @@ flag_string_T glv_flag_strings[] = { * execute_instructions: ISN_LOCKUNLOCK - sets lval_root from stack. */ static void -get_lval_root(lval_T *lp, lval_root_T *lr) +fill_lval_from_lval_root(lval_T *lp, lval_root_T *lr) { #ifdef LOG_LOCKVAR - ch_log(NULL, "LKVAR: get_lval_root(): name %s", lp->ll_name); + ch_log(NULL, "LKVAR: fill_lval_from_lval_root(): name %s, tv %p", + lp->ll_name, (void*)lr->lr_tv); #endif + if (lr->lr_tv == NULL) + return; if (!lr->lr_is_arg && lr->lr_tv->v_type == VAR_CLASS) { if (lr->lr_tv->vval.v_class != NULL) @@ -1177,15 +1180,14 @@ get_lval( #ifdef LOG_LOCKVAR if (lval_root == NULL) - ch_log(NULL, - "LKVAR: get_lval(): name %s, lval_root (nil)", name); + ch_log(NULL, "LKVAR: get_lval(): name: %s, lval_root (nil)", name); else - ch_log(NULL, - "LKVAR: get_lval(): name %s, lr_tv %p lr_is_arg %d", - name, (void*)lval_root->lr_tv, lval_root->lr_is_arg); + ch_log(NULL, "LKVAR: get_lval(): name: %s, lr_tv %p lr_is_arg %d", + name, (void*)lval_root->lr_tv, lval_root->lr_is_arg); char buf[80]; - ch_log(NULL, "LKVAR: ...: GLV flags %s", + ch_log(NULL, "LKVAR: ...: GLV flags: %s", flags_tostring(flags, glv_flag_strings, buf, sizeof(buf))); + int log_sync_root_key = FALSE; #endif // Clear everything in "lp". @@ -1324,20 +1326,26 @@ get_lval( } } - // Without [idx] or .key we are done. - if ((*p != '[' && *p != '.')) + int sync_root = FALSE; + if (vim9script && lval_root != NULL) + { + cl_exec = lval_root->lr_cl_exec; + sync_root = lval_root->lr_sync_root; + } + + // Without [idx] or .key we are done, unless doing sync_root. + if (*p != '[' && *p != '.' && (*name == NUL || !sync_root)) { if (lval_root != NULL) - get_lval_root(lp, lval_root); + fill_lval_from_lval_root(lp, lval_root); return p; } - if (vim9script && lval_root != NULL) + if (vim9script && lval_root != NULL && lval_root->lr_tv != NULL) { // using local variable lp->ll_tv = lval_root->lr_tv; v = NULL; - cl_exec = lval_root->lr_cl_exec; } else { @@ -1367,7 +1375,7 @@ get_lval( */ var1.v_type = VAR_UNKNOWN; var2.v_type = VAR_UNKNOWN; - while (*p == '[' || (*p == '.' && p[1] != '=' && p[1] != '.')) + while (*p == '[' || (*p == '.' && p[1] != '=' && p[1] != '.') || sync_root) { vartype_T v_type = lp->ll_tv->v_type; @@ -1407,6 +1415,10 @@ get_lval( emsg(_(e_slice_must_come_last)); return NULL; } +#ifdef LOG_LOCKVAR + ch_log(NULL, "LKVAR: get_lval() loop: p: %s, type: %s", p, + vartype_name(v_type)); +#endif if (vim9script && lp->ll_valtype == NULL && v != NULL @@ -1417,11 +1429,29 @@ get_lval( // Vim9 script local variable: get the type if (sv != NULL) + { lp->ll_valtype = sv->sv_type; +#ifdef LOG_LOCKVAR + ch_log(NULL, "LKVAR: ... loop: vim9 assign type: %s", + vartype_name(lp->ll_valtype->tt_type)); +#endif + } } len = -1; - if (*p == '.') + if (sync_root) + { + // For example, the first token is a member variable name and + // lp->ll_tv is a class/object. + // Process it directly without looking for "[idx]" or ".name". + key = name; + sync_root = FALSE; // only first time through +#ifdef LOG_LOCKVAR + log_sync_root_key = TRUE; + ch_log(NULL, "LKVAR: ... loop: name: %s, sync_root", name); +#endif + } + else if (*p == '.') { key = p + 1; for (len = 0; ASCII_ISALNUM(key[len]) || key[len] == '_'; ++len) @@ -1512,6 +1542,17 @@ get_lval( // Skip to past ']'. ++p; } +#ifdef LOG_LOCKVAR + if (log_sync_root_key) + ch_log(NULL, "LKVAR: ... loop: p: %s, sync_root key: %s", p, + key); + else if (len == -1) + ch_log(NULL, "LKVAR: ... loop: p: %s, '[' key: %s", p, + empty1 ? ":" : (char*)tv_get_string(&var1)); + else + ch_log(NULL, "LKVAR: ... loop: p: %s, '.' key: %s", p, key); + log_sync_root_key = FALSE; +#endif if (v_type == VAR_DICT) { @@ -1700,8 +1741,14 @@ get_lval( lp->ll_list = NULL; class_T *cl; - if (v_type == VAR_OBJECT && lp->ll_tv->vval.v_object != NULL) + if (v_type == VAR_OBJECT) { + if (lp->ll_tv->vval.v_object == NULL) + { + if (!quiet) + emsg(_(e_using_null_object)); + return NULL; + } cl = lp->ll_tv->vval.v_object->obj_class; lp->ll_object = lp->ll_tv->vval.v_object; } diff --git a/src/evalfunc.c b/src/evalfunc.c --- a/src/evalfunc.c +++ b/src/evalfunc.c @@ -7308,6 +7308,83 @@ f_invert(typval_T *argvars, typval_T *re } /* + * Free resources in lval_root allocated by fill_exec_lval_root(). + */ + static void +free_lval_root(lval_root_T *root) +{ + if (root->lr_tv != NULL) + free_tv(root->lr_tv); + class_unref(root->lr_cl_exec); + root->lr_tv = NULL; + root->lr_cl_exec = NULL; +} + +/* + * This is used if executing in a method, the argument string is a + * variable/item expr/reference. If it starts with a potential class/object + * variable then return OK, may get later errors in get_lval. + * + * Adjust "root" as needed. Note that name may change (for example to skip + * "this") and is returned. lr_tv may be changed or freed. + * + * Always returns OK. + * Free resources and return FAIL if the root should not be used. Otherwise OK. + */ + + static int +fix_variable_reference_lval_root(lval_root_T *root, char_u **p_name) +{ + char_u *name = *p_name; + char_u *end; + dictitem_T *di; + + // Only set lr_sync_root and lr_tv if the name is an object/class + // reference: object ("this.") or class because name is class variable. + if (root->lr_tv->v_type == VAR_OBJECT) + { + if (STRNCMP("this.", name, 5) == 0) + { + name += 5; + root->lr_sync_root = TRUE; + } + else if (STRCMP("this", name) == 0) + { + name += 4; + root->lr_sync_root = TRUE; + } + } + if (!root->lr_sync_root) // not object member, try class member + { + // Explicitly check if the name is a class member. + // If it's not then do nothing. + for (end = name; ASCII_ISALNUM(*end) || *end == '_'; ++end) + ; + if (class_member_lookup(root->lr_cl_exec, name, end - name, NULL) + != NULL) + { + // Using a class, so reference the class tv. + di = find_var(root->lr_cl_exec->class_name, NULL, FALSE); + if (di != NULL) + { + // replace the lr_tv + clear_tv(root->lr_tv); + copy_tv(&di->di_tv, root->lr_tv); + root->lr_sync_root = TRUE; + } + } + } + if (!root->lr_sync_root) + { + free_tv(root->lr_tv); + root->lr_tv = NULL; // Not a member variable + } + *p_name = name; + // If FAIL, then must free_lval_root(root); + return OK; +} + +/* * "islocked()" function */ static void @@ -7322,9 +7399,34 @@ f_islocked(typval_T *argvars, typval_T * if (in_vim9script() && check_for_string_arg(argvars, 0) == FAIL) return; - end = get_lval(tv_get_string(&argvars[0]), NULL, &lv, FALSE, FALSE, + char_u *name = tv_get_string(&argvars[0]); +#ifdef LOG_LOCKVAR + ch_log(NULL, "LKVAR: f_islocked(): name: %s", name); +#endif + + lval_root_T aroot; // fully initialized in fill_exec_lval_root + lval_root_T *root = NULL; + + // Set up lval_root if executing in a method. + if (fill_exec_lval_root(&aroot) == OK) + { + // Almost always produces a valid lval_root since lr_cl_exec is used + // for access verification, lr_tv may be set to NULL. + char_u *tname = name; + if (fix_variable_reference_lval_root(&aroot, &tname) == OK) + { + name = tname; + root = &aroot; + } + } + + lval_root_T *lval_root_save = lval_root; + lval_root = root; + end = get_lval(name, NULL, &lv, FALSE, FALSE, GLV_NO_AUTOLOAD | GLV_READ_ONLY | GLV_NO_DECL, FNE_CHECK_START); + lval_root = lval_root_save; + if (end != NULL && lv.ll_name != NULL) { if (*end != NUL) @@ -7347,6 +7449,10 @@ f_islocked(typval_T *argvars, typval_T * || tv_islocked(&di->di_tv)); } } + else if (lv.ll_is_root) + { + rettv->vval.v_number = tv_islocked(lv.ll_tv); + } else if (lv.ll_object != NULL) { typval_T *tv = ((typval_T *)(lv.ll_object + 1)) + lv.ll_oi; @@ -7376,6 +7482,8 @@ f_islocked(typval_T *argvars, typval_T * } } + if (root != NULL) + free_lval_root(root); clear_lval(&lv); } diff --git a/src/proto/vim9execute.pro b/src/proto/vim9execute.pro --- a/src/proto/vim9execute.pro +++ b/src/proto/vim9execute.pro @@ -4,6 +4,7 @@ void update_has_breakpoint(ufunc_T *ufun int funcstack_check_refcount(funcstack_T *funcstack); int set_ref_in_funcstacks(int copyID); int in_def_function(void); +int fill_exec_lval_root(lval_root_T *lr); ectx_T *clear_current_ectx(void); void restore_current_ectx(ectx_T *ectx); int add_defer_function(char_u *name, int argcount, typval_T *argvars); diff --git a/src/structs.h b/src/structs.h --- a/src/structs.h +++ b/src/structs.h @@ -4604,13 +4604,16 @@ typedef struct lval_S } lval_T; /** - * This may be used to specify the base type that get_lval() uses when + * This may be used to specify the base typval that get_lval() uses when * following a chain, for example a[idx1][idx2]. + * The lr_sync_root flags signals get_lval that the first time through + * the indexing loop, skip handling '.' and '[idx]'. */ typedef struct lval_root_S { typval_T *lr_tv; class_T *lr_cl_exec; // executing class for access checking int lr_is_arg; + int lr_sync_root; } lval_root_T; // Structure used to save the current state. Used when executing Normal mode diff --git a/src/testdir/test_vim9_class.vim b/src/testdir/test_vim9_class.vim --- a/src/testdir/test_vim9_class.vim +++ b/src/testdir/test_vim9_class.vim @@ -1704,8 +1704,7 @@ def Test_class_member() var obj: A obj.val = "" END - # FIXME(in source): this should give E1360 as well! - v9.CheckSourceFailure(lines, 'E1012: Type mismatch; expected object but got string', 7) + v9.CheckSourceFailure(lines, 'E1360: Using a null object', 7) # Test for accessing a member on a null object, at script level lines =<< trim END @@ -4259,8 +4258,249 @@ def Test_lockvar_islocked() assert_equal(0, islocked("C.c1[0]")) END v9.CheckSourceSuccess(lines) - lines =<< trim END - END + + # Do islocked() from an object method + # and then from a class method + lines =<< trim END + vim9script + + var l0o0 = [ [0], [1], [2]] + var l0o1 = [ [10], [11], [12]] + var l0c0 = [[120], [121], [122]] + var l0c1 = [[130], [131], [132]] + + class C0 + this.o0: list> = l0o0 + this.o1: list> = l0o1 + static c0: list> = l0c0 + static c1: list> = l0c1 + def Islocked(arg: string): number + return islocked(arg) + enddef + static def SIslocked(arg: string): number + return islocked(arg) + enddef + endclass + + var l2o0 = [[20000], [20001], [20002]] + var l2o1 = [[20010], [20011], [20012]] + var l2c0 = [[20120], [20121], [20122]] + var l2c1 = [[20130], [20131], [20132]] + + class C2 + this.o0: list> = l2o0 + this.o1: list> = l2o1 + static c0: list> = l2c0 + static c1: list> = l2c1 + def Islocked(arg: string): number + return islocked(arg) + enddef + static def SIslocked(arg: string): number + return islocked(arg) + enddef + endclass + + var obj0 = C0.new() + var obj2 = C2.new() + + var l = [ obj0, null_object, obj2 ] + + # lock list, object func access through script var expr + assert_equal(0, obj0.Islocked("l[0].o0")) + assert_equal(0, obj0.Islocked("l[0].o0[2]")) + lockvar l0o0 + assert_equal(1, obj0.Islocked("l[0].o0")) + assert_equal(1, obj0.Islocked("l[0].o0[2]")) + + #echo "check-b" obj2.Islocked("l[1].o1") # NULL OBJECT + + # lock list element, object func access through script var expr + lockvar l0o1[1] + assert_equal(0, obj0.Islocked("this.o1[0]")) + assert_equal(1, obj0.Islocked("this.o1[1]")) + + assert_equal(0, obj0.Islocked("this.o1")) + lockvar l0o1 + assert_equal(1, obj0.Islocked("this.o1")) + unlockvar l0o1 + + lockvar l0c1[1] + + # static by class name member expr from same class + assert_equal(0, obj0.Islocked("C0.c1[0]")) + assert_equal(1, obj0.Islocked("C0.c1[1]")) + # static by bare name member expr from same class + assert_equal(0, obj0.Islocked("c1[0]")) + assert_equal(1, obj0.Islocked("c1[1]")) + + # static by class name member expr from other class + assert_equal(0, obj2.Islocked("C0.c1[0]")) + assert_equal(1, obj2.Islocked("C0.c1[1]")) + # static by bare name member expr from other class + assert_equal(0, obj2.Islocked("c1[0]")) + assert_equal(0, obj2.Islocked("c1[1]")) + + + # static by bare name in same class + assert_equal(0, obj0.Islocked("c0")) + lockvar l0c0 + assert_equal(1, obj0.Islocked("c0")) + + # + # similar stuff, but use static method + # + + unlockvar l0o0 + + # lock list, object func access through script var expr + assert_equal(0, C0.SIslocked("l[0].o0")) + assert_equal(0, C0.SIslocked("l[0].o0[2]")) + lockvar l0o0 + assert_equal(1, C0.SIslocked("l[0].o0")) + assert_equal(1, C0.SIslocked("l[0].o0[2]")) + + unlockvar l0o1 + + # can't access "this" from class method + try + C0.SIslocked("this.o1[0]") + call assert_0(1, '"C0.SIslocked("this.o1[0]")" should have failed') + catch + call assert_exception('E121: Undefined variable: this') + endtry + + lockvar l0c1[1] + + # static by class name member expr from same class + assert_equal(0, C0.SIslocked("C0.c1[0]")) + assert_equal(1, C0.SIslocked("C0.c1[1]")) + # static by bare name member expr from same class + assert_equal(0, C0.SIslocked("c1[0]")) + assert_equal(1, C0.SIslocked("c1[1]")) + + # static by class name member expr from other class + assert_equal(0, C2.SIslocked("C0.c1[0]")) + assert_equal(1, C2.SIslocked("C0.c1[1]")) + # static by bare name member expr from other class + assert_equal(0, C2.SIslocked("c1[0]")) + assert_equal(0, C2.SIslocked("c1[1]")) + + + # static by bare name in same class + unlockvar l0c0 + assert_equal(0, C0.SIslocked("c0")) + lockvar l0c0 + assert_equal(1, C0.SIslocked("c0")) + END + v9.CheckSourceSuccess(lines) + + # Check islocked class/object from various places. + lines =<< trim END + vim9script + + class C + def Islocked(arg: string): number + return islocked(arg) + enddef + static def SIslocked(arg: string): number + return islocked(arg) + enddef + endclass + var obj = C.new() + + # object method + assert_equal(0, obj.Islocked("this")) + assert_equal(0, obj.Islocked("C")) + + # class method + ### assert_equal(0, C.SIslocked("this")) + assert_equal(0, C.SIslocked("C")) + + #script level + var v: number + v = islocked("C") + assert_equal(0, v) + v = islocked("obj") + assert_equal(0, v) + END + v9.CheckSourceSuccess(lines) +enddef + +def Test_lockvar_islocked_notfound() + # Try non-existent things + var lines =<< trim END + vim9script + + class C + def Islocked(arg: string): number + return islocked(arg) + enddef + static def SIslocked(arg: string): number + return islocked(arg) + enddef + endclass + var obj = C.new() + assert_equal(-1, obj.Islocked("anywhere")) + assert_equal(-1, C.SIslocked("notanywhere")) + END + v9.CheckSourceSuccess(lines) + + # Something not found of the form "name1.name2" is an error + lines =<< trim END + vim9script + + islocked("one.two") + END + v9.CheckSourceFailure(lines, 'E121: Undefined variable: one') + + lines =<< trim END + vim9script + + class C + this.val = { key: "value" } + def Islocked(arg: string): number + return islocked(arg) + enddef + endclass + var obj = C.new() + obj.Islocked("this.val.not_there")) + END + v9.CheckSourceFailure(lines, 'E716: Key not present in Dictionary: "not_there"') + + lines =<< trim END + vim9script + + class C + def Islocked(arg: string): number + return islocked(arg) + enddef + endclass + var obj = C.new() + obj.Islocked("this.notobjmember") + END + v9.CheckSourceFailure(lines, 'E1326: Variable not found on object "C": notobjmember') + + # access a script variable through methods + lines =<< trim END + vim9script + + var l = [1] + class C + def Islocked(arg: string): number + return islocked(arg) + enddef + static def SIslocked(arg: string): number + return islocked(arg) + enddef + endclass + var obj = C.new() + assert_equal(0, obj.Islocked("l")) + assert_equal(0, C.SIslocked("l")) + lockvar l + assert_equal(1, obj.Islocked("l")) + assert_equal(1, C.SIslocked("l")) + END + v9.CheckSourceSuccess(lines) enddef " Test for a private object method diff --git a/src/version.c b/src/version.c --- a/src/version.c +++ b/src/version.c @@ -705,6 +705,8 @@ static char *(features[]) = static int included_patches[] = { /* Add new patch number below this line */ /**/ + 2015, +/**/ 2014, /**/ 2013, diff --git a/src/vim9execute.c b/src/vim9execute.c --- a/src/vim9execute.c +++ b/src/vim9execute.c @@ -926,6 +926,41 @@ in_def_function(void) } /* + * If executing a class/object method, then fill in the lval_T. + * Set lr_tv to the executing item, and lr_exec_class to the executing class; + * use free_tv and class_unref when finished with the lval_root. + * For use by builtin functions. + * + * Return FAIL and do nothing if not executing in a class; otherwise OK. + */ + int +fill_exec_lval_root(lval_root_T *root) +{ + ectx_T *ectx = current_ectx; + if (ectx != NULL) + { + dfunc_T *dfunc = ((dfunc_T *)def_functions.ga_data) + + current_ectx->ec_dfunc_idx; + ufunc_T *ufunc = dfunc->df_ufunc; + if (ufunc->uf_class != NULL) // executing a method? + { + typval_T *tv = alloc_tv(); + if (tv != NULL) + { + CLEAR_POINTER(root); + root->lr_tv = tv; + copy_tv(STACK_TV_VAR(0), root->lr_tv); + root->lr_cl_exec = ufunc->uf_class; + ++root->lr_cl_exec->class_refcount; + return OK; + } + } + } + + return FAIL; +} + +/* * Clear "current_ectx" and return the previous value. To be used when calling * a user function. */ @@ -4185,21 +4220,20 @@ exec_instructions(ectx_T *ectx) case ISN_LOCKUNLOCK: { - lval_root_T *lval_root_save = lval_root; - int res; #ifdef LOG_LOCKVAR ch_log(NULL, "LKVAR: execute INS_LOCKUNLOCK isn_arg %s", iptr->isn_arg.string); #endif + lval_root_T *lval_root_save = lval_root; // Stack has the local variable, argument the whole :lock // or :unlock command, like ISN_EXEC. --ectx->ec_stack.ga_len; - lval_root_T root = { STACK_TV_BOT(0), - iptr->isn_arg.lockunlock.lu_cl_exec, - iptr->isn_arg.lockunlock.lu_is_arg }; + lval_root_T root = { .lr_tv = STACK_TV_BOT(0), + .lr_cl_exec = iptr->isn_arg.lockunlock.lu_cl_exec, + .lr_is_arg = iptr->isn_arg.lockunlock.lu_is_arg }; lval_root = &root; - res = exec_command(iptr, + int res = exec_command(iptr, iptr->isn_arg.lockunlock.lu_string); clear_tv(root.lr_tv); lval_root = lval_root_save;