# HG changeset patch # User Bram Moolenaar # Date 1586631604 -7200 # Node ID 4e8e0ce576af43338cf0e44a4fa9387fc54a2bef # Parent 7b71d96d6582691d82444f80b10460c32400f9b3 patch 8.2.0543: Vim9: function with varargs does not work properly Commit: https://github.com/vim/vim/commit/1378fbc4591b77186c90beda37bdac628add4cb6 Author: Bram Moolenaar Date: Sat Apr 11 20:50:33 2020 +0200 patch 8.2.0543: Vim9: function with varargs does not work properly Problem: Vim9: function with varargs does not work properly. Solution: Improve function type spec and add tests. Fix bugs. diff --git a/src/structs.h b/src/structs.h --- a/src/structs.h +++ b/src/structs.h @@ -1346,7 +1346,7 @@ typedef enum typedef struct type_S type_T; struct type_S { vartype_T tt_type; - int8_T tt_argcount; // for func, -1 for unknown + 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 diff --git a/src/testdir/test_vim9_func.vim b/src/testdir/test_vim9_func.vim --- a/src/testdir/test_vim9_func.vim +++ b/src/testdir/test_vim9_func.vim @@ -131,6 +131,47 @@ def Test_call_def_varargs() 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, ',') diff --git a/src/version.c b/src/version.c --- a/src/version.c +++ b/src/version.c @@ -739,6 +739,8 @@ static char *(features[]) = static int included_patches[] = { /* Add new patch number below this line */ /**/ + 543, +/**/ 542, /**/ 541, diff --git a/src/vim9compile.c b/src/vim9compile.c --- a/src/vim9compile.c +++ b/src/vim9compile.c @@ -304,6 +304,23 @@ get_dict_type(type_T *member_type, garra } /* + * 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,8 +328,6 @@ get_dict_type(type_T *member_type, garra 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) { @@ -351,15 +366,7 @@ get_func_type(type_T *ret_type, int argc } } - // 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; + return alloc_func_type(ret_type, argcount, type_gap); } /* @@ -370,9 +377,10 @@ get_func_type(type_T *ret_type, int argc func_type_add_arg_types( type_T *functype, int argcount, - int min_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,9 +389,6 @@ func_type_add_arg_types( ((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; } @@ -1251,20 +1256,6 @@ generate_CALL(cctx_T *cctx, ufunc_T *ufu } } - // 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; @@ -1326,6 +1317,7 @@ generate_PCALL(cctx_T *cctx, int argcoun 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,7 +1604,7 @@ parse_type(char_u **arg, garray_T *type_ int first_optional = -1; type_T *arg_type[MAX_FUNC_ARGS + 1]; - // func({type}, ...): {type} + // func({type}, ...{type}): {type} *arg += len; if (**arg == '(') { @@ -1624,13 +1616,6 @@ parse_type(char_u **arg, garray_T *type_ 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) @@ -1642,6 +1627,17 @@ parse_type(char_u **arg, garray_T *type_ 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,19 +1675,23 @@ parse_type(char_u **arg, garray_T *type_ *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 (flags == 0 && first_optional == -1) + type = get_func_type(ret_type, argcount, type_gap); + else { - if (func_type_add_arg_types(type, argcount, - first_optional == -1 ? argcount : first_optional, + 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, + return &t_any; + mch_memmove(type->tt_args, arg_type, sizeof(type_T *) * argcount); + } } return type; } @@ -1857,6 +1857,7 @@ type_name(type_T *type, char **tofree) { 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,7 +1866,7 @@ type_name(type_T *type, char **tofree) STRCPY(ga.ga_data, "func("); ga.ga_len += 5; - for (i = 0; i < type->tt_argcount; ++i) + for (i = 0; i < type->tt_argcount + varargs; ++i) { char *arg_free; char *arg_type = type_name(type->tt_args[i], &arg_free); @@ -1877,12 +1878,19 @@ type_name(type_T *type, char **tofree) ga.ga_len += 2; } len = (int)STRLEN(arg_type); - if (ga_grow(&ga, len + 6) == FAIL) + 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,18 +5581,18 @@ compile_def_function(ufunc_T *ufunc, int 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; } + // 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,24 +5902,27 @@ compile_def_function(ufunc_T *ufunc, int { int varargs = ufunc->uf_va_name != NULL; - int argcount = ufunc->uf_args.ga_len - (varargs ? 1 : 0); + 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". - 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 (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, - ufunc->uf_args.ga_len, - argcount - ufunc->uf_def_args.ga_len, - &ufunc->uf_type_list) == FAIL) + 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,9 +5935,17 @@ compile_def_function(ufunc_T *ufunc, int 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; diff --git a/src/vim9execute.c b/src/vim9execute.c --- a/src/vim9execute.c +++ b/src/vim9execute.c @@ -120,11 +120,13 @@ init_instr_idx(ufunc_T *ufunc, int argco * - reserved space for local variables */ static int -call_dfunc(int cdf_idx, int argcount, ectx_T *ectx) +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 optcount = ufunc_argcount(ufunc) - argcount; + int arg_to_add; + int vararg_count = 0; int idx; if (dfunc->df_deleted) @@ -133,18 +135,72 @@ call_dfunc(int cdf_idx, int argcount, ec return FAIL; } - if (ga_grow(&ectx->ec_stack, optcount + 3 + dfunc->df_varcount) == 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 (optcount < 0) + if (vararg_count > 0) { - emsg("argument count wrong?"); - return FAIL; + 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 any empty varargs argument. - ectx->ec_stack.ga_len += optcount; + // 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,7 +213,8 @@ call_dfunc(int cdf_idx, int argcount, ec // 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; + 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,7 +487,7 @@ call_def_function( #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. +// 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));