patch 8.2.3442: Vim9: || and && are not handled at compile time

Problem:    Vim9: || and && are not handled at compile time when possible.
Solution:   When using constants generate fewer instructions.

diff --git a/src/testdir/test_vim9_disassemble.vim b/src/testdir/test_vim9_disassemble.vim
index 840017418cace1e1d5e3ece78dc82ff9c6ac06fc..ba2d0a16ece045bf3e0f03aaa2495584f9526ca0 100644 (file)
--- a/src/testdir/test_vim9_disassemble.vim
+++ b/src/testdir/test_vim9_disassemble.vim
@@ -1218,6 +1218,38 @@ def Test_disassemble_and_or()
         instr)
 enddef
 
+def AndConstant(arg: any): string
+  if true && arg
+    return "yes"
+  endif
+  if false && arg
+    return "never"
+  endif
+  return "no"
+enddef
+
+def Test_disassemble_and_constant()
+  assert_equal("yes", AndConstant(1))
+  assert_equal("no", AndConstant(false))
+  var instr = execute('disassemble AndConstant')
+  assert_match('AndConstant\_s*' ..
+      'if true && arg\_s*' ..
+      '0 LOAD arg\[-1\]\_s*' ..
+      '1 COND2BOOL\_s*' ..
+      '2 JUMP_IF_FALSE -> 5\_s*' ..
+      'return "yes"\_s*' ..
+      '3 PUSHS "yes"\_s*' ..
+      '4 RETURN\_s*' ..
+      'endif\_s*' ..
+      'if false && arg\_s*' ..
+      'return "never"\_s*' ..
+      'endif\_s*' ..
+      'return "no"\_s*' ..
+      '5 PUSHS "no"\_s*' ..
+      '6 RETURN',
+      instr)
+enddef
+
 def ForLoop(): list<number>
   var res: list<number>
   for i in range(3)
@@ -1734,25 +1766,31 @@ def Test_disassemble_invert_bool()
 enddef
 
 def ReturnBool(): bool
-  var name: bool = 1 && 0 || 1
+  var one = 1
+  var zero = 0
+  var name: bool = one && zero || one
   return name
 enddef
 
 def Test_disassemble_return_bool()
   var instr = execute('disassemble ReturnBool')
   assert_match('ReturnBool\_s*' ..
-        'var name: bool = 1 && 0 || 1\_s*' ..
-        '0 PUSHNR 1\_s*' ..
-        '1 COND2BOOL\_s*' ..
-        '2 JUMP_IF_COND_FALSE -> 5\_s*' ..
-        '3 PUSHNR 0\_s*' ..
-        '4 COND2BOOL\_s*' ..
-        '5 JUMP_IF_COND_TRUE -> 8\_s*' ..
-        '6 PUSHNR 1\_s*' ..
-        '7 COND2BOOL\_s*' ..
-        '\d STORE $0\_s*' ..
+        'var one = 1\_s*' ..
+        '0 STORE 1 in $0\_s*' ..
+        'var zero = 0\_s*' ..
+        '1 STORE 0 in $1\_s*' ..
+        'var name: bool = one && zero || one\_s*' ..
+        '2 LOAD $0\_s*' ..
+        '3 COND2BOOL\_s*' ..
+        '4 JUMP_IF_COND_FALSE -> 7\_s*' ..
+        '5 LOAD $1\_s*' ..
+        '6 COND2BOOL\_s*' ..
+        '7 JUMP_IF_COND_TRUE -> 10\_s*' ..
+        '8 LOAD $0\_s*' ..
+        '9 COND2BOOL\_s*' ..
+        '10 STORE $2\_s*' ..
         'return name\_s*' ..
-        '\d\+ LOAD $0\_s*' ..   
+        '\d\+ LOAD $2\_s*' ..   
         '\d\+ RETURN',
         instr)
   assert_equal(true, InvertBool())

diff --git a/src/version.c b/src/version.c
index 7c8a254fea5b9a6e188d10ee18a9ac331de08dd9..ba2b475ca9f7198161e1f7e90b8927512aba3d65 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -755,6 +755,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    3442,
 /**/
     3441,
 /**/

diff --git a/src/vim9.h b/src/vim9.h
index 66fc6cc39129480bfff5bfdd97a5a94676b54d58..cffa71b381689c5a98457952aa906ed22d0d8e15 100644 (file)
--- a/src/vim9.h
+++ b/src/vim9.h
@@ -221,6 +221,7 @@ typedef struct {
 
 typedef enum {
     JUMP_ALWAYS,
+    JUMP_NEVER,
     JUMP_IF_FALSE,             // pop and jump if false
     JUMP_AND_KEEP_IF_TRUE,     // jump if top of stack is truthy, drop if not
     JUMP_AND_KEEP_IF_FALSE,    // jump if top of stack is falsy, drop if not

diff --git a/src/vim9compile.c b/src/vim9compile.c
index 4807e3aa096cbf9a09c1b0a90f401ed251569b98..0d64f0134d279403bc586f8ea586d31cc6885b16 100644 (file)
--- a/src/vim9compile.c
+++ b/src/vim9compile.c
@@ -2847,7 +2847,7 @@ generate_ppconst(cctx_T *cctx, ppconst_T *ppconst)
 }
 
 /*
- * Check that the last item of "ppconst" is a bool.
+ * Check that the last item of "ppconst" is a bool, if there is an item.
  */
     static int
 check_ppconst_bool(ppconst_T *ppconst)
@@ -4845,7 +4845,8 @@ compile_expr7(
        }
        else
        {
-           if (generate_ppconst(cctx, ppconst) == FAIL)
+           if (cctx->ctx_skip != SKIP_YES
+                                   && generate_ppconst(cctx, ppconst) == FAIL)
                return FAIL;
            r = compile_load(arg, p, cctx, TRUE, TRUE);
        }
@@ -5240,6 +5241,7 @@ compile_and_or(
     {
        garray_T        *instr = &cctx->ctx_instr;
        garray_T        end_ga;
+       int             save_skip = cctx->ctx_skip;
 
        /*
         * Repeat until there is no following "||" or "&&"
@@ -5251,7 +5253,10 @@ compile_and_or(
            long        save_sourcing_lnum;
            int         start_ctx_lnum = cctx->ctx_lnum;
            int         save_lnum;
+           int         const_used;
            int         status;
+           jumpwhen_T  jump_when = opchar == '|'
+                                     ? JUMP_IF_COND_TRUE : JUMP_IF_COND_FALSE;
 
            if (next != NULL)
            {
@@ -5274,14 +5279,38 @@ compile_and_or(
            status = check_ppconst_bool(ppconst);
            if (status != FAIL)
            {
-               // TODO: use ppconst if the value is a constant
-               generate_ppconst(cctx, ppconst);
+               // Use the last ppconst if possible.
+               if (ppconst->pp_used > 0)
+               {
+                   typval_T    *tv = &ppconst->pp_tv[ppconst->pp_used - 1];
+                   int         is_true = tv2bool(tv);
 
-               // Every part must evaluate to a bool.
-               status = bool_on_stack(cctx);
-               if (status != FAIL)
-                   status = ga_grow(&end_ga, 1);
+                   if ((is_true && opchar == '|')
+                                               || (!is_true && opchar == '&'))
+                   {
+                       // For "false && expr" and "true || expr" the "expr"
+                       // does not need to be evaluated.
+                       cctx->ctx_skip = SKIP_YES;
+                       clear_tv(tv);
+                       tv->v_type = VAR_BOOL;
+                       tv->vval.v_number = is_true ? VVAL_TRUE : VVAL_FALSE;
+                   }
+                   else
+                   {
+                       // For "true && expr" and "false || expr" only "expr"
+                       // needs to be evaluated.
+                       --ppconst->pp_used;
+                       jump_when = JUMP_NEVER;
+                   }
+               }
+               else
+               {
+                   // Every part must evaluate to a bool.
+                   status = bool_on_stack(cctx);
+               }
            }
+           if (status != FAIL)
+               status = ga_grow(&end_ga, 1);
            cctx->ctx_lnum = save_lnum;
            if (status == FAIL)
            {
@@ -5289,10 +5318,15 @@ compile_and_or(
                return FAIL;
            }
 
-           *(((int *)end_ga.ga_data) + end_ga.ga_len) = instr->ga_len;
-           ++end_ga.ga_len;
-           generate_JUMP(cctx, opchar == '|'
-                                ?  JUMP_IF_COND_TRUE : JUMP_IF_COND_FALSE, 0);
+           if (jump_when != JUMP_NEVER)
+           {
+               if (cctx->ctx_skip != SKIP_YES)
+               {
+                   *(((int *)end_ga.ga_data) + end_ga.ga_len) = instr->ga_len;
+                   ++end_ga.ga_len;
+               }
+               generate_JUMP(cctx, jump_when, 0);
+           }
 
            // eval the next expression
            SOURCING_LNUM = save_sourcing_lnum;
@@ -5302,6 +5336,7 @@ compile_and_or(
                return FAIL;
            }
 
+           const_used = ppconst->pp_used;
            if ((opchar == '|' ? compile_expr3(arg, cctx, ppconst)
                                  : compile_expr4(arg, cctx, ppconst)) == FAIL)
            {
@@ -5309,6 +5344,20 @@ compile_and_or(
                return FAIL;
            }
 
+           // "0 || 1" results in true, "1 && 0" results in false.
+           if (ppconst->pp_used == const_used + 1)
+           {
+               typval_T        *tv = &ppconst->pp_tv[ppconst->pp_used - 1];
+
+               if (tv->v_type == VAR_NUMBER
+                        && (tv->vval.v_number == 1 || tv->vval.v_number == 0))
+               {
+                   tv->vval.v_number = tv->vval.v_number == 1
+                                                     ? VVAL_TRUE : VVAL_FALSE;
+                   tv->v_type = VAR_BOOL;
+               }
+           }
+
            p = may_peek_next_line(cctx, *arg, &next);
        }
 
@@ -5317,26 +5366,32 @@ compile_and_or(
            ga_clear(&end_ga);
            return FAIL;
        }
-       generate_ppconst(cctx, ppconst);
 
-       // Every part must evaluate to a bool.
-       if (bool_on_stack(cctx) == FAIL)
-       {
-           ga_clear(&end_ga);
-           return FAIL;
-       }
+       if (cctx->ctx_skip != SKIP_YES && ppconst->pp_used == 0)
+           // 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)
+       if (end_ga.ga_len > 0)
        {
-           isn_T       *isn;
+           // Fill in the end label in all jumps.
+           generate_ppconst(cctx, ppconst);
+           while (end_ga.ga_len > 0)
+           {
+               isn_T   *isn;
 
-           --end_ga.ga_len;
-           isn = ((isn_T *)instr->ga_data)
+               --end_ga.ga_len;
+               isn = ((isn_T *)instr->ga_data)
                                  + *(((int *)end_ga.ga_data) + end_ga.ga_len);
-           isn->isn_arg.jump.jump_where = instr->ga_len;
+               isn->isn_arg.jump.jump_where = instr->ga_len;
+           }
+           ga_clear(&end_ga);
        }
-       ga_clear(&end_ga);
+
+       cctx->ctx_skip = save_skip;
     }
 
     return OK;

diff --git a/src/vim9execute.c b/src/vim9execute.c
index 0fa1a12f4cf2d7b0214661219989f0e098a166a3..481c94fde64dbb4f6b176e2210d8db70f0bbe3a1 100644 (file)
--- a/src/vim9execute.c
+++ b/src/vim9execute.c
@@ -5487,6 +5487,9 @@ list_instructions(char *pfx, isn_T *instr, int instr_count, ufunc_T *ufunc)
                        case JUMP_ALWAYS:
                            when = "JUMP";
                            break;
+                       case JUMP_NEVER:
+                           iemsg("JUMP_NEVER should not be used");
+                           break;
                        case JUMP_AND_KEEP_IF_TRUE:
                            when = "JUMP_AND_KEEP_IF_TRUE";
                            break;