To: vim_dev@googlegroups.com Subject: Patch 8.2.1977 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ------------ 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 -- 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 ///