thac0/vim
1
0
Fork 0
forked from aniani/vim
Code

patch 8.2.1006: Vim9: require unnecessary return statement

Browse Source 
Problem:    Vim9: require unnecessary return statement.
Solution:   Improve the use of the had_return flag. (closes #6270)
This commit is contained in:
Bram Moolenaar 2020-06-18 20:50:10 +02:00
parent 9b68c82b7c
commit efd8855594
4 changed files with 130 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
src/testdir/test_vim9_disassemble.vim
View File

@ -533,6 +533,30 @@ def Test_disassemble_const_expr()
assert_notmatch('JUMP', instr) assert_notmatch('JUMP', instr)
enddef enddef
def ReturnInIf(): string
if g:cond
return "yes"
else
return "no"
endif
enddef
def Test_disassemble_return_in_if()
let instr = execute('disassemble ReturnInIf')
assert_match('ReturnInIf\_s*' ..
'if g:cond\_s*' ..
'0 LOADG g:cond\_s*' ..
'1 JUMP_IF_FALSE -> 4\_s*' ..
'return "yes"\_s*' ..
'2 PUSHS "yes"\_s*' ..
'3 RETURN\_s*' ..
'else\_s*' ..
' return "no"\_s*' ..
'4 PUSHS "no"\_s*' ..
'5 RETURN$',
instr)
enddef
def WithFunc() def WithFunc()
let Funky1: func let Funky1: func
let Funky2: func = function("len") let Funky2: func = function("len")

25
src/testdir/test_vim9_func.vim
View File

@ -31,6 +31,31 @@ def Test_return_something()
assert_fails('call ReturnGlobal()', 'E1029: Expected number but got string') assert_fails('call ReturnGlobal()', 'E1029: Expected number but got string')
enddef enddef
def Test_missing_return()
CheckDefFailure(['def Missing(): number',
' if g:cond',
' echo "no return"',
' else',
' return 0',
' endif'
'enddef'], 'E1027:')
CheckDefFailure(['def Missing(): number',
' if g:cond',
' return 1',
' else',
' echo "no return"',
' endif'
'enddef'], 'E1027:')
CheckDefFailure(['def Missing(): number',
' if g:cond',
' return 1',
' else',
' return 2',
' endif'
' return 3'
'enddef'], 'E1095:')
enddef
let s:nothing = 0 let s:nothing = 0
def ReturnNothing() def ReturnNothing()
s:nothing = 1 s:nothing = 1

2
src/version.c
View File

@ -754,6 +754,8 @@ static char *(features[]) =
static int included_patches[] = static int included_patches[] =
{ /* Add new patch number below this line */ { /* Add new patch number below this line */
/**/
1006,
/**/ /**/
1005, 1005,
/**/ /**/

112
src/vim9compile.c
View File

@ -23,6 +23,13 @@
#define DEFINE_VIM9_GLOBALS #define DEFINE_VIM9_GLOBALS
#include "vim9.h" #include "vim9.h"
// values for ctx_skip
typedef enum {
SKIP_NOT, // condition is a constant, produce code
SKIP_YES, // condition is a constant, do NOT produce code
SKIP_UNKNONW // condition is not a constant, produce code
} skip_T;
/* /*
* Chain of jump instructions where the end label needs to be set. * Chain of jump instructions where the end label needs to be set.
*/ */
@ -36,6 +43,8 @@ struct endlabel_S {
* info specific for the scope of :if / elseif / else * info specific for the scope of :if / elseif / else
*/ */
typedef struct { typedef struct {
int is_seen_else;
int is_had_return; // every block ends in :return
int is_if_label; // instruction idx at IF or ELSEIF int is_if_label; // instruction idx at IF or ELSEIF
endlabel_T *is_end_label; // instructions to set end label endlabel_T *is_end_label; // instructions to set end label
} ifscope_T; } ifscope_T;
@ -83,6 +92,7 @@ struct scope_S {
scope_T *se_outer; // scope containing this one scope_T *se_outer; // scope containing this one
scopetype_T se_type; scopetype_T se_type;
int se_local_count; // ctx_locals.ga_len before scope int se_local_count; // ctx_locals.ga_len before scope
skip_T se_skip_save; // ctx_skip before the block
union { union {
ifscope_T se_if; ifscope_T se_if;
whilescope_T se_while; whilescope_T se_while;
@ -103,13 +113,6 @@ typedef struct {
int lv_arg; // when TRUE this is an argument int lv_arg; // when TRUE this is an argument
} lvar_T; } lvar_T;
// values for ctx_skip
typedef enum {
SKIP_NOT, // condition is a constant, produce code
SKIP_YES, // condition is a constant, do NOT produce code
SKIP_UNKNONW // condition is not a constant, produce code
} skip_T;
/* /*
* Context for compiling lines of Vim script. * Context for compiling lines of Vim script.
* Stores info about the local variables and condition stack. * Stores info about the local variables and condition stack.
@ -130,6 +133,7 @@ struct cctx_S {
skip_T ctx_skip; skip_T ctx_skip;
scope_T *ctx_scope; // current scope, NULL at toplevel scope_T *ctx_scope; // current scope, NULL at toplevel
int ctx_had_return; // last seen statement was "return"
cctx_T *ctx_outer; // outer scope for lambda or nested cctx_T *ctx_outer; // outer scope for lambda or nested
// function // function
@ -5664,6 +5668,7 @@ compile_if(char_u *arg, cctx_T *cctx)
garray_T *instr = &cctx->ctx_instr; garray_T *instr = &cctx->ctx_instr;
int instr_count = instr->ga_len; int instr_count = instr->ga_len;
scope_T *scope; scope_T *scope;
skip_T skip_save = cctx->ctx_skip;
ppconst_T ppconst; ppconst_T ppconst;
CLEAR_FIELD(ppconst); CLEAR_FIELD(ppconst);
@ -5672,10 +5677,11 @@ compile_if(char_u *arg, cctx_T *cctx)
clear_ppconst(&ppconst); clear_ppconst(&ppconst);
return NULL; return NULL;
} }
if (instr->ga_len == instr_count && ppconst.pp_used == 1) if (cctx->ctx_skip == SKIP_YES)
clear_ppconst(&ppconst);
else if (instr->ga_len == instr_count && ppconst.pp_used == 1)
{ {
// The expression results in a constant. // The expression results in a constant.
// TODO: how about nesting?
cctx->ctx_skip = tv2bool(&ppconst.pp_tv[0]) ? SKIP_NOT : SKIP_YES; cctx->ctx_skip = tv2bool(&ppconst.pp_tv[0]) ? SKIP_NOT : SKIP_YES;
clear_ppconst(&ppconst); clear_ppconst(&ppconst);
} }
@ -5690,6 +5696,9 @@ compile_if(char_u *arg, cctx_T *cctx)
scope = new_scope(cctx, IF_SCOPE); scope = new_scope(cctx, IF_SCOPE);
if (scope == NULL) if (scope == NULL)
return NULL; return NULL;
scope->se_skip_save = skip_save;
// "is_had_return" will be reset if any block does not end in :return
scope->se_u.se_if.is_had_return = TRUE;
if (cctx->ctx_skip == SKIP_UNKNONW) if (cctx->ctx_skip == SKIP_UNKNONW)
{ {
@ -5719,6 +5728,8 @@ compile_elseif(char_u *arg, cctx_T *cctx)
return NULL; return NULL;
} }
unwind_locals(cctx, scope->se_local_count); unwind_locals(cctx, scope->se_local_count);
if (!cctx->ctx_had_return)
scope->se_u.se_if.is_had_return = FALSE;
if (cctx->ctx_skip == SKIP_UNKNONW) if (cctx->ctx_skip == SKIP_UNKNONW)
{ {
@ -5737,7 +5748,9 @@ compile_elseif(char_u *arg, cctx_T *cctx)
clear_ppconst(&ppconst); clear_ppconst(&ppconst);
return NULL; return NULL;
} }
if (instr->ga_len == instr_count && ppconst.pp_used == 1) if (scope->se_skip_save == SKIP_YES)
clear_ppconst(&ppconst);
else if (instr->ga_len == instr_count && ppconst.pp_used == 1)
{ {
// The expression results in a constant. // The expression results in a constant.
// TODO: how about nesting? // TODO: how about nesting?
@ -5774,28 +5787,35 @@ compile_else(char_u *arg, cctx_T *cctx)
return NULL; return NULL;
} }
unwind_locals(cctx, scope->se_local_count); unwind_locals(cctx, scope->se_local_count);
if (!cctx->ctx_had_return)
scope->se_u.se_if.is_had_return = FALSE;
scope->se_u.se_if.is_seen_else = TRUE;
// jump from previous block to the end, unless the else block is empty if (scope->se_skip_save != SKIP_YES)
if (cctx->ctx_skip == SKIP_UNKNONW)
{ {
if (compile_jump_to_end(&scope->se_u.se_if.is_end_label, // jump from previous block to the end, unless the else block is empty
JUMP_ALWAYS, cctx) == FAIL) if (cctx->ctx_skip == SKIP_UNKNONW)
return NULL;
}
if (cctx->ctx_skip == SKIP_UNKNONW)
{
if (scope->se_u.se_if.is_if_label >= 0)
{ {
// previous "if" or "elseif" jumps here if (!cctx->ctx_had_return
isn = ((isn_T *)instr->ga_data) + scope->se_u.se_if.is_if_label; && compile_jump_to_end(&scope->se_u.se_if.is_end_label,
isn->isn_arg.jump.jump_where = instr->ga_len; JUMP_ALWAYS, cctx) == FAIL)
scope->se_u.se_if.is_if_label = -1; return NULL;
} }
}
if (cctx->ctx_skip != SKIP_UNKNONW) if (cctx->ctx_skip == SKIP_UNKNONW)
cctx->ctx_skip = cctx->ctx_skip == SKIP_YES ? SKIP_NOT : SKIP_YES; {
if (scope->se_u.se_if.is_if_label >= 0)
{
// 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;
scope->se_u.se_if.is_if_label = -1;
}
}
if (cctx->ctx_skip != SKIP_UNKNONW)
cctx->ctx_skip = cctx->ctx_skip == SKIP_YES ? SKIP_NOT : SKIP_YES;
}
return p; return p;
} }
@ -5815,6 +5835,8 @@ compile_endif(char_u *arg, cctx_T *cctx)
} }
ifscope = &scope->se_u.se_if; ifscope = &scope->se_u.se_if;
unwind_locals(cctx, scope->se_local_count); unwind_locals(cctx, scope->se_local_count);
if (!cctx->ctx_had_return)
ifscope->is_had_return = FALSE;
if (scope->se_u.se_if.is_if_label >= 0) if (scope->se_u.se_if.is_if_label >= 0)
{ {
@ -5824,8 +5846,11 @@ compile_endif(char_u *arg, cctx_T *cctx)
} }
// Fill in the "end" label in jumps at the end of the blocks. // Fill in the "end" label in jumps at the end of the blocks.
compile_fill_jump_to_end(&ifscope->is_end_label, cctx); compile_fill_jump_to_end(&ifscope->is_end_label, cctx);
// TODO: this should restore the value from before the :if cctx->ctx_skip = scope->se_skip_save;
cctx->ctx_skip = SKIP_UNKNONW;
// If all the blocks end in :return and there is an :else then the
// had_return flag is set.
cctx->ctx_had_return = ifscope->is_had_return && ifscope->is_seen_else;
drop_scope(cctx); drop_scope(cctx);
return arg; return arg;
@ -6528,7 +6553,6 @@ compile_def_function(ufunc_T *ufunc, int set_return_type, cctx_T *outer_cctx)
char_u *line = NULL; char_u *line = NULL;
char_u *p; char_u *p;
char *errormsg = NULL; // error message char *errormsg = NULL; // error message
int had_return = FALSE;
cctx_T cctx; cctx_T cctx;
garray_T *instr; garray_T *instr;
int called_emsg_before = called_emsg; int called_emsg_before = called_emsg;
@ -6648,7 +6672,6 @@ compile_def_function(ufunc_T *ufunc, int set_return_type, cctx_T *outer_cctx)
} }
emsg_before = called_emsg; emsg_before = called_emsg;
had_return = FALSE;
CLEAR_FIELD(ea); CLEAR_FIELD(ea);
ea.cmdlinep = &line; ea.cmdlinep = &line;
ea.cmd = skipwhite(line); ea.cmd = skipwhite(line);
@ -6823,6 +6846,22 @@ compile_def_function(ufunc_T *ufunc, int set_return_type, cctx_T *outer_cctx)
continue; continue;
} }
if (ea.cmdidx != CMD_elseif
&& ea.cmdidx != CMD_else
&& ea.cmdidx != CMD_endif
&& ea.cmdidx != CMD_endfor
&& ea.cmdidx != CMD_endwhile
&& ea.cmdidx != CMD_catch
&& ea.cmdidx != CMD_finally
&& ea.cmdidx != CMD_endtry)
{
if (cctx.ctx_had_return)
{
emsg(_("E1095: Unreachable code after :return"));
goto erret;
}
}
switch (ea.cmdidx) switch (ea.cmdidx)
{ {
case CMD_def: case CMD_def:
@ -6836,7 +6875,7 @@ compile_def_function(ufunc_T *ufunc, int set_return_type, cctx_T *outer_cctx)
case CMD_return: case CMD_return:
line = compile_return(p, set_return_type, &cctx); line = compile_return(p, set_return_type, &cctx);
had_return = TRUE; cctx.ctx_had_return = TRUE;
break; break;
case CMD_let: case CMD_let:
@ -6861,9 +6900,11 @@ compile_def_function(ufunc_T *ufunc, int set_return_type, cctx_T *outer_cctx)
break; break;
case CMD_elseif: case CMD_elseif:
line = compile_elseif(p, &cctx); line = compile_elseif(p, &cctx);
cctx.ctx_had_return = FALSE;
break; break;
case CMD_else: case CMD_else:
line = compile_else(p, &cctx); line = compile_else(p, &cctx);
cctx.ctx_had_return = FALSE;
break; break;
case CMD_endif: case CMD_endif:
line = compile_endif(p, &cctx); line = compile_endif(p, &cctx);
@ -6874,6 +6915,7 @@ compile_def_function(ufunc_T *ufunc, int set_return_type, cctx_T *outer_cctx)
break; break;
case CMD_endwhile: case CMD_endwhile:
line = compile_endwhile(p, &cctx); line = compile_endwhile(p, &cctx);
cctx.ctx_had_return = FALSE;
break; break;
case CMD_for: case CMD_for:
@ -6881,6 +6923,7 @@ compile_def_function(ufunc_T *ufunc, int set_return_type, cctx_T *outer_cctx)
break; break;
case CMD_endfor: case CMD_endfor:
line = compile_endfor(p, &cctx); line = compile_endfor(p, &cctx);
cctx.ctx_had_return = FALSE;
break; break;
case CMD_continue: case CMD_continue:
line = compile_continue(p, &cctx); line = compile_continue(p, &cctx);
@ -6894,12 +6937,15 @@ compile_def_function(ufunc_T *ufunc, int set_return_type, cctx_T *outer_cctx)
break; break;
case CMD_catch: case CMD_catch:
line = compile_catch(p, &cctx); line = compile_catch(p, &cctx);
cctx.ctx_had_return = FALSE;
break; break;
case CMD_finally: case CMD_finally:
line = compile_finally(p, &cctx); line = compile_finally(p, &cctx);
cctx.ctx_had_return = FALSE;
break; break;
case CMD_endtry: case CMD_endtry:
line = compile_endtry(p, &cctx); line = compile_endtry(p, &cctx);
cctx.ctx_had_return = FALSE;
break; break;
case CMD_throw: case CMD_throw:
line = compile_throw(p, &cctx); line = compile_throw(p, &cctx);
@ -6944,7 +6990,7 @@ compile_def_function(ufunc_T *ufunc, int set_return_type, cctx_T *outer_cctx)
goto erret; goto erret;
} }
if (!had_return) if (!cctx.ctx_had_return)
{ {
if (ufunc->uf_ret_type->tt_type != VAR_VOID) if (ufunc->uf_ret_type->tt_type != VAR_VOID)
{ {