Patch 8.2.1977

[Bram Moolenaar]

Patch 8.2.1977
Problem: Vim9: error for using a string in a condition is confusing.
Solution: Give a more specific error. Also adjust the compile time type
checking for || and &&.
Files: src/vim9compile.c, src/vim9execute.c, src/proto/vim9execute.pro,
src/typval.c, src/errors.h, src/testdir/test_vim9_cmd.vim,
src/testdir/test_vim9_disassemble.vim,
src/testdir/test_vim9_expr.vim


*** ../vim-8.2.1976/src/vim9compile.c 2020-11-08 12:49:43.116372865 +0100
--- src/vim9compile.c 2020-11-12 11:59:03.566372251 +0100
***************
*** 880,885 ****
--- 880,907 ----
}

/*
+ * Check that the top of the type stack has a type that can be used as a
+ * condition. Give an error and return FAIL if not.
+ */
+ static int
+ bool_on_stack(cctx_T *cctx)
+ {
+ garray_T *stack = &cctx->ctx_type_stack;
+ type_T *type;
+
+ type = ((type_T **)stack->ga_data)[stack->ga_len - 1];
+ if (type == &t_bool)
+ return OK;
+
+ if (type == &t_any || type == &t_number)
+ // Number 0 and 1 are OK to use as a bool. "any" could also be a bool.
+ // This requires a runtime type check.
+ return generate_COND2BOOL(cctx);
+
+ return need_type(type, &t_bool, -1, cctx, FALSE, FALSE);
+ }
+
+ /*
* Generate an ISN_PUSHNR instruction.
*/
static int
***************
*** 4306,4313 ****
{
garray_T *instr = &cctx->ctx_instr;
garray_T end_ga;
- garray_T *stack = &cctx->ctx_type_stack;
- int all_bool_values = TRUE;

/*
* Repeat until there is no following "||" or "&&"
--- 4328,4333 ----
***************
*** 4331,4338 ****
// evaluating to bool
generate_ppconst(cctx, ppconst);

! if (((type_T **)stack->ga_data)[stack->ga_len - 1] != &t_bool)
! all_bool_values = FALSE;

if (ga_grow(&end_ga, 1) == FAIL)
{
--- 4351,4362 ----
// evaluating to bool
generate_ppconst(cctx, ppconst);

! // Every part must evaluate to a bool.
! if (bool_on_stack(cctx) == FAIL)
! {
! ga_clear(&end_ga);
! return FAIL;
! }

if (ga_grow(&end_ga, 1) == FAIL)
{
***************
*** 4360,4365 ****
--- 4384,4396 ----
}
generate_ppconst(cctx, ppconst);

+ // Every part must evaluate to a bool.
+ if (bool_on_stack(cctx) == FAIL)
+ {
+ ga_clear(&end_ga);
+ return FAIL;
+ }
+
// Fill in the end label in all jumps.
while (end_ga.ga_len > 0)
{
***************
*** 4371,4380 ****
isn->isn_arg.jump.jump_where = instr->ga_len;
}
ga_clear(&end_ga);
-
- // The resulting type is converted to bool if needed.
- if (!all_bool_values)
- generate_COND2BOOL(cctx);
}

return OK;
--- 4402,4407 ----
***************
*** 4385,4395 ****
*
* Produces instructions:
* EVAL expr4a Push result of "expr4a"
* JUMP_IF_COND_FALSE end
* EVAL expr4b Push result of "expr4b"
* JUMP_IF_COND_FALSE end
* EVAL expr4c Push result of "expr4c"
- * COND2BOOL
* end:
*/
static int
--- 4412,4422 ----
*
* Produces instructions:
* EVAL expr4a Push result of "expr4a"
+ * COND2BOOL convert to bool if needed
* JUMP_IF_COND_FALSE end
* EVAL expr4b Push result of "expr4b"
* JUMP_IF_COND_FALSE end
* EVAL expr4c Push result of "expr4c"
* end:
*/
static int
***************
*** 4410,4420 ****
*
* Produces instructions:
* EVAL expr3a Push result of "expr3a"
* JUMP_IF_COND_TRUE end
* EVAL expr3b Push result of "expr3b"
* JUMP_IF_COND_TRUE end
* EVAL expr3c Push result of "expr3c"
- * COND2BOOL
* end:
*/
static int
--- 4437,4447 ----
*
* Produces instructions:
* EVAL expr3a Push result of "expr3a"
+ * COND2BOOL convert to bool if needed
* JUMP_IF_COND_TRUE end
* EVAL expr3b Push result of "expr3b"
* JUMP_IF_COND_TRUE end
* EVAL expr3c Push result of "expr3c"
* end:
*/
static int
***************
*** 5968,5990 ****
}

/*
- * Check that the top of the type stack has a type that can be used as a
- * condition. Give an error and return FAIL if not.
- */
- static int
- bool_on_stack(cctx_T *cctx)
- {
- garray_T *stack = &cctx->ctx_type_stack;
- type_T *type;
-
- type = ((type_T **)stack->ga_data)[stack->ga_len - 1];
- if (type != &t_bool && type != &t_number && type != &t_any
- && need_type(type, &t_bool, -1, cctx, FALSE, FALSE) == FAIL)
- return FAIL;
- return OK;
- }
-
- /*
* compile "if expr"
*
* "if expr" Produces instructions:
--- 5995,6000 ----
*** ../vim-8.2.1976/src/vim9execute.c 2020-11-04 15:07:13.057780706 +0100
--- src/vim9execute.c 2020-11-12 11:23:09.464704598 +0100
***************
*** 3630,3635 ****
--- 3630,3644 ----
return FALSE;
}

+ void
+ emsg_using_string_as(typval_T *tv, int as_number)
+ {
+ semsg(_(as_number ? e_using_string_as_number_str
+ : e_using_string_as_bool_str),
+ tv->vval.v_string == NULL
+ ? (char_u *)"" : tv->vval.v_string);
+ }
+
/*
* If "tv" is a string give an error and return FAIL.
*/
***************
*** 3638,3644 ****
{
if (tv->v_type == VAR_STRING)
{
! emsg(_(e_using_string_as_number));
clear_tv(tv);
return FAIL;
}
--- 3647,3653 ----
{
if (tv->v_type == VAR_STRING)
{
! emsg_using_string_as(tv, TRUE);
clear_tv(tv);
return FAIL;
}
*** ../vim-8.2.1976/src/proto/vim9execute.pro 2020-10-10 14:12:58.024646147 +0200
--- src/proto/vim9execute.pro 2020-11-11 22:13:28.016748482 +0100
***************
*** 4,8 ****
--- 4,9 ----
int call_def_function(ufunc_T *ufunc, int argc_arg, typval_T *argv, partial_T *partial, typval_T *rettv);
void ex_disassemble(exarg_T *eap);
int tv2bool(typval_T *tv);
+ void emsg_using_string_as(typval_T *tv, int as_number);
int check_not_string(typval_T *tv);
/* vim: set ft=c : */
*** ../vim-8.2.1976/src/typval.c 2020-09-26 22:39:18.427725844 +0200
--- src/typval.c 2020-11-11 22:13:20.572768226 +0100
***************
*** 196,202 ****
case VAR_STRING:
if (in_vim9script())
{
! emsg(_(e_using_string_as_number));
break;
}
if (varp->vval.v_string != NULL)
--- 196,202 ----
case VAR_STRING:
if (in_vim9script())
{
! emsg_using_string_as(varp, !want_bool);
break;
}
if (varp->vval.v_string != NULL)
*** ../vim-8.2.1976/src/errors.h 2020-11-04 11:36:31.412190527 +0100
--- src/errors.h 2020-11-11 22:18:12.871992576 +0100
***************
*** 89,96 ****
INIT(= N_("E1028: Compiling :def function failed"));
EXTERN char e_expected_str_but_got_str[]
INIT(= N_("E1029: Expected %s but got %s"));
! EXTERN char e_using_string_as_number[]
! INIT(= N_("E1030: Using a String as a Number"));
EXTERN char e_cannot_use_void_value[]
INIT(= N_("E1031: Cannot use void value"));
EXTERN char e_missing_catch_or_finally[]
--- 89,96 ----
INIT(= N_("E1028: Compiling :def function failed"));
EXTERN char e_expected_str_but_got_str[]
INIT(= N_("E1029: Expected %s but got %s"));
! EXTERN char e_using_string_as_number_str[]
! INIT(= N_("E1030: Using a String as a Number: \"%s\""));
EXTERN char e_cannot_use_void_value[]
INIT(= N_("E1031: Cannot use void value"));
EXTERN char e_missing_catch_or_finally[]
***************
*** 292,295 ****
--- 292,297 ----
INIT(= N_("E1133: Cannot extend a null dict"));
EXTERN char e_cannot_extend_null_list[]
INIT(= N_("E1134: Cannot extend a null list"));
+ EXTERN char e_using_string_as_bool_str[]
+ INIT(= N_("E1135: Using a String as a Bool: \"%s\""));
#endif
*** ../vim-8.2.1976/src/testdir/test_vim9_cmd.vim 2020-11-04 18:53:31.554111047 +0100
--- src/testdir/test_vim9_cmd.vim 2020-11-12 11:37:22.096547011 +0100
***************
*** 74,80 ****
if 'text'
endif
END
! CheckDefAndScriptFailure(lines, 'E1030:', 1)

lines =<< trim END
if [1]
--- 74,80 ----
if 'text'
endif
END
! CheckDefAndScriptFailure(lines, 'E1135:', 1)

lines =<< trim END
if [1]
***************
*** 88,94 ****
if g:cond
endif
END
! CheckDefExecAndScriptFailure(lines, 'E1030:', 2)

lines =<< trim END
g:cond = 0
--- 88,94 ----
if g:cond
endif
END
! CheckDefExecAndScriptFailure(lines, 'E1135:', 2)

lines =<< trim END
g:cond = 0
***************
*** 97,103 ****
endif
END
CheckDefFailure(lines, 'E1012:', 3)
! CheckScriptFailure(['vim9script'] + lines, 'E1030:', 4)

lines =<< trim END
if g:cond
--- 97,103 ----
endif
END
CheckDefFailure(lines, 'E1012:', 3)
! CheckScriptFailure(['vim9script'] + lines, 'E1135:', 4)

lines =<< trim END
if g:cond
***************
*** 113,126 ****
elseif g:cond
endif
END
! CheckDefExecAndScriptFailure(lines, 'E1030:', 3)

lines =<< trim END
while 'text'
endwhile
END
CheckDefFailure(lines, 'E1012:', 1)
! CheckScriptFailure(['vim9script'] + lines, 'E1030:', 2)

lines =<< trim END
while [1]
--- 113,126 ----
elseif g:cond
endif
END
! CheckDefExecAndScriptFailure(lines, 'E1135:', 3)

lines =<< trim END
while 'text'
endwhile
END
CheckDefFailure(lines, 'E1012:', 1)
! CheckScriptFailure(['vim9script'] + lines, 'E1135:', 2)

lines =<< trim END
while [1]
***************
*** 134,140 ****
while g:cond
endwhile
END
! CheckDefExecAndScriptFailure(lines, 'E1030:', 2)
enddef

def Test_if_linebreak()
--- 134,140 ----
while g:cond
endwhile
END
! CheckDefExecAndScriptFailure(lines, 'E1135:', 2)
enddef

def Test_if_linebreak()
*** ../vim-8.2.1976/src/testdir/test_vim9_disassemble.vim 2020-11-08 12:49:43.120372854 +0100
--- src/testdir/test_vim9_disassemble.vim 2020-11-12 12:05:50.799951366 +0100
***************
*** 707,712 ****
--- 707,713 ----
'if has("gui_running")\_s*' ..
'\d PUSHS "gui_running"\_s*' ..
'\d BCALL has(argc 1)\_s*' ..
+ '\d COND2BOOL\_s*' ..
'\d JUMP_IF_FALSE -> \d\_s*' ..
' echo "yes"\_s*' ..
'\d PUSHS "yes"\_s*' ..
***************
*** 760,773 ****
assert_match('ReturnInIf\_s*' ..
'if g:cond\_s*' ..
'0 LOADG g:cond\_s*' ..
! '1 JUMP_IF_FALSE -> 4\_s*' ..
'return "yes"\_s*' ..
! '2 PUSHS "yes"\_s*' ..
! '3 RETURN\_s*' ..
'else\_s*' ..
' return "no"\_s*' ..
! '4 PUSHS "no"\_s*' ..
! '5 RETURN$',
instr)
enddef

--- 761,775 ----
assert_match('ReturnInIf\_s*' ..
'if g:cond\_s*' ..
'0 LOADG g:cond\_s*' ..
! '1 COND2BOOL\_s*' ..
! '2 JUMP_IF_FALSE -> 5\_s*' ..
'return "yes"\_s*' ..
! '3 PUSHS "yes"\_s*' ..
! '4 RETURN\_s*' ..
'else\_s*' ..
' return "no"\_s*' ..
! '5 PUSHS "no"\_s*' ..
! '6 RETURN$',
instr)
enddef

***************
*** 1357,1372 ****
assert_match('ReturnBool\_s*' ..
'var name: bool = 1 && 0 || 1\_s*' ..
'0 PUSHNR 1\_s*' ..
! '1 JUMP_IF_COND_FALSE -> 3\_s*' ..
! '2 PUSHNR 0\_s*' ..
! '3 COND2BOOL\_s*' ..
! '4 JUMP_IF_COND_TRUE -> 6\_s*' ..
! '5 PUSHNR 1\_s*' ..
! '6 2BOOL (!!val)\_s*' ..
'\d STORE $0\_s*' ..
'return name\_s*' ..
! '\d LOAD $0\_s*' ..
! '\d RETURN',
instr)
assert_equal(true, InvertBool())
enddef
--- 1359,1375 ----
assert_match('ReturnBool\_s*' ..
'var name: bool = 1 && 0 || 1\_s*' ..
'0 PUSHNR 1\_s*' ..
! '1 2BOOL (!!val)\_s*' ..
! '2 JUMP_IF_COND_FALSE -> 5\_s*' ..
! '3 PUSHNR 0\_s*' ..
! '4 2BOOL (!!val)\_s*' ..
! '5 JUMP_IF_COND_TRUE -> 8\_s*' ..
! '6 PUSHNR 1\_s*' ..
! '7 2BOOL (!!val)\_s*' ..
'\d STORE $0\_s*' ..
'return name\_s*' ..
! '\d\+ LOAD $0\_s*' ..
! '\d\+ RETURN',
instr)
assert_equal(true, InvertBool())
enddef
*** ../vim-8.2.1976/src/testdir/test_vim9_expr.vim 2020-11-07 13:09:09.209143607 +0100
--- src/testdir/test_vim9_expr.vim 2020-11-12 12:03:29.577389089 +0100
***************
*** 131,137 ****
vim9script
var name = 'x' ? 1 : 2
END
! CheckScriptFailure(lines, 'E1030:', 2)

lines =<< trim END
vim9script
--- 131,137 ----
vim9script
var name = 'x' ? 1 : 2
END
! CheckScriptFailure(lines, 'E1135:', 2)

lines =<< trim END
vim9script
***************
*** 180,186 ****
call CheckDefFailure(["var x = 1 ? 'one' :'two'"], msg, 1)
call CheckDefFailure(["var x = 1 ? 'one':'two'"], msg, 1)

! call CheckDefFailure(["var x = 'x' ? 'one' : 'two'"], 'E1030:', 1)
call CheckDefFailure(["var x = 0z1234 ? 'one' : 'two'"], 'E974:', 1)
call CheckDefExecFailure(["var x = [] ? 'one' : 'two'"], 'E745:', 1)
call CheckDefExecFailure(["var x = {} ? 'one' : 'two'"], 'E728:', 1)
--- 180,186 ----
call CheckDefFailure(["var x = 1 ? 'one' :'two'"], msg, 1)
call CheckDefFailure(["var x = 1 ? 'one':'two'"], msg, 1)

! call CheckDefFailure(["var x = 'x' ? 'one' : 'two'"], 'E1135:', 1)
call CheckDefFailure(["var x = 0z1234 ? 'one' : 'two'"], 'E974:', 1)
call CheckDefExecFailure(["var x = [] ? 'one' : 'two'"], 'E745:', 1)
call CheckDefExecFailure(["var x = {} ? 'one' : 'two'"], 'E728:', 1)
***************
*** 356,366 ****
call CheckDefFailure(["var x = 1|| 2"], msg, 1)

call CheckDefFailure(["var x = 1 || xxx"], 'E1001:', 1)

# TODO: should fail at compile time
call CheckDefExecFailure(["var x = 3 || 7"], 'E1023:', 1)
call CheckScriptFailure(["vim9script", "var x = 3 || 7"], 'E1023:', 2)
- call CheckDefExecFailure(["var x = [] || false"], 'E745:', 1)
call CheckScriptFailure(["vim9script", "var x = [] || false"], 'E745:', 2)
enddef

--- 356,367 ----
call CheckDefFailure(["var x = 1|| 2"], msg, 1)

call CheckDefFailure(["var x = 1 || xxx"], 'E1001:', 1)
+ call CheckDefFailure(["var x = [] || false"], 'E1012:', 1)
+ call CheckDefFailure(["if 'yes' || 0", 'echo 0', 'endif'], 'E1012: Type mismatch; expected bool but got string', 1)

# TODO: should fail at compile time
call CheckDefExecFailure(["var x = 3 || 7"], 'E1023:', 1)
call CheckScriptFailure(["vim9script", "var x = 3 || 7"], 'E1023:', 2)
call CheckScriptFailure(["vim9script", "var x = [] || false"], 'E745:', 2)
enddef

***************
*** 492,497 ****
--- 493,500 ----
call CheckDefFailure(["var x = 1&&2"], msg, 1)
call CheckDefFailure(["var x = 1 &&2"], msg, 1)
call CheckDefFailure(["var x = 1&& 2"], msg, 1)
+
+ call CheckDefFailure(["if 'yes' && 0", 'echo 0', 'endif'], 'E1012: Type mismatch; expected bool but got string', 1)
endfunc

" global variables to use for tests with the "any" type
*** ../vim-8.2.1976/src/version.c 2020-11-11 20:52:36.970181354 +0100
--- src/version.c 2020-11-12 11:34:39.841324659 +0100
***************
*** 752,753 ****
--- 752,755 ----
{ /* Add new patch number below this line */
+ /**/
+ 1977,
/**/

--
You are Dead. Do you wish to restart, load, or quit?

/// Bram Moolenaar -- Br...@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 ///