To: vim_dev@googlegroups.com Subject: Patch 8.2.0543 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ------------ Patch 8.2.0543 Problem: Vim9: function with varargs does not work properly. Solution: Improve function type spec and add tests. Fix bugs. Files: runtime/doc/vim9.txt, src/vim9compile.c, src/vim9execute.c, src/structs.h, src/testdir/test_vim9_func.vim *** ../vim-8.2.0542/runtime/doc/vim9.txt 2020-04-03 21:59:29.329665634 +0200 --- runtime/doc/vim9.txt 2020-04-09 22:47:45.452632670 +0200 *************** *** 120,125 **** --- 120,132 ---- Variables cannot shadow previously defined variables. Variables may shadow Ex commands, rename the variable if needed. + Global variables must be prefixed with "g:", also at the script level. + However, global user defined functions are used without "g:". > + vim9script + let script_local = 'text' + let g:global = 'value' + let Funcref = ThatFunction + Since "&opt = value" is now assigning a value to option "opt", ":&" cannot be used to repeat a `:substitute` command. *************** *** 156,161 **** --- 163,180 ---- name will only be found when the call is executed. + Omitting function() ~ + + A user defined function can be used as a function reference in an expression + without `function()`. The argument types and return type will then be checked. + The function must already have been defined. > + + let Funcref = MyFunction + + When using `function()` the resulting type is "func", a function with any + number of arguments and any return type. The function can be defined later. + + No curly braces expansion ~ |curly-braces-names| cannot be used. *************** *** 213,220 **** blob non-empty list non-empty (different from JavaScript) dictionary non-empty (different from JavaScript) ! func when not NULL ! partial when not NULL special v:true job when not NULL channel when not NULL --- 232,238 ---- blob non-empty list non-empty (different from JavaScript) dictionary non-empty (different from JavaScript) ! func when there is a function name special v:true job when not NULL channel when not NULL *************** *** 301,306 **** --- 319,325 ---- job channel func + func: {type} func({type}, ...) func({type}, ...): {type} *************** *** 318,329 **** A partial and function can be declared in more or less specific ways: func any kind of function reference, no type ! checking func: {type} any number and type of arguments with specific return type ! func({type} ...) function with argument types, does not return a value ! func({type} ...): {type} function with argument types and return type If the return type is "void" the function does not return a value. --- 337,358 ---- A partial and function can be declared in more or less specific ways: func any kind of function reference, no type ! checking for arguments or return value func: {type} any number and type of arguments with specific return type ! func({type}) function with argument type, does not return a value ! func({type}): {type} function with argument type and return type ! func(?{type}) function with type of optional argument, does ! not return a value ! func(...{type}) function with type of variable number of ! arguments, does not return a value ! func({type}, ?{type}, ...{type}): {type} ! function with: ! - type of mandatory argument ! - type of optional argument ! - type of variable number of arguments ! - return type If the return type is "void" the function does not return a value. *** ../vim-8.2.0542/src/vim9compile.c 2020-04-09 21:08:06.298505479 +0200 --- src/vim9compile.c 2020-04-11 20:48:41.300343827 +0200 *************** *** 304,309 **** --- 304,326 ---- } /* + * Allocate a new type for a function. + */ + static type_T * + alloc_func_type(type_T *ret_type, int argcount, garray_T *type_gap) + { + type_T *type = alloc_type(type_gap); + + if (type == NULL) + return &t_any; + type->tt_type = VAR_FUNC; + type->tt_member = ret_type; + type->tt_argcount = argcount; + type->tt_args = NULL; + return type; + } + + /* * Get a function type, based on the return type "ret_type". * If "argcount" is -1 or 0 a predefined type can be used. * If "argcount" > 0 always create a new type, so that arguments can be added. *************** *** 311,318 **** static type_T * get_func_type(type_T *ret_type, int argcount, garray_T *type_gap) { - type_T *type; - // recognize commonly used types if (argcount <= 0) { --- 328,333 ---- *************** *** 351,365 **** } } ! // Not a common type or has arguments, create a new entry. ! type = alloc_type(type_gap); ! if (type == NULL) ! return &t_any; ! type->tt_type = VAR_FUNC; ! type->tt_member = ret_type; ! type->tt_argcount = argcount; ! type->tt_args = NULL; ! return type; } /* --- 366,372 ---- } } ! return alloc_func_type(ret_type, argcount, type_gap); } /* *************** *** 370,378 **** func_type_add_arg_types( type_T *functype, int argcount, - int min_argcount, garray_T *type_gap) { if (ga_grow(type_gap, 1) == FAIL) return FAIL; functype->tt_args = ALLOC_CLEAR_MULT(type_T *, argcount); --- 377,386 ---- func_type_add_arg_types( type_T *functype, int argcount, garray_T *type_gap) { + // To make it easy to free the space needed for the argument types, add the + // pointer to type_gap. if (ga_grow(type_gap, 1) == FAIL) return FAIL; functype->tt_args = ALLOC_CLEAR_MULT(type_T *, argcount); *************** *** 381,389 **** ((type_T **)type_gap->ga_data)[type_gap->ga_len] = (void *)functype->tt_args; ++type_gap->ga_len; - - functype->tt_argcount = argcount; - functype->tt_min_argcount = min_argcount; return OK; } --- 389,394 ---- *************** *** 1251,1270 **** } } - // Turn varargs into a list. - if (ufunc->uf_va_name != NULL) - { - int count = argcount - regular_args; - - // If count is negative an empty list will be added after evaluating - // default values for missing optional arguments. - if (count >= 0) - { - generate_NEWLIST(cctx, count); - argcount = regular_args + 1; - } - } - if ((isn = generate_instr(cctx, ufunc->uf_dfunc_idx >= 0 ? ISN_DCALL : ISN_UCALL)) == NULL) return FAIL; --- 1256,1261 ---- *************** *** 1326,1331 **** --- 1317,1323 ---- garray_T *stack = &cctx->ctx_type_stack; RETURN_OK_IF_SKIP(cctx); + if ((isn = generate_instr(cctx, ISN_PCALL)) == NULL) return FAIL; isn->isn_arg.pfunc.cpf_top = at_top; *************** *** 1612,1618 **** int first_optional = -1; type_T *arg_type[MAX_FUNC_ARGS + 1]; ! // func({type}, ...): {type} *arg += len; if (**arg == '(') { --- 1604,1610 ---- int first_optional = -1; type_T *arg_type[MAX_FUNC_ARGS + 1]; ! // func({type}, ...{type}): {type} *arg += len; if (**arg == '(') { *************** *** 1624,1636 **** argcount = 0; while (*p != NUL && *p != ')') { - if (STRNCMP(p, "...", 3) == 0) - { - flags |= TTFLAG_VARARGS; - break; - } - arg_type[argcount++] = parse_type(&p, type_gap); - if (*p == '?') { if (first_optional == -1) --- 1616,1621 ---- *************** *** 1642,1647 **** --- 1627,1643 ---- emsg(_("E1007: mandatory argument after optional argument")); return &t_any; } + else if (STRNCMP(p, "...", 3) == 0) + { + flags |= TTFLAG_VARARGS; + p += 3; + } + + arg_type[argcount++] = parse_type(&p, type_gap); + + // Nothing comes after "...{type}". + if (flags & TTFLAG_VARARGS) + break; if (*p != ',' && *skipwhite(p) == ',') { *************** *** 1679,1697 **** *arg = skipwhite(*arg); ret_type = parse_type(arg, type_gap); } ! type = get_func_type(ret_type, ! flags == 0 && first_optional == -1 ? argcount : 99, ! type_gap); ! if (flags != 0) ! type->tt_flags = flags; ! if (argcount > 0) { ! if (func_type_add_arg_types(type, argcount, ! first_optional == -1 ? argcount : first_optional, type_gap) == FAIL) ! return &t_any; ! mch_memmove(type->tt_args, arg_type, sizeof(type_T *) * argcount); } return type; } --- 1675,1697 ---- *arg = skipwhite(*arg); ret_type = parse_type(arg, type_gap); } ! if (flags == 0 && first_optional == -1) ! type = get_func_type(ret_type, argcount, type_gap); ! else { ! type = alloc_func_type(ret_type, argcount, type_gap); ! type->tt_flags = flags; ! if (argcount > 0) ! { ! type->tt_argcount = argcount; ! type->tt_min_argcount = first_optional == -1 ! ? argcount : first_optional; ! if (func_type_add_arg_types(type, argcount, type_gap) == FAIL) ! return &t_any; ! mch_memmove(type->tt_args, arg_type, sizeof(type_T *) * argcount); + } } return type; } *************** *** 1857,1862 **** --- 1857,1863 ---- { garray_T ga; int i; + int varargs = (type->tt_flags & TTFLAG_VARARGS) ? 1 : 0; ga_init2(&ga, 1, 100); if (ga_grow(&ga, 20) == FAIL) *************** *** 1865,1871 **** STRCPY(ga.ga_data, "func("); ga.ga_len += 5; ! for (i = 0; i < type->tt_argcount; ++i) { char *arg_free; char *arg_type = type_name(type->tt_args[i], &arg_free); --- 1866,1872 ---- STRCPY(ga.ga_data, "func("); ga.ga_len += 5; ! for (i = 0; i < type->tt_argcount + varargs; ++i) { char *arg_free; char *arg_type = type_name(type->tt_args[i], &arg_free); *************** *** 1877,1888 **** ga.ga_len += 2; } len = (int)STRLEN(arg_type); ! if (ga_grow(&ga, len + 6) == FAIL) { vim_free(arg_free); return "[unknown]"; } *tofree = ga.ga_data; STRCPY((char *)ga.ga_data + ga.ga_len, arg_type); ga.ga_len += len; vim_free(arg_free); --- 1878,1896 ---- ga.ga_len += 2; } len = (int)STRLEN(arg_type); ! if (ga_grow(&ga, len + 8) == FAIL) { vim_free(arg_free); return "[unknown]"; } *tofree = ga.ga_data; + if (i == type->tt_argcount) + { + STRCPY((char *)ga.ga_data + ga.ga_len, "..."); + ga.ga_len += 3; + } + else if (i >= type->tt_min_argcount) + *((char *)ga.ga_data + ga.ga_len++) = '?'; STRCPY((char *)ga.ga_data + ga.ga_len, arg_type); ga.ga_len += len; vim_free(arg_free); *************** *** 5573,5590 **** if (generate_STORE(&cctx, ISN_STORE, i - count - off, NULL) == FAIL) goto erret; } - - // If a varargs is following, push an empty list. - if (ufunc->uf_va_name != NULL) - { - if (generate_NEWLIST(&cctx, 0) == FAIL - || generate_STORE(&cctx, ISN_STORE, -off, NULL) == FAIL) - goto erret; - } - ufunc->uf_def_arg_idx[count] = instr->ga_len; } /* * Loop over all the lines of the function and generate instructions. */ --- 5581,5598 ---- if (generate_STORE(&cctx, ISN_STORE, i - count - off, NULL) == FAIL) goto erret; } ufunc->uf_def_arg_idx[count] = instr->ga_len; } + // If varargs is use, push a list. Empty if no more arguments. + if (ufunc->uf_va_name != NULL) + { + if (generate_NEWLIST(&cctx, 0) == FAIL + || generate_STORE(&cctx, ISN_STORE, + -STACK_FRAME_SIZE - 1, NULL) == FAIL) + goto erret; + } + /* * Loop over all the lines of the function and generate instructions. */ *************** *** 5894,5917 **** { int varargs = ufunc->uf_va_name != NULL; ! int argcount = ufunc->uf_args.ga_len - (varargs ? 1 : 0); // Create a type for the function, with the return type and any // argument types. // A vararg is included in uf_args.ga_len but not in uf_arg_types. // The type is included in "tt_args". ! ufunc->uf_func_type = get_func_type(ufunc->uf_ret_type, ! ufunc->uf_args.ga_len, &ufunc->uf_type_list); ! if (ufunc->uf_args.ga_len > 0) { if (func_type_add_arg_types(ufunc->uf_func_type, ! ufunc->uf_args.ga_len, ! argcount - ufunc->uf_def_args.ga_len, ! &ufunc->uf_type_list) == FAIL) { ret = FAIL; goto erret; } if (ufunc->uf_arg_types == NULL) { int i; --- 5902,5928 ---- { int varargs = ufunc->uf_va_name != NULL; ! int argcount = ufunc->uf_args.ga_len; // Create a type for the function, with the return type and any // argument types. // A vararg is included in uf_args.ga_len but not in uf_arg_types. // The type is included in "tt_args". ! if (argcount > 0 || varargs) { + ufunc->uf_func_type = alloc_func_type(ufunc->uf_ret_type, + argcount, &ufunc->uf_type_list); + // Add argument types to the function type. if (func_type_add_arg_types(ufunc->uf_func_type, ! argcount + varargs, ! &ufunc->uf_type_list) == FAIL) { ret = FAIL; goto erret; } + ufunc->uf_func_type->tt_argcount = argcount + varargs; + ufunc->uf_func_type->tt_min_argcount = + argcount - ufunc->uf_def_args.ga_len; if (ufunc->uf_arg_types == NULL) { int i; *************** *** 5924,5932 **** --- 5935,5951 ---- mch_memmove(ufunc->uf_func_type->tt_args, ufunc->uf_arg_types, sizeof(type_T *) * argcount); if (varargs) + { ufunc->uf_func_type->tt_args[argcount] = ufunc->uf_va_type == NULL ? &t_any : ufunc->uf_va_type; + ufunc->uf_func_type->tt_flags = TTFLAG_VARARGS; + } } + else + // No arguments, can use a predefined type. + ufunc->uf_func_type = get_func_type(ufunc->uf_ret_type, + argcount, &ufunc->uf_type_list); + } ret = OK; *** ../vim-8.2.0542/src/vim9execute.c 2020-04-09 21:08:06.298505479 +0200 --- src/vim9execute.c 2020-04-11 20:42:43.885078038 +0200 *************** *** 120,130 **** * - reserved space for local variables */ static int ! call_dfunc(int cdf_idx, int argcount, ectx_T *ectx) { dfunc_T *dfunc = ((dfunc_T *)def_functions.ga_data) + cdf_idx; ufunc_T *ufunc = dfunc->df_ufunc; ! int optcount = ufunc_argcount(ufunc) - argcount; int idx; if (dfunc->df_deleted) --- 120,132 ---- * - reserved space for local variables */ static int ! call_dfunc(int cdf_idx, int argcount_arg, ectx_T *ectx) { + int argcount = argcount_arg; dfunc_T *dfunc = ((dfunc_T *)def_functions.ga_data) + cdf_idx; ufunc_T *ufunc = dfunc->df_ufunc; ! int arg_to_add; ! int vararg_count = 0; int idx; if (dfunc->df_deleted) *************** *** 133,150 **** return FAIL; } ! if (ga_grow(&ectx->ec_stack, optcount + 3 + dfunc->df_varcount) == FAIL) ! return FAIL; ! if (optcount < 0) { ! emsg("argument count wrong?"); return FAIL; } // Reserve space for omitted optional arguments, filled in soon. ! // Also any empty varargs argument. ! ectx->ec_stack.ga_len += optcount; // Store current execution state in stack frame for ISN_RETURN. // TODO: If the actual number of arguments doesn't match what the called --- 135,206 ---- return FAIL; } ! if (ufunc->uf_va_name != NULL) ! { ! int iidx; ! isn_T *iptr; ! ! // Need to make a list out of the vararg arguments. There is a ! // ISN_NEWLIST instruction at the start of the function body, we need ! // to move the arguments below the stack frame and pass the count. ! // Stack at time of call with 2 varargs: ! // normal_arg ! // optional_arg ! // vararg_1 ! // vararg_2 ! // When starting execution: ! // normal_arg ! // optional_arg ! // space list of varargs ! // STACK_FRAME ! // [local variables] ! // vararg_1 ! // vararg_2 ! // TODO: This doesn't work if the same function is used for a default ! // argument value. Forbid that somehow? ! vararg_count = argcount - ufunc->uf_args.ga_len; ! if (vararg_count < 0) ! vararg_count = 0; ! else ! argcount -= vararg_count; ! if (ufunc->uf_def_arg_idx == NULL) ! iidx = 0; ! else ! iidx = ufunc->uf_def_arg_idx[ufunc->uf_def_args.ga_len]; ! iptr = &dfunc->df_instr[iidx]; ! if (iptr->isn_type != ISN_NEWLIST) ! { ! iemsg("Not a ISN_NEWLIST instruction"); ! return FAIL; ! } ! iptr->isn_arg.number = vararg_count; ! } ! arg_to_add = ufunc_argcount(ufunc) - argcount; ! if (arg_to_add < 0) { ! iemsg("Argument count wrong?"); ! return FAIL; ! } ! if (ga_grow(&ectx->ec_stack, arg_to_add + 3 + dfunc->df_varcount) == FAIL) return FAIL; + + if (vararg_count > 0) + { + int stack_added = arg_to_add + STACK_FRAME_SIZE + dfunc->df_varcount; + + // Move the varargs to below the stack frame. + // TODO: use mch_memmove() + for (idx = 1; idx <= vararg_count; ++idx) + *STACK_TV_BOT(stack_added - idx) = *STACK_TV_BOT(-idx); + ectx->ec_stack.ga_len -= vararg_count; } // Reserve space for omitted optional arguments, filled in soon. ! // Also room for a list of varargs, if there is one. ! for (idx = 0; idx < arg_to_add; ++idx) ! STACK_TV_BOT(idx)->v_type = VAR_UNKNOWN; ! ectx->ec_stack.ga_len += arg_to_add; // Store current execution state in stack frame for ISN_RETURN. // TODO: If the actual number of arguments doesn't match what the called *************** *** 157,163 **** // Initialize local variables for (idx = 0; idx < dfunc->df_varcount; ++idx) STACK_TV_BOT(STACK_FRAME_SIZE + idx)->v_type = VAR_UNKNOWN; ! ectx->ec_stack.ga_len += STACK_FRAME_SIZE + dfunc->df_varcount; // Set execution state to the start of the called function. ectx->ec_dfunc_idx = cdf_idx; --- 213,220 ---- // Initialize local variables for (idx = 0; idx < dfunc->df_varcount; ++idx) STACK_TV_BOT(STACK_FRAME_SIZE + idx)->v_type = VAR_UNKNOWN; ! ectx->ec_stack.ga_len += STACK_FRAME_SIZE + dfunc->df_varcount ! + vararg_count; // Set execution state to the start of the called function. ectx->ec_dfunc_idx = cdf_idx; *************** *** 430,436 **** #undef STACK_TV_BOT #define STACK_TV_BOT(idx) (((typval_T *)ectx.ec_stack.ga_data) + ectx.ec_stack.ga_len + idx) ! // Get pointer to local variable on the stack. #define STACK_TV_VAR(idx) (((typval_T *)ectx.ec_stack.ga_data) + ectx.ec_frame + STACK_FRAME_SIZE + idx) vim_memset(&ectx, 0, sizeof(ectx)); --- 487,493 ---- #undef STACK_TV_BOT #define STACK_TV_BOT(idx) (((typval_T *)ectx.ec_stack.ga_data) + ectx.ec_stack.ga_len + idx) ! // Get pointer to a local variable on the stack. Negative for arguments. #define STACK_TV_VAR(idx) (((typval_T *)ectx.ec_stack.ga_data) + ectx.ec_frame + STACK_FRAME_SIZE + idx) vim_memset(&ectx, 0, sizeof(ectx)); *** ../vim-8.2.0542/src/structs.h 2020-04-08 11:31:45.468620014 +0200 --- src/structs.h 2020-04-10 23:07:22.579343467 +0200 *************** *** 1346,1352 **** typedef struct type_S type_T; struct type_S { vartype_T tt_type; ! int8_T tt_argcount; // for func, -1 for unknown char tt_min_argcount; // number of non-optional arguments char tt_flags; // TTFLAG_ values type_T *tt_member; // for list, dict, func return type --- 1346,1352 ---- typedef struct type_S type_T; struct type_S { vartype_T tt_type; ! int8_T tt_argcount; // for func, incl. vararg, -1 for unknown char tt_min_argcount; // number of non-optional arguments char tt_flags; // TTFLAG_ values type_T *tt_member; // for list, dict, func return type *** ../vim-8.2.0542/src/testdir/test_vim9_func.vim 2020-04-07 22:44:56.778289142 +0200 --- src/testdir/test_vim9_func.vim 2020-04-10 22:35:29.747809412 +0200 *************** *** 131,136 **** --- 131,177 ---- call CheckDefFailure(['MyDefVarargs("one", 22)'], 'E1013: argument 2: type mismatch, expected string but got number') enddef + let s:value = '' + + def FuncOneDefArg(opt = 'text') + s:value = opt + enddef + + def FuncTwoDefArg(nr = 123, opt = 'text'): string + return nr .. opt + enddef + + def FuncVarargs(...arg: list): string + return join(arg, ',') + enddef + + def Test_func_type_varargs() + let RefDefArg: func(?string) + RefDefArg = FuncOneDefArg + RefDefArg() + assert_equal('text', s:value) + RefDefArg('some') + assert_equal('some', s:value) + + let RefDef2Arg: func(?number, ?string): string + RefDef2Arg = FuncTwoDefArg + assert_equal('123text', RefDef2Arg()) + assert_equal('99text', RefDef2Arg(99)) + assert_equal('77some', RefDef2Arg(77, 'some')) + + call CheckDefFailure(['let RefWrong: func(string?)'], 'E1010:') + call CheckDefFailure(['let RefWrong: func(?string, string)'], 'E1007:') + + let RefVarargs: func(...list): string + RefVarargs = FuncVarargs + assert_equal('', RefVarargs()) + assert_equal('one', RefVarargs('one')) + assert_equal('one,two', RefVarargs('one', 'two')) + + call CheckDefFailure(['let RefWrong: func(...list, string)'], 'E110:') + call CheckDefFailure(['let RefWrong: func(...list, ?string)'], 'E110:') + enddef + " Only varargs def MyVarargsOnly(...args: list): string return join(args, ',') *** ../vim-8.2.0542/src/version.c 2020-04-11 18:36:35.051150839 +0200 --- src/version.c 2020-04-11 20:46:09.980662076 +0200 *************** *** 740,741 **** --- 740,743 ---- { /* Add new patch number below this line */ + /**/ + 543, /**/ -- NEIL INNES PLAYED: THE FIRST SELF-DESTRUCTIVE MONK, ROBIN'S LEAST FAVORITE MINSTREL, THE PAGE CRUSHED BY A RABBIT, THE OWNER OF A DUCK "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 ///