To: vim_dev@googlegroups.com Subject: Patch 7.4.2136 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ------------ Patch 7.4.2136 Problem: Closure function fails. Solution: Don't reset uf_scoped when it points to another funccal. Files: src/userfunc.c, src/testdir/test_lambda.vim *** ../vim-7.4.2135/src/userfunc.c 2016-07-31 14:11:55.178542370 +0200 --- src/userfunc.c 2016-07-31 18:10:19.831429099 +0200 *************** *** 150,155 **** --- 150,156 ---- # 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,263 **** --- 259,281 ---- } /* + * 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,324 **** sprintf((char*)name, "%d", ++lambda_no); ! fp = (ufunc_T *)alloc((unsigned)(sizeof(ufunc_T) + STRLEN(name))); if (fp == NULL) goto errret; --- 336,342 ---- sprintf((char*)name, "%d", ++lambda_no); ! fp = (ufunc_T *)alloc_clear((unsigned)(sizeof(ufunc_T) + STRLEN(name))); if (fp == NULL) goto errret; *************** *** 343,355 **** 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) 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; --- 361,368 ---- if (current_funccal != NULL && eval_lavars) { flags |= FC_CLOSURE; ! if (register_closure(fp) == FAIL) goto errret; } else fp->uf_scoped = NULL; *************** *** 660,667 **** ufunc_T *fp = ((ufunc_T **)(fc->fc_funcs.ga_data))[i]; if (fp != NULL) ! fp->uf_scoped = NULL; } /* The a: variables typevals may not have been allocated, only free the * allocated variables. */ --- 673,687 ---- ufunc_T *fp = ((ufunc_T **)(fc->fc_funcs.ga_data))[i]; if (fp != 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,689 **** 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); } --- 695,700 ---- *************** *** 1046,1053 **** 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 (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 --- 1057,1064 ---- current_funccal = fc->caller; --depth; ! /* 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,1068 **** listitem_T *li; int todo; ! /* "fc" is still in use. This can happen when returning "a:000" or ! * assigning "l:" to a global variable. * Link "fc" in the list for garbage collection later. */ fc->caller = previous_funccal; previous_funccal = fc; --- 1072,1079 ---- listitem_T *li; int todo; ! /* "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,1133 **** 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; } - } } } --- 1132,1142 ---- *************** *** 1976,1981 **** --- 1985,1996 ---- { 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,2271 **** } } ! fp = (ufunc_T *)alloc((unsigned)(sizeof(ufunc_T) + STRLEN(name))); if (fp == NULL) goto erret; --- 2280,2286 ---- } } ! fp = (ufunc_T *)alloc_clear((unsigned)(sizeof(ufunc_T) + STRLEN(name))); if (fp == NULL) goto erret; *************** *** 2311,2329 **** 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); 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; --- 2326,2334 ---- fp->uf_lines = newlines; if ((flags & FC_CLOSURE) != 0) { ! ++fp->uf_refcount; ! if (register_closure(fp) == FAIL) goto erret; } else fp->uf_scoped = NULL; *************** *** 3582,3602 **** /* Search in parent scope which is possible to reference from lambda */ current_funccal = current_funccal->func->uf_scoped; ! while (current_funccal) { ! 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; --- 3587,3607 ---- /* Search in parent scope which is possible to reference from lambda */ current_funccal = current_funccal->func->uf_scoped; ! 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; } current_funccal = old_current_funccal; *** ../vim-7.4.2135/src/testdir/test_lambda.vim 2016-07-29 22:50:31.851971861 +0200 --- src/testdir/test_lambda.vim 2016-07-31 18:05:55.089733517 +0200 *************** *** 247,249 **** --- 247,273 ---- 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 *** ../vim-7.4.2135/src/version.c 2016-07-31 14:17:22.907502194 +0200 --- src/version.c 2016-07-31 18:12:22.826362486 +0200 *************** *** 765,766 **** --- 765,768 ---- { /* Add new patch number below this line */ + /**/ + 2136, /**/ -- JOHN CLEESE PLAYED: SECOND SOLDIER WITH A KEEN INTEREST IN BIRDS, LARGE MAN WITH DEAD BODY, BLACK KNIGHT, MR NEWT (A VILLAGE BLACKSMITH INTERESTED IN BURNING WITCHES), A QUITE EXTRAORDINARILY RUDE FRENCHMAN, TIM THE WIZARD, SIR LAUNCELOT "Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD /// 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 ///