To: vim_dev@googlegroups.com Subject: Patch 8.2.1798 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ------------ Patch 8.2.1798 Problem: Vim9: trinary operator condition is too permissive. Solution: Use tv_get_bool_chk(). Files: runtime/doc/vim9.txt, src/eval.c, src/vim9compile.c, src/vim9execute.c, src/testdir/vim9.vim, src/testdir/test_expr.vim, src/testdir/test_vim9_expr.vim, src/testdir/test_vim9_cmd.vim, src/testdir/test_vim9_script.vim *** ../vim-8.2.1797/runtime/doc/vim9.txt 2020-10-03 22:51:42.890813408 +0200 --- runtime/doc/vim9.txt 2020-10-04 15:21:54.182614262 +0200 *************** *** 164,178 **** When using `:function` or `:def` to specify a nested function inside a `:def` function, this nested function is local to the code block it is defined in. ! In a `:def` function IT is not possible to define a script-local function. it is possible to define a global function by using the "g:" prefix. When referring to a function and no "s:" or "g:" prefix is used, Vim will ! prefer using a local function (in the function scope, script scope or ! imported) before looking for a global function. However, it is recommended to ! always use "g:" to refer to a local function for clarity. In all cases the ! function must be defined before used. That is when it is first called or when ! `:defcompile` causes the call to be compiled. The result is that functions and variables without a namespace can usually be found in the script, either defined there or imported. Global functions and --- 164,183 ---- When using `:function` or `:def` to specify a nested function inside a `:def` function, this nested function is local to the code block it is defined in. ! In a `:def` function it is not possible to define a script-local function. it is possible to define a global function by using the "g:" prefix. When referring to a function and no "s:" or "g:" prefix is used, Vim will ! search for the function: ! - in the function scope ! - in the script scope, possibly imported ! - in the list of global functions ! However, it is recommended to always use "g:" to refer to a global function ! for clarity. ! ! In all cases the function must be defined before used. That is when it is ! called, when `:defcompile` causes the it to be compiled, or when code that ! calls it is being compiled (to figure out the return type). The result is that functions and variables without a namespace can usually be found in the script, either defined there or imported. Global functions and *************** *** 467,477 **** Conditions and expressions ~ ! Conditions and expressions are mostly working like they do in JavaScript. A ! difference is made where JavaScript does not work like most people expect. ! Specifically, an empty list is falsy. ! type TRUE when ~ bool v:true or 1 number non-zero float non-zero --- 472,492 ---- Conditions and expressions ~ ! Conditions and expressions are mostly working like they do in other languages. ! Some values are different from legacy Vim script: ! value legacy Vim script Vim9 script ~ ! 0 falsy falsy ! 1 truthy truthy ! 99 truthy Error! ! "0" falsy Error! ! "99" truthy Error! ! "text" falsy Error! ! ! For the "??" operator and when using "!" then there is no error, every value ! is either falsy or truthy. This is mostly like JavaScript, except that an ! empty list and dict is falsy: ! type truthy when ~ bool v:true or 1 number non-zero float non-zero *************** *** 498,510 **** [] || 99 Error! When using "!" for inverting, there is no error for using any type and the ! result is a boolean: > !'yes' == false ! var myList = [1, 2, 3] ! !!myList == true When using "`.."` for string concatenation arguments of simple types are ! always converted to string. > 'hello ' .. 123 == 'hello 123' 'hello ' .. v:true == 'hello v:true' --- 513,525 ---- [] || 99 Error! When using "!" for inverting, there is no error for using any type and the ! result is a boolean. "!!" can be used to turn any value into boolean: > !'yes' == false ! !![] == false ! !![1, 2, 3] == true When using "`.."` for string concatenation arguments of simple types are ! always converted to string: > 'hello ' .. 123 == 'hello 123' 'hello ' .. v:true == 'hello v:true' *** ../vim-8.2.1797/src/eval.c 2020-10-03 22:51:42.890813408 +0200 --- src/eval.c 2020-10-04 15:32:53.648263271 +0200 *************** *** 191,197 **** if (!skip) { if (in_vim9script()) ! retval = tv2bool(&tv); else retval = (tv_get_number_chk(&tv, error) != 0); clear_tv(&tv); --- 191,197 ---- if (!skip) { if (in_vim9script()) ! retval = tv_get_bool_chk(&tv, error); else retval = (tv_get_number_chk(&tv, error) != 0); clear_tv(&tv); *************** *** 2143,2148 **** --- 2143,2149 ---- evalarg_T local_evalarg; int orig_flags; int evaluate; + int vim9script = in_vim9script(); if (evalarg == NULL) { *************** *** 2156,2162 **** *arg = eval_next_line(evalarg_used); else { ! if (evaluate && in_vim9script() && !VIM_ISWHITE(p[-1])) { error_white_both(p, 1); clear_tv(rettv); --- 2157,2163 ---- *arg = eval_next_line(evalarg_used); else { ! if (evaluate && vim9script && !VIM_ISWHITE(p[-1])) { error_white_both(p, 1); clear_tv(rettv); *************** *** 2170,2177 **** { int error = FALSE; ! if (in_vim9script() || op_falsy) result = tv2bool(rettv); else if (tv_get_number_chk(rettv, &error) != 0) result = TRUE; if (error || !op_falsy || !result) --- 2171,2180 ---- { int error = FALSE; ! if (op_falsy) result = tv2bool(rettv); + else if (vim9script) + result = tv_get_bool_chk(rettv, &error); else if (tv_get_number_chk(rettv, &error) != 0) result = TRUE; if (error || !op_falsy || !result) *************** *** 2185,2191 **** */ if (op_falsy) ++*arg; ! if (evaluate && in_vim9script() && !IS_WHITE_OR_NUL((*arg)[1])) { error_white_both(p, 1); clear_tv(rettv); --- 2188,2194 ---- */ if (op_falsy) ++*arg; ! if (evaluate && vim9script && !IS_WHITE_OR_NUL((*arg)[1])) { error_white_both(p, 1); clear_tv(rettv); *************** *** 2220,2226 **** *arg = eval_next_line(evalarg_used); else { ! if (evaluate && in_vim9script() && !VIM_ISWHITE(p[-1])) { error_white_both(p, 1); clear_tv(rettv); --- 2223,2229 ---- *arg = eval_next_line(evalarg_used); else { ! if (evaluate && vim9script && !VIM_ISWHITE(p[-1])) { error_white_both(p, 1); clear_tv(rettv); *************** *** 2233,2239 **** /* * Get the third variable. Recursive! */ ! if (evaluate && in_vim9script() && !IS_WHITE_OR_NUL((*arg)[1])) { error_white_both(p, 1); clear_tv(rettv); --- 2236,2242 ---- /* * Get the third variable. Recursive! */ ! if (evaluate && vim9script && !IS_WHITE_OR_NUL((*arg)[1])) { error_white_both(p, 1); clear_tv(rettv); *** ../vim-8.2.1797/src/vim9compile.c 2020-10-03 22:51:42.890813408 +0200 --- src/vim9compile.c 2020-10-04 15:58:05.471431295 +0200 *************** *** 4212,4218 **** // the condition is a constant, we know whether the ? or the : // expression is to be evaluated. has_const_expr = TRUE; ! const_value = tv2bool(&ppconst->pp_tv[ppconst_used]); cctx->ctx_skip = save_skip == SKIP_YES || (op_falsy ? const_value : !const_value) ? SKIP_YES : SKIP_NOT; --- 4212,4228 ---- // the condition is a constant, we know whether the ? or the : // expression is to be evaluated. has_const_expr = TRUE; ! if (op_falsy) ! const_value = tv2bool(&ppconst->pp_tv[ppconst_used]); ! else ! { ! int error = FALSE; ! ! const_value = tv_get_bool_chk(&ppconst->pp_tv[ppconst_used], ! &error); ! if (error) ! return FAIL; ! } cctx->ctx_skip = save_skip == SKIP_YES || (op_falsy ? const_value : !const_value) ? SKIP_YES : SKIP_NOT; *************** *** 5638,5643 **** --- 5648,5670 ---- } /* + * 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) == FAIL) + return FAIL; + return OK; + } + + /* * compile "if expr" * * "if expr" Produces instructions: *************** *** 5689,5696 **** clear_ppconst(&ppconst); else if (instr->ga_len == instr_count && ppconst.pp_used == 1) { // The expression results in a constant. ! cctx->ctx_skip = tv2bool(&ppconst.pp_tv[0]) ? SKIP_NOT : SKIP_YES; clear_ppconst(&ppconst); } else --- 5716,5729 ---- clear_ppconst(&ppconst); else if (instr->ga_len == instr_count && ppconst.pp_used == 1) { + int error = FALSE; + int v; + // The expression results in a constant. ! v = tv_get_bool_chk(&ppconst.pp_tv[0], &error); ! if (error) ! return NULL; ! cctx->ctx_skip = v ? SKIP_NOT : SKIP_YES; clear_ppconst(&ppconst); } else *************** *** 5699,5704 **** --- 5732,5739 ---- cctx->ctx_skip = SKIP_UNKNOWN; if (generate_ppconst(cctx, &ppconst) == FAIL) return NULL; + if (bool_on_stack(cctx) == FAIL) + return NULL; } scope = new_scope(cctx, IF_SCOPE); *************** *** 5764,5772 **** clear_ppconst(&ppconst); else if (instr->ga_len == instr_count && ppconst.pp_used == 1) { // The expression results in a constant. // TODO: how about nesting? ! cctx->ctx_skip = tv2bool(&ppconst.pp_tv[0]) ? SKIP_NOT : SKIP_YES; clear_ppconst(&ppconst); scope->se_u.se_if.is_if_label = -1; } --- 5799,5813 ---- clear_ppconst(&ppconst); else if (instr->ga_len == instr_count && ppconst.pp_used == 1) { + int error = FALSE; + int v; + // The expression results in a constant. // TODO: how about nesting? ! v = tv_get_bool_chk(&ppconst.pp_tv[0], &error); ! if (error) ! return NULL; ! cctx->ctx_skip = v ? SKIP_NOT : SKIP_YES; clear_ppconst(&ppconst); scope->se_u.se_if.is_if_label = -1; } *************** *** 5776,5781 **** --- 5817,5824 ---- cctx->ctx_skip = SKIP_UNKNOWN; if (generate_ppconst(cctx, &ppconst) == FAIL) return NULL; + if (bool_on_stack(cctx) == FAIL) + return NULL; // "where" is set when ":elseif", "else" or ":endif" is found scope->se_u.se_if.is_if_label = instr->ga_len; *************** *** 6037,6042 **** --- 6080,6088 ---- if (compile_expr0(&p, cctx) == FAIL) return NULL; + if (bool_on_stack(cctx) == FAIL) + return FAIL; + // "while_end" is set when ":endwhile" is found if (compile_jump_to_end(&scope->se_u.se_while.ws_end_label, JUMP_IF_FALSE, cctx) == FAIL) *** ../vim-8.2.1797/src/vim9execute.c 2020-10-03 22:51:42.890813408 +0200 --- src/vim9execute.c 2020-10-04 15:03:09.806014728 +0200 *************** *** 1908,1913 **** --- 1908,1914 ---- { tv = STACK_TV_BOT(-1); if (when == JUMP_IF_COND_FALSE + || when == JUMP_IF_FALSE || when == JUMP_IF_COND_TRUE) { SOURCING_LNUM = iptr->isn_lnum; *************** *** 3403,3409 **** } /* ! * Return TRUE when "tv" is not falsey: non-zero, non-empty string, non-empty * list, etc. Mostly like what JavaScript does, except that empty list and * empty dictionary are FALSE. */ --- 3404,3410 ---- } /* ! * Return TRUE when "tv" is not falsy: non-zero, non-empty string, non-empty * list, etc. Mostly like what JavaScript does, except that empty list and * empty dictionary are FALSE. */ *** ../vim-8.2.1797/src/testdir/vim9.vim 2020-09-09 18:54:39.170253618 +0200 --- src/testdir/vim9.vim 2020-10-04 15:34:40.947913054 +0200 *************** *** 52,54 **** --- 52,61 ---- CheckDefFailure(lines, error, lnum) CheckScriptFailure(['vim9script'] + lines, error, lnum + 1) enddef + + " Check that a command fails both when executed in a :def function and when + " used in Vim9 script. + def CheckDefExecAndScriptFailure(lines: list, error: string, lnum = -3) + CheckDefExecFailure(lines, error, lnum) + CheckScriptFailure(['vim9script'] + lines, error, lnum + 1) + enddef *** ../vim-8.2.1797/src/testdir/test_expr.vim 2020-10-03 20:16:48.775216665 +0200 --- src/testdir/test_expr.vim 2020-10-04 14:42:27.509542060 +0200 *************** *** 42,47 **** --- 42,57 ---- call assert_false(has('patch-9.9.1')) endfunc + func Test_op_trinary() + call assert_equal('yes', 1 ? 'yes' : 'no') + call assert_equal('no', 0 ? 'yes' : 'no') + call assert_equal('no', 'x' ? 'yes' : 'no') + call assert_equal('yes', '1x' ? 'yes' : 'no') + + call assert_fails('echo [1] ? "yes" : "no"', 'E745:') + call assert_fails('echo {} ? "yes" : "no"', 'E728:') + endfunc + func Test_op_falsy() call assert_equal(v:true, v:true ?? 456) call assert_equal(123, 123 ?? 456) *** ../vim-8.2.1797/src/testdir/test_vim9_expr.vim 2020-10-03 22:51:42.894813399 +0200 --- src/testdir/test_vim9_expr.vim 2020-10-04 15:04:46.805744662 +0200 *************** *** 18,44 **** 'one' : 'two') if has('float') ! assert_equal('one', 0.1 ? 'one' : 'two') endif ! assert_equal('one', 'x' ? 'one' : 'two') ! assert_equal('one', 'x' ? 'one' : 'two') ! assert_equal('one', 0z1234 ? 'one' : 'two') ! assert_equal('one', [0] ? 'one' : 'two') ! assert_equal('one', #{x: 0} ? 'one' : 'two') var name = 1 assert_equal('one', name ? 'one' : 'two') assert_equal('two', false ? 'one' : 'two') assert_equal('two', 0 ? 'one' : 'two') if has('float') ! assert_equal('two', 0.0 ? 'one' : 'two') endif ! assert_equal('two', '' ? 'one' : 'two') ! assert_equal('two', 0z ? 'one' : 'two') ! assert_equal('two', [] ? 'one' : 'two') ! assert_equal('two', {} ? 'one' : 'two') name = 0 assert_equal('two', name ? 'one' : 'two') --- 18,44 ---- 'one' : 'two') if has('float') ! assert_equal('one', !!0.1 ? 'one' : 'two') endif ! assert_equal('one', !!'x' ? 'one' : 'two') ! assert_equal('one', !!'x' ? 'one' : 'two') ! assert_equal('one', !!0z1234 ? 'one' : 'two') ! assert_equal('one', !![0] ? 'one' : 'two') ! assert_equal('one', !!#{x: 0} ? 'one' : 'two') var name = 1 assert_equal('one', name ? 'one' : 'two') assert_equal('two', false ? 'one' : 'two') assert_equal('two', 0 ? 'one' : 'two') if has('float') ! assert_equal('two', !!0.0 ? 'one' : 'two') endif ! assert_equal('two', !!'' ? 'one' : 'two') ! assert_equal('two', !!0z ? 'one' : 'two') ! assert_equal('two', !![] ? 'one' : 'two') ! assert_equal('two', !!{} ? 'one' : 'two') name = 0 assert_equal('two', name ? 'one' : 'two') *************** *** 117,122 **** --- 117,140 ---- END CheckScriptFailure(lines, 'E1004:', 2) + lines =<< trim END + vim9script + var name = 'x' ? 1 : 2 + END + CheckScriptFailure(lines, 'E1030:', 2) + + lines =<< trim END + vim9script + var name = [] ? 1 : 2 + END + CheckScriptFailure(lines, 'E745:', 2) + + lines =<< trim END + vim9script + var name = {} ? 1 : 2 + END + CheckScriptFailure(lines, 'E728:', 2) + # check after failure eval_flags is reset lines =<< trim END vim9script *************** *** 152,157 **** --- 170,184 ---- 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) + + if has('float') + call CheckDefFailure(["var x = 0.1 ? 'one' : 'two'"], 'E805:', 1) + endif + " missing argument detected even when common type is used call CheckDefFailure([ \ 'var X = FuncOne', *** ../vim-8.2.1797/src/testdir/test_vim9_cmd.vim 2020-10-03 22:51:42.894813399 +0200 --- src/testdir/test_vim9_cmd.vim 2020-10-04 16:00:22.230977948 +0200 *************** *** 68,73 **** --- 68,141 ---- CheckScriptSuccess(lines) enddef + def Test_condition_types() + var lines =<< trim END + if 'text' + endif + END + CheckDefAndScriptFailure(lines, 'E1030:', 1) + + lines =<< trim END + if [1] + endif + END + CheckDefFailure(lines, 'E1012:', 1) + CheckScriptFailure(['vim9script'] + lines, 'E745:', 2) + + lines =<< trim END + g:cond = 'text' + if g:cond + endif + END + CheckDefExecAndScriptFailure(lines, 'E1030:', 2) + + lines =<< trim END + g:cond = 0 + if g:cond + elseif 'text' + endif + END + CheckDefFailure(lines, 'E1012:', 3) + CheckScriptFailure(['vim9script'] + lines, 'E1030:', 4) + + lines =<< trim END + if g:cond + elseif [1] + endif + END + CheckDefFailure(lines, 'E1012:', 2) + CheckScriptFailure(['vim9script'] + lines, 'E745:', 3) + + lines =<< trim END + g:cond = 'text' + if 0 + 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] + endwhile + END + CheckDefFailure(lines, 'E1012:', 1) + CheckScriptFailure(['vim9script'] + lines, 'E745:', 2) + + lines =<< trim END + g:cond = 'text' + while g:cond + endwhile + END + CheckDefExecAndScriptFailure(lines, 'E1030:', 2) + enddef + def Test_if_linebreak() var lines =<< trim END vim9script *** ../vim-8.2.1797/src/testdir/test_vim9_script.vim 2020-09-30 22:59:37.904243979 +0200 --- src/testdir/test_vim9_script.vim 2020-10-04 16:01:46.966700783 +0200 *************** *** 543,549 **** CheckDefFailure(['endtry'], 'E602:') CheckDefFailure(['while 1', 'endtry'], 'E170:') CheckDefFailure(['for i in range(5)', 'endtry'], 'E170:') ! CheckDefFailure(['if 2', 'endtry'], 'E171:') CheckDefFailure(['try', 'echo 1', 'endtry'], 'E1032:') CheckDefFailure(['throw'], 'E1015:') --- 543,549 ---- CheckDefFailure(['endtry'], 'E602:') CheckDefFailure(['while 1', 'endtry'], 'E170:') CheckDefFailure(['for i in range(5)', 'endtry'], 'E170:') ! CheckDefFailure(['if 1', 'endtry'], 'E171:') CheckDefFailure(['try', 'echo 1', 'endtry'], 'E1032:') CheckDefFailure(['throw'], 'E1015:') *** ../vim-8.2.1797/src/version.c 2020-10-04 14:17:28.965363295 +0200 --- src/version.c 2020-10-04 14:37:59.738262979 +0200 *************** *** 752,753 **** --- 752,755 ---- { /* Add new patch number below this line */ + /**/ + 1798, /**/ -- The only backup you need is the one that you didn't have time for. (Murphy) /// 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 ///