changeset 23723:14e92f4c98c9 v8.2.2403

patch 8.2.2403: Vim9: profiling if/elseif/endif not correct Commit: https://github.com/vim/vim/commit/ced68a0070dac059fc978a1a99e2893edf158116 Author: Bram Moolenaar <Bram@vim.org> Date: Sun Jan 24 17:53:47 2021 +0100 patch 8.2.2403: Vim9: profiling if/elseif/endif not correct Problem: Vim9: profiling if/elseif/endif not correct. Solution: Add profile instructions. Fix that "elseif" was wrong.
author Bram Moolenaar <Bram@vim.org>
date Sun, 24 Jan 2021 18:00:04 +0100
parents a889925d20fd
children 85e6fa8b86f7
files src/testdir/test_profile.vim src/testdir/test_vim9_disassemble.vim src/testdir/test_vim9_script.vim src/version.c src/vim9compile.c
diffstat 5 files changed, 120 insertions(+), 40 deletions(-) [+]
line wrap: on
line diff
--- a/src/testdir/test_profile.vim
+++ b/src/testdir/test_profile.vim
@@ -100,39 +100,48 @@ func RunProfileFunc(command, declare, as
 endfunc
 
 func Test_profile_func_with_ifelse()
+  call Run_profile_func_with_ifelse('func', 'let', 'let')
+  call Run_profile_func_with_ifelse('def', 'var', '')
+endfunc
+
+func Run_profile_func_with_ifelse(command, declare, assign)
   let lines =<< trim [CODE]
-    func! Foo1()
+    XXX Foo1()
       if 1
-        let x = 0
-      elseif 1
-        let x = 1
-      else
-        let x = 2
-      endif
-    endfunc
-    func! Foo2()
-      if 0
-        let x = 0
+        DDD x = 0
       elseif 1
-        let x = 1
+        DDD x = 1
       else
-        let x = 2
+        DDD x = 2
       endif
-    endfunc
-    func! Foo3()
+    endXXX
+    XXX Foo2()
       if 0
-        let x = 0
-      elseif 0
-        let x = 1
+        DDD x = 0
+      elseif 1
+        DDD x = 1
       else
-        let x = 2
+        DDD x = 2
       endif
-    endfunc
+    endXXX
+    XXX Foo3()
+      if 0
+        DDD x = 0
+      elseif 0
+        DDD x = 1
+      else
+        DDD x = 2
+      endif
+    endXXX
     call Foo1()
     call Foo2()
     call Foo3()
   [CODE]
 
+  call map(lines, {k, v -> substitute(v, 'XXX', a:command, '') })
+  call map(lines, {k, v -> substitute(v, 'DDD', a:declare, '') })
+  call map(lines, {k, v -> substitute(v, 'AAA', a:assign, '') })
+
   call writefile(lines, 'Xprofile_func.vim')
   call system(GetVimCommand()
     \ . ' -es -i NONE --noplugin'
@@ -157,11 +166,11 @@ func Test_profile_func_with_ifelse()
   call assert_equal('',                                            lines[5])
   call assert_equal('count  total (s)   self (s)',                 lines[6])
   call assert_match('^\s*1\s\+.*\sif 1$',                          lines[7])
-  call assert_match('^\s*1\s\+.*\s  let x = 0$',                   lines[8])
+  call assert_match('^\s*1\s\+.*\s  \(let\|var\) x = 0$',          lines[8])
   call assert_match(        '^\s\+elseif 1$',                      lines[9])
-  call assert_match(          '^\s\+let x = 1$',                   lines[10])
+  call assert_match(          '^\s\+\(let\|var\) x = 1$',          lines[10])
   call assert_match(        '^\s\+else$',                          lines[11])
-  call assert_match(          '^\s\+let x = 2$',                   lines[12])
+  call assert_match(          '^\s\+\(let\|var\) x = 2$',          lines[12])
   call assert_match('^\s*1\s\+.*\sendif$',                         lines[13])
   call assert_equal('',                                            lines[14])
   call assert_equal('FUNCTION  Foo2()',                            lines[15])
@@ -171,11 +180,11 @@ func Test_profile_func_with_ifelse()
   call assert_equal('',                                            lines[20])
   call assert_equal('count  total (s)   self (s)',                 lines[21])
   call assert_match('^\s*1\s\+.*\sif 0$',                          lines[22])
-  call assert_match(          '^\s\+let x = 0$',                   lines[23])
+  call assert_match(          '^\s\+\(let\|var\) x = 0$',          lines[23])
   call assert_match('^\s*1\s\+.*\selseif 1$',                      lines[24])
-  call assert_match('^\s*1\s\+.*\s  let x = 1$',                   lines[25])
+  call assert_match('^\s*1\s\+.*\s  \(let\|var\) x = 1$',          lines[25])
   call assert_match(        '^\s\+else$',                          lines[26])
-  call assert_match(          '^\s\+let x = 2$',                   lines[27])
+  call assert_match(          '^\s\+\(let\|var\) x = 2$',          lines[27])
   call assert_match('^\s*1\s\+.*\sendif$',                         lines[28])
   call assert_equal('',                                            lines[29])
   call assert_equal('FUNCTION  Foo3()',                            lines[30])
@@ -185,11 +194,11 @@ func Test_profile_func_with_ifelse()
   call assert_equal('',                                            lines[35])
   call assert_equal('count  total (s)   self (s)',                 lines[36])
   call assert_match('^\s*1\s\+.*\sif 0$',                          lines[37])
-  call assert_match(          '^\s\+let x = 0$',                   lines[38])
+  call assert_match(          '^\s\+\(let\|var\) x = 0$',          lines[38])
   call assert_match('^\s*1\s\+.*\selseif 0$',                      lines[39])
-  call assert_match(          '^\s\+let x = 1$',                   lines[40])
+  call assert_match(          '^\s\+\(let\|var\) x = 1$',          lines[40])
   call assert_match('^\s*1\s\+.*\selse$',                          lines[41])
-  call assert_match('^\s*1\s\+.*\s  let x = 2$',                   lines[42])
+  call assert_match('^\s*1\s\+.*\s  \(let\|var\) x = 2$',          lines[42])
   call assert_match('^\s*1\s\+.*\sendif$',                         lines[43])
   call assert_equal('',                                            lines[44])
   call assert_equal('FUNCTIONS SORTED ON TOTAL TIME',              lines[45])
--- a/src/testdir/test_vim9_disassemble.vim
+++ b/src/testdir/test_vim9_disassemble.vim
@@ -1857,8 +1857,8 @@ def Test_profiled()
         '\d PROFILE START line 1\_s*' ..
         '\d PUSHS "profiled"\_s*' ..
         '\d ECHO 1\_s*' ..
+        'return "done"\_s*' ..
         '\d PROFILE END\_s*' ..
-        'return "done"\_s*' ..
         '\d PROFILE START line 2\_s*' ..
         '\d PUSHS "done"\_s*' ..
         '\d RETURN\_s*' ..
--- a/src/testdir/test_vim9_script.vim
+++ b/src/testdir/test_vim9_script.vim
@@ -1741,7 +1741,7 @@ def Test_if_elseif_else_fails()
   CheckDefFailure(['elseif true'], 'E582:')
   CheckDefFailure(['else'], 'E581:')
   CheckDefFailure(['endif'], 'E580:')
-  CheckDefFailure(['if true', 'elseif xxx'], 'E1001:')
+  CheckDefFailure(['if g:abool', 'elseif xxx'], 'E1001:')
   CheckDefFailure(['if true', 'echo 1'], 'E171:')
 enddef
 
--- a/src/version.c
+++ b/src/version.c
@@ -751,6 +751,8 @@ static char *(features[]) =
 static int included_patches[] =
 {   /* Add new patch number below this line */
 /**/
+    2403,
+/**/
     2402,
 /**/
     2401,
--- a/src/vim9compile.c
+++ b/src/vim9compile.c
@@ -44,6 +44,7 @@ struct endlabel_S {
  */
 typedef struct {
     int		is_seen_else;
+    int		is_seen_skip_not;   // a block was unconditionally executed
     int		is_had_return;	    // every block ends in :return
     int		is_if_label;	    // instruction idx at IF or ELSEIF
     endlabel_T	*is_end_label;	    // instructions to set end label
@@ -2098,13 +2099,7 @@ generate_undo_cmdmods(cctx_T *cctx)
 may_generate_prof_end(cctx_T *cctx, int prof_lnum)
 {
     if (cctx->ctx_profiling && prof_lnum >= 0)
-    {
-	int save_lnum = cctx->ctx_lnum;
-
-	cctx->ctx_lnum = prof_lnum;
 	generate_instr(cctx, ISN_PROF_END);
-	cctx->ctx_lnum = save_lnum;
-    }
 }
 #endif
 
@@ -6735,6 +6730,18 @@ compile_if(char_u *arg, cctx_T *cctx)
     else
 	scope->se_u.se_if.is_if_label = -1;
 
+#ifdef FEAT_PROFILE
+    if (cctx->ctx_profiling && cctx->ctx_skip == SKIP_YES
+						      && skip_save != SKIP_YES)
+    {
+	// generated a profile start, need to generate a profile end, since it
+	// won't be done after returning
+	cctx->ctx_skip = SKIP_NOT;
+	generate_instr(cctx, ISN_PROF_END);
+	cctx->ctx_skip = SKIP_YES;
+    }
+#endif
+
     return p;
 }
 
@@ -6758,6 +6765,25 @@ compile_elseif(char_u *arg, cctx_T *cctx
     if (!cctx->ctx_had_return)
 	scope->se_u.se_if.is_had_return = FALSE;
 
+    if (cctx->ctx_skip == SKIP_NOT)
+    {
+	// previous block was executed, this one and following will not
+	cctx->ctx_skip = SKIP_YES;
+	scope->se_u.se_if.is_seen_skip_not = TRUE;
+    }
+    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
+	skip_expr_cctx(&p, cctx);
+	return p;
+    }
+
     if (cctx->ctx_skip == SKIP_UNKNOWN)
     {
 	if (compile_jump_to_end(&scope->se_u.se_if.is_end_label,
@@ -6771,7 +6797,17 @@ compile_elseif(char_u *arg, cctx_T *cctx
     // compile "expr"; if we know it evaluates to FALSE skip the block
     CLEAR_FIELD(ppconst);
     if (cctx->ctx_skip == SKIP_YES)
+    {
 	cctx->ctx_skip = SKIP_UNKNOWN;
+#ifdef FEAT_PROFILE
+	if (cctx->ctx_profiling)
+	{
+	    // the previous block was skipped, need to profile this line
+	    generate_instr(cctx, ISN_PROF_START);
+	    instr_count = instr->ga_len;
+	}
+#endif
+    }
     if (compile_expr1(&p, cctx, &ppconst) == FAIL)
     {
 	clear_ppconst(&ppconst);
@@ -6829,7 +6865,27 @@ compile_else(char_u *arg, cctx_T *cctx)
 	scope->se_u.se_if.is_had_return = FALSE;
     scope->se_u.se_if.is_seen_else = TRUE;
 
-    if (scope->se_skip_save != SKIP_YES)
+#ifdef FEAT_PROFILE
+    if (cctx->ctx_profiling)
+    {
+	if (cctx->ctx_skip == SKIP_NOT
+		&& ((isn_T *)instr->ga_data)[instr->ga_len - 1]
+						   .isn_type == ISN_PROF_START)
+	    // the previous block was executed, do not count "else" for profiling
+	    --instr->ga_len;
+	if (cctx->ctx_skip == SKIP_YES && !scope->se_u.se_if.is_seen_skip_not)
+	{
+	    // the previous block was not executed, this one will, do count the
+	    // "else" for profiling
+	    cctx->ctx_skip = SKIP_NOT;
+	    generate_instr(cctx, ISN_PROF_END);
+	    generate_instr(cctx, ISN_PROF_START);
+	    cctx->ctx_skip = SKIP_YES;
+	}
+    }
+#endif
+
+    if (!scope->se_u.se_if.is_seen_skip_not && scope->se_skip_save != SKIP_YES)
     {
 	// jump from previous block to the end, unless the else block is empty
 	if (cctx->ctx_skip == SKIP_UNKNOWN)
@@ -6884,6 +6940,17 @@ compile_endif(char_u *arg, cctx_T *cctx)
     }
     // Fill in the "end" label in jumps at the end of the blocks.
     compile_fill_jump_to_end(&ifscope->is_end_label, cctx);
+
+#ifdef FEAT_PROFILE
+    // even when skipping we count the endif as executed, unless the block it's
+    // in is skipped
+    if (cctx->ctx_profiling && cctx->ctx_skip == SKIP_YES
+					    && scope->se_skip_save != SKIP_YES)
+    {
+	cctx->ctx_skip = SKIP_NOT;
+	generate_instr(cctx, ISN_PROF_START);
+    }
+#endif
     cctx->ctx_skip = scope->se_skip_save;
 
     // If all the blocks end in :return and there is an :else then the
@@ -8005,7 +8072,8 @@ compile_def_function(
 	    {
 		// beyond the last line
 #ifdef FEAT_PROFILE
-		may_generate_prof_end(&cctx, prof_lnum);
+		if (cctx.ctx_skip != SKIP_YES)
+		    may_generate_prof_end(&cctx, prof_lnum);
 #endif
 		break;
 	    }
@@ -8023,7 +8091,8 @@ compile_def_function(
 	}
 
 #ifdef FEAT_PROFILE
-	if (cctx.ctx_profiling && cctx.ctx_lnum != prof_lnum)
+	if (cctx.ctx_profiling && cctx.ctx_lnum != prof_lnum &&
+						     cctx.ctx_skip != SKIP_YES)
 	{
 	    may_generate_prof_end(&cctx, prof_lnum);