# HG changeset patch # User Christian Brabandt # Date 1469983506 -7200 # Node ID 79862f44c6471462c72de41fb12fd0b9e109c466 # Parent 8c110d08b87f6194fb8a28b903583f4c9b759079 commit https://github.com/vim/vim/commit/580164481924ed8611eb79f0247a0eb1ca0b3b9a Author: Bram Moolenaar Date: Sun Jul 31 18:30:22 2016 +0200 patch 7.4.2136 Problem: Closure function fails. Solution: Don't reset uf_scoped when it points to another funccal. diff --git a/src/testdir/test_lambda.vim b/src/testdir/test_lambda.vim --- a/src/testdir/test_lambda.vim +++ b/src/testdir/test_lambda.vim @@ -247,3 +247,27 @@ function! Test_closure_unlet() call assert_false(has_key(s:foo(), 'x')) call test_garbagecollect_now() endfunction + +function! LambdaFoo() + let x = 0 + function! LambdaBar() closure + let x += 1 + return x + endfunction + return function('LambdaBar') +endfunction + +func Test_closure_refcount() + let g:Count = LambdaFoo() + call test_garbagecollect_now() + call assert_equal(1, g:Count()) + let g:Count2 = LambdaFoo() + call test_garbagecollect_now() + call assert_equal(1, g:Count2()) + call assert_equal(2, g:Count()) + call assert_equal(3, g:Count2()) + + " This causes memory access errors. + " delfunc LambdaFoo + " delfunc LambdaBar +endfunc diff --git a/src/userfunc.c b/src/userfunc.c --- a/src/userfunc.c +++ b/src/userfunc.c @@ -150,6 +150,7 @@ static int # endif prof_self_cmp(const void *s1, const void *s2); #endif +static void funccal_unref(funccall_T *fc, ufunc_T *fp); void func_init() @@ -258,6 +259,23 @@ err_ret: } /* + * Register function "fp" as using "current_funccal" as its scope. + */ + static int +register_closure(ufunc_T *fp) +{ + funccal_unref(fp->uf_scoped, NULL); + fp->uf_scoped = current_funccal; + current_funccal->fc_refcount++; + if (ga_grow(¤t_funccal->fc_funcs, 1) == FAIL) + return FAIL; + ((ufunc_T **)current_funccal->fc_funcs.ga_data) + [current_funccal->fc_funcs.ga_len++] = fp; + func_ref(current_funccal->func->uf_name); + return OK; +} + +/* * Parse a lambda expression and get a Funcref from "*arg". * Return OK or FAIL. Returns NOTDONE for dict or {expr}. */ @@ -318,7 +336,7 @@ get_lambda_tv(char_u **arg, typval_T *re sprintf((char*)name, "%d", ++lambda_no); - fp = (ufunc_T *)alloc((unsigned)(sizeof(ufunc_T) + STRLEN(name))); + fp = (ufunc_T *)alloc_clear((unsigned)(sizeof(ufunc_T) + STRLEN(name))); if (fp == NULL) goto errret; @@ -343,13 +361,8 @@ get_lambda_tv(char_u **arg, typval_T *re if (current_funccal != NULL && eval_lavars) { flags |= FC_CLOSURE; - fp->uf_scoped = current_funccal; - current_funccal->fc_refcount++; - if (ga_grow(¤t_funccal->fc_funcs, 1) == FAIL) + if (register_closure(fp) == FAIL) goto errret; - ((ufunc_T **)current_funccal->fc_funcs.ga_data) - [current_funccal->fc_funcs.ga_len++] = fp; - func_ref(current_funccal->func->uf_name); } else fp->uf_scoped = NULL; @@ -660,8 +673,15 @@ free_funccal( ufunc_T *fp = ((ufunc_T **)(fc->fc_funcs.ga_data))[i]; if (fp != NULL) - fp->uf_scoped = NULL; + { + /* Function may have been redefined and point to another + * funccall_T, don't clear it then. */ + if (fp->uf_scoped == fc) + fp->uf_scoped = NULL; + func_unref(fc->func->uf_name); + } } + ga_clear(&fc->fc_funcs); /* The a: variables typevals may not have been allocated, only free the * allocated variables. */ @@ -675,15 +695,6 @@ free_funccal( for (li = fc->l_varlist.lv_first; li != NULL; li = li->li_next) clear_tv(&li->li_tv); - for (i = 0; i < fc->fc_funcs.ga_len; ++i) - { - ufunc_T *fp = ((ufunc_T **)(fc->fc_funcs.ga_data))[i]; - - if (fp != NULL) - func_unref(fc->func->uf_name); - } - ga_clear(&fc->fc_funcs); - func_unref(fc->func->uf_name); vim_free(fc); } @@ -1046,8 +1057,8 @@ call_user_func( current_funccal = fc->caller; --depth; - /* If the a:000 list and the l: and a: dicts are not referenced we can - * free the funccall_T and what's in it. */ + /* If the a:000 list and the l: and a: dicts are not referenced and there + * is no closure using it, we can free the funccall_T and what's in it. */ if (fc->l_varlist.lv_refcount == DO_NOT_FREE_CNT && fc->l_vars.dv_refcount == DO_NOT_FREE_CNT && fc->l_avars.dv_refcount == DO_NOT_FREE_CNT @@ -1061,8 +1072,8 @@ call_user_func( listitem_T *li; int todo; - /* "fc" is still in use. This can happen when returning "a:000" or - * assigning "l:" to a global variable. + /* "fc" is still in use. This can happen when returning "a:000", + * assigning "l:" to a global variable or defining a closure. * Link "fc" in the list for garbage collection later. */ fc->caller = previous_funccal; previous_funccal = fc; @@ -1121,13 +1132,11 @@ funccal_unref(funccall_T *fc, ufunc_T *f func_unref(fc->func->uf_name); if (fp != NULL) - { for (i = 0; i < fc->fc_funcs.ga_len; ++i) { if (((ufunc_T **)(fc->fc_funcs.ga_data))[i] == fp) ((ufunc_T **)(fc->fc_funcs.ga_data))[i] = NULL; } - } } } @@ -1976,6 +1985,12 @@ ex_function(exarg_T *eap) { flags |= FC_CLOSURE; p += 7; + if (current_funccal == NULL) + { + emsg_funcname(N_("E932 Closure function should not be at top level: %s"), + name == NULL ? (char_u *)"" : name); + goto erret; + } } else break; @@ -2265,7 +2280,7 @@ ex_function(exarg_T *eap) } } - fp = (ufunc_T *)alloc((unsigned)(sizeof(ufunc_T) + STRLEN(name))); + fp = (ufunc_T *)alloc_clear((unsigned)(sizeof(ufunc_T) + STRLEN(name))); if (fp == NULL) goto erret; @@ -2311,19 +2326,9 @@ ex_function(exarg_T *eap) fp->uf_lines = newlines; if ((flags & FC_CLOSURE) != 0) { - if (current_funccal == NULL) - { - emsg_funcname(N_("E932 Closure function should not be at top level: %s"), - name); + ++fp->uf_refcount; + if (register_closure(fp) == FAIL) goto erret; - } - fp->uf_scoped = current_funccal; - current_funccal->fc_refcount++; - if (ga_grow(¤t_funccal->fc_funcs, 1) == FAIL) - goto erret; - ((ufunc_T **)current_funccal->fc_funcs.ga_data) - [current_funccal->fc_funcs.ga_len++] = fp; - func_ref(current_funccal->func->uf_name); } else fp->uf_scoped = NULL; @@ -3582,21 +3587,21 @@ find_hi_in_scoped_ht(char_u *name, char_ /* Search in parent scope which is possible to reference from lambda */ current_funccal = current_funccal->func->uf_scoped; - while (current_funccal) + while (current_funccal != NULL) { - ht = find_var_ht(name, varname); - if (ht != NULL && **varname != NUL) - { - hi = hash_find(ht, *varname); - if (!HASHITEM_EMPTY(hi)) - { - *pht = ht; - break; - } - } - if (current_funccal == current_funccal->func->uf_scoped) - break; - current_funccal = current_funccal->func->uf_scoped; + ht = find_var_ht(name, varname); + if (ht != NULL && **varname != NUL) + { + hi = hash_find(ht, *varname); + if (!HASHITEM_EMPTY(hi)) + { + *pht = ht; + break; + } + } + if (current_funccal == current_funccal->func->uf_scoped) + break; + current_funccal = current_funccal->func->uf_scoped; } current_funccal = old_current_funccal; diff --git a/src/version.c b/src/version.c --- a/src/version.c +++ b/src/version.c @@ -764,6 +764,8 @@ static char *(features[]) = static int included_patches[] = { /* Add new patch number below this line */ /**/ + 2136, +/**/ 2135, /**/ 2134,