]> granicus.if.org Git - vim/commitdiff
patch 8.2.0543: Vim9: function with varargs does not work properly v8.2.0543
authorBram Moolenaar <Bram@vim.org>
Sat, 11 Apr 2020 18:50:33 +0000 (20:50 +0200)
committerBram Moolenaar <Bram@vim.org>
Sat, 11 Apr 2020 18:50:33 +0000 (20:50 +0200)
Problem:    Vim9: function with varargs does not work properly.
Solution:   Improve function type spec and add tests.  Fix bugs.

src/structs.h
src/testdir/test_vim9_func.vim
src/version.c
src/vim9compile.c
src/vim9execute.c

index 851ee6310e40a63f00a2f051fb430151112a6d5f..1d3d411172edbc7837d075cdb87eda937a75c85c 100644 (file)
@@ -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
index 6f9aa0cca4846eeb19a3fc0fdd8c75709d9ec88f..470e880e0fdd5ee154fd8524f69636fe7cba2cdd 100644 (file)
@@ -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>): 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>): 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>, string)'], 'E110:')
+  call CheckDefFailure(['let RefWrong: func(...list<string>, ?string)'], 'E110:')
+enddef
+
 " Only varargs
 def MyVarargsOnly(...args: list<string>): string
   return join(args, ',')
index e85bc31333526a75b4dca9d1285de68b9c448e5a..ae324b3616fd44af6ac273086546e09a10450256 100644 (file)
@@ -738,6 +738,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    543,
 /**/
     542,
 /**/
index 90f7d2841331e73934dbb3fe66247067d656db7c..69543b52ebdd02aa46fd09076a95575359c975b8 100644 (file)
@@ -303,6 +303,23 @@ get_dict_type(type_T *member_type, garray_T *type_gap)
     return type;
 }
 
+/*
+ * 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.
@@ -311,8 +328,6 @@ get_dict_type(type_T *member_type, garray_T *type_gap)
     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 argcount, garray_T *type_gap)
        }
     }
 
-    // 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 argcount, garray_T *type_gap)
 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 *ufunc, int pushed_argcount)
        }
     }
 
-    // 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 argcount, int at_top)
     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_gap)
                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_gap)
                    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_gap)
                            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_gap)
                    *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 set_return_type)
            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 set_return_type)
 
     {
        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 set_return_type)
                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;
index d9b87c18dcf5425bb76433721f77b13ff998e9e1..310be87f7e8dcb7c375e32bb34eb6bfd3c0d3e1b 100644 (file)
@@ -120,11 +120,13 @@ init_instr_idx(ufunc_T *ufunc, int argcount, ectx_T *ectx)
  * - 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, ectx_T *ectx)
        return FAIL;
     }
 
-    if (ga_grow(&ectx->ec_stack, optcount + 3 + dfunc->df_varcount) == FAIL)
-       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;
+    }
 
-    if (optcount < 0)
+    arg_to_add = ufunc_argcount(ufunc) - argcount;
+    if (arg_to_add < 0)
     {
-       emsg("argument count wrong?");
+       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 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, ectx_T *ectx)
     // 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));