Skip to content

Commit

Permalink
patch 8.2.2362: Vim9: check of builtin function argument type is inco…
Browse files Browse the repository at this point in the history
…mplete

Problem:    Vim9: check of builtin function argument type is incomplete.
Solution:   Use need_type() instead of check_arg_type().
  • Loading branch information
brammool committed Jan 16, 2021
1 parent 7c886db commit 351ead0
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 44 deletions.
34 changes: 28 additions & 6 deletions src/evalfunc.c
Original file line number Diff line number Diff line change
Expand Up @@ -275,13 +275,30 @@ typedef struct {
int arg_count; // actual argument count
type_T **arg_types; // list of argument types
int arg_idx; // current argument index (first arg is zero)
cctx_T *arg_cctx;
} argcontext_T;

// A function to check one argument type. The first argument is the type to
// check. If needed, other argument types can be obtained with the context.
// E.g. if "arg_idx" is 1, then (type - 1) is the first argument type.
typedef int (*argcheck_T)(type_T *, argcontext_T *);

/*
* Call need_type() to check an argument type.
*/
static int
check_arg_type(
type_T *expected,
type_T *actual,
argcontext_T *context)
{
// TODO: would be useful to know if "actual" is a constant and pass it to
// need_type() to get a compile time error if possible.
return need_type(actual, expected,
context->arg_idx - context->arg_count, context->arg_idx + 1,
context->arg_cctx, FALSE, FALSE);
}

/*
* Check "type" is a float or a number.
*/
Expand All @@ -301,7 +318,7 @@ arg_float_or_nr(type_T *type, argcontext_T *context)
static int
arg_number(type_T *type, argcontext_T *context)
{
return check_arg_type(&t_number, type, context->arg_idx + 1);
return check_arg_type(&t_number, type, context);
}

/*
Expand All @@ -310,7 +327,7 @@ arg_number(type_T *type, argcontext_T *context)
static int
arg_string(type_T *type, argcontext_T *context)
{
return check_arg_type(&t_string, type, context->arg_idx + 1);
return check_arg_type(&t_string, type, context);
}

/*
Expand Down Expand Up @@ -348,7 +365,7 @@ arg_same_as_prev(type_T *type, argcontext_T *context)
{
type_T *prev_type = context->arg_types[context->arg_idx - 1];

return check_arg_type(prev_type, type, context->arg_idx + 1);
return check_arg_type(prev_type, type, context);
}

/*
Expand All @@ -362,7 +379,7 @@ arg_same_struct_as_prev(type_T *type, argcontext_T *context)
type_T *prev_type = context->arg_types[context->arg_idx - 1];

if (prev_type->tt_type != context->arg_types[context->arg_idx]->tt_type)
return check_arg_type(prev_type, type, context->arg_idx + 1);
return check_arg_type(prev_type, type, context);
return OK;
}

Expand All @@ -384,7 +401,7 @@ arg_item_of_prev(type_T *type, argcontext_T *context)
// probably VAR_ANY, can't check
return OK;

return check_arg_type(expected, type, context->arg_idx + 1);
return check_arg_type(expected, type, context);
}

/*
Expand Down Expand Up @@ -1931,7 +1948,11 @@ internal_func_name(int idx)
* Return FAIL and gives an error message when a type is wrong.
*/
int
internal_func_check_arg_types(type_T **types, int idx, int argcount)
internal_func_check_arg_types(
type_T **types,
int idx,
int argcount,
cctx_T *cctx)
{
argcheck_T *argchecks = global_functions[idx].f_argcheck;
int i;
Expand All @@ -1942,6 +1963,7 @@ internal_func_check_arg_types(type_T **types, int idx, int argcount)

context.arg_count = argcount;
context.arg_types = types;
context.arg_cctx = cctx;
for (i = 0; i < argcount; ++i)
if (argchecks[i] != NULL)
{
Expand Down
2 changes: 1 addition & 1 deletion src/proto/evalfunc.pro
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ char_u *get_expr_name(expand_T *xp, int idx);
int find_internal_func(char_u *name);
int has_internal_func(char_u *name);
char *internal_func_name(int idx);
int internal_func_check_arg_types(type_T **types, int idx, int argcount);
int internal_func_check_arg_types(type_T **types, int idx, int argcount, cctx_T *cctx);
type_T *internal_func_ret_type(int idx, int argcount, type_T **argtypes);
int internal_func_is_map(int idx);
int check_internal_func(int idx, int argcount);
Expand Down
1 change: 1 addition & 0 deletions src/proto/vim9compile.pro
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
int check_defined(char_u *p, size_t len, cctx_T *cctx);
int check_compare_types(exprtype_T type, typval_T *tv1, typval_T *tv2);
int use_typecheck(type_T *actual, type_T *expected);
int need_type(type_T *actual, type_T *expected, int offset, int arg_idx, cctx_T *cctx, int silent, int actual_is_const);
int get_script_item_idx(int sid, char_u *name, int check_writable, cctx_T *cctx);
imported_T *find_imported(char_u *name, size_t len, cctx_T *cctx);
imported_T *find_imported_in_script(char_u *name, size_t len, int sid);
Expand Down
1 change: 0 additions & 1 deletion src/proto/vim9type.pro
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ int check_typval_type(type_T *expected, typval_T *actual_tv, int argidx);
void type_mismatch(type_T *expected, type_T *actual);
void arg_type_mismatch(type_T *expected, type_T *actual, int argidx);
int check_type(type_T *expected, type_T *actual, int give_msg, int argidx);
int check_arg_type(type_T *expected, type_T *actual, int argidx);
int check_argument_types(type_T *type, typval_T *argvars, int argcount, char_u *name);
char_u *skip_type(char_u *start, int optional);
type_T *parse_type(char_u **arg, garray_T *type_gap, int give_error);
Expand Down
3 changes: 3 additions & 0 deletions src/testdir/test_vim9_builtin.vim
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,9 @@ def Test_extend_arg_types()
CheckDefFailure(['extend({a: 1}, 42)'], 'E1013: Argument 2: type mismatch, expected dict<number> but got number')
CheckDefFailure(['extend({a: 1}, {b: "x"})'], 'E1013: Argument 2: type mismatch, expected dict<number> but got dict<string>')
CheckDefFailure(['extend({a: 1}, {b: 2}, 1)'], 'E1013: Argument 3: type mismatch, expected string but got number')

CheckDefFailure(['extend([1], ["b"])'], 'E1013: Argument 2: type mismatch, expected list<number> but got list<string>')
CheckDefExecFailure(['extend([1], ["b", 1])'], 'E1012: Type mismatch; expected list<number> but got list<any>')
enddef

def Test_extendnew()
Expand Down
2 changes: 2 additions & 0 deletions src/version.c
Original file line number Diff line number Diff line change
Expand Up @@ -750,6 +750,8 @@ static char *(features[]) =

static int included_patches[] =
{ /* Add new patch number below this line */
/**/
2362,
/**/
2361,
/**/
Expand Down
44 changes: 23 additions & 21 deletions src/vim9compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -878,11 +878,12 @@ use_typecheck(type_T *actual, type_T *expected)
* If "actual_is_const" is TRUE then the type won't change at runtime, do not
* generate a TYPECHECK.
*/
static int
int
need_type(
type_T *actual,
type_T *expected,
int offset,
int arg_idx,
cctx_T *cctx,
int silent,
int actual_is_const)
Expand All @@ -896,7 +897,7 @@ need_type(
return OK;
}

if (check_type(expected, actual, FALSE, 0) == OK)
if (check_type(expected, actual, FALSE, arg_idx) == OK)
return OK;

// If the actual type can be the expected type add a runtime check.
Expand All @@ -908,7 +909,7 @@ need_type(
}

if (!silent)
type_mismatch(expected, actual);
arg_type_mismatch(expected, actual, arg_idx);
return FAIL;
}

Expand All @@ -931,7 +932,7 @@ bool_on_stack(cctx_T *cctx)
// This requires a runtime type check.
return generate_COND2BOOL(cctx);

return need_type(type, &t_bool, -1, cctx, FALSE, FALSE);
return need_type(type, &t_bool, -1, 0, cctx, FALSE, FALSE);
}

/*
Expand Down Expand Up @@ -1613,7 +1614,8 @@ generate_BCALL(cctx_T *cctx, int func_idx, int argcount, int method_call)
{
// Check the types of the arguments.
argtypes = ((type_T **)stack->ga_data) + stack->ga_len - argcount;
if (internal_func_check_arg_types(argtypes, func_idx, argcount) == FAIL)
if (internal_func_check_arg_types(argtypes, func_idx, argcount,
cctx) == FAIL)
return FAIL;
if (internal_func_is_map(func_idx))
maptype = *argtypes;
Expand Down Expand Up @@ -1656,7 +1658,7 @@ generate_LISTAPPEND(cctx_T *cctx)
list_type = ((type_T **)stack->ga_data)[stack->ga_len - 2];
item_type = ((type_T **)stack->ga_data)[stack->ga_len - 1];
expected = list_type->tt_member;
if (need_type(item_type, expected, -1, cctx, FALSE, FALSE) == FAIL)
if (need_type(item_type, expected, -1, 0, cctx, FALSE, FALSE) == FAIL)
return FAIL;

if (generate_instr(cctx, ISN_LISTAPPEND) == NULL)
Expand All @@ -1678,7 +1680,7 @@ generate_BLOBAPPEND(cctx_T *cctx)

// Caller already checked that blob_type is a blob.
item_type = ((type_T **)stack->ga_data)[stack->ga_len - 1];
if (need_type(item_type, &t_number, -1, cctx, FALSE, FALSE) == FAIL)
if (need_type(item_type, &t_number, -1, 0, cctx, FALSE, FALSE) == FAIL)
return FAIL;

if (generate_instr(cctx, ISN_BLOBAPPEND) == NULL)
Expand Down Expand Up @@ -1733,7 +1735,7 @@ generate_CALL(cctx_T *cctx, ufunc_T *ufunc, int pushed_argcount)
else
expected = ufunc->uf_va_type->tt_member;
actual = ((type_T **)stack->ga_data)[stack->ga_len - argcount + i];
if (need_type(actual, expected, -argcount + i, cctx,
if (need_type(actual, expected, -argcount + i, 0, cctx,
TRUE, FALSE) == FAIL)
{
arg_type_mismatch(expected, actual, i + 1);
Expand Down Expand Up @@ -1850,7 +1852,7 @@ generate_PCALL(
type->tt_argcount - 1]->tt_member;
else
expected = type->tt_args[i];
if (need_type(actual, expected, offset,
if (need_type(actual, expected, offset, 0,
cctx, TRUE, FALSE) == FAIL)
{
arg_type_mismatch(expected, actual, i + 1);
Expand Down Expand Up @@ -3135,7 +3137,7 @@ compile_dict(char_u **arg, cctx_T *cctx, ppconst_T *ppconst)
{
type_T *keytype = ((type_T **)stack->ga_data)
[stack->ga_len - 1];
if (need_type(keytype, &t_string, -1, cctx,
if (need_type(keytype, &t_string, -1, 0, cctx,
FALSE, FALSE) == FAIL)
return FAIL;
}
Expand Down Expand Up @@ -3808,13 +3810,13 @@ compile_subscript(
vtype = VAR_DICT;
if (vtype == VAR_STRING || vtype == VAR_LIST || vtype == VAR_BLOB)
{
if (need_type(valtype, &t_number, -1, cctx,
if (need_type(valtype, &t_number, -1, 0, cctx,
FALSE, FALSE) == FAIL)
return FAIL;
if (is_slice)
{
valtype = ((type_T **)stack->ga_data)[stack->ga_len - 2];
if (need_type(valtype, &t_number, -2, cctx,
if (need_type(valtype, &t_number, -2, 0, cctx,
FALSE, FALSE) == FAIL)
return FAIL;
}
Expand All @@ -3836,7 +3838,7 @@ compile_subscript(
}
else
{
if (need_type(*typep, &t_dict_any, -2, cctx,
if (need_type(*typep, &t_dict_any, -2, 0, cctx,
FALSE, FALSE) == FAIL)
return FAIL;
*typep = &t_any;
Expand Down Expand Up @@ -4235,7 +4237,7 @@ compile_expr7t(char_u **arg, cctx_T *cctx, ppconst_T *ppconst)
actual = ((type_T **)stack->ga_data)[stack->ga_len - 1];
if (check_type(want_type, actual, FALSE, 0) == FAIL)
{
if (need_type(actual, want_type, -1, cctx, FALSE, FALSE) == FAIL)
if (need_type(actual, want_type, -1, 0, cctx, FALSE, FALSE) == FAIL)
return FAIL;
}
}
Expand Down Expand Up @@ -4917,7 +4919,7 @@ compile_return(char_u *arg, int check_return_type, cctx_T *cctx)
return NULL;
}
if (need_type(stack_type, cctx->ctx_ufunc->uf_ret_type, -1,
cctx, FALSE, FALSE) == FAIL)
0, cctx, FALSE, FALSE) == FAIL)
return NULL;
}
}
Expand Down Expand Up @@ -5831,7 +5833,7 @@ compile_assign_unlet(
: ((type_T **)stack->ga_data)[stack->ga_len - 1];
// now we can properly check the type
if (lhs->lhs_type->tt_member != NULL && rhs_type != &t_void
&& need_type(rhs_type, lhs->lhs_type->tt_member, -2, cctx,
&& need_type(rhs_type, lhs->lhs_type->tt_member, -2, 0, cctx,
FALSE, FALSE) == FAIL)
return FAIL;
}
Expand Down Expand Up @@ -5976,7 +5978,7 @@ compile_assignment(char_u *arg, exarg_T *eap, cmdidx_T cmdidx, cctx_T *cctx)
emsg(_(e_cannot_use_void_value));
goto theend;
}
if (need_type(stacktype, &t_list_any, -1, cctx,
if (need_type(stacktype, &t_list_any, -1, 0, cctx,
FALSE, FALSE) == FAIL)
goto theend;
// TODO: check the length of a constant list here
Expand Down Expand Up @@ -6123,13 +6125,13 @@ compile_assignment(char_u *arg, exarg_T *eap, cmdidx_T cmdidx, cctx_T *cctx)
// without operator check type here, otherwise below
if (lhs.lhs_has_index)
use_type = lhs.lhs_member_type;
if (need_type(rhs_type, use_type, -1, cctx,
if (need_type(rhs_type, use_type, -1, 0, cctx,
FALSE, is_const) == FAIL)
goto theend;
}
}
else if (*p != '=' && need_type(rhs_type, lhs.lhs_member_type,
-1, cctx, FALSE, FALSE) == FAIL)
-1, 0, cctx, FALSE, FALSE) == FAIL)
goto theend;
}
else if (cmdidx == CMD_final)
Expand Down Expand Up @@ -6216,7 +6218,7 @@ compile_assignment(char_u *arg, exarg_T *eap, cmdidx_T cmdidx, cctx_T *cctx)
// If variable is float operation with number is OK.
!(expected == &t_float && stacktype == &t_number) &&
#endif
need_type(stacktype, expected, -1, cctx,
need_type(stacktype, expected, -1, 0, cctx,
FALSE, FALSE) == FAIL)
goto theend;

Expand Down Expand Up @@ -6925,7 +6927,7 @@ compile_for(char_u *arg_start, cctx_T *cctx)
// Now that we know the type of "var", check that it is a list, now or at
// runtime.
vartype = ((type_T **)stack->ga_data)[stack->ga_len - 1];
if (need_type(vartype, &t_list_any, -1, cctx, FALSE, FALSE) == FAIL)
if (need_type(vartype, &t_list_any, -1, 0, cctx, FALSE, FALSE) == FAIL)
{
drop_scope(cctx);
return NULL;
Expand Down
20 changes: 5 additions & 15 deletions src/vim9type.c
Original file line number Diff line number Diff line change
Expand Up @@ -513,26 +513,16 @@ check_type(type_T *expected, type_T *actual, int give_msg, int argidx)
return ret;
}

/*
* Like check_type() but also allow for a runtime type check. E.g. "any" can be
* used for "number".
*/
int
check_arg_type(type_T *expected, type_T *actual, int argidx)
{
if (check_type(expected, actual, FALSE, 0) == OK
|| use_typecheck(actual, expected))
return OK;
// TODO: should generate a TYPECHECK instruction.
return check_type(expected, actual, TRUE, argidx);
}

/*
* Check that the arguments of "type" match "argvars[argcount]".
* Return OK/FAIL.
*/
int
check_argument_types(type_T *type, typval_T *argvars, int argcount, char_u *name)
check_argument_types(
type_T *type,
typval_T *argvars,
int argcount,
char_u *name)
{
int varargs = (type->tt_flags & TTFLAG_VARARGS) ? 1 : 0;
int i;
Expand Down

0 comments on commit 351ead0

Please sign in to comment.