patch 8.2.2651: Vim9: restoring command modifiers happens after jump

Problem:    Vim9: restoring command modifiers happens after jump.
Solution:   Move the restore instruction to before the jump. (closes #8006)
            Also handle for and while.

diff --git a/src/testdir/test_vim9_disassemble.vim b/src/testdir/test_vim9_disassemble.vim
index 7470ac3ce7b4f44cca548efd261401df56ea2c1c..5d3e4faa69f4b484cee228946655fc90804e681c 100644 (file)
--- a/src/testdir/test_vim9_disassemble.vim
+++ b/src/testdir/test_vim9_disassemble.vim
@@ -1896,7 +1896,95 @@ def Test_silent()
         '\d PUSHS "error"\_s*' ..
         '\d ECHOERR 1\_s*' ..
         '\d CMDMOD_REV\_s*' ..
-        '\d RETURN 0',
+        '\d\+ RETURN 0',
+        res)
+enddef
+
+def s:SilentIf()
+  silent if 4 == g:five
+  silent elseif 4 == g:five
+  silent endif
+enddef
+
+def Test_silent_if()
+  var res = execute('disass s:SilentIf')
+  assert_match('<SNR>\d*_SilentIf\_s*' ..
+        'silent if 4 == g:five\_s*' ..
+        '\d\+ CMDMOD silent\_s*' ..
+        '\d\+ PUSHNR 4\_s*' ..
+        '\d\+ LOADG g:five\_s*' ..
+        '\d\+ COMPAREANY ==\_s*' ..
+        '\d\+ CMDMOD_REV\_s*' ..
+        '\d\+ JUMP_IF_FALSE -> \d\+\_s*' ..
+        'silent elseif 4 == g:five\_s*' ..
+        '\d\+ JUMP -> \d\+\_s*' ..
+        '\d\+ CMDMOD silent\_s*' ..
+        '\d\+ PUSHNR 4\_s*' ..
+        '\d\+ LOADG g:five\_s*' ..
+        '\d\+ COMPAREANY ==\_s*' ..
+        '\d\+ CMDMOD_REV\_s*' ..
+        '\d\+ JUMP_IF_FALSE -> \d\+\_s*' ..
+        'silent endif\_s*' ..
+        '\d\+ RETURN 0',
+        res)
+enddef
+
+def s:SilentFor()
+  silent for i in [0]
+  silent endfor
+enddef
+
+def Test_silent_for()
+  var res = execute('disass s:SilentFor')
+  assert_match('<SNR>\d*_SilentFor\_s*' ..
+        'silent for i in \[0\]\_s*' ..
+        '\d CMDMOD silent\_s*' ..
+        '\d STORE -1 in $0\_s*' ..
+        '\d PUSHNR 0\_s*' ..
+        '\d NEWLIST size 1\_s*' ..
+        '\d CMDMOD_REV\_s*' ..
+        '5 FOR $0 -> 8\_s*' ..
+        '\d STORE $1\_s*' ..
+        'silent endfor\_s*' ..
+        '\d JUMP -> 5\_s*' ..
+        '8 DROP\_s*' ..
+        '\d RETURN 0\_s*',
+        res)
+enddef
+
+def s:SilentWhile()
+  silent while g:not
+  silent endwhile
+enddef
+
+def Test_silent_while()
+  var res = execute('disass s:SilentWhile')
+  assert_match('<SNR>\d*_SilentWhile\_s*' ..
+        'silent while g:not\_s*' ..
+        '0 CMDMOD silent\_s*' ..
+        '\d LOADG g:not\_s*' ..
+        '\d COND2BOOL\_s*' ..
+        '\d CMDMOD_REV\_s*' ..
+        '\d JUMP_IF_FALSE -> 6\_s*' ..
+
+        'silent endwhile\_s*' ..
+        '\d JUMP -> 0\_s*' ..
+        '6 RETURN 0\_s*',
+         res)
+enddef
+
+def s:SilentReturn(): string
+  silent return "done"
+enddef
+
+def Test_silent_return()
+  var res = execute('disass s:SilentReturn')
+  assert_match('<SNR>\d*_SilentReturn\_s*' ..
+        'silent return "done"\_s*' ..
+        '\d CMDMOD silent\_s*' ..
+        '\d PUSHS "done"\_s*' ..
+        '\d CMDMOD_REV\_s*' ..
+        '\d RETURN',
         res)
 enddef
 
@@ -1924,19 +2012,5 @@ def Test_profiled()
         res)
 enddef
 
-def s:SilentReturn(): string
-  silent return "done"
-enddef
-
-def Test_silent_return()
-  var res = execute('disass s:SilentReturn')
-  assert_match('<SNR>\d*_SilentReturn\_s*' ..
-        'silent return "done"\_s*' ..
-        '\d CMDMOD silent\_s*' ..
-        '\d PUSHS "done"\_s*' ..
-        '\d CMDMOD_REV\_s*' ..
-        '\d RETURN',
-        res)
-enddef
 
 " vim: ts=8 sw=2 sts=2 expandtab tw=80 fdm=marker

diff --git a/src/version.c b/src/version.c
index 594f88febe75cbd371d2740732fb2130b8a8f6f7..9e80bd42f2770fd657f9869a2fc9599feeae0461 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 */
+/**/
+    2651,
 /**/
     2650,
 /**/

diff --git a/src/vim9compile.c b/src/vim9compile.c
index 535de057c62dc937652f5c789a2de7ddcdba4c93..41db5e669147bdb07ea8569d68b9be840090af48 100644 (file)
--- a/src/vim9compile.c
+++ b/src/vim9compile.c
@@ -2172,6 +2172,45 @@ generate_undo_cmdmods(cctx_T *cctx)
     return OK;
 }
 
+/*
+ * If an ISN_CMDMOD was just generated drop it.
+ */
+    static void
+drop_cmdmod(cctx_T *cctx)
+{
+    garray_T   *instr = &cctx->ctx_instr;
+
+    // Drop any CMDMOD instruction
+    if (cctx->ctx_has_cmdmod
+           && ((isn_T *)instr->ga_data)[instr->ga_len - 1].isn_type
+                                                                == ISN_CMDMOD)
+    {
+       --instr->ga_len;
+       cctx->ctx_has_cmdmod = FALSE;
+    }
+}
+
+/*
+ * Get the index of the current instruction.
+ * This compenstates for a preceding ISN_CMDMOD and ISN_PROF_START.
+ */
+    static int
+current_instr_idx(cctx_T *cctx)
+{
+    garray_T   *instr = &cctx->ctx_instr;
+    int                idx = instr->ga_len;
+
+    if (cctx->ctx_has_cmdmod && ((isn_T *)instr->ga_data)[idx - 1]
+                                                      .isn_type == ISN_CMDMOD)
+       --idx;
+#ifdef FEAT_PROFILE
+    if (cctx->ctx_profiling && ((isn_T *)instr->ga_data)[idx - 1]
+                                                  .isn_type == ISN_PROF_START)
+       --idx;
+#endif
+    return idx;
+}
+
 #ifdef FEAT_PROFILE
     static void
 may_generate_prof_end(cctx_T *cctx, int prof_lnum)
@@ -6877,6 +6916,9 @@ compile_if(char_u *arg, cctx_T *cctx)
            return NULL;
     }
 
+    // CMDMOD_REV must come before the jump
+    generate_undo_cmdmods(cctx);
+
     scope = new_scope(cctx, IF_SCOPE);
     if (scope == NULL)
        return NULL;
@@ -6937,24 +6979,36 @@ compile_elseif(char_u *arg, cctx_T *cctx)
     if (scope->se_u.se_if.is_seen_skip_not)
     {
        // A previous block was executed, skip over expression and bail out.
-       // Do not count the "elseif" for profiling.
-#ifdef FEAT_PROFILE
-       if (cctx->ctx_profiling && ((isn_T *)instr->ga_data)[instr->ga_len - 1]
-                                                  .isn_type == ISN_PROF_START)
-           --instr->ga_len;
-#endif
+       // Do not count the "elseif" for profiling and cmdmod
+       instr->ga_len = current_instr_idx(cctx);
+
        skip_expr_cctx(&p, cctx);
        return p;
     }
 
     if (cctx->ctx_skip == SKIP_UNKNOWN)
     {
+       int moved_cmdmod = FALSE;
+
+       // Move any CMDMOD instruction to after the jump
+       if (((isn_T *)instr->ga_data)[instr->ga_len - 1].isn_type == ISN_CMDMOD)
+       {
+           if (ga_grow(instr, 1) == FAIL)
+               return NULL;
+           ((isn_T *)instr->ga_data)[instr->ga_len] =
+                                 ((isn_T *)instr->ga_data)[instr->ga_len - 1];
+           --instr->ga_len;
+           moved_cmdmod = TRUE;
+       }
+
        if (compile_jump_to_end(&scope->se_u.se_if.is_end_label,
                                                    JUMP_ALWAYS, cctx) == FAIL)
            return NULL;
        // previous "if" or "elseif" jumps here
        isn = ((isn_T *)instr->ga_data) + scope->se_u.se_if.is_if_label;
        isn->isn_arg.jump.jump_where = instr->ga_len;
+       if (moved_cmdmod)
+           ++instr->ga_len;
     }
 
     // compile "expr"; if we know it evaluates to FALSE skip the block
@@ -7007,6 +7061,9 @@ compile_elseif(char_u *arg, cctx_T *cctx)
        if (bool_on_stack(cctx) == FAIL)
            return NULL;
 
+       // CMDMOD_REV must come before the jump
+       generate_undo_cmdmods(cctx);
+
        // "where" is set when ":elseif", "else" or ":endif" is found
        scope->se_u.se_if.is_if_label = instr->ga_len;
        generate_JUMP(cctx, JUMP_IF_FALSE, 0);
@@ -7090,6 +7147,7 @@ compile_endif(char_u *arg, cctx_T *cctx)
     garray_T   *instr = &cctx->ctx_instr;
     isn_T      *isn;
 
+    drop_cmdmod(cctx);
     if (scope == NULL || scope->se_type != IF_SCOPE)
     {
        emsg(_(e_endif_without_if));
@@ -7160,7 +7218,6 @@ compile_for(char_u *arg_start, cctx_T *cctx)
     int                var_count = 0;
     int                semicolon = FALSE;
     size_t     varlen;
-    garray_T   *instr = &cctx->ctx_instr;
     garray_T   *stack = &cctx->ctx_type_stack;
     scope_T    *scope;
     lvar_T     *loop_lvar;     // loop iteration variable
@@ -7230,8 +7287,11 @@ compile_for(char_u *arg_start, cctx_T *cctx)
            item_type = vartype->tt_member->tt_member;
     }
 
+    // CMDMOD_REV must come before the FOR instruction
+    generate_undo_cmdmods(cctx);
+
     // "for_end" is set when ":endfor" is found
-    scope->se_u.se_for.fs_top_label = instr->ga_len;
+    scope->se_u.se_for.fs_top_label = current_instr_idx(cctx);
     generate_FOR(cctx, loop_lvar->lv_idx);
 
     arg = arg_start;
@@ -7333,6 +7393,8 @@ compile_endfor(char_u *arg, cctx_T *cctx)
     forscope_T *forscope;
     isn_T      *isn;
 
+    drop_cmdmod(cctx);
+
     if (scope == NULL || scope->se_type != FOR_SCOPE)
     {
        emsg(_(e_for));
@@ -7376,20 +7438,14 @@ compile_endfor(char_u *arg, cctx_T *cctx)
 compile_while(char_u *arg, cctx_T *cctx)
 {
     char_u     *p = arg;
-    garray_T   *instr = &cctx->ctx_instr;
     scope_T    *scope;
 
     scope = new_scope(cctx, WHILE_SCOPE);
     if (scope == NULL)
        return NULL;
 
-    // "endwhile" jumps back here, one before when profiling
-    scope->se_u.se_while.ws_top_label = instr->ga_len;
-#ifdef FEAT_PROFILE
-    if (cctx->ctx_profiling && ((isn_T *)instr->ga_data)[instr->ga_len - 1]
-                                                  .isn_type == ISN_PROF_START)
-       --scope->se_u.se_while.ws_top_label;
-#endif
+    // "endwhile" jumps back here, one before when profiling or using cmdmods
+    scope->se_u.se_while.ws_top_label = current_instr_idx(cctx);
 
     // compile "expr"
     if (compile_expr0(&p, cctx) == FAIL)
@@ -7403,6 +7459,9 @@ compile_while(char_u *arg, cctx_T *cctx)
     if (bool_on_stack(cctx) == FAIL)
        return FAIL;
 
+    // CMDMOD_REV must come before the jump
+    generate_undo_cmdmods(cctx);
+
     // "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)
@@ -7420,6 +7479,7 @@ compile_endwhile(char_u *arg, cctx_T *cctx)
     scope_T    *scope = cctx->ctx_scope;
     garray_T   *instr = &cctx->ctx_instr;
 
+    drop_cmdmod(cctx);
     if (scope == NULL || scope->se_type != WHILE_SCOPE)
     {
        emsg(_(e_while));

diff --git a/src/vim9execute.c b/src/vim9execute.c
index 52c4193c6a55e9944e0c36411a395286599d2912..1ae17d97bfc4d3f8ae4d8a8ed5aa76523a687de8 100644 (file)
--- a/src/vim9execute.c
+++ b/src/vim9execute.c
@@ -795,6 +795,21 @@ call_ufunc(
     return OK;
 }
 
+/*
+ * If command modifiers were applied restore them.
+ */
+    static void
+may_restore_cmdmod(funclocal_T *funclocal)
+{
+    if (funclocal->floc_restore_cmdmod)
+    {
+       cmdmod.cmod_filter_regmatch.regprog = NULL;
+       undo_cmdmod(&cmdmod);
+       cmdmod = funclocal->floc_save_cmdmod;
+       funclocal->floc_restore_cmdmod = FALSE;
+    }
+}
+
 /*
  * Return TRUE if an error was given or CTRL-C was pressed.
  */
@@ -2719,8 +2734,11 @@ call_def_function(
                        goto failed;
                    ++idxtv->vval.v_number;
                    if (list == NULL || idxtv->vval.v_number >= list->lv_len)
+                   {
                        // past the end of the list, jump to "endfor"
                        ectx.ec_iidx = iptr->isn_arg.forloop.for_end;
+                       may_restore_cmdmod(&funclocal);
+                   }
                    else if (list->lv_first == &range_list_item)
                    {
                        // non-materialized range() list
@@ -2755,9 +2773,12 @@ call_def_function(
                    CLEAR_POINTER(trycmd);
                    trycmd->tcd_frame_idx = ectx.ec_frame_idx;
                    trycmd->tcd_stack_len = ectx.ec_stack.ga_len;
-                   trycmd->tcd_catch_idx = iptr->isn_arg.try.try_ref->try_catch;
-                   trycmd->tcd_finally_idx = iptr->isn_arg.try.try_ref->try_finally;
-                   trycmd->tcd_endtry_idx = iptr->isn_arg.try.try_ref->try_endtry;
+                   trycmd->tcd_catch_idx =
+                                         iptr->isn_arg.try.try_ref->try_catch;
+                   trycmd->tcd_finally_idx =
+                                       iptr->isn_arg.try.try_ref->try_finally;
+                   trycmd->tcd_endtry_idx =
+                                        iptr->isn_arg.try.try_ref->try_endtry;
                }
                break;
 
@@ -2782,13 +2803,7 @@ call_def_function(
                {
                    garray_T    *trystack = &ectx.ec_trystack;
 
-                   if (funclocal.floc_restore_cmdmod)
-                   {
-                       cmdmod.cmod_filter_regmatch.regprog = NULL;
-                       undo_cmdmod(&cmdmod);
-                       cmdmod = funclocal.floc_save_cmdmod;
-                       funclocal.floc_restore_cmdmod = FALSE;
-                   }
+                   may_restore_cmdmod(&funclocal);
                    if (trystack->ga_len > 0)
                    {
                        trycmd_T    *trycmd = ((trycmd_T *)trystack->ga_data)