From 1310660557470a669cc64b359e20666b116e5dbd Mon Sep 17 00:00:00 2001 From: Bram Moolenaar Date: Sun, 4 Oct 2020 16:06:05 +0200 Subject: patch 8.2.1798: Vim9: trinary operator condition is too permissive Problem: Vim9: trinary operator condition is too permissive. Solution: Use tv_get_bool_chk(). --- src/eval.c | 15 +++++---- src/testdir/test_expr.vim | 10 ++++++ src/testdir/test_vim9_cmd.vim | 68 ++++++++++++++++++++++++++++++++++++++++ src/testdir/test_vim9_expr.vim | 49 ++++++++++++++++++++++------- src/testdir/test_vim9_script.vim | 2 +- src/testdir/vim9.vim | 7 +++++ src/version.c | 2 ++ src/vim9compile.c | 52 ++++++++++++++++++++++++++++-- src/vim9execute.c | 3 +- 9 files changed, 186 insertions(+), 22 deletions(-) (limited to 'src') diff --git a/src/eval.c b/src/eval.c index a678d32e64..285558df8f 100644 --- a/src/eval.c +++ b/src/eval.c @@ -191,7 +191,7 @@ eval_to_bool( if (!skip) { if (in_vim9script()) - retval = tv2bool(&tv); + retval = tv_get_bool_chk(&tv, error); else retval = (tv_get_number_chk(&tv, error) != 0); clear_tv(&tv); @@ -2143,6 +2143,7 @@ eval1(char_u **arg, typval_T *rettv, evalarg_T *evalarg) evalarg_T local_evalarg; int orig_flags; int evaluate; + int vim9script = in_vim9script(); if (evalarg == NULL) { @@ -2156,7 +2157,7 @@ eval1(char_u **arg, typval_T *rettv, evalarg_T *evalarg) *arg = eval_next_line(evalarg_used); else { - if (evaluate && in_vim9script() && !VIM_ISWHITE(p[-1])) + if (evaluate && vim9script && !VIM_ISWHITE(p[-1])) { error_white_both(p, 1); clear_tv(rettv); @@ -2170,8 +2171,10 @@ eval1(char_u **arg, typval_T *rettv, evalarg_T *evalarg) { int error = FALSE; - if (in_vim9script() || op_falsy) + 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,7 +2188,7 @@ eval1(char_u **arg, typval_T *rettv, evalarg_T *evalarg) */ if (op_falsy) ++*arg; - if (evaluate && in_vim9script() && !IS_WHITE_OR_NUL((*arg)[1])) + if (evaluate && vim9script && !IS_WHITE_OR_NUL((*arg)[1])) { error_white_both(p, 1); clear_tv(rettv); @@ -2220,7 +2223,7 @@ eval1(char_u **arg, typval_T *rettv, evalarg_T *evalarg) *arg = eval_next_line(evalarg_used); else { - if (evaluate && in_vim9script() && !VIM_ISWHITE(p[-1])) + if (evaluate && vim9script && !VIM_ISWHITE(p[-1])) { error_white_both(p, 1); clear_tv(rettv); @@ -2233,7 +2236,7 @@ eval1(char_u **arg, typval_T *rettv, evalarg_T *evalarg) /* * Get the third variable. Recursive! */ - if (evaluate && in_vim9script() && !IS_WHITE_OR_NUL((*arg)[1])) + if (evaluate && vim9script && !IS_WHITE_OR_NUL((*arg)[1])) { error_white_both(p, 1); clear_tv(rettv); diff --git a/src/testdir/test_expr.vim b/src/testdir/test_expr.vim index 1086534dca..754f856c22 100644 --- a/src/testdir/test_expr.vim +++ b/src/testdir/test_expr.vim @@ -42,6 +42,16 @@ func Test_version() 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) diff --git a/src/testdir/test_vim9_cmd.vim b/src/testdir/test_vim9_cmd.vim index 82d5f325e4..599c287e23 100644 --- a/src/testdir/test_vim9_cmd.vim +++ b/src/testdir/test_vim9_cmd.vim @@ -68,6 +68,74 @@ def Test_echo_linebreak() 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 diff --git a/src/testdir/test_vim9_expr.vim b/src/testdir/test_vim9_expr.vim index 5158e1093f..2cbb2854a6 100644 --- a/src/testdir/test_vim9_expr.vim +++ b/src/testdir/test_vim9_expr.vim @@ -18,27 +18,27 @@ def Test_expr1_trinary() 'one' : 'two') if has('float') - assert_equal('one', 0.1 ? 'one' : 'two') + assert_equal('one', !!0.1 ? 'one' : 'two') endif - assert_equal('one', 'x' ? 'one' : 'two') - assert_equal('one', 'x' + 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') + 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') + 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') + 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,6 +117,24 @@ def Test_expr1_trinary_vimscript() 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,6 +170,15 @@ func Test_expr1_trinary_fails() 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', diff --git a/src/testdir/test_vim9_script.vim b/src/testdir/test_vim9_script.vim index 6dbabe3892..7fc522362d 100644 --- a/src/testdir/test_vim9_script.vim +++ b/src/testdir/test_vim9_script.vim @@ -543,7 +543,7 @@ def Test_try_catch_fails() CheckDefFailure(['endtry'], 'E602:') CheckDefFailure(['while 1', 'endtry'], 'E170:') CheckDefFailure(['for i in range(5)', 'endtry'], 'E170:') - CheckDefFailure(['if 2', 'endtry'], 'E171:') + CheckDefFailure(['if 1', 'endtry'], 'E171:') CheckDefFailure(['try', 'echo 1', 'endtry'], 'E1032:') CheckDefFailure(['throw'], 'E1015:') diff --git a/src/testdir/vim9.vim b/src/testdir/vim9.vim index 2e4b03f495..aeeb5a349b 100644 --- a/src/testdir/vim9.vim +++ b/src/testdir/vim9.vim @@ -52,3 +52,10 @@ def CheckDefAndScriptFailure(lines: list, error: string, lnum = -3) 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 diff --git a/src/version.c b/src/version.c index 914e4f0844..58eb3d6b60 100644 --- a/src/version.c +++ b/src/version.c @@ -750,6 +750,8 @@ static char *(features[]) = static int included_patches[] = { /* Add new patch number below this line */ +/**/ + 1798, /**/ 1797, /**/ diff --git a/src/vim9compile.c b/src/vim9compile.c index f7dc9df203..8d362ab1d0 100644 --- a/src/vim9compile.c +++ b/src/vim9compile.c @@ -4212,7 +4212,17 @@ compile_expr1(char_u **arg, cctx_T *cctx, ppconst_T *ppconst) // 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]); + 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; @@ -5637,6 +5647,23 @@ drop_scope(cctx_T *cctx) vim_free(scope); } +/* + * 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" * @@ -5689,8 +5716,14 @@ compile_if(char_u *arg, cctx_T *cctx) 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. - cctx->ctx_skip = tv2bool(&ppconst.pp_tv[0]) ? SKIP_NOT : SKIP_YES; + 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,6 +5732,8 @@ compile_if(char_u *arg, cctx_T *cctx) 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,9 +5799,15 @@ compile_elseif(char_u *arg, cctx_T *cctx) 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? - cctx->ctx_skip = tv2bool(&ppconst.pp_tv[0]) ? SKIP_NOT : SKIP_YES; + 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,6 +5817,8 @@ compile_elseif(char_u *arg, cctx_T *cctx) 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,6 +6080,9 @@ compile_while(char_u *arg, cctx_T *cctx) 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) diff --git a/src/vim9execute.c b/src/vim9execute.c index b1065ca8e8..7afc5c27cd 100644 --- a/src/vim9execute.c +++ b/src/vim9execute.c @@ -1908,6 +1908,7 @@ call_def_function( { 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,7 +3404,7 @@ ex_disassemble(exarg_T *eap) } /* - * Return TRUE when "tv" is not falsey: non-zero, non-empty string, non-empty + * 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. */ -- cgit v1.2.3