Skip to content

[MERGED] Improvements for detection of unused assignments #480

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 12 commits into from
Closed

[MERGED] Improvements for detection of unused assignments #480

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
c4ab5f3
Warn if a value assigned to a variable is not used upon the next assi…
Daniel-Cortez Nov 14, 2019
b86e957
Update the array size test to accomodate for the new warning
Daniel-Cortez Nov 18, 2019
4a1e04a
Add tests
Daniel-Cortez Dec 3, 2019
d410d09
Suppress warning 240 for the cases whence the previous value is a zer…
Daniel-Cortez Dec 4, 2019
111a8f7
Issue warning 204 at the line where the assignment appears, not at th…
Daniel-Cortez Feb 13, 2020
5d3f51c
Don't issue warning 204 if the assignment operator is overloaded
Daniel-Cortez Feb 14, 2020
456e837
Detect more unused assignments for function arguments by marking them…
Daniel-Cortez Apr 8, 2020
8fae330
Clear assignments before the "else" branch, not after the "if" branch
Daniel-Cortez May 20, 2020
fcc47bd
Don't clear assignments after switch "case" branches
Daniel-Cortez May 20, 2020
35850a4
Clear the assignment flag for local variables when using `goto` on a …
Daniel-Cortez May 21, 2020
a4f87f8
Add more test cases
Daniel-Cortez May 21, 2020
375a095
Merge branch 'dev' into w240
Y-Less May 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion source/compiler/sc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,9 @@ typedef struct s_symbol {
* used during parsing a function, to detect a mix of "return;" and
* "return value;" in a few special cases.
*/
#define uRETNONE 0x10
#define uRETNONE 0x010
/* uASSIGNED indicates that a value assigned to the variable is not used yet */
#define uASSIGNED 0x080

#define flagDEPRECATED 0x01 /* symbol is deprecated (avoid use) */
#define flagNAKED 0x10 /* function is naked */
Expand Down Expand Up @@ -684,6 +686,8 @@ SC_FUNC void delete_symbol(symbol *root,symbol *sym);
SC_FUNC void delete_symbols(symbol *root,int level,int del_labels,int delete_functions);
SC_FUNC int refer_symbol(symbol *entry,symbol *bywhom);
SC_FUNC void markusage(symbol *sym,int usage);
SC_FUNC void markinitialized(symbol *sym,int assignment);
SC_FUNC void clearassignments(symbol *root);
SC_FUNC void rename_symbol(symbol *sym,const char *newname);
SC_FUNC symbol *findglb(const char *name,int filter);
SC_FUNC symbol *findloc(const char *name);
Expand Down Expand Up @@ -934,6 +938,7 @@ SC_VDECL short fnumber; /* number of files in the file table (debugging) *
SC_VDECL short fcurrent; /* current file being processed (debugging) */
SC_VDECL short sc_intest; /* true if inside a test */
SC_VDECL int pc_sideeffect; /* true if an expression causes a side-effect */
SC_VDECL int pc_ovlassignment;/* true if an expression contains an overloaded assignment */
SC_VDECL int stmtindent; /* current indent of the statement */
SC_VDECL int indent_nowarn; /* skip warning "217 loose indentation" */
SC_VDECL int sc_tabsize; /* number of spaces that a TAB represents */
Expand Down
79 changes: 59 additions & 20 deletions source/compiler/sc1.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ static void decl_const(int table);
static void decl_enum(int table,int fstatic);
static cell needsub(int *tag,constvalue_root **enumroot);
static void initials(int ident,int tag,cell *size,int dim[],int numdim,
constvalue_root *enumroot);
constvalue_root *enumroot,int *explicit_init);
static cell initarray(int ident,int tag,int dim[],int numdim,int cur,
int startlit,int counteddim[],constvalue_root *lastdim,
constvalue_root *enumroot,int *errorfound);
Expand All @@ -123,7 +123,7 @@ static void statement(int *lastindent,int allow_decl);
static void compound(int stmt_sameline,int starttok);
static int test(int label,int parens,int invert);
static int doexpr(int comma,int chkeffect,int allowarray,int mark_endexpr,
int *tag,symbol **symptr,int chkfuncresult);
int *tag,symbol **symptr,int chkfuncresult,cell *val);
static void doassert(void);
static void doexit(void);
static int doif(void);
Expand Down Expand Up @@ -893,6 +893,7 @@ static void resetglobals(void)
fcurrent=0; /* current file being processed (debugging) */
sc_intest=FALSE; /* true if inside a test */
pc_sideeffect=FALSE; /* true if an expression causes a side-effect */
pc_ovlassignment=FALSE;/* true if an expression contains an overloaded assignment */
stmtindent=0; /* current indent of the statement */
indent_nowarn=FALSE; /* do not skip warning "217 loose indentation" */
sc_allowtags=TRUE; /* allow/detect tagnames */
Expand Down Expand Up @@ -2018,6 +2019,7 @@ static void declglb(char *firstname,int firsttag,int fpublic,int fstatic,int fst
char *str;
int dim[sDIMEN_MAX];
int numdim;
int explicit_init;
short filenum;
symbol *sym;
constvalue_root *enumroot=NULL;
Expand Down Expand Up @@ -2122,7 +2124,7 @@ static void declglb(char *firstname,int firsttag,int fpublic,int fstatic,int fst
litidx=0; /* global initial data is dumped, so restart at zero */
} /* if */
assert(litidx==0); /* literal queue should be empty (again) */
initials(ident,tag,&size,dim,numdim,enumroot);/* stores values in the literal queue */
initials(ident,tag,&size,dim,numdim,enumroot,&explicit_init);/* stores values in the literal queue */
assert(size>=litidx);
if (numdim==1)
dim[0]=(int)size;
Expand Down Expand Up @@ -2246,6 +2248,8 @@ static void declglb(char *firstname,int firsttag,int fpublic,int fstatic,int fst
sym->usage|=uSTOCK;
if (fstatic)
sym->fnumber=filenum;
if (explicit_init)
markinitialized(sym,TRUE);
sc_attachdocumentation(sym);/* attach any documenation to the variable */
if (sc_status==statSKIP) {
sc_status=statWRITE;
Expand Down Expand Up @@ -2282,6 +2286,8 @@ static int declloc(int fstatic)
int numdim;
int fconst;
int staging_start;
int explicit_init; /* is the variable explicitly initialized? */
int suppress_w240=FALSE;

fconst=matchtoken(tCONST);
do {
Expand Down Expand Up @@ -2332,7 +2338,7 @@ static int declloc(int fstatic)
sc_alignnext=FALSE;
} /* if */
cur_lit=litidx; /* save current index in the literal table */
initials(ident,tag,&size,dim,numdim,enumroot);
initials(ident,tag,&size,dim,numdim,enumroot,&explicit_init);
if (size==0)
return ident; /* error message already given */
if (numdim==1)
Expand Down Expand Up @@ -2371,13 +2377,16 @@ static int declloc(int fstatic)
if (ident==iVARIABLE) {
/* simple variable, also supports initialization */
int ctag = tag; /* set to "tag" by default */
int explicit_init=FALSE;/* is the variable explicitly initialized? */
explicit_init=FALSE;
if (matchtoken('=')) {
int initexpr_ident;
cell val;
sym->usage &= ~uDEFINE; /* temporarily mark the variable as undefined to prevent
* possible self-assignment through its initialization expression */
doexpr(FALSE,FALSE,FALSE,FALSE,&ctag,NULL,TRUE);
initexpr_ident=doexpr(FALSE,FALSE,FALSE,FALSE,&ctag,NULL,TRUE,&val);
sym->usage |= uDEFINE;
explicit_init=TRUE;
suppress_w240=(initexpr_ident==iCONSTEXPR && val==0);
} else {
ldconst(0,sPRI); /* uninitialized variable, set to zero */
} /* if */
Expand All @@ -2386,7 +2395,7 @@ static int declloc(int fstatic)
lval.ident=iVARIABLE;
lval.constval=0;
lval.tag=tag;
check_userop(NULL,ctag,lval.tag,2,NULL,&ctag);
suppress_w240 |= check_userop(NULL,ctag,lval.tag,2,NULL,&ctag);
store(&lval);
markexpr(sEXPR,NULL,0); /* full expression ends after the store */
assert(staging); /* end staging phase (optimize expression) */
Expand Down Expand Up @@ -2430,6 +2439,10 @@ static int declloc(int fstatic)
} /* if */
} /* if */
} /* if */
if (explicit_init)
markinitialized(sym,!suppress_w240);
if (pc_ovlassignment)
sym->usage |= uREAD;
} while (matchtoken(',')); /* enddo */ /* more? */
needtoken(tTERM); /* if not comma, must be semicolumn */
return ident;
Expand Down Expand Up @@ -2510,14 +2523,16 @@ static int base;
* Global references: litidx (altered)
*/
static void initials(int ident,int tag,cell *size,int dim[],int numdim,
constvalue_root *enumroot)
constvalue_root *enumroot,int *explicit_init)
{
int ctag;
cell tablesize;
int curlit=litidx;
int err=0;
int i;

if (explicit_init!=NULL)
*explicit_init=FALSE;
if (!matchtoken('=')) {
assert(ident!=iARRAY || numdim>0);
if (ident==iARRAY) {
Expand Down Expand Up @@ -2548,6 +2563,8 @@ static void initials(int ident,int tag,cell *size,int dim[],int numdim,
return;
} /* if */

if (explicit_init!=NULL)
*explicit_init=TRUE;
if (ident==iVARIABLE) {
assert(*size==1);
init(ident,&ctag,NULL);
Expand Down Expand Up @@ -4203,7 +4220,7 @@ static void doarg(char *name,int ident,int offset,int tags[],int numtags,
lexpush(); /* initials() needs the "=" token again */
assert(litidx==0); /* at the start of a function, this is reset */
assert(numtags>0);
initials(ident,tags[0],&size,arg->dim,arg->numdim,enumroot);
initials(ident,tags[0],&size,arg->dim,arg->numdim,enumroot,NULL);
assert(size>=litidx);
/* allocate memory to hold the initial values */
arg->defvalue.array.data=(cell *)malloc(litidx*sizeof(cell));
Expand Down Expand Up @@ -4291,7 +4308,9 @@ static void doarg(char *name,int ident,int offset,int tags[],int numtags,
argsym->usage|=uREAD; /* arguments of public functions are always "used" */
if(argsym->ident==iREFARRAY || argsym->ident==iREFERENCE)
argsym->usage|=uWRITTEN;
}
} else if (argsym->ident==iVARIABLE) {
argsym->usage|=uASSIGNED;
} /* if */

if (fconst)
argsym->usage|=uCONST;
Expand Down Expand Up @@ -5077,6 +5096,14 @@ static void destructsymbols(symbol *root,int level)
if ((opsym->usage & uNATIVE)!=0 && opsym->x.lib!=NULL)
opsym->x.lib->value += 1; /* increment "usage count" of the library */
} /* if */
/* check that the assigned value was used, but don't show the warning
* if the variable is completely unused (we already have warning 203 for that) */
if ((sym->usage & (uASSIGNED | uREAD | uWRITTEN))==(uASSIGNED | uREAD | uWRITTEN)
&& sym->vclass!=sSTATIC) {
errorset(sSETPOS,sym->lnumber);
error(204,sym->name); /* symbol is assigned a value that is never used */
errorset(sSETPOS,-1);
} /* if */
} /* if */
sym=sym->next;
} /* while */
Expand Down Expand Up @@ -5440,7 +5467,7 @@ static void statement(int *lastindent,int allow_decl)
default: /* non-empty expression */
sc_allowproccall=optproccall;
lexpush(); /* analyze token later */
doexpr(TRUE,TRUE,TRUE,TRUE,NULL,NULL,FALSE);
doexpr(TRUE,TRUE,TRUE,TRUE,NULL,NULL,FALSE,NULL);
needtoken(tTERM);
lastst=tEXPR;
sc_allowproccall=FALSE;
Expand Down Expand Up @@ -5510,11 +5537,10 @@ static void compound(int stmt_sameline,int starttok)
* Global references: stgidx (referred to only)
*/
static int doexpr(int comma,int chkeffect,int allowarray,int mark_endexpr,
int *tag,symbol **symptr,int chkfuncresult)
int *tag,symbol **symptr,int chkfuncresult,cell *val)
{
int index,ident;
int localstaging=FALSE;
cell val;

if (!staging) {
stgset(TRUE); /* start stage-buffering */
Expand All @@ -5528,7 +5554,8 @@ static int doexpr(int comma,int chkeffect,int allowarray,int mark_endexpr,
if (index!=stgidx)
markexpr(sEXPR,NULL,0);
pc_sideeffect=FALSE;
ident=expression(&val,tag,symptr,chkfuncresult);
pc_ovlassignment=FALSE;
ident=expression(val,tag,symptr,chkfuncresult);
if (!allowarray && (ident==iARRAY || ident==iREFARRAY))
error(33,"-unknown-"); /* array must be indexed */
if (chkeffect && !pc_sideeffect)
Expand Down Expand Up @@ -5668,6 +5695,7 @@ static int doif(void)
ifindent=stmtindent; /* save the indent of the "if" instruction */
flab1=getlabel(); /* get label number for false branch */
test(flab1,TEST_THEN,FALSE); /* get expression, branch to flab1 if false */
clearassignments(&loctab);
statement(NULL,FALSE); /* if true, do a statement */
if (!matchtoken(tELSE)) { /* if...else ? */
setlabel(flab1); /* no, simple if..., print false label */
Expand All @@ -5677,6 +5705,7 @@ static int doif(void)
* has a lower indent than the matching "if" */
if (stmtindent<ifindent && sc_tabsize>0)
error(217); /* loose indentation */
clearassignments(&loctab);
flab2=getlabel();
if ((lastst!=tRETURN) && (lastst!=tGOTO))
jumplabel(flab2); /* "true" branch jumps around "else" clause, unless the "true" branch statement already jumped */
Expand Down Expand Up @@ -5707,7 +5736,9 @@ static int dowhile(void)
*/
setline(TRUE);
endlessloop=test(wq[wqEXIT],TEST_DO,FALSE);/* branch to wq[wqEXIT] if false */
clearassignments(&loctab);
statement(NULL,FALSE); /* if so, do a statement */
clearassignments(&loctab);
jumplabel(wq[wqLOOP]); /* and loop to "while" start */
setlabel(wq[wqEXIT]); /* exit label */
delwhile(); /* delete queue entry */
Expand All @@ -5730,7 +5761,9 @@ static int dodo(void)
addwhile(wq); /* see "dowhile" for more info */
top=getlabel(); /* make a label first */
setlabel(top); /* loop label */
clearassignments(&loctab);
statement(NULL,FALSE);
clearassignments(&loctab);
needtoken(tWHILE);
setlabel(wq[wqLOOP]); /* "continue" always jumps to WQLOOP. */
setline(TRUE);
Expand Down Expand Up @@ -5769,7 +5802,7 @@ static int dofor(void)
nestlevel++;
declloc(FALSE); /* declare local variable */
} else {
doexpr(TRUE,TRUE,TRUE,TRUE,NULL,NULL,FALSE); /* expression 1 */
doexpr(TRUE,TRUE,TRUE,TRUE,NULL,NULL,FALSE,NULL); /* expression 1 */
needtoken(';');
} /* if */
} /* if */
Expand Down Expand Up @@ -5804,13 +5837,15 @@ static int dofor(void)
} /* if */
stgmark((char)(sEXPRSTART+1)); /* mark start of 3th expression in stage */
if (!matchtoken(endtok)) {
doexpr(TRUE,TRUE,TRUE,TRUE,NULL,NULL,FALSE); /* expression 3 */
doexpr(TRUE,TRUE,TRUE,TRUE,NULL,NULL,FALSE,NULL); /* expression 3 */
needtoken(endtok);
} /* if */
stgmark(sENDREORDER); /* mark end of reversed evaluation */
stgout(index);
stgset(FALSE); /* stop staging */
clearassignments(&loctab);
statement(NULL,FALSE);
clearassignments(&loctab);
jumplabel(wq[wqLOOP]);
setlabel(wq[wqEXIT]);
delwhile();
Expand Down Expand Up @@ -5858,7 +5893,7 @@ static void doswitch(void)
char labelname[sNAMEMAX+1];

endtok= matchtoken('(') ? ')' : tDO;
doexpr(TRUE,FALSE,FALSE,FALSE,&swtag,NULL,TRUE);/* evaluate switch expression */
doexpr(TRUE,FALSE,FALSE,FALSE,&swtag,NULL,TRUE,NULL);/* evaluate switch expression */
needtoken(endtok);
/* generate the code for the switch statement, the label is the address
* of the case table (to be generated later).
Expand All @@ -5880,6 +5915,7 @@ static void doswitch(void)
tok=lex(&val,&str); /* read in (new) token */
switch (tok) {
case tCASE:
clearassignments(&loctab);
if (swdefault!=FALSE)
error(15); /* "default" case must be last in switch statement */
lbl_case=getlabel();
Expand Down Expand Up @@ -5947,6 +5983,7 @@ static void doswitch(void)
jumplabel(lbl_exit);
break;
case tDEFAULT:
clearassignments(&loctab);
if (swdefault!=FALSE)
error(16); /* multiple defaults in switch */
lbl_case=getlabel();
Expand Down Expand Up @@ -6029,6 +6066,8 @@ static void dogoto(void)

if (lex(&val,&st)==tSYMBOL) {
sym=fetchlab(st);
if ((sym->usage & uDEFINE)!=0)
clearassignments(&loctab);
jumplabel((int)sym->addr);
sym->usage|=uREAD; /* set "uREAD" bit */
// ??? if the label is defined (check sym->usage & uDEFINE), check
Expand Down Expand Up @@ -7587,7 +7626,7 @@ static void doreturn(void)
/* "return <value>" */
if ((rettype & uRETNONE)!=0)
error(78); /* mix "return;" and "return value;" */
ident=doexpr(TRUE,FALSE,TRUE,FALSE,&tag,&sym,TRUE);
ident=doexpr(TRUE,FALSE,TRUE,FALSE,&tag,&sym,TRUE,NULL);
needtoken(tTERM);
/* see if this function already has a sub type (an array attached) */
assert(curfunc!=NULL);
Expand Down Expand Up @@ -7759,7 +7798,7 @@ static void doexit(void)
int tag=0;

if (matchtoken(tTERM)==0){
doexpr(TRUE,FALSE,FALSE,FALSE,&tag,NULL,TRUE);
doexpr(TRUE,FALSE,FALSE,FALSE,&tag,NULL,TRUE,NULL);
needtoken(tTERM);
} else {
ldconst(0,sPRI);
Expand All @@ -7775,7 +7814,7 @@ static void dosleep(void)
int tag=0;

if (matchtoken(tTERM)==0){
doexpr(TRUE,FALSE,FALSE,FALSE,&tag,NULL,TRUE);
doexpr(TRUE,FALSE,FALSE,FALSE,&tag,NULL,TRUE,NULL);
needtoken(tTERM);
} else {
ldconst(0,sPRI);
Expand Down
Loading