Skip to content

Commit

Permalink
patch 8.2.3442: Vim9: || and && are not handled at compile time
Browse files Browse the repository at this point in the history
Problem:    Vim9: || and && are not handled at compile time when possible.
Solution:   When using constants generate fewer instructions.
  • Loading branch information
brammool committed Sep 16, 2021
1 parent ee2cbcd commit 1a7ee4d
Show file tree
Hide file tree
Showing 5 changed files with 137 additions and 38 deletions.
62 changes: 50 additions & 12 deletions src/testdir/test_vim9_disassemble.vim
Expand Up @@ -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)
Expand Down Expand Up @@ -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())
Expand Down
2 changes: 2 additions & 0 deletions src/version.c
Expand Up @@ -755,6 +755,8 @@ static char *(features[]) =

static int included_patches[] =
{ /* Add new patch number below this line */
/**/
3442,
/**/
3441,
/**/
Expand Down
1 change: 1 addition & 0 deletions src/vim9.h
Expand Up @@ -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
Expand Down
107 changes: 81 additions & 26 deletions src/vim9compile.c
Expand Up @@ -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)
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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 "&&"
Expand All @@ -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)
{
Expand All @@ -5274,25 +5279,54 @@ 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)
{
ga_clear(&end_ga);
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;
Expand All @@ -5302,13 +5336,28 @@ compile_and_or(
return FAIL;
}

const_used = ppconst->pp_used;
if ((opchar == '|' ? compile_expr3(arg, cctx, ppconst)
: compile_expr4(arg, cctx, ppconst)) == FAIL)
{
ga_clear(&end_ga);
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);
}

Expand All @@ -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;
Expand Down
3 changes: 3 additions & 0 deletions src/vim9execute.c
Expand Up @@ -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;
Expand Down

0 comments on commit 1a7ee4d

Please sign in to comment.