Warn when variables used inside of a loop condition aren't modified in the loop body

This commit is contained in:
Stanislav Gromov 2020-10-07 20:43:45 +08:00
parent 0b721bfa1c
commit e0cde0583e
6 changed files with 183 additions and 7 deletions

View File

@ -231,6 +231,16 @@ typedef struct s_symbol {
#define uRETNONE 0x010
/* uASSIGNED indicates that a value assigned to the variable is not used yet */
#define uASSIGNED 0x080
/* uLOOPVAR is set when a variable is read inside of a loop condition. This is
* used to detect situations when a variable is used in a loop condition, but
* not modified inside of a loop body. */
#define uLOOPVAR 0x1000
/* uNOLOOPVAR is set when a variable is
* * modified inside of a loop condition before being read, or
* * used in an enclosing loop and should be excluded from checks in an inner loop,
* so the compiler would know it shouldn't set the uLOOPVAR flag when the variable
* is read inside a loop condition */
#define uNOLOOPVAR 0x2000
#define flagDEPRECATED 0x01 /* symbol is deprecated (avoid use) */
#define flagNAKED 0x10 /* function is naked */
@ -305,7 +315,9 @@ typedef struct s_valuepair {
/* struct "symstate" is used to:
* * synchronize the status of assignments between all "if" branches or "switch"
* cases, so the compiler could detect unused assignments in all of those
* branches/cases, not only in the last one */
* branches/cases, not only in the last one;
* * back up the "uNOLOOPVAR" flag when scanning for variables that were used
* in a loop exit condition, but weren't modified inside the loop body */
typedef struct s_assigninfo {
int lnumber; /* line number of the first unused assignment made in one of
* the branches (used for error messages) */
@ -1011,6 +1023,8 @@ SC_VDECL int pc_retexpr; /* true if the current expression is a part of a "
SC_VDECL int pc_retheap; /* heap space (in bytes) to be manually freed when returning an array returned by another function */
SC_VDECL int pc_nestlevel; /* number of active (open) compound statements */
SC_VDECL unsigned int pc_attributes;/* currently set attribute flags (for the "__pragma" operator) */
SC_VDECL int pc_loopcond; /* true if the current expression is a loop condition */
SC_VDECL int pc_numloopvars; /* number of variables used inside a loop condition */
SC_VDECL constvalue_root sc_automaton_tab; /* automaton table */
SC_VDECL constvalue_root sc_state_tab; /* state table */

View File

@ -116,6 +116,8 @@ static void make_report(symbol *root,FILE *log,char *sourcefile);
static void reduce_referrers(symbol *root);
static long max_stacksize(symbol *root,int *recursion);
static int testsymbols(symbol *root,int level,int testlabs,int testconst);
static void scanloopvariables(symstate **loopvars);
static void testloopvariables(symstate *loopvars,int line);
static void destructsymbols(symbol *root,int level);
static constvalue *find_constval_byval(constvalue_root *table,cell val);
static symbol *fetchlab(char *name);
@ -919,6 +921,7 @@ static void resetglobals(void)
pc_naked=FALSE;
pc_retexpr=FALSE;
pc_attributes=0;
pc_loopcond=FALSE;
emit_flags=0;
emit_stgbuf_idx=-1;
}
@ -5174,6 +5177,105 @@ static int testsymbols(symbol *root,int level,int testlabs,int testconst)
return entry;
}
static void scanloopvariables(symstate **loopvars)
{
symbol *start,*sym;
int num;
/* error messages are only printed on the "writing" pass,
* so if we are not writing yet, then we have a quick exit */
if (sc_status!=statWRITE)
return;
/* if there's only one active loop entry (the current loop),
* then there's no enclosing loop and we have an early exit */
if (wqptr-wqSIZE==wq)
return;
/* skip labels */
start=&loctab;
while ((start=start->next)!=NULL && start->ident==iLABEL)
/* nothing */;
/* if there are no other local symbols, we have an early exit */
if (start==NULL)
return;
/* count the number of local symbols */
for (num=0,sym=start; sym!=NULL; num++,sym=sym->next)
/* nothing */;
assert(*loopvars==NULL);
assert(num!=0);
*loopvars=(symstate *)calloc((size_t)num,sizeof(symstate));
if (*loopvars==NULL)
error(103); /* insufficient memory */
for (num=0,sym=start; sym!=NULL; num++,sym=sym->next) {
/* if the variable already has the uLOOPVAR flag set (from being used
* in an enclosing loop), we have to set the uNOLOOPVAR to exclude it
* from checks in the current loop */
if ((sym->ident==iVARIABLE || sym->ident==iREFERENCE || sym->ident==iARRAY
|| sym->ident==iREFARRAY) && (sym->usage & uLOOPVAR)!=0) {
/* ... but it might be already set from an enclosing loop,
* so we need to temporarily store it in "loopvars[num]" first */
(*loopvars)[num].usage |= (sym->usage & uNOLOOPVAR);
sym->usage |= uNOLOOPVAR;
} /* if */
} /* if */
}
static void testloopvariables(symstate *loopvars,int line)
{
symbol *start,*sym;
int num,warnnum=0;
/* the error messages are only printed on the "writing" pass,
* so if we are not writing yet, then we have a quick exit */
if (sc_status!=statWRITE)
return;
/* skip labels */
start=&loctab;
while ((start=start->next)!=NULL && start->ident==iLABEL)
/* nothing */;
/* decrement pc_numloopvars by 1 for each variable that wasn't modified
* inside the loop body; if pc_numloopvars gets zeroed after this, it would
* mean none of the variables used inside the loop condition were modified */
if (pc_numloopvars!=0) {
warnnum=(pc_numloopvars==1) ? 250 : 251;
for (sym=start; sym!=NULL; sym=sym->next)
if ((sym->ident==iVARIABLE || sym->ident==iREFERENCE || sym->ident==iARRAY
|| sym->ident==iREFARRAY) && (sym->usage & (uLOOPVAR | uNOLOOPVAR))==uLOOPVAR)
pc_numloopvars--;
if (pc_numloopvars==0 && warnnum==251) {
errorset(sSETPOS,line);
error(251); /* none of the variables used in loop condition are modified in loop body */
errorset(sSETPOS,-1);
} /* if */
} /* if */
for (num=0,sym=start; sym!=NULL; num++,sym=sym->next) {
if (sym->ident==iVARIABLE || sym->ident==iREFERENCE
|| sym->ident==iARRAY || sym->ident==iREFARRAY) {
if ((sym->usage & (uLOOPVAR | uNOLOOPVAR))==uLOOPVAR) {
sym->usage &= ~uLOOPVAR;
/* warn only if none of the variables used inside the loop condition
* were modified inside the loop body */
if (pc_numloopvars==0 && warnnum==250) {
errorset(sSETPOS,line);
error(250,sym->name); /* variable used in loop condition not modified in loop body */
errorset(sSETPOS,-1);
} /* if */
} /* if */
sym->usage &= ~uNOLOOPVAR;
if (loopvars!=NULL)
sym->usage |= (loopvars[num].usage & uNOLOOPVAR);
} /* if */
} /* for */
free(loopvars);
}
static cell calc_array_datasize(symbol *sym, cell *offset)
{
cell length;
@ -5925,9 +6027,13 @@ static int doif(void)
static int dowhile(void)
{
int wq[wqSIZE]; /* allocate local queue */
int save_endlessloop,retcode;
int save_endlessloop,save_numloopvars,retcode;
int loopline=fline;
symstate *loopvars=NULL;
save_endlessloop=endlessloop;
save_numloopvars=pc_numloopvars;
pc_numloopvars=0;
addwhile(wq); /* add entry to queue for "break" */
setlabel(wq[wqLOOP]); /* loop label */
/* The debugger uses the "break" opcode to be able to "break" out of
@ -5935,18 +6041,23 @@ static int dowhile(void)
* tiniest loop, set it below the top of the loop
*/
setline(TRUE);
scanloopvariables(&loopvars);
pc_nestlevel++; /* temporarily increase the "compound statement" nesting level,
* so any assignments made inside the loop control expression
* could be cleaned up later */
pc_loopcond=TRUE;
endlessloop=test(wq[wqEXIT],TEST_DO,FALSE);/* branch to wq[wqEXIT] if false */
pc_loopcond=FALSE;
pc_nestlevel--;
statement(NULL,FALSE); /* if so, do a statement */
clearassignments(pc_nestlevel+1);
testloopvariables(loopvars,loopline);
jumplabel(wq[wqLOOP]); /* and loop to "while" start */
setlabel(wq[wqEXIT]); /* exit label */
delwhile(); /* delete queue entry */
retcode=endlessloop ? tENDLESS : tWHILE;
pc_numloopvars=save_numloopvars;
endlessloop=save_endlessloop;
return retcode;
}
@ -5959,6 +6070,8 @@ static int dodo(void)
{
int wq[wqSIZE],top;
int save_endlessloop,retcode;
int loopline=fline;
symstate *loopvars=NULL;
save_endlessloop=endlessloop;
addwhile(wq); /* see "dowhile" for more info */
@ -5968,12 +6081,14 @@ static int dodo(void)
needtoken(tWHILE);
setlabel(wq[wqLOOP]); /* "continue" always jumps to WQLOOP. */
setline(TRUE);
scanloopvariables(&loopvars);
pc_nestlevel++; /* temporarily increase the "compound statement" nesting level,
* so any assignments made inside the loop control expression
* could be cleaned up later */
endlessloop=test(wq[wqEXIT],TEST_OPT,FALSE);
pc_nestlevel--;
clearassignments(pc_nestlevel+1);
testloopvariables(loopvars,loopline);
jumplabel(top);
setlabel(wq[wqEXIT]);
delwhile();
@ -5988,13 +6103,17 @@ static int dofor(void)
{
int wq[wqSIZE],skiplab;
cell save_decl;
int save_nestlevel,save_endlessloop;
int save_nestlevel,save_endlessloop,save_numloopvars;
int index,endtok;
int *ptr;
int loopline=fline;
symstate *loopvars=NULL;
save_decl=declared;
save_nestlevel=pc_nestlevel;
save_endlessloop=endlessloop;
save_numloopvars=pc_numloopvars;
pc_numloopvars=0;
addwhile(wq);
skiplab=getlabel();
@ -6027,6 +6146,7 @@ static int dofor(void)
jumplabel(skiplab); /* skip expression 3 1st time */
setlabel(wq[wqLOOP]); /* "continue" goes to this label: expr3 */
setline(TRUE);
scanloopvariables(&loopvars);
/* Expressions 2 and 3 are reversed in the generated code: expression 3
* precedes expression 2. When parsing, the code is buffered and marks for
* the start of each expression are insterted in the buffer.
@ -6041,7 +6161,9 @@ static int dofor(void)
if (matchtoken(';')) {
endlessloop=1;
} else {
pc_loopcond=TRUE;
endlessloop=test(wq[wqEXIT],TEST_PLAIN,FALSE);/* expression 2 (jump to wq[wqEXIT] if false) */
pc_loopcond=FALSE;
needtoken(';');
} /* if */
stgmark((char)(sEXPRSTART+1)); /* mark start of 3th expression in stage */
@ -6054,6 +6176,7 @@ static int dofor(void)
stgset(FALSE); /* stop staging */
statement(NULL,FALSE);
clearassignments(save_nestlevel+1);
testloopvariables(loopvars,loopline);
jumplabel(wq[wqLOOP]);
setlabel(wq[wqEXIT]);
delwhile();
@ -6072,6 +6195,7 @@ static int dofor(void)
pc_nestlevel=save_nestlevel; /* reset 'compound statement' nesting level */
index=endlessloop ? tENDLESS : tFOR;
pc_numloopvars=save_numloopvars;
endlessloop=save_endlessloop;
return index;
}

View File

@ -55,6 +55,7 @@ static symbol *find_symbol(const symbol *root,const char *name,int fnumber,int a
static void substallpatterns(unsigned char *line,int buffersize);
static int match(char *st,int end);
static int alpha(char c);
static void markloopvariable(symbol *sym,int usage);
#define SKIPMODE 1 /* bit field in "#if" stack */
#define PARSEMODE 2 /* bit field in "#if" stack */
@ -3308,6 +3309,11 @@ SC_FUNC void markusage(symbol *sym,int usage)
sym->lnumber=fline;
if ((usage & uREAD)!=0 && (sym->ident==iVARIABLE || sym->ident==iREFERENCE))
sym->usage &= ~uASSIGNED;
if ((usage & (uREAD | uWRITTEN))!=0
&& (sym->vclass==sLOCAL || sym->vclass==sSTATIC)
&& (sym->ident==iVARIABLE || sym->ident==iREFERENCE
|| sym->ident==iARRAY || sym->ident==iREFARRAY))
markloopvariable(sym,usage);
/* check if (global) reference must be added to the symbol */
if ((usage & (uREAD | uWRITTEN))!=0) {
/* only do this for global symbols */
@ -3338,7 +3344,7 @@ SC_FUNC void clearassignments(int fromlevel)
{
symbol *sym;
/* the error messages are only printed on the "writing" pass,
/* error messages are only printed on the "writing" pass,
* so if we are not writing yet, then we have a quick exit */
if (sc_status!=statWRITE)
return;
@ -3355,7 +3361,7 @@ SC_FUNC void memoizeassignments(int fromlevel,symstate **assignments)
symbol *sym;
int num;
/* the error messages are only printed on the "writing" pass,
/* error messages are only printed on the "writing" pass,
* so if we are not writing yet, then we have a quick exit */
if (sc_status!=statWRITE)
return;
@ -3415,6 +3421,28 @@ SC_FUNC void restoreassignments(int fromlevel,symstate *assignments)
free(assignments);
}
static void markloopvariable(symbol *sym,int usage)
{
while (sym->parent!=NULL)
sym=sym->parent;
/* check if the variable used inside a loop condition */
if (pc_loopcond) {
if ((usage & uWRITTEN)!=0) {
/* the symbol is being modified inside a loop condition before being read;
* set the uNOLOOPVAR flag, so later we'll know we shouldn't mark the symbol
* with the uLOOPVAR flag */
sym->usage |= uNOLOOPVAR;
pc_numloopvars++;
} else if ((usage & uREAD)!=0 && (sym->usage & (uNOLOOPVAR | uLOOPVAR))==0) {
sym->usage |= uLOOPVAR;
pc_numloopvars++;
} /* if */
} /* if */
/* unset the uLOOPVAR flag if the variable is being modified */
if ((usage & uWRITTEN)!=0)
sym->usage &= ~uLOOPVAR;
}
/* findglb
*

View File

@ -2177,6 +2177,7 @@ static int nesting=0;
int nargs=0; /* number of arguments */
int heapalloc=0;
int namedparams=FALSE;
int save_loopcond;
value lval = {0};
arginfo *arg;
char arglist[sMAXARGS];
@ -2195,7 +2196,11 @@ static int nesting=0;
/* functions cannot be called at global scope */
error(29); /* invalid expression, assumed zero */
return;
}
} /* if */
/* make the compiler not count variables passed as function arguments
* as being used inside a loop condition */
save_loopcond=pc_loopcond;
pc_loopcond=FALSE;
/* check whether this is a function that returns an array */
symret=finddepend(sym);
assert(symret==NULL || symret->ident==iREFARRAY);
@ -2609,6 +2614,7 @@ static int nesting=0;
if (symret!=NULL)
popreg(sPRI); /* pop hidden parameter as function result */
pc_sideeffect=TRUE; /* assume functions carry out a side-effect */
pc_loopcond=save_loopcond;
delete_consttable(&arrayszlst); /* clear list of array sizes */
delete_consttable(&taglst); /* clear list of parameter tags */

View File

@ -207,7 +207,9 @@ static char *warnmsg[] = {
/*246*/ "multiplication overflow in enum element declaration (symbol \"%s\")\n",
/*247*/ "use of operator \"~\" on a \"bool:\" value always results in \"true\"\n",
/*248*/ "possible misuse of comma operator\n",
/*249*/ ""
/*249*/ "",
/*250*/ "variable \"%s\" used in loop condition not modified in loop body\n",
/*251*/ "none of the variables used in loop condition are modified in loop body\n"
};
static char *noticemsg[] = {

View File

@ -102,6 +102,8 @@ SC_VDEFINE int pc_retexpr=FALSE; /* true if the current expression is
SC_VDEFINE int pc_retheap=0; /* heap space (in bytes) to be manually freed when returning an array returned by another function */
SC_VDEFINE int pc_nestlevel=0; /* number of active (open) compound statements */
SC_VDEFINE unsigned int pc_attributes=0; /* currently set attribute flags (for the "__pragma" operator) */
SC_VDEFINE int pc_loopcond=FALSE; /* true if the current expression is a loop condition */
SC_VDEFINE int pc_numloopvars=0; /* number of variables used inside a loop condition */
SC_VDEFINE constvalue_root sc_automaton_tab = { NULL, NULL}; /* automaton table */
SC_VDEFINE constvalue_root sc_state_tab = { NULL, NULL}; /* state table */