To: vim_dev@googlegroups.com Subject: Patch 9.0.0485 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ------------ Patch 9.0.0485 Problem: In a :def function all closures in a loop get the same variables. Solution: Make a copy of loop variables used in a closure. Files: src/structs.h, src/vim9execute.c, src/proto/vim9execute.pro, src/eval.c, src/testdir/test_vim9_script.vim *** ../vim-9.0.0484/src/structs.h 2022-09-16 19:04:19.957886512 +0100 --- src/structs.h 2022-09-17 14:06:43.698042183 +0100 *************** *** 2077,2083 **** * defined in that function. */ typedef struct funcstack_S funcstack_T; - struct funcstack_S { funcstack_T *fs_next; // linked list at "first_funcstack" --- 2077,2082 ---- *************** *** 2092,2098 **** int fs_refcount; // nr of closures referencing this funcstack int fs_min_refcount; // nr of closures on this funcstack ! int fs_copyID; // for garray_T collection }; typedef struct outer_S outer_T; --- 2091,2113 ---- int fs_refcount; // nr of closures referencing this funcstack int fs_min_refcount; // nr of closures on this funcstack ! int fs_copyID; // for garbage collection ! }; ! ! /* ! * Structure to hold the variables declared in a loop that are possiblly used ! * in a closure. ! */ ! typedef struct loopvars_S loopvars_T; ! struct loopvars_S ! { ! loopvars_T *lvs_next; // linked list at "first_loopvars" ! loopvars_T *lvs_prev; ! ! garray_T lvs_ga; // contains the variables ! int lvs_refcount; // nr of closures referencing this loopvars ! int lvs_min_refcount; // nr of closures on this loopvars ! int lvs_copyID; // for garbage collection }; typedef struct outer_S outer_T; *************** *** 2128,2133 **** --- 2143,2150 ---- funcstack_T *pt_funcstack; // copy of stack, used after context // function returns + loopvars_T *pt_loopvars; // copy of loop variables, used after loop + // block ends typval_T *pt_argv; // arguments in allocated array int pt_argc; // number of arguments *** ../vim-9.0.0484/src/vim9execute.c 2022-09-16 19:04:19.957886512 +0100 --- src/vim9execute.c 2022-09-17 15:24:58.795524927 +0100 *************** *** 774,790 **** - closure_count + idx]; if (pt->pt_refcount > 1) { - int prev_frame_idx = pt->pt_outer.out_frame_idx; - ++funcstack->fs_refcount; pt->pt_funcstack = funcstack; pt->pt_outer.out_stack = &funcstack->fs_ga; pt->pt_outer.out_frame_idx = ectx->ec_frame_idx - top; - - // TODO: drop this, should be done at ISN_ENDLOOP - pt->pt_outer.out_loop_stack = &funcstack->fs_ga; - pt->pt_outer.out_loop_var_idx -= - prev_frame_idx - pt->pt_outer.out_frame_idx; } } } --- 774,783 ---- *************** *** 2622,2638 **** } /* * End of a for or while loop: Handle any variables used by a closure. */ static int ! execute_endloop(isn_T *iptr UNUSED, ectx_T *ectx UNUSED) { ! // TODO return OK; } /* * Load instruction for w:/b:/g:/t: variable. * "isn_type" is used instead of "iptr->isn_type". */ --- 2615,2802 ---- } /* + * Code for handling variables declared inside a loop and used in a closure. + * This is very similar to what is done with funcstack_T. The difference is + * that the funcstack_T has the scope of a function, while a loopvars_T has the + * scope of the block inside a loop and each loop may have its own. + */ + + // Double linked list of loopvars_T in use. + static loopvars_T *first_loopvars = NULL; + + static void + add_loopvars_to_list(loopvars_T *loopvars) + { + // Link in list of loopvarss. + if (first_loopvars != NULL) + first_loopvars->lvs_prev = loopvars; + loopvars->lvs_next = first_loopvars; + loopvars->lvs_prev = NULL; + first_loopvars = loopvars; + } + + static void + remove_loopvars_from_list(loopvars_T *loopvars) + { + if (loopvars->lvs_prev == NULL) + first_loopvars = loopvars->lvs_next; + else + loopvars->lvs_prev->lvs_next = loopvars->lvs_next; + if (loopvars->lvs_next != NULL) + loopvars->lvs_next->lvs_prev = loopvars->lvs_prev; + } + + /* * End of a for or while loop: Handle any variables used by a closure. */ static int ! execute_endloop(isn_T *iptr, ectx_T *ectx) { ! endloop_T *endloop = &iptr->isn_arg.endloop; ! typval_T *tv_refcount = STACK_TV_VAR(endloop->end_funcref_idx); ! int prev_closure_count = tv_refcount->vval.v_number; ! garray_T *gap = &ectx->ec_funcrefs; ! int closure_in_use = FALSE; ! loopvars_T *loopvars; ! typval_T *stack; ! int idx; ! ! // Check if any created closure is still being referenced. ! for (idx = prev_closure_count; idx < gap->ga_len; ++idx) ! { ! partial_T *pt = ((partial_T **)gap->ga_data)[idx]; ! ! if (pt->pt_refcount > 1) ! { ! int refcount = pt->pt_refcount; ! int i; ! ! // A Reference in a variable inside the loop doesn't count, it gets ! // unreferenced at the end of the loop. ! for (i = 0; i < endloop->end_var_count; ++i) ! { ! typval_T *stv = STACK_TV_VAR(endloop->end_var_idx + i); ! ! if (stv->v_type == VAR_PARTIAL && pt == stv->vval.v_partial) ! --refcount; ! } ! if (refcount > 1) ! { ! closure_in_use = TRUE; ! break; ! } ! } ! } ! ! // If no function reference were created since the start of the loop block ! // or it is no longer referenced there is nothing to do. ! if (!closure_in_use) ! return OK; ! ! // A closure is using variables declared inside the loop. ! // Move them to the called function. ! loopvars = ALLOC_CLEAR_ONE(loopvars_T); ! if (loopvars == NULL) ! return FAIL; ! ! loopvars->lvs_ga.ga_len = endloop->end_var_count; ! stack = ALLOC_CLEAR_MULT(typval_T, loopvars->lvs_ga.ga_len); ! loopvars->lvs_ga.ga_data = stack; ! if (stack == NULL) ! { ! vim_free(loopvars); ! return FAIL; ! } ! add_loopvars_to_list(loopvars); ! ! // Move the variable values. ! for (idx = 0; idx < endloop->end_var_count; ++idx) ! { ! typval_T *tv = STACK_TV_VAR(endloop->end_var_idx + idx); ! ! *(stack + idx) = *tv; ! tv->v_type = VAR_UNKNOWN; ! } ! ! for (idx = prev_closure_count; idx < gap->ga_len; ++idx) ! { ! partial_T *pt = ((partial_T **)gap->ga_data)[idx]; ! ! if (pt->pt_refcount > 1) ! { ! ++loopvars->lvs_refcount; ! pt->pt_loopvars = loopvars; ! ! pt->pt_outer.out_loop_stack = &loopvars->lvs_ga; ! pt->pt_outer.out_loop_var_idx -= ectx->ec_frame_idx ! + STACK_FRAME_SIZE + endloop->end_var_idx; ! } ! } return OK; } /* + * Called when a partial is freed or its reference count goes down to one. The + * loopvars may be the only reference to the partials in the local variables. + * Go over all of them, the funcref and can be freed if all partials + * referencing the loopvars have a reference count of one. + */ + void + loopvars_check_refcount(loopvars_T *loopvars) + { + int i; + garray_T *gap = &loopvars->lvs_ga; + int done = 0; + + if (loopvars->lvs_refcount > loopvars->lvs_min_refcount) + return; + for (i = 0; i < gap->ga_len; ++i) + { + typval_T *tv = ((typval_T *)gap->ga_data) + i; + + if (tv->v_type == VAR_PARTIAL && tv->vval.v_partial != NULL + && tv->vval.v_partial->pt_loopvars == loopvars + && tv->vval.v_partial->pt_refcount == 1) + ++done; + } + if (done == loopvars->lvs_min_refcount) + { + typval_T *stack = gap->ga_data; + + // All partials referencing the loopvars have a reference count of + // one, thus the loopvars is no longer of use. + for (i = 0; i < gap->ga_len; ++i) + clear_tv(stack + i); + vim_free(stack); + remove_loopvars_from_list(loopvars); + vim_free(loopvars); + } + } + + /* + * For garbage collecting: set references in all variables referenced by + * all loopvars. + */ + int + set_ref_in_loopvars(int copyID) + { + loopvars_T *loopvars; + + for (loopvars = first_loopvars; loopvars != NULL; + loopvars = loopvars->lvs_next) + { + typval_T *stack = loopvars->lvs_ga.ga_data; + int i; + + for (i = 0; i < loopvars->lvs_ga.ga_len; ++i) + if (set_ref_in_item(stack + i, copyID, NULL, NULL)) + return TRUE; // abort + } + return FALSE; + } + + /* * Load instruction for w:/b:/g:/t: variable. * "isn_type" is used instead of "iptr->isn_type". */ *************** *** 3609,3615 **** goto on_error; break; ! // load or store variable or argument from outer scope case ISN_LOADOUTER: case ISN_STOREOUTER: { --- 3773,3779 ---- goto on_error; break; ! // Load or store variable or argument from outer scope. case ISN_LOADOUTER: case ISN_STOREOUTER: { *************** *** 3635,3645 **** goto theend; } if (depth == OUTER_LOOP_DEPTH) ! // variable declared in loop tv = ((typval_T *)outer->out_loop_stack->ga_data) + outer->out_loop_var_idx + iptr->isn_arg.outer.outer_idx; else tv = ((typval_T *)outer->out_stack->ga_data) + outer->out_frame_idx + STACK_FRAME_SIZE + iptr->isn_arg.outer.outer_idx; --- 3799,3812 ---- goto theend; } if (depth == OUTER_LOOP_DEPTH) ! // Variable declared in loop. May be copied if the ! // loop block has already ended. tv = ((typval_T *)outer->out_loop_stack->ga_data) + outer->out_loop_var_idx + iptr->isn_arg.outer.outer_idx; else + // Variable declared in a function. May be copied if + // the function has already returned. tv = ((typval_T *)outer->out_stack->ga_data) + outer->out_frame_idx + STACK_FRAME_SIZE + iptr->isn_arg.outer.outer_idx; *************** *** 5563,5569 **** { outer_T *outer = get_pt_outer(partial); ! if (outer->out_stack == NULL) { if (current_ectx != NULL) { --- 5730,5736 ---- { outer_T *outer = get_pt_outer(partial); ! if (outer->out_stack == NULL && outer->out_loop_stack == NULL) { if (current_ectx != NULL) { *************** *** 5572,5578 **** ectx.ec_outer_ref->or_outer = current_ectx->ec_outer_ref->or_outer; } ! // Should there be an error here? } else { --- 5739,5745 ---- ectx.ec_outer_ref->or_outer = current_ectx->ec_outer_ref->or_outer; } ! // else: should there be an error here? } else { *** ../vim-9.0.0484/src/proto/vim9execute.pro 2022-09-16 19:04:19.957886512 +0100 --- src/proto/vim9execute.pro 2022-09-17 15:17:05.496048965 +0100 *************** *** 13,18 **** --- 13,20 ---- int may_load_script(int sid, int *loaded); typval_T *lookup_debug_var(char_u *name); int may_break_in_function(ufunc_T *ufunc); + void loopvars_check_refcount(loopvars_T *loopvars); + int set_ref_in_loopvars(int copyID); int exe_typval_instr(typval_T *tv, typval_T *rettv); char_u *exe_substitute_instr(void); int call_def_function(ufunc_T *ufunc, int argc_arg, typval_T *argv, int flags, partial_T *partial, funccall_T *funccal, typval_T *rettv); *** ../vim-9.0.0484/src/eval.c 2022-09-14 00:30:47.077316538 +0100 --- src/eval.c 2022-09-17 15:14:03.556196785 +0100 *************** *** 4857,4862 **** --- 4857,4868 ---- --pt->pt_funcstack->fs_refcount; funcstack_check_refcount(pt->pt_funcstack); } + // Similarly for loop variables. + if (pt->pt_loopvars != NULL) + { + --pt->pt_loopvars->lvs_refcount; + loopvars_check_refcount(pt->pt_loopvars); + } vim_free(pt); } *************** *** 4875,4882 **** // If the reference count goes down to one, the funcstack may be the // only reference and can be freed if no other partials reference it. ! else if (pt->pt_refcount == 1 && pt->pt_funcstack != NULL) ! funcstack_check_refcount(pt->pt_funcstack); } } --- 4881,4893 ---- // If the reference count goes down to one, the funcstack may be the // only reference and can be freed if no other partials reference it. ! else if (pt->pt_refcount == 1) ! { ! if (pt->pt_funcstack != NULL) ! funcstack_check_refcount(pt->pt_funcstack); ! if (pt->pt_loopvars != NULL) ! loopvars_check_refcount(pt->pt_loopvars); ! } } } *************** *** 5016,5021 **** --- 5027,5035 ---- // funcstacks keep variables for closures abort = abort || set_ref_in_funcstacks(copyID); + // loopvars keep variables for loop blocks + abort = abort || set_ref_in_loopvars(copyID); + // v: vars abort = abort || garbage_collect_vimvars(copyID); *************** *** 5379,5384 **** --- 5393,5399 ---- abort = abort || set_ref_in_item(&pt->pt_argv[i], copyID, ht_stack, list_stack); // pt_funcstack is handled in set_ref_in_funcstacks() + // pt_loopvars is handled in set_ref_in_loopvars() } } #ifdef FEAT_JOB_CHANNEL *** ../vim-9.0.0484/src/testdir/test_vim9_script.vim 2022-09-15 22:26:13.166294520 +0100 --- src/testdir/test_vim9_script.vim 2022-09-17 15:33:49.686821275 +0100 *************** *** 2285,2293 **** assert_equal(i, flist[i]()) endfor END ! # FIXME ! # v9.CheckDefAndScriptSuccess(lines) ! v9.CheckScriptSuccess(['vim9script'] + lines) lines =<< trim END var flist: list --- 2285,2291 ---- assert_equal(i, flist[i]()) endfor END ! v9.CheckDefAndScriptSuccess(lines) lines =<< trim END var flist: list *************** *** 2301,2309 **** assert_equal(i, flist[i]()) endfor END ! # FIXME ! # v9.CheckDefAndScriptSuccess(lines) ! v9.CheckScriptSuccess(['vim9script'] + lines) enddef def Test_for_loop_fails() --- 2299,2305 ---- assert_equal(i, flist[i]()) endfor END ! v9.CheckDefAndScriptSuccess(lines) enddef def Test_for_loop_fails() *** ../vim-9.0.0484/src/version.c 2022-09-17 12:39:54.996464478 +0100 --- src/version.c 2022-09-17 13:51:45.275096217 +0100 *************** *** 705,706 **** --- 705,708 ---- { /* Add new patch number below this line */ + /**/ + 485, /**/ -- From "know your smileys": (X0||) Double hamburger with lettuce and tomato /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net \\\ /// \\\ \\\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ /// \\\ help me help AIDS victims -- http://ICCF-Holland.org ///