patch 8.2.1977: Vim9: error for using a string in a condition is confusing

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 &&.

diff --git a/src/errors.h b/src/errors.h
index eec45dff2abde3241033dbf5908411de39e6b9d3..426164998d959f3491d36033cf944483eb2031d9 100644 (file)
--- a/src/errors.h
+++ b/src/errors.h
@@ -89,8 +89,8 @@ EXTERN char e_compiling_def_function_failed[]
        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_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,4 +292,6 @@ EXTERN char e_cannot_extend_null_dict[]
        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

diff --git a/src/proto/vim9execute.pro b/src/proto/vim9execute.pro
index 2f0ef5454329482b74c0e1dc1812af580b51cedf..882456df80c58d5e7e391a17473b19dcf19b428e 100644 (file)
--- a/src/proto/vim9execute.pro
+++ b/src/proto/vim9execute.pro
@@ -4,5 +4,6 @@ void funcstack_check_refcount(funcstack_T *funcstack);
 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 : */

diff --git a/src/testdir/test_vim9_cmd.vim b/src/testdir/test_vim9_cmd.vim
index 6751483f04c402b2af7764b18cdc2b2fb32fc0f3..0e6e94649e44022c7a5548acb0fca8190c8399c4 100644 (file)
--- a/src/testdir/test_vim9_cmd.vim
+++ b/src/testdir/test_vim9_cmd.vim
@@ -74,7 +74,7 @@ def Test_condition_types()
       if 'text'
       endif
   END
-  CheckDefAndScriptFailure(lines, 'E1030:', 1)
+  CheckDefAndScriptFailure(lines, 'E1135:', 1)
 
   lines =<< trim END
       if [1]
@@ -88,7 +88,7 @@ def Test_condition_types()
       if g:cond
       endif
   END
-  CheckDefExecAndScriptFailure(lines, 'E1030:', 2)
+  CheckDefExecAndScriptFailure(lines, 'E1135:', 2)
 
   lines =<< trim END
       g:cond = 0
@@ -97,7 +97,7 @@ def Test_condition_types()
       endif
   END
   CheckDefFailure(lines, 'E1012:', 3)
-  CheckScriptFailure(['vim9script'] + lines, 'E1030:', 4)
+  CheckScriptFailure(['vim9script'] + lines, 'E1135:', 4)
 
   lines =<< trim END
       if g:cond
@@ -113,14 +113,14 @@ def Test_condition_types()
       elseif g:cond
       endif
   END
-  CheckDefExecAndScriptFailure(lines, 'E1030:', 3)
+  CheckDefExecAndScriptFailure(lines, 'E1135:', 3)
 
   lines =<< trim END
       while 'text'
       endwhile
   END
   CheckDefFailure(lines, 'E1012:', 1)
-  CheckScriptFailure(['vim9script'] + lines, 'E1030:', 2)
+  CheckScriptFailure(['vim9script'] + lines, 'E1135:', 2)
 
   lines =<< trim END
       while [1]
@@ -134,7 +134,7 @@ def Test_condition_types()
       while g:cond
       endwhile
   END
-  CheckDefExecAndScriptFailure(lines, 'E1030:', 2)
+  CheckDefExecAndScriptFailure(lines, 'E1135:', 2)
 enddef
 
 def Test_if_linebreak()

diff --git a/src/testdir/test_vim9_disassemble.vim b/src/testdir/test_vim9_disassemble.vim
index 07974d39ab837744341a0a68a6b3cda9dbfe67bc..37748a1d3f1ca5f1c99953cf0073a9a673482604 100644 (file)
--- a/src/testdir/test_vim9_disassemble.vim
+++ b/src/testdir/test_vim9_disassemble.vim
@@ -707,6 +707,7 @@ def Test_disassemble_const_expr()
             '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,14 +761,15 @@ def Test_disassemble_return_in_if()
   assert_match('ReturnInIf\_s*' ..
         'if g:cond\_s*' ..
         '0 LOADG g:cond\_s*' ..
-        '1 JUMP_IF_FALSE -> 4\_s*' ..
+        '1 COND2BOOL\_s*' ..
+        '2 JUMP_IF_FALSE -> 5\_s*' ..
         'return "yes"\_s*' ..
-        '2 PUSHS "yes"\_s*' ..
-        '3 RETURN\_s*' ..
+        '3 PUSHS "yes"\_s*' ..
+        '4 RETURN\_s*' ..
         'else\_s*' ..
         ' return "no"\_s*' ..
-        '4 PUSHS "no"\_s*' ..
-        '5 RETURN$',
+        '5 PUSHS "no"\_s*' ..
+        '6 RETURN$',
         instr)
 enddef
 
@@ -1357,16 +1359,17 @@ def Test_disassemble_return_bool()
   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*' ..
+        '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',
+        '\d\+ LOAD $0\_s*' ..   
+        '\d\+ RETURN',
         instr)
   assert_equal(true, InvertBool())
 enddef

diff --git a/src/testdir/test_vim9_expr.vim b/src/testdir/test_vim9_expr.vim
index 83db715305a4a979affa9916b1419fcf5b3bb5dd..24d30e25a7cb6d5565ecb5e8236acaf02747b881 100644 (file)
--- a/src/testdir/test_vim9_expr.vim
+++ b/src/testdir/test_vim9_expr.vim
@@ -131,7 +131,7 @@ def Test_expr1_trinary_vimscript()
       vim9script
       var name = 'x' ? 1 : 2
   END
-  CheckScriptFailure(lines, 'E1030:', 2)
+  CheckScriptFailure(lines, 'E1135:', 2)
 
   lines =<< trim END
       vim9script
@@ -180,7 +180,7 @@ 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 = '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,11 +356,12 @@ def Test_expr2_fails()
   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 CheckDefExecFailure(["var x = [] || false"], 'E745:', 1)
   call CheckScriptFailure(["vim9script", "var x = [] || false"], 'E745:', 2)
 enddef
 
@@ -492,6 +493,8 @@ func Test_expr3_fails()
   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

diff --git a/src/typval.c b/src/typval.c
index 5151f724dc03ccdba43b27a49979b952e96ca0b9..7c55d3a5e1ee21fba740d21e1e6ae1810de96c02 100644 (file)
--- a/src/typval.c
+++ b/src/typval.c
@@ -196,7 +196,7 @@ tv_get_bool_or_number_chk(typval_T *varp, int *denote, int want_bool)
        case VAR_STRING:
            if (in_vim9script())
            {
-               emsg(_(e_using_string_as_number));
+               emsg_using_string_as(varp, !want_bool);
                break;
            }
            if (varp->vval.v_string != NULL)

diff --git a/src/version.c b/src/version.c
index 28f95cfbb6eed7bee9647a2be2bfa270081fc649..092801cc1fc9069de88f5bfdbdcca89bff6764fb 100644 (file)
--- 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 */
+/**/
+    1977,
 /**/
     1976,
 /**/

diff --git a/src/vim9compile.c b/src/vim9compile.c
index bda1ab3e47790f0290ac9206a3393f66e9a034db..9a38f056a425712d46314421efd6eb5bdfd2442f 100644 (file)
--- a/src/vim9compile.c
+++ b/src/vim9compile.c
@@ -879,6 +879,28 @@ need_type(
     return FAIL;
 }
 
+/*
+ * 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.
  */
@@ -4306,8 +4328,6 @@ compile_and_or(
     {
        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 "&&"
@@ -4331,8 +4351,12 @@ compile_and_or(
            // evaluating to bool
            generate_ppconst(cctx, ppconst);
 
-           if (((type_T **)stack->ga_data)[stack->ga_len - 1] != &t_bool)
-               all_bool_values = FALSE;
+           // 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,6 +4384,13 @@ compile_and_or(
        }
        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,10 +4402,6 @@ compile_and_or(
            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;
@@ -4385,11 +4412,11 @@ compile_and_or(
  *
  * 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"
- *     COND2BOOL
  * end:
  */
     static int
@@ -4410,11 +4437,11 @@ compile_expr3(char_u **arg, cctx_T *cctx, ppconst_T *ppconst)
  *
  * 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"
- *     COND2BOOL
  * end:
  */
     static int
@@ -5967,23 +5994,6 @@ 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, FALSE) == FAIL)
-       return FAIL;
-    return OK;
-}
-
 /*
  * compile "if expr"
  *

diff --git a/src/vim9execute.c b/src/vim9execute.c
index c9ae709d63d0567af908e52edba86d2b4c0a3b2f..d8f9cfe02dfb6eea68326148b56426ee9eca935c 100644 (file)
--- a/src/vim9execute.c
+++ b/src/vim9execute.c
@@ -3630,6 +3630,15 @@ tv2bool(typval_T *tv)
     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,7 +3647,7 @@ check_not_string(typval_T *tv)
 {
     if (tv->v_type == VAR_STRING)
     {
-       emsg(_(e_using_string_as_number));
+       emsg_using_string_as(tv, TRUE);
        clear_tv(tv);
        return FAIL;
     }