Skip to content

Commit ced68a0

Browse files
brammoolbrammool
committed
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.
1 parent 8323cab commit ced68a0

5 files changed

Lines changed: 115 additions & 35 deletions

File tree

‎src/testdir/test_profile.vim‎

Lines changed: 33 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -100,39 +100,48 @@ func RunProfileFunc(command, declare, assign)
100100
endfunc
101101

102102
func Test_profile_func_with_ifelse()
103+
call Run_profile_func_with_ifelse('func', 'let', 'let')
104+
call Run_profile_func_with_ifelse('def', 'var', '')
105+
endfunc
106+
107+
func Run_profile_func_with_ifelse(command, declare, assign)
103108
let lines =<< trim [CODE]
104-
func! Foo1()
109+
XXX Foo1()
105110
if 1
106-
let x = 0
111+
DDD x = 0
107112
elseif 1
108-
let x = 1
113+
DDD x = 1
109114
else
110-
let x = 2
115+
DDD x = 2
111116
endif
112-
endfunc
113-
func! Foo2()
117+
endXXX
118+
XXX Foo2()
114119
if 0
115-
let x = 0
120+
DDD x = 0
116121
elseif 1
117-
let x = 1
122+
DDD x = 1
118123
else
119-
let x = 2
124+
DDD x = 2
120125
endif
121-
endfunc
122-
func! Foo3()
126+
endXXX
127+
XXX Foo3()
123128
if 0
124-
let x = 0
129+
DDD x = 0
125130
elseif 0
126-
let x = 1
131+
DDD x = 1
127132
else
128-
let x = 2
133+
DDD x = 2
129134
endif
130-
endfunc
135+
endXXX
131136
call Foo1()
132137
call Foo2()
133138
call Foo3()
134139
[CODE]
135140

141+
call map(lines, {k, v -> substitute(v, 'XXX', a:command, '') })
142+
call map(lines, {k, v -> substitute(v, 'DDD', a:declare, '') })
143+
call map(lines, {k, v -> substitute(v, 'AAA', a:assign, '') })
144+
136145
call writefile(lines, 'Xprofile_func.vim')
137146
call system(GetVimCommand()
138147
\ . ' -es -i NONE --noplugin'
@@ -157,11 +166,11 @@ func Test_profile_func_with_ifelse()
157166
call assert_equal('', lines[5])
158167
call assert_equal('count total (s) self (s)', lines[6])
159168
call assert_match('^\s*1\s\+.*\sif 1$', lines[7])
160-
call assert_match('^\s*1\s\+.*\s let x = 0$', lines[8])
169+
call assert_match('^\s*1\s\+.*\s \(let\|var\) x = 0$', lines[8])
161170
call assert_match( '^\s\+elseif 1$', lines[9])
162-
call assert_match( '^\s\+let x = 1$', lines[10])
171+
call assert_match( '^\s\+\(let\|var\) x = 1$', lines[10])
163172
call assert_match( '^\s\+else$', lines[11])
164-
call assert_match( '^\s\+let x = 2$', lines[12])
173+
call assert_match( '^\s\+\(let\|var\) x = 2$', lines[12])
165174
call assert_match('^\s*1\s\+.*\sendif$', lines[13])
166175
call assert_equal('', lines[14])
167176
call assert_equal('FUNCTION Foo2()', lines[15])
@@ -171,11 +180,11 @@ func Test_profile_func_with_ifelse()
171180
call assert_equal('', lines[20])
172181
call assert_equal('count total (s) self (s)', lines[21])
173182
call assert_match('^\s*1\s\+.*\sif 0$', lines[22])
174-
call assert_match( '^\s\+let x = 0$', lines[23])
183+
call assert_match( '^\s\+\(let\|var\) x = 0$', lines[23])
175184
call assert_match('^\s*1\s\+.*\selseif 1$', lines[24])
176-
call assert_match('^\s*1\s\+.*\s let x = 1$', lines[25])
185+
call assert_match('^\s*1\s\+.*\s \(let\|var\) x = 1$', lines[25])
177186
call assert_match( '^\s\+else$', lines[26])
178-
call assert_match( '^\s\+let x = 2$', lines[27])
187+
call assert_match( '^\s\+\(let\|var\) x = 2$', lines[27])
179188
call assert_match('^\s*1\s\+.*\sendif$', lines[28])
180189
call assert_equal('', lines[29])
181190
call assert_equal('FUNCTION Foo3()', lines[30])
@@ -185,11 +194,11 @@ func Test_profile_func_with_ifelse()
185194
call assert_equal('', lines[35])
186195
call assert_equal('count total (s) self (s)', lines[36])
187196
call assert_match('^\s*1\s\+.*\sif 0$', lines[37])
188-
call assert_match( '^\s\+let x = 0$', lines[38])
197+
call assert_match( '^\s\+\(let\|var\) x = 0$', lines[38])
189198
call assert_match('^\s*1\s\+.*\selseif 0$', lines[39])
190-
call assert_match( '^\s\+let x = 1$', lines[40])
199+
call assert_match( '^\s\+\(let\|var\) x = 1$', lines[40])
191200
call assert_match('^\s*1\s\+.*\selse$', lines[41])
192-
call assert_match('^\s*1\s\+.*\s let x = 2$', lines[42])
201+
call assert_match('^\s*1\s\+.*\s \(let\|var\) x = 2$', lines[42])
193202
call assert_match('^\s*1\s\+.*\sendif$', lines[43])
194203
call assert_equal('', lines[44])
195204
call assert_equal('FUNCTIONS SORTED ON TOTAL TIME', lines[45])

‎src/testdir/test_vim9_disassemble.vim‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1857,8 +1857,8 @@ def Test_profiled()
18571857
'\d PROFILE START line 1\_s*' ..
18581858
'\d PUSHS "profiled"\_s*' ..
18591859
'\d ECHO 1\_s*' ..
1860-
'\d PROFILE END\_s*' ..
18611860
'return "done"\_s*' ..
1861+
'\d PROFILE END\_s*' ..
18621862
'\d PROFILE START line 2\_s*' ..
18631863
'\d PUSHS "done"\_s*' ..
18641864
'\d RETURN\_s*' ..

‎src/testdir/test_vim9_script.vim‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1741,7 +1741,7 @@ def Test_if_elseif_else_fails()
17411741
CheckDefFailure(['elseif true'], 'E582:')
17421742
CheckDefFailure(['else'], 'E581:')
17431743
CheckDefFailure(['endif'], 'E580:')
1744-
CheckDefFailure(['if true', 'elseif xxx'], 'E1001:')
1744+
CheckDefFailure(['if g:abool', 'elseif xxx'], 'E1001:')
17451745
CheckDefFailure(['if true', 'echo 1'], 'E171:')
17461746
enddef
17471747

‎src/version.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -750,6 +750,8 @@ static char *(features[]) =
750750

751751
static int included_patches[] =
752752
{ /* Add new patch number below this line */
753+
/**/
754+
2403,
753755
/**/
754756
2402,
755757
/**/

‎src/vim9compile.c‎

Lines changed: 78 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ struct endlabel_S {
4444
*/
4545
typedef struct {
4646
int is_seen_else;
47+
int is_seen_skip_not; // a block was unconditionally executed
4748
int is_had_return; // every block ends in :return
4849
int is_if_label; // instruction idx at IF or ELSEIF
4950
endlabel_T *is_end_label; // instructions to set end label
@@ -2098,13 +2099,7 @@ generate_undo_cmdmods(cctx_T *cctx)
20982099
may_generate_prof_end(cctx_T *cctx, int prof_lnum)
20992100
{
21002101
if (cctx->ctx_profiling && prof_lnum >= 0)
2101-
{
2102-
int save_lnum = cctx->ctx_lnum;
2103-
2104-
cctx->ctx_lnum = prof_lnum;
21052102
generate_instr(cctx, ISN_PROF_END);
2106-
cctx->ctx_lnum = save_lnum;
2107-
}
21082103
}
21092104
#endif
21102105

@@ -6735,6 +6730,18 @@ compile_if(char_u *arg, cctx_T *cctx)
67356730
else
67366731
scope->se_u.se_if.is_if_label = -1;
67376732

6733+
#ifdef FEAT_PROFILE
6734+
if (cctx->ctx_profiling && cctx->ctx_skip == SKIP_YES
6735+
&& skip_save != SKIP_YES)
6736+
{
6737+
// generated a profile start, need to generate a profile end, since it
6738+
// won't be done after returning
6739+
cctx->ctx_skip = SKIP_NOT;
6740+
generate_instr(cctx, ISN_PROF_END);
6741+
cctx->ctx_skip = SKIP_YES;
6742+
}
6743+
#endif
6744+
67386745
return p;
67396746
}
67406747

@@ -6758,6 +6765,25 @@ compile_elseif(char_u *arg, cctx_T *cctx)
67586765
if (!cctx->ctx_had_return)
67596766
scope->se_u.se_if.is_had_return = FALSE;
67606767

6768+
if (cctx->ctx_skip == SKIP_NOT)
6769+
{
6770+
// previous block was executed, this one and following will not
6771+
cctx->ctx_skip = SKIP_YES;
6772+
scope->se_u.se_if.is_seen_skip_not = TRUE;
6773+
}
6774+
if (scope->se_u.se_if.is_seen_skip_not)
6775+
{
6776+
// A previous block was executed, skip over expression and bail out.
6777+
// Do not count the "elseif" for profiling.
6778+
#ifdef FEAT_PROFILE
6779+
if (cctx->ctx_profiling && ((isn_T *)instr->ga_data)[instr->ga_len - 1]
6780+
.isn_type == ISN_PROF_START)
6781+
--instr->ga_len;
6782+
#endif
6783+
skip_expr_cctx(&p, cctx);
6784+
return p;
6785+
}
6786+
67616787
if (cctx->ctx_skip == SKIP_UNKNOWN)
67626788
{
67636789
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)
67716797
// compile "expr"; if we know it evaluates to FALSE skip the block
67726798
CLEAR_FIELD(ppconst);
67736799
if (cctx->ctx_skip == SKIP_YES)
6800+
{
67746801
cctx->ctx_skip = SKIP_UNKNOWN;
6802+
#ifdef FEAT_PROFILE
6803+
if (cctx->ctx_profiling)
6804+
{
6805+
// the previous block was skipped, need to profile this line
6806+
generate_instr(cctx, ISN_PROF_START);
6807+
instr_count = instr->ga_len;
6808+
}
6809+
#endif
6810+
}
67756811
if (compile_expr1(&p, cctx, &ppconst) == FAIL)
67766812
{
67776813
clear_ppconst(&ppconst);
@@ -6829,7 +6865,27 @@ compile_else(char_u *arg, cctx_T *cctx)
68296865
scope->se_u.se_if.is_had_return = FALSE;
68306866
scope->se_u.se_if.is_seen_else = TRUE;
68316867

6832-
if (scope->se_skip_save != SKIP_YES)
6868+
#ifdef FEAT_PROFILE
6869+
if (cctx->ctx_profiling)
6870+
{
6871+
if (cctx->ctx_skip == SKIP_NOT
6872+
&& ((isn_T *)instr->ga_data)[instr->ga_len - 1]
6873+
.isn_type == ISN_PROF_START)
6874+
// the previous block was executed, do not count "else" for profiling
6875+
--instr->ga_len;
6876+
if (cctx->ctx_skip == SKIP_YES && !scope->se_u.se_if.is_seen_skip_not)
6877+
{
6878+
// the previous block was not executed, this one will, do count the
6879+
// "else" for profiling
6880+
cctx->ctx_skip = SKIP_NOT;
6881+
generate_instr(cctx, ISN_PROF_END);
6882+
generate_instr(cctx, ISN_PROF_START);
6883+
cctx->ctx_skip = SKIP_YES;
6884+
}
6885+
}
6886+
#endif
6887+
6888+
if (!scope->se_u.se_if.is_seen_skip_not && scope->se_skip_save != SKIP_YES)
68336889
{
68346890
// jump from previous block to the end, unless the else block is empty
68356891
if (cctx->ctx_skip == SKIP_UNKNOWN)
@@ -6884,6 +6940,17 @@ compile_endif(char_u *arg, cctx_T *cctx)
68846940
}
68856941
// Fill in the "end" label in jumps at the end of the blocks.
68866942
compile_fill_jump_to_end(&ifscope->is_end_label, cctx);
6943+
6944+
#ifdef FEAT_PROFILE
6945+
// even when skipping we count the endif as executed, unless the block it's
6946+
// in is skipped
6947+
if (cctx->ctx_profiling && cctx->ctx_skip == SKIP_YES
6948+
&& scope->se_skip_save != SKIP_YES)
6949+
{
6950+
cctx->ctx_skip = SKIP_NOT;
6951+
generate_instr(cctx, ISN_PROF_START);
6952+
}
6953+
#endif
68876954
cctx->ctx_skip = scope->se_skip_save;
68886955

68896956
// If all the blocks end in :return and there is an :else then the
@@ -8005,7 +8072,8 @@ compile_def_function(
80058072
{
80068073
// beyond the last line
80078074
#ifdef FEAT_PROFILE
8008-
may_generate_prof_end(&cctx, prof_lnum);
8075+
if (cctx.ctx_skip != SKIP_YES)
8076+
may_generate_prof_end(&cctx, prof_lnum);
80098077
#endif
80108078
break;
80118079
}
@@ -8023,7 +8091,8 @@ compile_def_function(
80238091
}
80248092

80258093
#ifdef FEAT_PROFILE
8026-
if (cctx.ctx_profiling && cctx.ctx_lnum != prof_lnum)
8094+
if (cctx.ctx_profiling && cctx.ctx_lnum != prof_lnum &&
8095+
cctx.ctx_skip != SKIP_YES)
80278096
{
80288097
may_generate_prof_end(&cctx, prof_lnum);
80298098

0 commit comments

Comments
 (0)