From c4ab5f38bd5eb80ead755a157ac08840dc59029c Mon Sep 17 00:00:00 2001 From: Stanislav Gromov Date: Thu, 14 Nov 2019 20:58:16 +0700 Subject: [PATCH 01/11] Warn if a value assigned to a variable is not used upon the next assignment or symbol destruction --- source/compiler/sc.h | 6 +++++- source/compiler/sc1.c | 37 +++++++++++++++++++++++++++++++------ source/compiler/sc2.c | 30 +++++++++++++++++++++++++++++- source/compiler/sc3.c | 9 +++++++++ source/compiler/sc5.c | 3 ++- 5 files changed, 76 insertions(+), 9 deletions(-) diff --git a/source/compiler/sc.h b/source/compiler/sc.h index 86703ad6..6ea19a0f 100644 --- a/source/compiler/sc.h +++ b/source/compiler/sc.h @@ -243,7 +243,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 */ @@ -667,6 +669,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); diff --git a/source/compiler/sc1.c b/source/compiler/sc1.c index 83ded706..7c3ed0a2 100644 --- a/source/compiler/sc1.c +++ b/source/compiler/sc1.c @@ -96,7 +96,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); @@ -2004,6 +2004,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; @@ -2108,7 +2109,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; @@ -2232,6 +2233,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; @@ -2268,6 +2271,7 @@ static int declloc(int fstatic) int numdim; int fconst; int staging_start; + int explicit_init; /* is the variable explicitly initialized? */ fconst=matchtoken(tCONST); do { @@ -2318,7 +2322,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) @@ -2357,7 +2361,7 @@ 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('=')) { sym->usage &= ~uDEFINE; /* temporarily mark the variable as undefined to prevent * possible self-assignment through its initialization expression */ @@ -2416,6 +2420,8 @@ static int declloc(int fstatic) } /* if */ } /* if */ } /* if */ + if (explicit_init) + markinitialized(sym,TRUE); } while (matchtoken(',')); /* enddo */ /* more? */ needtoken(tTERM); /* if not comma, must be semicolumn */ return ident; @@ -2496,7 +2502,7 @@ 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; @@ -2504,6 +2510,8 @@ static void initials(int ident,int tag,cell *size,int dim[],int numdim, int err=0; int i; + if (explicit_init!=NULL) + *explicit_init=FALSE; if (!matchtoken('=')) { assert(ident!=iARRAY || numdim>0); if (ident==iARRAY) { @@ -2534,6 +2542,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); @@ -4189,7 +4199,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)); @@ -5063,6 +5073,11 @@ 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) + error(204,sym->name); /* symbol is assigned a value that is never used */ } /* if */ sym=sym->next; } /* while */ @@ -5654,7 +5669,9 @@ 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 */ + clearassignments(&loctab); if (!matchtoken(tELSE)) { /* if...else ? */ setlabel(flab1); /* no, simple if..., print false label */ } else { @@ -5693,7 +5710,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 */ @@ -5716,7 +5735,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); @@ -5796,7 +5817,9 @@ static int dofor(void) 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(); @@ -5863,6 +5886,7 @@ static void doswitch(void) swdefault=FALSE; casecount=0; do { + clearassignments(&loctab); tok=lex(&val,&str); /* read in (new) token */ switch (tok) { case tCASE: @@ -5954,6 +5978,7 @@ static void doswitch(void) } /* if */ } /* switch */ } while (tok!=endtok); + clearassignments(&loctab); #if !defined NDEBUG /* verify that the case table is sorted (unfortunatly, duplicates can diff --git a/source/compiler/sc2.c b/source/compiler/sc2.c index 13d464ee..62813379 100644 --- a/source/compiler/sc2.c +++ b/source/compiler/sc2.c @@ -1011,6 +1011,7 @@ static int command(void) iflevel++; if (SKIPPING) break; /* break out of switch */ + clearassignments(&loctab); skiplevel=iflevel; preproc_expr(&val,NULL); /* get value (or 0 on error) */ ifstack[iflevel-1]=(char)(val ? PARSEMODE : SKIPMODE); @@ -1050,6 +1051,7 @@ static int command(void) } /* if */ } else { /* previous conditions were all FALSE */ + clearassignments(&loctab); if (tok==tpELSEIF) { /* if we were already skipping this section, allow expressions with * undefined symbols; otherwise check the expression to catch errors @@ -1076,6 +1078,7 @@ static int command(void) error(26); /* no matching "#if" */ errorset(sRESET,0); } else { + clearassignments(&loctab); iflevel--; if (iflevelusage |= read; if (sym->ident==iVARIABLE || sym->ident==iREFERENCE - || sym->ident==iARRAY || sym->ident==iREFARRAY) + || sym->ident==iARRAY || sym->ident==iREFARRAY) { sym->usage |= write; + sym->usage &= ~uASSIGNED; + } /* if */ } else { error(17,name); /* undefined symbol */ } /* if */ @@ -3178,6 +3183,8 @@ SC_FUNC void markusage(symbol *sym,int usage) sym->usage |= (char)usage; if ((usage & uWRITTEN)!=0) sym->lnumber=fline; + if ((usage & uREAD)!=0 && (sym->ident==iVARIABLE || sym->ident==iREFERENCE)) + sym->usage &= ~uASSIGNED; /* check if (global) reference must be added to the symbol */ if ((usage & (uREAD | uWRITTEN))!=0) { /* only do this for global symbols */ @@ -3190,6 +3197,27 @@ SC_FUNC void markusage(symbol *sym,int usage) } /* if */ } +SC_FUNC void markinitialized(symbol *sym,int assignment) +{ + assert(sym!=NULL); + if (sym->ident!=iVARIABLE && sym->ident!=iARRAY) + return; + if (sc_status==statFIRST && (sym->vclass==sLOCAL || sym->vclass==sSTATIC)) + return; + if (assignment && sym->ident==iVARIABLE) + sym->usage |= uASSIGNED; +} + +SC_FUNC void clearassignments(symbol *root) +{ + /* clear the unused assignment flag for all variables in the table */ + symbol *sym=root->next; + while (sym!=NULL) { + sym->usage &= ~uASSIGNED; + sym=sym->next; + } /* while */ +} + /* findglb * diff --git a/source/compiler/sc3.c b/source/compiler/sc3.c index 1185bcb2..111a4ead 100644 --- a/source/compiler/sc3.c +++ b/source/compiler/sc3.c @@ -823,6 +823,7 @@ static int hier14(value *lval1) int bwcount,leftarray; cell arrayidx1[sDIMEN_MAX],arrayidx2[sDIMEN_MAX]; /* last used array indices */ cell *org_arrayidx; + int assignment=FALSE; bwcount=bitwise_opercount; bitwise_opercount=0; @@ -878,6 +879,7 @@ static int hier14(value *lval1) break; case '=': /* simple assignment */ oper=NULL; + assignment=TRUE; if (sc_intest) error(211); /* possibly unintended assignment */ break; @@ -1063,6 +1065,13 @@ static int hier14(value *lval1) pc_sideeffect=TRUE; bitwise_opercount=bwcount; lval1->ident=iEXPRESSION; + if (assignment) { + symbol *sym=lval3.sym; + assert(sym!=NULL); + if ((sym->usage & uASSIGNED)!=0 && (sym->vclass==sLOCAL || sym->vclass==sSTATIC)) + error(240,sym->name); /* previously assigned value is unused */ + markinitialized(sym,TRUE); + } /* if */ return FALSE; /* expression result is never an lvalue */ } diff --git a/source/compiler/sc5.c b/source/compiler/sc5.c index df567f90..d03b272b 100644 --- a/source/compiler/sc5.c +++ b/source/compiler/sc5.c @@ -196,7 +196,8 @@ static char *warnmsg[] = { /*236*/ "unknown parameter in substitution (incorrect #define pattern)\n", /*237*/ "user warning: %s\n", /*238*/ "meaningless combination of class specifiers (%s)\n", -/*239*/ "literal array/string passed to a non-const parameter\n" +/*239*/ "literal array/string passed to a non-const parameter\n", +/*240*/ "previously assigned value is never used (symbol \"%s\")\n" }; static char *noticemsg[] = { From b86e95798024ec0c8eb0e1344a1071e292d619d0 Mon Sep 17 00:00:00 2001 From: Stanislav Gromov Date: Mon, 18 Nov 2019 20:20:33 +0700 Subject: [PATCH 02/11] Update the array size test to accomodate for the new warning --- source/compiler/tests/md_array_size_chk_gh_314.pwn | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/source/compiler/tests/md_array_size_chk_gh_314.pwn b/source/compiler/tests/md_array_size_chk_gh_314.pwn index f926648c..c1a5c0e1 100644 --- a/source/compiler/tests/md_array_size_chk_gh_314.pwn +++ b/source/compiler/tests/md_array_size_chk_gh_314.pwn @@ -28,9 +28,9 @@ main () { arr5[0][0] = 0; new a = sizeof(arr1); - a = sizeof(arr1[]); - a = sizeof(arr5[][]); - #pragma unused a + new b = sizeof(arr1[]); + new c = sizeof(arr5[][]); + #pragma unused a, b, c f1(arr1); f2(arr2); From 4a1e04a137d3f6ec0cc597b23d9e6d36e00c61d5 Mon Sep 17 00:00:00 2001 From: Stanislav Gromov Date: Tue, 3 Dec 2019 22:15:46 +0700 Subject: [PATCH 03/11] Add tests --- source/compiler/tests/warning_240.meta | 12 ++++++ source/compiler/tests/warning_240.pwn | 60 ++++++++++++++++++++++++++ 2 files changed, 72 insertions(+) create mode 100644 source/compiler/tests/warning_240.meta create mode 100644 source/compiler/tests/warning_240.pwn diff --git a/source/compiler/tests/warning_240.meta b/source/compiler/tests/warning_240.meta new file mode 100644 index 00000000..9eac0561 --- /dev/null +++ b/source/compiler/tests/warning_240.meta @@ -0,0 +1,12 @@ +{ + 'test_type': 'output_check', + 'errors': """ +warning_240.pwn(11) : warning 240: previously assigned value is never used (symbol "local_var") +warning_240.pwn(12) : warning 240: previously assigned value is never used (symbol "local_var") +warning_240.pwn(15) : warning 204: symbol is assigned a value that is never used: "local_var" +warning_240.pwn(22) : warning 240: previously assigned value is never used (symbol "local_static_var") +warning_240.pwn(23) : warning 240: previously assigned value is never used (symbol "local_static_var") +warning_240.pwn(49) : warning 240: previously assigned value is never used (symbol "arg") +warning_240.pwn(49) : warning 204: symbol is assigned a value that is never used: "arg" +""" +} diff --git a/source/compiler/tests/warning_240.pwn b/source/compiler/tests/warning_240.pwn new file mode 100644 index 00000000..432d8316 --- /dev/null +++ b/source/compiler/tests/warning_240.pwn @@ -0,0 +1,60 @@ +#include +#include + +new global_var; +static global_static_var; + +test_local() +{ + new local_var; + local_var = 1; + local_var = 2; // warning + local_var = 3; // warning + printf("x: %d\n", local_var); + local_var = 4; +} // warning (value assigned to "local_var" + // wasn't used upon symbol destruction) + +test_local_static() +{ + static local_static_var; + local_static_var = 1; + local_static_var = 2; // warning + local_static_var = 3; // warning + printf("x: %d\n", local_static_var); + local_static_var = 4; +} + +test_global() +{ + global_var = 1; + global_var = 2; + global_var = 3; + printf("x: %d\n", global_var); + global_var = 4; +} + +test_global_static() +{ + global_static_var = 1; + global_static_var = 2; + global_static_var = 3; + printf("x: %d\n", global_static_var); + global_static_var = 4; +} + +test_arg(arg) +{ + arg = 0; + arg = 1; // warning (240, 204) +} + +main() +{ + test_local(); + test_local_static(); + test_global(); + test_global_static(); + new x = 0; + test_arg(x); +} From d410d09dc22f2f02b7237201e3ecbb950d9ad810 Mon Sep 17 00:00:00 2001 From: Stanislav Gromov Date: Wed, 4 Dec 2019 18:53:10 +0700 Subject: [PATCH 04/11] Suppress warning 240 for the cases whence the previous value is a zero value assigned at variable definition --- source/compiler/sc1.c | 29 ++++++++++++++------------ source/compiler/tests/warning_240.meta | 10 ++++----- source/compiler/tests/warning_240.pwn | 16 +++++++------- 3 files changed, 30 insertions(+), 25 deletions(-) diff --git a/source/compiler/sc1.c b/source/compiler/sc1.c index 7c3ed0a2..33b9d7a8 100644 --- a/source/compiler/sc1.c +++ b/source/compiler/sc1.c @@ -121,7 +121,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); @@ -2272,6 +2272,7 @@ static int declloc(int fstatic) int fconst; int staging_start; int explicit_init; /* is the variable explicitly initialized? */ + int suppress_w240=FALSE; fconst=matchtoken(tCONST); do { @@ -2363,11 +2364,14 @@ static int declloc(int fstatic) int ctag = tag; /* set to "tag" by default */ 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 */ @@ -2421,7 +2425,7 @@ static int declloc(int fstatic) } /* if */ } /* if */ if (explicit_init) - markinitialized(sym,TRUE); + markinitialized(sym,!suppress_w240); } while (matchtoken(',')); /* enddo */ /* more? */ needtoken(tTERM); /* if not comma, must be semicolumn */ return ident; @@ -5441,7 +5445,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; @@ -5511,11 +5515,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 */ @@ -5529,7 +5532,7 @@ 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); + ident=expression(val,tag,symptr,chkfuncresult); if (!allowarray && (ident==iARRAY || ident==iREFARRAY)) error(33,"-unknown-"); /* array must be indexed */ if (chkeffect && !pc_sideeffect) @@ -5776,7 +5779,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 */ @@ -5811,7 +5814,7 @@ 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 */ @@ -5867,7 +5870,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). @@ -7598,7 +7601,7 @@ static void doreturn(void) /* "return " */ 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); @@ -7770,7 +7773,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); @@ -7786,7 +7789,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); diff --git a/source/compiler/tests/warning_240.meta b/source/compiler/tests/warning_240.meta index 9eac0561..02d7a5e1 100644 --- a/source/compiler/tests/warning_240.meta +++ b/source/compiler/tests/warning_240.meta @@ -3,10 +3,10 @@ 'errors': """ warning_240.pwn(11) : warning 240: previously assigned value is never used (symbol "local_var") warning_240.pwn(12) : warning 240: previously assigned value is never used (symbol "local_var") -warning_240.pwn(15) : warning 204: symbol is assigned a value that is never used: "local_var" -warning_240.pwn(22) : warning 240: previously assigned value is never used (symbol "local_static_var") -warning_240.pwn(23) : warning 240: previously assigned value is never used (symbol "local_static_var") -warning_240.pwn(49) : warning 240: previously assigned value is never used (symbol "arg") -warning_240.pwn(49) : warning 204: symbol is assigned a value that is never used: "arg" +warning_240.pwn(18) : warning 204: symbol is assigned a value that is never used: "local_var" +warning_240.pwn(24) : warning 240: previously assigned value is never used (symbol "local_static_var") +warning_240.pwn(25) : warning 240: previously assigned value is never used (symbol "local_static_var") +warning_240.pwn(51) : warning 240: previously assigned value is never used (symbol "arg") +warning_240.pwn(51) : warning 204: symbol is assigned a value that is never used: "arg" """ } diff --git a/source/compiler/tests/warning_240.pwn b/source/compiler/tests/warning_240.pwn index 432d8316..64f3eaf6 100644 --- a/source/compiler/tests/warning_240.pwn +++ b/source/compiler/tests/warning_240.pwn @@ -10,18 +10,20 @@ test_local() local_var = 1; local_var = 2; // warning local_var = 3; // warning - printf("x: %d\n", local_var); + #pragma unused local_var local_var = 4; + new local_var2 = 0; + local_var2 = 1; + #pragma unused local_var2 } // warning (value assigned to "local_var" // wasn't used upon symbol destruction) test_local_static() { - static local_static_var; - local_static_var = 1; + static local_static_var = 0; + local_static_var = 1; // warning local_static_var = 2; // warning - local_static_var = 3; // warning - printf("x: %d\n", local_static_var); + #pragma unused local_static_var local_static_var = 4; } @@ -30,7 +32,7 @@ test_global() global_var = 1; global_var = 2; global_var = 3; - printf("x: %d\n", global_var); + #pragma unused global_var global_var = 4; } @@ -39,7 +41,7 @@ test_global_static() global_static_var = 1; global_static_var = 2; global_static_var = 3; - printf("x: %d\n", global_static_var); + #pragma unused global_static_var global_static_var = 4; } From 111a8f7d2b997fe183533069c1bcc2c21c14f998 Mon Sep 17 00:00:00 2001 From: Stanislav Gromov Date: Thu, 13 Feb 2020 20:27:24 +0700 Subject: [PATCH 05/11] Issue warning 204 at the line where the assignment appears, not at the end of function --- source/compiler/sc1.c | 5 ++++- source/compiler/tests/warning_240.meta | 8 ++++---- source/compiler/tests/warning_240.pwn | 15 +++++++-------- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/source/compiler/sc1.c b/source/compiler/sc1.c index 33b9d7a8..169c8bd5 100644 --- a/source/compiler/sc1.c +++ b/source/compiler/sc1.c @@ -5080,8 +5080,11 @@ static void destructsymbols(symbol *root,int level) /* 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) + && 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 */ diff --git a/source/compiler/tests/warning_240.meta b/source/compiler/tests/warning_240.meta index 02d7a5e1..c13b38b0 100644 --- a/source/compiler/tests/warning_240.meta +++ b/source/compiler/tests/warning_240.meta @@ -3,10 +3,10 @@ 'errors': """ warning_240.pwn(11) : warning 240: previously assigned value is never used (symbol "local_var") warning_240.pwn(12) : warning 240: previously assigned value is never used (symbol "local_var") -warning_240.pwn(18) : warning 204: symbol is assigned a value that is never used: "local_var" +warning_240.pwn(14) : warning 204: symbol is assigned a value that is never used: "local_var" +warning_240.pwn(23) : warning 240: previously assigned value is never used (symbol "local_static_var") warning_240.pwn(24) : warning 240: previously assigned value is never used (symbol "local_static_var") -warning_240.pwn(25) : warning 240: previously assigned value is never used (symbol "local_static_var") -warning_240.pwn(51) : warning 240: previously assigned value is never used (symbol "arg") -warning_240.pwn(51) : warning 204: symbol is assigned a value that is never used: "arg" +warning_240.pwn(50) : warning 240: previously assigned value is never used (symbol "arg") +warning_240.pwn(50) : warning 204: symbol is assigned a value that is never used: "arg" """ } diff --git a/source/compiler/tests/warning_240.pwn b/source/compiler/tests/warning_240.pwn index 64f3eaf6..773deaa2 100644 --- a/source/compiler/tests/warning_240.pwn +++ b/source/compiler/tests/warning_240.pwn @@ -8,21 +8,20 @@ test_local() { new local_var; local_var = 1; - local_var = 2; // warning - local_var = 3; // warning + local_var = 2; // warning 240 + local_var = 3; // warning 240 #pragma unused local_var - local_var = 4; + local_var = 4; // warning 204 new local_var2 = 0; local_var2 = 1; #pragma unused local_var2 -} // warning (value assigned to "local_var" - // wasn't used upon symbol destruction) +} test_local_static() { static local_static_var = 0; - local_static_var = 1; // warning - local_static_var = 2; // warning + local_static_var = 1; // warning 240 + local_static_var = 2; // warning 240 #pragma unused local_static_var local_static_var = 4; } @@ -48,7 +47,7 @@ test_global_static() test_arg(arg) { arg = 0; - arg = 1; // warning (240, 204) + arg = 1; // warning 240, warning 204 } main() From 5d3f51c34835abd0077feee371ff32ae2352dff9 Mon Sep 17 00:00:00 2001 From: Stanislav Gromov Date: Sat, 15 Feb 2020 00:11:37 +0700 Subject: [PATCH 06/11] Don't issue warning 204 if the assignment operator is overloaded --- source/compiler/sc.h | 1 + source/compiler/sc1.c | 6 +++++- source/compiler/sc3.c | 8 +++++--- source/compiler/scvars.c | 1 + source/compiler/tests/warning_240.pwn | 13 +++++++++++++ 5 files changed, 25 insertions(+), 4 deletions(-) diff --git a/source/compiler/sc.h b/source/compiler/sc.h index 6ea19a0f..e8457b7d 100644 --- a/source/compiler/sc.h +++ b/source/compiler/sc.h @@ -925,6 +925,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 */ diff --git a/source/compiler/sc1.c b/source/compiler/sc1.c index 169c8bd5..a812bc98 100644 --- a/source/compiler/sc1.c +++ b/source/compiler/sc1.c @@ -891,6 +891,7 @@ static void resetglobals(void) fcurrent=0; /* current file being processed (debugging) */ sc_intest=FALSE; /* true if inside a test */ pc_sideeffect=0; /* 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 */ @@ -2380,7 +2381,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) */ @@ -2426,6 +2427,8 @@ static int declloc(int fstatic) } /* 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; @@ -5535,6 +5538,7 @@ static int doexpr(int comma,int chkeffect,int allowarray,int mark_endexpr, if (index!=stgidx) markexpr(sEXPR,NULL,0); pc_sideeffect=FALSE; + pc_ovlassignment=FALSE; ident=expression(val,tag,symptr,chkfuncresult); if (!allowarray && (ident==iARRAY || ident==iREFARRAY)) error(33,"-unknown-"); /* array must be indexed */ diff --git a/source/compiler/sc3.c b/source/compiler/sc3.c index 111a4ead..aaad80a4 100644 --- a/source/compiler/sc3.c +++ b/source/compiler/sc3.c @@ -190,6 +190,8 @@ static void (*unopers[])(void) = { lneg, neg, user_inc, user_dec }; if (sym==NULL /*|| (sym->usage & uDEFINE)==0*/) return FALSE; } /* if */ + if (oper==NULL) + pc_ovlassignment=TRUE; /* check existance and the proper declaration of this function */ if ((sym->usage & uMISSING)!=0 || (sym->usage & uPROTOTYPED)==0) { @@ -823,7 +825,6 @@ static int hier14(value *lval1) int bwcount,leftarray; cell arrayidx1[sDIMEN_MAX],arrayidx2[sDIMEN_MAX]; /* last used array indices */ cell *org_arrayidx; - int assignment=FALSE; bwcount=bitwise_opercount; bitwise_opercount=0; @@ -879,7 +880,6 @@ static int hier14(value *lval1) break; case '=': /* simple assignment */ oper=NULL; - assignment=TRUE; if (sc_intest) error(211); /* possibly unintended assignment */ break; @@ -1065,12 +1065,14 @@ static int hier14(value *lval1) pc_sideeffect=TRUE; bitwise_opercount=bwcount; lval1->ident=iEXPRESSION; - if (assignment) { + if (oper==NULL) { symbol *sym=lval3.sym; assert(sym!=NULL); if ((sym->usage & uASSIGNED)!=0 && (sym->vclass==sLOCAL || sym->vclass==sSTATIC)) error(240,sym->name); /* previously assigned value is unused */ markinitialized(sym,TRUE); + if (pc_ovlassignment) + markusage(sym,uREAD); } /* if */ return FALSE; /* expression result is never an lvalue */ } diff --git a/source/compiler/scvars.c b/source/compiler/scvars.c index 05fefa1e..4e72a2bf 100644 --- a/source/compiler/scvars.c +++ b/source/compiler/scvars.c @@ -80,6 +80,7 @@ SC_VDEFINE short fnumber=0; /* the file number in the file table SC_VDEFINE short fcurrent=0; /* current file being processed (debugging) */ SC_VDEFINE short sc_intest=FALSE; /* true if inside a test */ SC_VDEFINE int pc_sideeffect=0; /* true if an expression causes a side-effect */ +SC_VDEFINE int pc_ovlassignment=FALSE; /* true if an expression contains an overloaded assignment */ SC_VDEFINE int stmtindent=0; /* current indent of the statement */ SC_VDEFINE int indent_nowarn=FALSE; /* skip warning "217 loose indentation" */ SC_VDEFINE int sc_tabsize=8; /* number of spaces that a TAB represents */ diff --git a/source/compiler/tests/warning_240.pwn b/source/compiler/tests/warning_240.pwn index 773deaa2..0e574f6c 100644 --- a/source/compiler/tests/warning_240.pwn +++ b/source/compiler/tests/warning_240.pwn @@ -50,6 +50,18 @@ test_arg(arg) arg = 1; // warning 240, warning 204 } +stock Tag:operator =(oper) + return Tag:oper; + +test_ovl_assignment() +{ + // Overloaded assignments are essentially function calls which may have + // desirable side effects, so they shouldn't trigger warning 204. + new Tag:a = 1; + new Tag:b; + b = 2; +} + main() { test_local(); @@ -58,4 +70,5 @@ main() test_global_static(); new x = 0; test_arg(x); + test_ovl_assignment(); } From 456e83788db5e071b11642ece7a870f9654b0081 Mon Sep 17 00:00:00 2001 From: Stanislav Gromov Date: Wed, 8 Apr 2020 22:22:28 +0800 Subject: [PATCH 07/11] Detect more unused assignments for function arguments by marking them as `uASSIGNED` at the beginning of function body --- source/compiler/sc1.c | 4 +++- source/compiler/tests/warning_240.meta | 1 + source/compiler/tests/warning_240.pwn | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/source/compiler/sc1.c b/source/compiler/sc1.c index a812bc98..c29ffac7 100644 --- a/source/compiler/sc1.c +++ b/source/compiler/sc1.c @@ -4294,7 +4294,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; diff --git a/source/compiler/tests/warning_240.meta b/source/compiler/tests/warning_240.meta index c13b38b0..81cf2ff9 100644 --- a/source/compiler/tests/warning_240.meta +++ b/source/compiler/tests/warning_240.meta @@ -6,6 +6,7 @@ warning_240.pwn(12) : warning 240: previously assigned value is never used (symb warning_240.pwn(14) : warning 204: symbol is assigned a value that is never used: "local_var" warning_240.pwn(23) : warning 240: previously assigned value is never used (symbol "local_static_var") warning_240.pwn(24) : warning 240: previously assigned value is never used (symbol "local_static_var") +warning_240.pwn(49) : warning 240: previously assigned value is never used (symbol "arg") warning_240.pwn(50) : warning 240: previously assigned value is never used (symbol "arg") warning_240.pwn(50) : warning 204: symbol is assigned a value that is never used: "arg" """ diff --git a/source/compiler/tests/warning_240.pwn b/source/compiler/tests/warning_240.pwn index 0e574f6c..641ea1bd 100644 --- a/source/compiler/tests/warning_240.pwn +++ b/source/compiler/tests/warning_240.pwn @@ -46,7 +46,7 @@ test_global_static() test_arg(arg) { - arg = 0; + arg = 0; // warning 240 arg = 1; // warning 240, warning 204 } From 8fae3308c9ad1275c4ea0303ab7bbe3f5aac6d09 Mon Sep 17 00:00:00 2001 From: Stanislav Gromov Date: Wed, 20 May 2020 18:12:19 +0800 Subject: [PATCH 08/11] Clear assignments before the "else" branch, not after the "if" branch This allows to detect unused assignments inside "if" statements, e.g. ``` main() { new var = 1; if (var) var = 0; // the value assigned to "var" is not used upon return } ``` --- source/compiler/sc1.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/compiler/sc1.c b/source/compiler/sc1.c index c29ffac7..392f35cb 100644 --- a/source/compiler/sc1.c +++ b/source/compiler/sc1.c @@ -5683,7 +5683,6 @@ static int doif(void) test(flab1,TEST_THEN,FALSE); /* get expression, branch to flab1 if false */ clearassignments(&loctab); statement(NULL,FALSE); /* if true, do a statement */ - clearassignments(&loctab); if (!matchtoken(tELSE)) { /* if...else ? */ setlabel(flab1); /* no, simple if..., print false label */ } else { @@ -5692,6 +5691,7 @@ static int doif(void) * has a lower indent than the matching "if" */ if (stmtindent0) 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 */ From fcc47bd2fa9a9038588339ec26abb41ee4fd6274 Mon Sep 17 00:00:00 2001 From: Stanislav Gromov Date: Wed, 20 May 2020 18:19:27 +0800 Subject: [PATCH 09/11] Don't clear assignments after switch "case" branches --- source/compiler/sc1.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/compiler/sc1.c b/source/compiler/sc1.c index 392f35cb..07fe26ff 100644 --- a/source/compiler/sc1.c +++ b/source/compiler/sc1.c @@ -5898,10 +5898,10 @@ static void doswitch(void) swdefault=FALSE; casecount=0; do { - clearassignments(&loctab); 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(); @@ -5969,6 +5969,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(); @@ -5990,7 +5991,6 @@ static void doswitch(void) } /* if */ } /* switch */ } while (tok!=endtok); - clearassignments(&loctab); #if !defined NDEBUG /* verify that the case table is sorted (unfortunatly, duplicates can From 35850a41a7a072307bed7430835c76f9c126ea43 Mon Sep 17 00:00:00 2001 From: Stanislav Gromov Date: Thu, 21 May 2020 19:57:11 +0800 Subject: [PATCH 10/11] Clear the assignment flag for local variables when using `goto` on a previously defined label --- source/compiler/sc1.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/source/compiler/sc1.c b/source/compiler/sc1.c index 07fe26ff..3ce08f7a 100644 --- a/source/compiler/sc1.c +++ b/source/compiler/sc1.c @@ -6052,6 +6052,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 From a4f87f84c6b9abdbae2f1fac78d436f061d52002 Mon Sep 17 00:00:00 2001 From: Stanislav Gromov Date: Thu, 21 May 2020 22:43:53 +0800 Subject: [PATCH 11/11] Add more test cases --- source/compiler/tests/warning_240.meta | 15 ++++-- source/compiler/tests/warning_240.pwn | 63 ++++++++++++++++++++++++-- 2 files changed, 70 insertions(+), 8 deletions(-) diff --git a/source/compiler/tests/warning_240.meta b/source/compiler/tests/warning_240.meta index 81cf2ff9..0758308d 100644 --- a/source/compiler/tests/warning_240.meta +++ b/source/compiler/tests/warning_240.meta @@ -4,10 +4,15 @@ warning_240.pwn(11) : warning 240: previously assigned value is never used (symbol "local_var") warning_240.pwn(12) : warning 240: previously assigned value is never used (symbol "local_var") warning_240.pwn(14) : warning 204: symbol is assigned a value that is never used: "local_var" -warning_240.pwn(23) : warning 240: previously assigned value is never used (symbol "local_static_var") -warning_240.pwn(24) : warning 240: previously assigned value is never used (symbol "local_static_var") -warning_240.pwn(49) : warning 240: previously assigned value is never used (symbol "arg") -warning_240.pwn(50) : warning 240: previously assigned value is never used (symbol "arg") -warning_240.pwn(50) : warning 204: symbol is assigned a value that is never used: "arg" +warning_240.pwn(31) : warning 204: symbol is assigned a value that is never used: "local_var2" +warning_240.pwn(25) : warning 204: symbol is assigned a value that is never used: "local_var" +warning_240.pwn(53) : warning 204: symbol is assigned a value that is never used: "local_var2" +warning_240.pwn(63) : warning 240: previously assigned value is never used (symbol "local_static_var") +warning_240.pwn(64) : warning 240: previously assigned value is never used (symbol "local_static_var") +warning_240.pwn(89) : warning 240: previously assigned value is never used (symbol "arg") +warning_240.pwn(90) : warning 240: previously assigned value is never used (symbol "arg") +warning_240.pwn(90) : warning 204: symbol is assigned a value that is never used: "arg" +warning_240.pwn(96) : warning 204: symbol is assigned a value that is never used: "arg" +warning_240.pwn(103) : warning 204: symbol is assigned a value that is never used: "arg" """ } diff --git a/source/compiler/tests/warning_240.pwn b/source/compiler/tests/warning_240.pwn index 641ea1bd..dc0f05b0 100644 --- a/source/compiler/tests/warning_240.pwn +++ b/source/compiler/tests/warning_240.pwn @@ -17,6 +17,46 @@ test_local() #pragma unused local_var2 } +test_local2() +{ + new local_var, local_var2; + if (random(2)) + { + local_var = 1; // warning 204 + } + switch (random(2)) + { + case 0: + { + local_var2 = 1; // warning 204 + } + } +} + +test_goto() +{ + { + new local_var; + #pragma unused local_var +lbl_1: + if (random(2)) + { + local_var = 1; // no warning(s) + goto lbl_1; + } + } + { + new local_var2; + #pragma unused local_var2 + if (random(2)) + { + local_var2 = 1; // warning 204 + goto lbl_2; + } +lbl_2: + } +} + test_local_static() { static local_static_var = 0; @@ -44,12 +84,26 @@ test_global_static() global_static_var = 4; } -test_arg(arg) +test_arg1(arg) { arg = 0; // warning 240 arg = 1; // warning 240, warning 204 } +test_arg2(arg) +{ + if (arg) + arg = 1; // warning 204 +} + +test_arg3(arg) +{ + switch (arg) + { + case 0: arg = 1; // warning 204 + } +} + stock Tag:operator =(oper) return Tag:oper; @@ -65,10 +119,13 @@ test_ovl_assignment() main() { test_local(); + test_local2(); + test_goto(); test_local_static(); test_global(); test_global_static(); - new x = 0; - test_arg(x); + test_arg1(1); + test_arg2(1); + test_arg3(1); test_ovl_assignment(); }