Skip to content

Commit 1a7ee4d

Browse files
committed
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.
1 parent ee2cbcd commit 1a7ee4d

File tree

5 files changed

+137
-38
lines changed

5 files changed

+137
-38
lines changed

src/testdir/test_vim9_disassemble.vim

Lines changed: 50 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1218,6 +1218,38 @@ def Test_disassemble_and_or()
12181218
instr)
12191219
enddef
12201220

1221+
def AndConstant(arg: any): string
1222+
if true && arg
1223+
return "yes"
1224+
endif
1225+
if false && arg
1226+
return "never"
1227+
endif
1228+
return "no"
1229+
enddef
1230+
1231+
def Test_disassemble_and_constant()
1232+
assert_equal("yes", AndConstant(1))
1233+
assert_equal("no", AndConstant(false))
1234+
var instr = execute('disassemble AndConstant')
1235+
assert_match('AndConstant\_s*' ..
1236+
'if true && arg\_s*' ..
1237+
'0 LOAD arg\[-1\]\_s*' ..
1238+
'1 COND2BOOL\_s*' ..
1239+
'2 JUMP_IF_FALSE -> 5\_s*' ..
1240+
'return "yes"\_s*' ..
1241+
'3 PUSHS "yes"\_s*' ..
1242+
'4 RETURN\_s*' ..
1243+
'endif\_s*' ..
1244+
'if false && arg\_s*' ..
1245+
'return "never"\_s*' ..
1246+
'endif\_s*' ..
1247+
'return "no"\_s*' ..
1248+
'5 PUSHS "no"\_s*' ..
1249+
'6 RETURN',
1250+
instr)
1251+
enddef
1252+
12211253
def ForLoop(): list<number>
12221254
var res: list<number>
12231255
for i in range(3)
@@ -1734,25 +1766,31 @@ def Test_disassemble_invert_bool()
17341766
enddef
17351767

17361768
def ReturnBool(): bool
1737-
var name: bool = 1 && 0 || 1
1769+
var one = 1
1770+
var zero = 0
1771+
var name: bool = one && zero || one
17381772
return name
17391773
enddef
17401774

17411775
def Test_disassemble_return_bool()
17421776
var instr = execute('disassemble ReturnBool')
17431777
assert_match('ReturnBool\_s*' ..
1744-
'var name: bool = 1 && 0 || 1\_s*' ..
1745-
'0 PUSHNR 1\_s*' ..
1746-
'1 COND2BOOL\_s*' ..
1747-
'2 JUMP_IF_COND_FALSE -> 5\_s*' ..
1748-
'3 PUSHNR 0\_s*' ..
1749-
'4 COND2BOOL\_s*' ..
1750-
'5 JUMP_IF_COND_TRUE -> 8\_s*' ..
1751-
'6 PUSHNR 1\_s*' ..
1752-
'7 COND2BOOL\_s*' ..
1753-
'\d STORE $0\_s*' ..
1778+
'var one = 1\_s*' ..
1779+
'0 STORE 1 in $0\_s*' ..
1780+
'var zero = 0\_s*' ..
1781+
'1 STORE 0 in $1\_s*' ..
1782+
'var name: bool = one && zero || one\_s*' ..
1783+
'2 LOAD $0\_s*' ..
1784+
'3 COND2BOOL\_s*' ..
1785+
'4 JUMP_IF_COND_FALSE -> 7\_s*' ..
1786+
'5 LOAD $1\_s*' ..
1787+
'6 COND2BOOL\_s*' ..
1788+
'7 JUMP_IF_COND_TRUE -> 10\_s*' ..
1789+
'8 LOAD $0\_s*' ..
1790+
'9 COND2BOOL\_s*' ..
1791+
'10 STORE $2\_s*' ..
17541792
'return name\_s*' ..
1755-
'\d\+ LOAD $0\_s*' ..
1793+
'\d\+ LOAD $2\_s*' ..
17561794
'\d\+ RETURN',
17571795
instr)
17581796
assert_equal(true, InvertBool())

src/version.c

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

756756
static int included_patches[] =
757757
{ /* Add new patch number below this line */
758+
/**/
759+
3442,
758760
/**/
759761
3441,
760762
/**/

src/vim9.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,7 @@ typedef struct {
221221

222222
typedef enum {
223223
JUMP_ALWAYS,
224+
JUMP_NEVER,
224225
JUMP_IF_FALSE, // pop and jump if false
225226
JUMP_AND_KEEP_IF_TRUE, // jump if top of stack is truthy, drop if not
226227
JUMP_AND_KEEP_IF_FALSE, // jump if top of stack is falsy, drop if not

src/vim9compile.c

Lines changed: 81 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2847,7 +2847,7 @@ generate_ppconst(cctx_T *cctx, ppconst_T *ppconst)
28472847
}
28482848

28492849
/*
2850-
* Check that the last item of "ppconst" is a bool.
2850+
* Check that the last item of "ppconst" is a bool, if there is an item.
28512851
*/
28522852
static int
28532853
check_ppconst_bool(ppconst_T *ppconst)
@@ -4845,7 +4845,8 @@ compile_expr7(
48454845
}
48464846
else
48474847
{
4848-
if (generate_ppconst(cctx, ppconst) == FAIL)
4848+
if (cctx->ctx_skip != SKIP_YES
4849+
&& generate_ppconst(cctx, ppconst) == FAIL)
48494850
return FAIL;
48504851
r = compile_load(arg, p, cctx, TRUE, TRUE);
48514852
}
@@ -5240,6 +5241,7 @@ compile_and_or(
52405241
{
52415242
garray_T *instr = &cctx->ctx_instr;
52425243
garray_T end_ga;
5244+
int save_skip = cctx->ctx_skip;
52435245

52445246
/*
52455247
* Repeat until there is no following "||" or "&&"
@@ -5251,7 +5253,10 @@ compile_and_or(
52515253
long save_sourcing_lnum;
52525254
int start_ctx_lnum = cctx->ctx_lnum;
52535255
int save_lnum;
5256+
int const_used;
52545257
int status;
5258+
jumpwhen_T jump_when = opchar == '|'
5259+
? JUMP_IF_COND_TRUE : JUMP_IF_COND_FALSE;
52555260

52565261
if (next != NULL)
52575262
{
@@ -5274,25 +5279,54 @@ compile_and_or(
52745279
status = check_ppconst_bool(ppconst);
52755280
if (status != FAIL)
52765281
{
5277-
// TODO: use ppconst if the value is a constant
5278-
generate_ppconst(cctx, ppconst);
5282+
// Use the last ppconst if possible.
5283+
if (ppconst->pp_used > 0)
5284+
{
5285+
typval_T *tv = &ppconst->pp_tv[ppconst->pp_used - 1];
5286+
int is_true = tv2bool(tv);
52795287

5280-
// Every part must evaluate to a bool.
5281-
status = bool_on_stack(cctx);
5282-
if (status != FAIL)
5283-
status = ga_grow(&end_ga, 1);
5288+
if ((is_true && opchar == '|')
5289+
|| (!is_true && opchar == '&'))
5290+
{
5291+
// For "false && expr" and "true || expr" the "expr"
5292+
// does not need to be evaluated.
5293+
cctx->ctx_skip = SKIP_YES;
5294+
clear_tv(tv);
5295+
tv->v_type = VAR_BOOL;
5296+
tv->vval.v_number = is_true ? VVAL_TRUE : VVAL_FALSE;
5297+
}
5298+
else
5299+
{
5300+
// For "true && expr" and "false || expr" only "expr"
5301+
// needs to be evaluated.
5302+
--ppconst->pp_used;
5303+
jump_when = JUMP_NEVER;
5304+
}
5305+
}
5306+
else
5307+
{
5308+
// Every part must evaluate to a bool.
5309+
status = bool_on_stack(cctx);
5310+
}
52845311
}
5312+
if (status != FAIL)
5313+
status = ga_grow(&end_ga, 1);
52855314
cctx->ctx_lnum = save_lnum;
52865315
if (status == FAIL)
52875316
{
52885317
ga_clear(&end_ga);
52895318
return FAIL;
52905319
}
52915320

5292-
*(((int *)end_ga.ga_data) + end_ga.ga_len) = instr->ga_len;
5293-
++end_ga.ga_len;
5294-
generate_JUMP(cctx, opchar == '|'
5295-
? JUMP_IF_COND_TRUE : JUMP_IF_COND_FALSE, 0);
5321+
if (jump_when != JUMP_NEVER)
5322+
{
5323+
if (cctx->ctx_skip != SKIP_YES)
5324+
{
5325+
*(((int *)end_ga.ga_data) + end_ga.ga_len) = instr->ga_len;
5326+
++end_ga.ga_len;
5327+
}
5328+
generate_JUMP(cctx, jump_when, 0);
5329+
}
52965330

52975331
// eval the next expression
52985332
SOURCING_LNUM = save_sourcing_lnum;
@@ -5302,13 +5336,28 @@ compile_and_or(
53025336
return FAIL;
53035337
}
53045338

5339+
const_used = ppconst->pp_used;
53055340
if ((opchar == '|' ? compile_expr3(arg, cctx, ppconst)
53065341
: compile_expr4(arg, cctx, ppconst)) == FAIL)
53075342
{
53085343
ga_clear(&end_ga);
53095344
return FAIL;
53105345
}
53115346

5347+
// "0 || 1" results in true, "1 && 0" results in false.
5348+
if (ppconst->pp_used == const_used + 1)
5349+
{
5350+
typval_T *tv = &ppconst->pp_tv[ppconst->pp_used - 1];
5351+
5352+
if (tv->v_type == VAR_NUMBER
5353+
&& (tv->vval.v_number == 1 || tv->vval.v_number == 0))
5354+
{
5355+
tv->vval.v_number = tv->vval.v_number == 1
5356+
? VVAL_TRUE : VVAL_FALSE;
5357+
tv->v_type = VAR_BOOL;
5358+
}
5359+
}
5360+
53125361
p = may_peek_next_line(cctx, *arg, &next);
53135362
}
53145363

@@ -5317,26 +5366,32 @@ compile_and_or(
53175366
ga_clear(&end_ga);
53185367
return FAIL;
53195368
}
5320-
generate_ppconst(cctx, ppconst);
53215369

5322-
// Every part must evaluate to a bool.
5323-
if (bool_on_stack(cctx) == FAIL)
5324-
{
5325-
ga_clear(&end_ga);
5326-
return FAIL;
5327-
}
5370+
if (cctx->ctx_skip != SKIP_YES && ppconst->pp_used == 0)
5371+
// Every part must evaluate to a bool.
5372+
if (bool_on_stack(cctx) == FAIL)
5373+
{
5374+
ga_clear(&end_ga);
5375+
return FAIL;
5376+
}
53285377

5329-
// Fill in the end label in all jumps.
5330-
while (end_ga.ga_len > 0)
5378+
if (end_ga.ga_len > 0)
53315379
{
5332-
isn_T *isn;
5380+
// Fill in the end label in all jumps.
5381+
generate_ppconst(cctx, ppconst);
5382+
while (end_ga.ga_len > 0)
5383+
{
5384+
isn_T *isn;
53335385

5334-
--end_ga.ga_len;
5335-
isn = ((isn_T *)instr->ga_data)
5386+
--end_ga.ga_len;
5387+
isn = ((isn_T *)instr->ga_data)
53365388
+ *(((int *)end_ga.ga_data) + end_ga.ga_len);
5337-
isn->isn_arg.jump.jump_where = instr->ga_len;
5389+
isn->isn_arg.jump.jump_where = instr->ga_len;
5390+
}
5391+
ga_clear(&end_ga);
53385392
}
5339-
ga_clear(&end_ga);
5393+
5394+
cctx->ctx_skip = save_skip;
53405395
}
53415396

53425397
return OK;

src/vim9execute.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5487,6 +5487,9 @@ list_instructions(char *pfx, isn_T *instr, int instr_count, ufunc_T *ufunc)
54875487
case JUMP_ALWAYS:
54885488
when = "JUMP";
54895489
break;
5490+
case JUMP_NEVER:
5491+
iemsg("JUMP_NEVER should not be used");
5492+
break;
54905493
case JUMP_AND_KEEP_IF_TRUE:
54915494
when = "JUMP_AND_KEEP_IF_TRUE";
54925495
break;

0 commit comments

Comments
 (0)