From 6739e2c5fe27b0f5d8711426851dc775aa8f0fe2 Mon Sep 17 00:00:00 2001 From: Stanislav Gromov Date: Sun, 11 Apr 2021 17:00:16 +0700 Subject: [PATCH 01/11] Decrease "compound statement" nesting level only after the body of a while loop, so the code inside the loop condition won't be mixed with the code inside the loop body --- 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 a50409ea..a301dc8c 100644 --- a/source/compiler/sc1.c +++ b/source/compiler/sc1.c @@ -6116,8 +6116,8 @@ static int dowhile(void) pc_loopcond=tWHILE; endlessloop=test(wq[wqEXIT],TEST_DO,FALSE);/* branch to wq[wqEXIT] if false */ pc_loopcond=0; - pc_nestlevel--; statement(NULL,FALSE); /* if so, do a statement */ + pc_nestlevel--; clearassignments(pc_nestlevel+1); testloopvariables(loopvars,FALSE,loopline); jumplabel(wq[wqLOOP]); /* and loop to "while" start */ From b42cfa395235df46c207b3e58d4b5465969d43ec Mon Sep 17 00:00:00 2001 From: Stanislav Gromov Date: Sun, 11 Apr 2021 18:37:33 +0700 Subject: [PATCH 02/11] Warn about unused modifications of variables (operators `++` and `--`) --- source/compiler/sc.h | 6 +++-- source/compiler/sc1.c | 4 ++-- source/compiler/sc2.c | 56 ++++++++++++++++++++++++++----------------- source/compiler/sc3.c | 20 ++++++++++------ source/compiler/sc5.c | 3 ++- 5 files changed, 55 insertions(+), 34 deletions(-) diff --git a/source/compiler/sc.h b/source/compiler/sc.h index d5576510..194f9f13 100644 --- a/source/compiler/sc.h +++ b/source/compiler/sc.h @@ -239,6 +239,8 @@ typedef struct s_symbol { * so the compiler would know it shouldn't set the uLOOPVAR flag when the variable * is read inside a loop condition */ #define uNOLOOPVAR 0x2000 +/* uMODIFIED indicates that the value of a variable was modified, but not used yet */ +#define uMODIFIED 0x4000 #define flagDEPRECATED 0x01 /* symbol is deprecated (avoid use) */ #define flagNAKED 0x10 /* function is naked */ @@ -319,7 +321,7 @@ typedef struct s_valuepair { typedef struct s_assigninfo { int lnumber; /* line number of the first unused assignment made in one of * the branches (used for error messages) */ - short usage; /* usage flags to memoize (currently only uASSIGNED) */ + short usage; /* usage flags to memoize */ } symstate; /* macros for code generation */ @@ -748,7 +750,7 @@ SC_FUNC void delete_symbol(symbol *root,symbol *sym); SC_FUNC void delete_symbols(symbol *root,int level,int delete_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 markinitialized(symbol *sym,int assignment,int modification); SC_FUNC void clearassignments(int fromlevel); SC_FUNC void memoizeassignments(int fromlevel,symstate **assignments); SC_FUNC void restoreassignments(int fromlevel,symstate *assignments); diff --git a/source/compiler/sc1.c b/source/compiler/sc1.c index a301dc8c..23dd9aab 100644 --- a/source/compiler/sc1.c +++ b/source/compiler/sc1.c @@ -2350,7 +2350,7 @@ static void declglb(char *firstname,int firsttag,int fpublic,int fstatic,int fst if (fstatic) sym->fnumber=filenum; if (explicit_init) - markinitialized(sym,TRUE); + markinitialized(sym,TRUE,FALSE); sc_attachdocumentation(sym);/* attach any documentation to the variable */ if (sc_status==statSKIP) { sc_status=statWRITE; @@ -2549,7 +2549,7 @@ static int declloc(int fstatic) } /* if */ } /* if */ if (explicit_init) - markinitialized(sym,!suppress_w240); + markinitialized(sym,!suppress_w240,FALSE); if (pc_ovlassignment) sym->usage |= uREAD; if (matchtoken(t__PRAGMA)) diff --git a/source/compiler/sc2.c b/source/compiler/sc2.c index 52d794f6..20eb17d3 100644 --- a/source/compiler/sc2.c +++ b/source/compiler/sc2.c @@ -3202,13 +3202,18 @@ SC_FUNC void delete_symbols(symbol *root,int level,int delete_labels,int delete_ mustdelete=delete_labels; break; case iVARIABLE: - /* check that the assigned value was used, but don't show the warning + /* check that the assigned/modified 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==sLOCAL) { - errorset(sSETPOS,sym->lnumber); - error(204,sym->name); /* symbol is assigned a value that is never used */ - errorset(sSETPOS,-1); + if (sym->vclass==sLOCAL) { + if ((sym->usage & (uREAD | uWRITTEN | uASSIGNED))==(uREAD | uWRITTEN | uASSIGNED)) { + errorset(sSETPOS,sym->lnumber); + error(204,sym->name); /* symbol is assigned a value that is never used */ + errorset(sSETPOS,-1); + } else if ((sym->usage & (uREAD | uWRITTEN | uMODIFIED))==(uREAD | uWRITTEN | uMODIFIED)) { + errorset(sSETPOS,sym->lnumber); + error(252,sym->name); /* symbol has its value modified but never used */ + errorset(sSETPOS,-1); + } /* if */ } /* if */ /* fallthrough */ case iARRAY: @@ -3387,7 +3392,7 @@ SC_FUNC void markusage(symbol *sym,int usage) if ((usage & uWRITTEN)!=0) sym->lnumber=fline; if ((usage & uREAD)!=0 && (sym->ident==iVARIABLE || sym->ident==iREFERENCE)) - sym->usage &= ~uASSIGNED; + sym->usage &= ~(uASSIGNED | uMODIFIED); if ((usage & (uREAD | uWRITTEN))!=0 && (sym->ident==iVARIABLE || sym->ident==iREFERENCE)) markloopvariable(sym,usage); /* check if (global) reference must be added to the symbol */ @@ -3402,15 +3407,17 @@ SC_FUNC void markusage(symbol *sym,int usage) } /* if */ } -SC_FUNC void markinitialized(symbol *sym,int assignment) +SC_FUNC void markinitialized(symbol *sym,int assignment,int modification) { assert(sym!=NULL); + assert(!assignment | !modification); if (sym->ident!=iVARIABLE && sym->ident!=iREFERENCE && sym->ident!=iARRAY) return; if (sc_status==statFIRST && (sym->vclass==sLOCAL || sym->vclass==sSTATIC)) return; - if (assignment && sym->vclass==sLOCAL && (sym->ident==iVARIABLE || sym->ident==iREFERENCE)) { - sym->usage |= uASSIGNED; + if ((assignment || modification) && sym->vclass==sLOCAL + && (sym->ident==iVARIABLE || sym->ident==iREFERENCE)) { + sym->usage |= assignment ? uASSIGNED : uMODIFIED; sym->assignlevel=pc_nestlevel; } /* if */ } @@ -3428,7 +3435,7 @@ SC_FUNC void clearassignments(int fromlevel) sym=&loctab; while ((sym=sym->next)!=NULL) if (sym->assignlevel>=fromlevel) - sym->usage &= ~uASSIGNED; + sym->usage &= ~(uASSIGNED | uMODIFIED); } /* memoizes all assignments done on the specified compound level and higher */ @@ -3460,18 +3467,22 @@ SC_FUNC void memoizeassignments(int fromlevel,symstate **assignments) sym=&loctab; while ((sym=sym->next)!=NULL && sym->ident==iLABEL) {} /* skip labels */ for (num=0; sym!=NULL; num++,sym=sym->next) { - /* if the assignment is unused and it was done inside the branch... */ - if ((sym->usage & uASSIGNED)!=0 && sym->assignlevel>=fromlevel) { - /* clear the assignment flag, so the compiler won't report this assignment as unused - * if the next "if" or "switch" branch also contains an assignment to this variable */ - sym->usage &= ~uASSIGNED; - /* memoize the assignment only if there was no other unused assignment - * in any other "if" or "switch" branch */ + int flag=sym->usage & (uASSIGNED | uMODIFIED); + /* can't have both flags set at the same time */ + assert((flag & uASSIGNED)==0 || (flag & uMODIFIED)==0); + /* if the assignment/modification is unused and it was done inside the branch... */ + if (flag!=0 && sym->assignlevel>=fromlevel) { + /* memoize the assignment/modification only if there was no other unused + * assignment or modification in any other "if" or "switch" branch */ assert_static(sizeof(sym->usage)==sizeof((*assignments)->usage)); - if (((*assignments)[num].usage & uASSIGNED)==0) { + if (((*assignments)[num].usage & (uASSIGNED | uMODIFIED))==0) { (*assignments)[num].lnumber=sym->lnumber; - (*assignments)[num].usage |= uASSIGNED; + (*assignments)[num].usage |= flag; } /* if */ + /* unset the assignment/modification flag for the variable, so the compiler + * won't report the assignment/modification as unused if the next "if" or + * "switch" branch also contains an assignment to this variable */ + sym->usage &= ~(uASSIGNED | uMODIFIED); } /* if */ } /* for */ } @@ -3485,8 +3496,9 @@ SC_FUNC void restoreassignments(int fromlevel,symstate *assignments) sym=&loctab; while ((sym=sym->next)!=NULL && sym->ident==iLABEL) {} /* skip labels */ for (num=0; sym!=NULL; num++,sym=sym->next) { - if (assignments!=NULL && (assignments[num].usage & uASSIGNED)!=0) { - sym->usage |= uASSIGNED; + int flag; + if (assignments!=NULL && (flag=(assignments[num].usage & (uASSIGNED | uMODIFIED)))!=0) { + sym->usage |= flag; sym->lnumber=assignments[num].lnumber; } /* if */ /* demote all assignments that were made inside any of the "if"/"switch" diff --git a/source/compiler/sc3.c b/source/compiler/sc3.c index 11d7c03e..73cb4a50 100644 --- a/source/compiler/sc3.c +++ b/source/compiler/sc3.c @@ -1118,14 +1118,16 @@ static int hier14(value *lval1) pc_sideeffect=TRUE; bitwise_opercount=bwcount; lval1->ident=iEXPRESSION; + /* register assignment/modification */ + assert(lval3.sym!=NULL); if (oper==NULL) { - symbol *sym=lval3.sym; - assert(sym!=NULL); - if ((sym->usage & uASSIGNED)!=0 && sym->assignlevel>=pc_nestlevel && sym->vclass==sLOCAL) - error(240,sym->name); /* previously assigned value is unused */ - markinitialized(sym,TRUE); - if (pc_ovlassignment) - markusage(sym,uREAD); + if (lval3.sym->vclass==sLOCAL && (lval3.sym->usage & uASSIGNED)!=0 + && lval3.sym->assignlevel>=pc_nestlevel) + error(240,lval3.sym->name); /* previously assigned value is unused */ + if (!pc_ovlassignment) + markinitialized(lval3.sym,TRUE,FALSE); + } else if (!pc_ovlassignment) { + markinitialized(lval3.sym,FALSE,TRUE); } /* if */ return FALSE; /* expression result is never an lvalue */ } @@ -1312,6 +1314,7 @@ static int hier2(value *lval) } /* if */ rvalue(lval); /* and read the result into PRI */ pc_sideeffect=TRUE; + markinitialized(lval->sym,FALSE,TRUE); return FALSE; /* result is no longer lvalue */ case tDEC: /* --lval */ if (!hier2(lval)) @@ -1326,6 +1329,7 @@ static int hier2(value *lval) } /* if */ rvalue(lval); /* and read the result into PRI */ pc_sideeffect=TRUE; + markinitialized(lval->sym,FALSE,TRUE); return FALSE; /* result is no longer lvalue */ case '~': /* ~ (one's complement) */ if (hier2(lval)) @@ -1724,6 +1728,7 @@ static int hier2(value *lval) if (saveresult) popreg(sPRI); /* restore PRI (result of rvalue()) */ pc_sideeffect=TRUE; + markinitialized(lval->sym,FALSE,TRUE); return FALSE; /* result is no longer lvalue */ case tDEC: /* lval-- */ if (!lvalue) @@ -1745,6 +1750,7 @@ static int hier2(value *lval) if (saveresult) popreg(sPRI); /* restore PRI (result of rvalue()) */ pc_sideeffect=TRUE; + markinitialized(lval->sym,FALSE,TRUE); return FALSE; case tCHAR: /* char (compute required # of cells */ if (lval->ident==iCONSTEXPR) { diff --git a/source/compiler/sc5.c b/source/compiler/sc5.c index ae126ec9..0e394af9 100644 --- a/source/compiler/sc5.c +++ b/source/compiler/sc5.c @@ -209,7 +209,8 @@ static char *warnmsg[] = { /*248*/ "possible misuse of comma operator\n", /*249*/ "check failed: %s\n", /*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" +/*251*/ "none of the variables used in loop condition are modified in loop body\n", +/*252*/ "variable has its value modified but never used: \"%s\"\n" }; static char *noticemsg[] = { From 7cdb55e2643b0b94616901fb66297b3413762e0e Mon Sep 17 00:00:00 2001 From: Stanislav Gromov Date: Thu, 6 May 2021 01:25:59 +0700 Subject: [PATCH 03/11] Add tests for warning 252 --- source/compiler/tests/warning_252.meta | 10 ++++++++++ source/compiler/tests/warning_252.pwn | 14 ++++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 source/compiler/tests/warning_252.meta create mode 100644 source/compiler/tests/warning_252.pwn diff --git a/source/compiler/tests/warning_252.meta b/source/compiler/tests/warning_252.meta new file mode 100644 index 00000000..f22220ca --- /dev/null +++ b/source/compiler/tests/warning_252.meta @@ -0,0 +1,10 @@ +{ + 'test_type': 'output_check', + 'errors': """ +warning_252.pwn(13) : warning 252: variable has its value modified but never used: "e" +warning_252.pwn(7) : warning 252: variable has its value modified but never used: "d" +warning_252.pwn(6) : warning 252: variable has its value modified but never used: "c" +warning_252.pwn(5) : warning 252: variable has its value modified but never used: "b" +warning_252.pwn(4) : warning 252: variable has its value modified but never used: "a" +""" +} diff --git a/source/compiler/tests/warning_252.pwn b/source/compiler/tests/warning_252.pwn new file mode 100644 index 00000000..1f1856a5 --- /dev/null +++ b/source/compiler/tests/warning_252.pwn @@ -0,0 +1,14 @@ +main() +{ + new a = 1, b = 2, c = 3, d = 4, e = 5; + a++; // warning 252: variable has its value modified but never used: "a" + b--; // warning 252: variable has its value modified but never used: "b" + ++c; // warning 252: variable has its value modified but never used: "c" + --d; // warning 252: variable has its value modified but never used: "d" + + // "e" is used as a return value, so it shouldn't trigger warning 252 + if (e--) + return e; + // but here it should cause a warning, as the post-increment is redundant + return e++; +} From e532513ad7fe9ac52e63afe48421a21000a383fe Mon Sep 17 00:00:00 2001 From: Stanislav Gromov Date: Sun, 29 Aug 2021 13:20:18 +0700 Subject: [PATCH 04/11] Fix warning 240 false-positives when `goto` is used inside of a loop, on a label defined after the use, inside of the same loop MCVE: ``` main() { for (new x = 5; --x != 0;) { if (x == 2) { x = 1; goto lbl_cont; } x = 3; // false-positive warning 240 lbl_cont: } } ``` --- source/compiler/sc1.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/source/compiler/sc1.c b/source/compiler/sc1.c index 23dd9aab..a384f7ab 100644 --- a/source/compiler/sc1.c +++ b/source/compiler/sc1.c @@ -6555,8 +6555,17 @@ static int dogoto(void) if (lex(&val,&st)==tSYMBOL) { sym=fetchlab(st); - if ((sym->usage & uDEFINE)!=0) + if ((sym->usage & uDEFINE)!=0) { clearassignments(1); + } else if (wqptr>wq) { + /* The label is not defined yet, which means it must be defined after the + * 'goto'. If we're inside of a loop, the target label may or may not be + * defined inside of the same loop - we can't know that ahead of time + * due to how the compiler works, so the best we can do for now is clear + * all the assignments at the current compound statement nesting level + * in order to avoid false-positives of warning 240. */ + clearassignments(pc_nestlevel); + } /* if */ jumplabel((int)sym->addr); sym->usage|=uREAD; /* set "uREAD" bit */ if ((sym->usage & uDEFINE)!=0) { From a39b248aeccbe5186ada58f81aa598363a94c0c4 Mon Sep 17 00:00:00 2001 From: Stanislav Gromov Date: Sun, 29 Aug 2021 15:34:19 +0700 Subject: [PATCH 05/11] Add a test --- source/compiler/tests/warning_240.pwn | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/source/compiler/tests/warning_240.pwn b/source/compiler/tests/warning_240.pwn index 8aaabf15..4b405722 100644 --- a/source/compiler/tests/warning_240.pwn +++ b/source/compiler/tests/warning_240.pwn @@ -214,6 +214,21 @@ lbl_1: } lbl_2: } + { + for (new x = 5; --x != 0;) + { + if (x == 2) + { + x = 1; + // The "x = 3;" line is used, as the target label of "goto" + // is located inside of the current loop, so this shouldn't + // trigger warning 240. + goto lbl_3; + } + x = 3; + lbl_3: + } + } } stock Tag:operator =(oper) From b14ddadfd537eb2d82839d0be391db5bbeffd8ca Mon Sep 17 00:00:00 2001 From: Stanislav Gromov Date: Sat, 25 Sep 2021 20:35:16 +0700 Subject: [PATCH 06/11] Print warning 240 at the line of the previous assignment, not of the current one --- source/compiler/sc3.c | 31 ++++++++++++++++++------------- source/compiler/sc5.c | 2 +- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/source/compiler/sc3.c b/source/compiler/sc3.c index 73cb4a50..8dc15997 100644 --- a/source/compiler/sc3.c +++ b/source/compiler/sc3.c @@ -194,8 +194,6 @@ 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 existence and the proper declaration of this function */ if ((sym->usage & uMISSING)!=0 || (sym->usage & uPROTOTYPED)==0) { @@ -285,6 +283,8 @@ static void (*unopers[])(void) = { lneg, neg, user_inc, user_dec }; store(lval); /* store PRI in the symbol */ moveto1(); /* make sure PRI is restored on exit */ } /* if */ + if (oper==NULL) + pc_ovlassignment=TRUE; return TRUE; } @@ -1104,6 +1104,17 @@ static int hier14(value *lval1) if (lval2.ident==iARRAY || lval2.ident==iREFARRAY) error(6); /* must be assigned to an array */ } /* if */ + /* check if the previously assigned value was used (it's important to do this + * before generating the code for storing the value into the variable, as the latter + * might clobber the line number of the previous assignment used by warning 240) */ + assert(lval3.sym!=NULL); + if (oper==NULL && lval3.sym->vclass==sLOCAL && (lval3.sym->usage & uASSIGNED)!=0 + && lval3.sym->assignlevel>=pc_nestlevel) { + errorset(sSETPOS,lval3.sym->lnumber); + error(240,lval3.sym->name); /* assigned/modified value is never used */ + errorset(sSETPOS,-1); + } /* if */ + /* store the expression result and mark the lvalue as modified */ if (leftarray) { memcopy(val*sizeof(cell)); } else { @@ -1113,21 +1124,15 @@ static int hier14(value *lval1) } /* if */ if (!oper) check_tagmismatch(lval3.tag,lval2.tag,TRUE,-1); /* tagname mismatch (if "oper", warning already given in plunge2()) */ - if (lval3.sym) - markusage(lval3.sym,uWRITTEN); + markusage(lval3.sym,uWRITTEN); pc_sideeffect=TRUE; bitwise_opercount=bwcount; lval1->ident=iEXPRESSION; - /* register assignment/modification */ - assert(lval3.sym!=NULL); - if (oper==NULL) { - if (lval3.sym->vclass==sLOCAL && (lval3.sym->usage & uASSIGNED)!=0 - && lval3.sym->assignlevel>=pc_nestlevel) - error(240,lval3.sym->name); /* previously assigned value is unused */ - if (!pc_ovlassignment) + if (!pc_ovlassignment) { + if (oper==NULL) markinitialized(lval3.sym,TRUE,FALSE); - } else if (!pc_ovlassignment) { - markinitialized(lval3.sym,FALSE,TRUE); + else + markinitialized(lval3.sym,FALSE,TRUE); } /* if */ return FALSE; /* expression result is never an lvalue */ } diff --git a/source/compiler/sc5.c b/source/compiler/sc5.c index 0e394af9..caabe560 100644 --- a/source/compiler/sc5.c +++ b/source/compiler/sc5.c @@ -198,7 +198,7 @@ static char *warnmsg[] = { /*237*/ "user warning: %s\n", /*238*/ "meaningless combination of class specifiers (%s)\n", /*239*/ "literal array/string passed to a non-const parameter\n", -/*240*/ "previously assigned value is never used (symbol \"%s\")\n", +/*240*/ "assigned value is never used (symbol \"%s\")\n", /*241*/ "negative or too big shift count\n", /*242*/ "shift overflow in enum element declaration (symbol \"%s\")\n", /*243*/ "redundant code: switch control expression is constant\n", From a9fbfe288c2dfc4ef4efc3cd5a213a9e42676f3d Mon Sep 17 00:00:00 2001 From: Stanislav Gromov Date: Sat, 25 Sep 2021 20:38:04 +0700 Subject: [PATCH 07/11] Update tests --- source/compiler/tests/warning_240.meta | 26 ++++++------ source/compiler/tests/warning_240.pwn | 56 +++++++++++++------------- 2 files changed, 41 insertions(+), 41 deletions(-) diff --git a/source/compiler/tests/warning_240.meta b/source/compiler/tests/warning_240.meta index 3b58bc4c..e67505d7 100644 --- a/source/compiler/tests/warning_240.meta +++ b/source/compiler/tests/warning_240.meta @@ -1,20 +1,20 @@ { 'test_type': 'output_check', 'errors': """ -warning_240.pwn(12) : warning 240: previously assigned value is never used (symbol "local_var") +warning_240.pwn(11) : warning 240: 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(38) : warning 240: previously assigned value is never used (symbol "local_var") -warning_240.pwn(52) : warning 240: previously assigned value is never used (symbol "local_var") -warning_240.pwn(66) : warning 240: previously assigned value is never used (symbol "local_var") -warning_240.pwn(74) : warning 240: previously assigned value is never used (symbol "local_var") -warning_240.pwn(85) : warning 240: previously assigned value is never used (symbol "local_var") -warning_240.pwn(97) : warning 240: previously assigned value is never used (symbol "local_var") -warning_240.pwn(109) : warning 240: previously assigned value is never used (symbol "local_var") -warning_240.pwn(120) : warning 240: previously assigned value is never used (symbol "local_var") -warning_240.pwn(139) : warning 240: previously assigned value is never used (symbol "local_var") -warning_240.pwn(174) : warning 240: previously assigned value is never used (symbol "arg") -warning_240.pwn(178) : warning 240: previously assigned value is never used (symbol "arg") -warning_240.pwn(183) : warning 240: previously assigned value is never used (symbol "refarg") +warning_240.pwn(35) : warning 240: assigned value is never used (symbol "local_var") +warning_240.pwn(46) : warning 240: assigned value is never used (symbol "local_var") +warning_240.pwn(63) : warning 240: assigned value is never used (symbol "local_var") +warning_240.pwn(71) : warning 240: assigned value is never used (symbol "local_var") +warning_240.pwn(82) : warning 240: assigned value is never used (symbol "local_var") +warning_240.pwn(93) : warning 240: assigned value is never used (symbol "local_var") +warning_240.pwn(106) : warning 240: assigned value is never used (symbol "local_var") +warning_240.pwn(114) : warning 240: assigned value is never used (symbol "local_var") +warning_240.pwn(136) : warning 240: assigned value is never used (symbol "local_var") +warning_240.pwn(168) : warning 240: assigned value is never used (symbol "arg") +warning_240.pwn(176) : warning 240: assigned value is never used (symbol "arg") +warning_240.pwn(182) : warning 240: assigned value is never used (symbol "refarg") warning_240.pwn(178) : warning 204: symbol is assigned a value that is never used: "arg" warning_240.pwn(212) : warning 204: symbol is assigned a value that is never used: "local_var" """ diff --git a/source/compiler/tests/warning_240.pwn b/source/compiler/tests/warning_240.pwn index 4b405722..46ca61bd 100644 --- a/source/compiler/tests/warning_240.pwn +++ b/source/compiler/tests/warning_240.pwn @@ -8,8 +8,8 @@ static global_static_var; test_locals() { { - new local_var = 1; - local_var = 2; // warning 240 + new local_var = 1; // warning 240 + local_var = 2; UseVariable(local_var); local_var = 3; // warning 204 } @@ -32,10 +32,10 @@ test_locals() new local_var = 1; if (TRUE) { - local_var = 2; + local_var = 2; // warning 240 } // The previous assignment ("local_var = 2") should be reported as unused. - local_var = 3; // warning 240 + local_var = 3; UseVariable(local_var); } @@ -43,13 +43,13 @@ test_locals() new local_var; if (TRUE) { - local_var = 1; + local_var = 1; // warning 240 } else { } // The previous assignment ("local_var = 1") should be reported as unused. - local_var = 2; // warning 240 + local_var = 2; UseVariable(local_var); } @@ -60,18 +60,18 @@ test_locals() } else { - local_var = 1; + local_var = 1; // warning 240 } // The previous assignment ("local_var = 1") should be reported as unused. - local_var = 2; // warning 240 + local_var = 2; UseVariable(local_var); } { - new local_var = 1; + new local_var = 1; // warning 240 if (TRUE) {} // The previous assignment ("local_var = 1") should be reported as unused. - local_var = 2; // warning 240 + local_var = 2; UseVariable(local_var); } @@ -79,10 +79,10 @@ test_locals() new local_var = 1; switch (TRUE) { - default: local_var = 2; + default: local_var = 2; // warning 240 } // The previous assignment ("local_var = 2") should be reported as unused. - local_var = 3; // warning 240 + local_var = 3; UseVariable(local_var); } @@ -90,11 +90,11 @@ test_locals() new local_var = 1; switch (TRUE) { - case true: local_var = 2; + case true: local_var = 2; // warning 240 default: {} } // The previous assignment ("local_var = 2") should be reported as unused. - local_var = 3; // warning 240 + local_var = 3; UseVariable(local_var); } @@ -103,21 +103,21 @@ test_locals() switch (TRUE) { case true: {} - default: local_var = 2; + default: local_var = 2; // warning 240 } // The previous assignment ("local_var = 2") should be reported as unused. - local_var = 3; // warning 240 + local_var = 3; UseVariable(local_var); } { - new local_var = 1; + new local_var = 1; // warning 240 switch (TRUE) { default: {} } // The previous assignment ("local_var = 1") should be reported as unused. - local_var = 2; // warning 240 + local_var = 2; UseVariable(local_var); } @@ -133,10 +133,10 @@ test_locals() } { - new local_var = 1; + new local_var = 1; // warning 240 do {} while (FALSE); // The previous assignment ("local_var = 1") should be reported as unused. - local_var = 1; // warning 240 + local_var = 1; UseVariable(local_var); } @@ -145,7 +145,7 @@ test_locals() // assigned value can be used on the next function call, which is why // assignment "local_static_var = 4" should NOT be reported as unused. static local_static_var = 1; - local_static_var = 1; // warning 240 + local_static_var = 1; UseVariable(local_static_var); local_static_var = 4; } @@ -165,22 +165,22 @@ test_globals() global_static_var = 3; } -test_args(arg, &refarg) +test_args(arg, &refarg) // warning 240 (symbol "arg") { // Technically function arguments are like local variables, except that they // have a value implicitly assigned to them at the start of the function body. // This is why on the subsequent assignment ("arg = 1") the compiler should warn // about the previously assigned value being unused. - arg = 1; // warning 240 + arg = 1; if (TRUE) - arg = 2; + arg = 2; // warning 240 do {} while (FALSE); - arg = 3; // warning 240, warning 204 + arg = 3; // warning 204 // "warning 203" is not applicable to references, as the value might be used - // outside of the function. - refarg = 1; - refarg = 2; // warning 240 + // outside of the function, but warning 240 should still work for them. + refarg = 1; // warning 240 + refarg = 2; UseVariable(refarg); refarg = 3; } From a6b5435c3b4f58d9b967df612cc039b9c5738d8f Mon Sep 17 00:00:00 2001 From: Stanislav Gromov Date: Sat, 18 Jun 2022 23:39:34 +0700 Subject: [PATCH 08/11] Clear the `uMODIFIED` flag when using `#pragma unused/unread` --- 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 a384f7ab..ddc2a1eb 100644 --- a/source/compiler/sc1.c +++ b/source/compiler/sc1.c @@ -8689,7 +8689,7 @@ SC_FUNC void pragma_unused(symbol *sym, int unread, int unwritten) /* mark as read if the pragma wasn't "unwritten" */ if (!unwritten) { sym->usage |= uREAD; - sym->usage &= ~uASSIGNED; + sym->usage &= ~(uASSIGNED | uMODIFIED); } /* if */ /* mark as written if the pragma wasn't "unread" */ if (sym->ident == iVARIABLE || sym->ident == iREFERENCE From 3527e5a49072b0198ab6571bd92179fdf3d8bd7a Mon Sep 17 00:00:00 2001 From: Stanislav Gromov Date: Sun, 19 Jun 2022 00:03:35 +0700 Subject: [PATCH 09/11] Move test cases for warning 252 into separate functions --- source/compiler/tests/warning_252.meta | 2 +- source/compiler/tests/warning_252.pwn | 22 ++++++++++++++++------ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/source/compiler/tests/warning_252.meta b/source/compiler/tests/warning_252.meta index f22220ca..f2d3826e 100644 --- a/source/compiler/tests/warning_252.meta +++ b/source/compiler/tests/warning_252.meta @@ -1,10 +1,10 @@ { 'test_type': 'output_check', 'errors': """ -warning_252.pwn(13) : warning 252: variable has its value modified but never used: "e" warning_252.pwn(7) : warning 252: variable has its value modified but never used: "d" warning_252.pwn(6) : warning 252: variable has its value modified but never used: "c" warning_252.pwn(5) : warning 252: variable has its value modified but never used: "b" warning_252.pwn(4) : warning 252: variable has its value modified but never used: "a" +warning_252.pwn(17) : warning 252: variable has its value modified but never used: "x" """ } diff --git a/source/compiler/tests/warning_252.pwn b/source/compiler/tests/warning_252.pwn index 1f1856a5..5a1c6da5 100644 --- a/source/compiler/tests/warning_252.pwn +++ b/source/compiler/tests/warning_252.pwn @@ -1,14 +1,24 @@ -main() +test_basic() { - new a = 1, b = 2, c = 3, d = 4, e = 5; + new a = 1, b = 2, c = 3, d = 4; a++; // warning 252: variable has its value modified but never used: "a" b--; // warning 252: variable has its value modified but never used: "b" ++c; // warning 252: variable has its value modified but never used: "c" --d; // warning 252: variable has its value modified but never used: "d" +} - // "e" is used as a return value, so it shouldn't trigger warning 252 - if (e--) - return e; +test_retval() +{ + new x = 1; + // "x" is used after it's modified, so it shouldn't trigger warning 252 + if (x--) + return x; // but here it should cause a warning, as the post-increment is redundant - return e++; + return x++; // warning 252: variable has its value modified but never used: "x" +} + +main() +{ + test_basic(); + test_retval(); } From 89f5736cb40994ee3299050b435dee15dd9ad77a Mon Sep 17 00:00:00 2001 From: Stanislav Gromov Date: Sun, 19 Jun 2022 00:08:24 +0700 Subject: [PATCH 10/11] Make sure `#pragma unused/unread` clears the `uMODIFIED` flag --- source/compiler/tests/warning_252.pwn | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/source/compiler/tests/warning_252.pwn b/source/compiler/tests/warning_252.pwn index 5a1c6da5..d0419b3f 100644 --- a/source/compiler/tests/warning_252.pwn +++ b/source/compiler/tests/warning_252.pwn @@ -17,8 +17,18 @@ test_retval() return x++; // warning 252: variable has its value modified but never used: "x" } +test_pragma_unused_unread() +{ + new x = 1, y = 2; + x += 2; + y -= 1; + #pragma unused x + #pragma unread y +} + main() { test_basic(); test_retval(); + test_pragma_unused_unread(); } From b89faa1f2497dd995ff4fb38a25add435d1ed935 Mon Sep 17 00:00:00 2001 From: Stanislav Gromov Date: Sun, 19 Jun 2022 12:19:02 +0700 Subject: [PATCH 11/11] Add a missing test case for ce6f7123dd8899dbb38e568fb706df7831d01109 --- source/compiler/tests/warning_240.pwn | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/source/compiler/tests/warning_240.pwn b/source/compiler/tests/warning_240.pwn index 46ca61bd..35bd6300 100644 --- a/source/compiler/tests/warning_240.pwn +++ b/source/compiler/tests/warning_240.pwn @@ -243,12 +243,33 @@ test_overloaded() b = 2; } +test_return_refarg(&refarg) +{ + // Assignment "refarg = 1" shouldn't be reported as unused, as it's followed + // by a "return" statement and "refarg" is a pass-by-reference argument, so + // we assume it might be used by the caller (although we can't check for + // that to be sure). "exit" statement works in a similar way, so assignment + // "refarg = 2" shouldn't be reported as unused either. + if (TRUE) + { + refarg = 1; + return; + } + if (TRUE) + { + refarg = 2; + exit; + } + refarg = 3; +} + main() { + new x; test_locals(); test_globals(); - new x; test_args(0,x); test_goto(); test_overloaded(); + test_return_refarg(x); }