From 355b324df46bc6569e0f3b8b0bc82b569e0d7e75 Mon Sep 17 00:00:00 2001
From: Stanislav Gromov <gromovstanislav@yandex.ru>
Date: Sat, 30 Jan 2021 13:25:42 +0700
Subject: [PATCH 01/11] Skip warning ID 249

---
 source/compiler/sc5.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/source/compiler/sc5.c b/source/compiler/sc5.c
index 3302cfa..d72803f 100644
--- a/source/compiler/sc5.c
+++ b/source/compiler/sc5.c
@@ -206,7 +206,8 @@ static char *warnmsg[] = {
 /*245*/  "enum increment \"%s %d\" has no effect on zero value (symbol \"%s\")\n",
 /*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"
+/*248*/  "possible misuse of comma operator\n",
+/*249*/  ""
 };
 
 static char *noticemsg[] = {

From 0b721bfa1cc4c63d366c0751f06e24f207092230 Mon Sep 17 00:00:00 2001
From: Stanislav Gromov <gromovstanislav@yandex.ru>
Date: Sat, 17 Oct 2020 16:09:44 +0800
Subject: [PATCH 02/11] Reformat struct "assigninfo" into "symstate", so it
 could be used to store multiple symbol usage flags

---
 source/compiler/sc.h  | 16 ++++++++--------
 source/compiler/sc1.c |  4 ++--
 source/compiler/sc2.c | 13 +++++++------
 3 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/source/compiler/sc.h b/source/compiler/sc.h
index 15cbd87..4d3cf54 100644
--- a/source/compiler/sc.h
+++ b/source/compiler/sc.h
@@ -302,15 +302,15 @@ typedef struct s_valuepair {
   long second;
 } valuepair;
 
-/* struct "assigninfo" is used to synchronize the status of assignments that
- * were made in multiple "if" and "switch" branches, so the compiler could
- * detect unused assignments in all of those branches, not only the last one */
+/* 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 */
 typedef struct s_assigninfo {
-  int unused;       /* true if the variable has an unused value assigned to it
-                     * in one of the branches" */
   int lnumber;      /* line number of the first unused assignment made in one of
                      * the branches (used for error messages) */
-} assigninfo;
+  short usage;      /* usage flags to memoize (currently only uASSIGNED) */
+} symstate;
 
 /* macros for code generation */
 #define opcodes(n)      ((n)*sizeof(cell))      /* opcode size */
@@ -735,8 +735,8 @@ 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(int fromlevel);
-SC_FUNC void memoizeassignments(int fromlevel,assigninfo **assignments);
-SC_FUNC void restoreassignments(int fromlevel,assigninfo *assignments);
+SC_FUNC void memoizeassignments(int fromlevel,symstate **assignments);
+SC_FUNC void restoreassignments(int fromlevel,symstate *assignments);
 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 85dafe7..5cba80c 100644
--- a/source/compiler/sc1.c
+++ b/source/compiler/sc1.c
@@ -5884,7 +5884,7 @@ static int doif(void)
   int ifindent;
   int lastst_true;
   int returnst=tIF;
-  assigninfo *assignments=NULL;
+  symstate *assignments=NULL;
 
   lastst=0;                     /* reset the last statement */
   ifindent=stmtindent;          /* save the indent of the "if" instruction */
@@ -6104,7 +6104,7 @@ static int doswitch(void)
   constvalue_root caselist = { NULL, NULL};   /* case list starts empty */
   constvalue *cse,*csp,*newval;
   char labelname[sNAMEMAX+1];
-  assigninfo *assignments=NULL;
+  symstate *assignments=NULL;
 
   endtok= matchtoken('(') ? ')' : tDO;
   ident=doexpr(TRUE,FALSE,FALSE,FALSE,&swtag,NULL,TRUE,NULL);   /* evaluate switch expression */
diff --git a/source/compiler/sc2.c b/source/compiler/sc2.c
index aeb6029..50523aa 100644
--- a/source/compiler/sc2.c
+++ b/source/compiler/sc2.c
@@ -3350,7 +3350,7 @@ SC_FUNC void clearassignments(int fromlevel)
 }
 
 /* memoizes all assignments done on the specified compound level and higher */
-SC_FUNC void memoizeassignments(int fromlevel,assigninfo **assignments)
+SC_FUNC void memoizeassignments(int fromlevel,symstate **assignments)
 {
   symbol *sym;
   int num;
@@ -3370,7 +3370,7 @@ SC_FUNC void memoizeassignments(int fromlevel,assigninfo **assignments)
     /* if there are no variables, then we have an early exit */
     if (num==0)
       return;
-    *assignments=(assigninfo *)calloc((size_t)num,sizeof(assigninfo));
+    *assignments=(symstate *)calloc((size_t)num,sizeof(symstate));
     if (*assignments==NULL)
       error(103); /* insufficient memory */
   } /* if */
@@ -3385,16 +3385,17 @@ SC_FUNC void memoizeassignments(int fromlevel,assigninfo **assignments)
       sym->usage &= ~uASSIGNED;
       /* memoize the assignment only if there was no other unused assignment
        * in any other "if" or "switch" branch */
-      if ((*assignments)[num].unused==FALSE) {
-        (*assignments)[num].unused=TRUE;
+      assert_static(sizeof(sym->usage)==sizeof((*assignments)->usage));
+      if (((*assignments)[num].usage & uASSIGNED)==0) {
         (*assignments)[num].lnumber=sym->lnumber;
+        (*assignments)[num].usage |= uASSIGNED;
       } /* if */
     } /* if */
   } /* for */
 }
 
 /* restores all memoized assignments */
-SC_FUNC void restoreassignments(int fromlevel,assigninfo *assignments)
+SC_FUNC void restoreassignments(int fromlevel,symstate *assignments)
 {
   symbol *sym;
   int num;
@@ -3402,7 +3403,7 @@ SC_FUNC void restoreassignments(int fromlevel,assigninfo *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].unused) {
+    if (assignments!=NULL && (assignments[num].usage & uASSIGNED)!=0) {
       sym->usage |= uASSIGNED;
       sym->lnumber=assignments[num].lnumber;
     } /* if */

From e0cde0583e68e2bef57d65148f1abd0852d21998 Mon Sep 17 00:00:00 2001
From: Stanislav Gromov <gromovstanislav@yandex.ru>
Date: Wed, 7 Oct 2020 20:43:45 +0800
Subject: [PATCH 03/11] Warn when variables used inside of a loop condition
 aren't modified in the loop body

---
 source/compiler/sc.h     |  16 ++++-
 source/compiler/sc1.c    | 128 ++++++++++++++++++++++++++++++++++++++-
 source/compiler/sc2.c    |  32 +++++++++-
 source/compiler/sc3.c    |   8 ++-
 source/compiler/sc5.c    |   4 +-
 source/compiler/scvars.c |   2 +
 6 files changed, 183 insertions(+), 7 deletions(-)

diff --git a/source/compiler/sc.h b/source/compiler/sc.h
index 4d3cf54..2a313c0 100644
--- a/source/compiler/sc.h
+++ b/source/compiler/sc.h
@@ -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 */
diff --git a/source/compiler/sc1.c b/source/compiler/sc1.c
index 5cba80c..f2a815e 100644
--- a/source/compiler/sc1.c
+++ b/source/compiler/sc1.c
@@ -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;
 }
diff --git a/source/compiler/sc2.c b/source/compiler/sc2.c
index 50523aa..028595f 100644
--- a/source/compiler/sc2.c
+++ b/source/compiler/sc2.c
@@ -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
  *
diff --git a/source/compiler/sc3.c b/source/compiler/sc3.c
index f54c545..7ed94e1 100644
--- a/source/compiler/sc3.c
+++ b/source/compiler/sc3.c
@@ -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 */
 
diff --git a/source/compiler/sc5.c b/source/compiler/sc5.c
index d72803f..77c30a3 100644
--- a/source/compiler/sc5.c
+++ b/source/compiler/sc5.c
@@ -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[] = {
diff --git a/source/compiler/scvars.c b/source/compiler/scvars.c
index 7afb577..57ea194 100644
--- a/source/compiler/scvars.c
+++ b/source/compiler/scvars.c
@@ -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 */

From 7d1b6c92942408c870c86ea367d2f995dca2e1f9 Mon Sep 17 00:00:00 2001
From: Stanislav Gromov <gromovstanislav@yandex.ru>
Date: Sun, 18 Oct 2020 20:59:35 +0800
Subject: [PATCH 04/11] Disable the new diagnostics when a function or an array
 is used inside a loop condition

---
 source/compiler/sc1.c | 10 ++++------
 source/compiler/sc2.c |  6 ++----
 source/compiler/sc3.c | 18 ++++++++++++------
 3 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/source/compiler/sc1.c b/source/compiler/sc1.c
index f2a815e..d7ffa55 100644
--- a/source/compiler/sc1.c
+++ b/source/compiler/sc1.c
@@ -5214,8 +5214,7 @@ static void scanloopvariables(symstate **loopvars)
     /* 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) {
+    if ((sym->ident==iVARIABLE || sym->ident==iREFERENCE) && (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);
@@ -5245,8 +5244,8 @@ static void testloopvariables(symstate *loopvars,int line)
   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)
+      if ((sym->ident==iVARIABLE || sym->ident==iREFERENCE)
+          && (sym->usage & (uLOOPVAR | uNOLOOPVAR))==uLOOPVAR)
         pc_numloopvars--;
     if (pc_numloopvars==0 && warnnum==251) {
       errorset(sSETPOS,line);
@@ -5256,8 +5255,7 @@ static void testloopvariables(symstate *loopvars,int line)
   } /* 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->ident==iVARIABLE || sym->ident==iREFERENCE) {
       if ((sym->usage & (uLOOPVAR | uNOLOOPVAR))==uLOOPVAR) {
         sym->usage &= ~uLOOPVAR;
         /* warn only if none of the variables used inside the loop condition
diff --git a/source/compiler/sc2.c b/source/compiler/sc2.c
index 028595f..07bd7bb 100644
--- a/source/compiler/sc2.c
+++ b/source/compiler/sc2.c
@@ -3309,10 +3309,8 @@ 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))
+  if ((usage & (uREAD | uWRITTEN))!=0 && (sym->vclass==sLOCAL || sym->vclass==sSTATIC)
+      && (sym->ident==iVARIABLE || sym->ident==iREFERENCE))
     markloopvariable(sym,usage);
   /* check if (global) reference must be added to the symbol */
   if ((usage & (uREAD | uWRITTEN))!=0) {
diff --git a/source/compiler/sc3.c b/source/compiler/sc3.c
index 7ed94e1..c6f2506 100644
--- a/source/compiler/sc3.c
+++ b/source/compiler/sc3.c
@@ -1770,6 +1770,12 @@ restart:
         error(51);      /* invalid subscript, must use [ ] */
         invsubscript=TRUE;
       } /* if */
+      if (pc_loopcond) {
+        /* stop counting variables that were used in loop condition,
+         * otherwise warnings 250 and 251 may be inaccurate */
+        pc_loopcond=FALSE;
+        pc_numloopvars=0;
+      } /* if */
       if (invsubscript) {
         if (sym!=NULL && sym->ident!=iFUNCTN)
           sym->usage |= uREAD;  /* avoid the "symbol is never used" warning */
@@ -2177,7 +2183,6 @@ 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];
@@ -2197,10 +2202,12 @@ static int nesting=0;
     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;
+  if (pc_loopcond) {
+    /* stop counting variables that were used in loop condition,
+     * otherwise warnings 249 and 250 may be inaccurate */
+    pc_loopcond=FALSE;
+    pc_numloopvars=0;
+  } /* if */
   /* check whether this is a function that returns an array */
   symret=finddepend(sym);
   assert(symret==NULL || symret->ident==iREFARRAY);
@@ -2614,7 +2621,6 @@ 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 */
 

From ae30cfb20b4f3ff0fd421510ce272ee147a348c6 Mon Sep 17 00:00:00 2001
From: Stanislav Gromov <gromovstanislav@yandex.ru>
Date: Sun, 18 Oct 2020 23:45:21 +0800
Subject: [PATCH 05/11] Add tests

---
 source/compiler/tests/warning_250_251.meta | 11 +++
 source/compiler/tests/warning_250_251.pwn  | 92 ++++++++++++++++++++++
 2 files changed, 103 insertions(+)
 create mode 100644 source/compiler/tests/warning_250_251.meta
 create mode 100644 source/compiler/tests/warning_250_251.pwn

diff --git a/source/compiler/tests/warning_250_251.meta b/source/compiler/tests/warning_250_251.meta
new file mode 100644
index 0000000..fa8369f
--- /dev/null
+++ b/source/compiler/tests/warning_250_251.meta
@@ -0,0 +1,11 @@
+{
+  'test_type': 'output_check',
+  'errors': """
+warning_250_251.pwn(12) : warning 250: variable "n" used in loop condition not modified in loop body
+warning_250_251.pwn(13) : warning 250: variable "i" used in loop condition not modified in loop body
+warning_250_251.pwn(31) : warning 250: variable "n" used in loop condition not modified in loop body
+warning_250_251.pwn(32) : warning 250: variable "i" used in loop condition not modified in loop body
+warning_250_251.pwn(46) : warning 251: none of the variables used in loop condition are modified in loop body
+warning_250_251.pwn(47) : warning 251: none of the variables used in loop condition are modified in loop body
+"""
+}
diff --git a/source/compiler/tests/warning_250_251.pwn b/source/compiler/tests/warning_250_251.pwn
new file mode 100644
index 0000000..1083e3f
--- /dev/null
+++ b/source/compiler/tests/warning_250_251.pwn
@@ -0,0 +1,92 @@
+#include <core>
+#include <file>
+
+new glbvar = 0;
+
+main()
+{
+	new n = 0, m = 10;
+	static st = 0;
+
+	// Case 1: Variable is used inside a loop condition without being modified.
+	while (n < 10) {}                      // warning 250: variable used in loop condition not modified in loop body (symbol "n")
+	for (new i = 0, j = 0; i < 10; ++j) {} // warning 250: variable used in loop condition not modified in loop body (symbol "i")
+
+	// Case 2: Variable is used inside a loop condition and modified in the loop body.
+	while (n != 0) { n++; }
+	for (new i = 0; i < 10; ) { i++; }
+
+	// Case 3: Variable is used inside a loop condition and modified in the
+	// loop counter increment/decrement section.
+	for (new i = 0; i < 10; i++) {}
+
+	// Case 4: Variable is used and modified inside a loop condition.
+	while (n++ != 0) {}
+	while (++n != 0) {}
+	for (new i = 0; i++ < 10; ) {}
+	for (new i = 0; ++i < 10; ) {}
+
+	// Case 5: Same variable is used inside a loop condition more than once
+	// and it's not modified.
+	while (n == 0 || n < 10) {}            // warning 250: variable used in loop condition not modified in loop body (symbol "n")
+	for (new i = 0; i == 0 || i < 10; ) {} // warning 250: variable used in loop condition not modified in loop body (symbol "i")
+
+	// Case 6: Same variable is used inside a loop condition more than once,
+	// but it's modified.
+	while (n == 0 || n < 10) { n++; }
+	for (new i = 0; i == 0 || i < 10; i++) {}
+
+	// Case 7: Two variables are used inside a loop condition, both aren't modified.
+	// Printing warning 250 for each unmodified variable wouldn't be productive, because:
+	//   1. the user would be spammed with multiple warnings, which can be annoying;
+	//   2. depending on the context, only one of the variables might be supposed
+	//      to be modified, but the compiler can't know which one exactly.
+	// Solution: introduce a warning that simply says that none of the variables
+	// were modified, and let the user decide which variable should be modified.
+	while (n < m) {}            // warning 251: none of the variables used in loop condition are modified in loop body
+	for (new i = 0; i < m; ) {} // warning 251: none of the variables used in loop condition are modified in loop body
+
+	// Case 7: Two variables are used in a loop condition, but one of them
+	// is modified inside the loop body (or the loop counter increment/decrement
+	// section of a "for" loop), and the other one is not modified.
+	while (n < m) { ++n; }
+	for (new i = 0; i < m; ) { i++; }
+	for (new i = 0; i < m; i++) {}
+
+	// Case 8: Two variables are used in a loop condition, but one of them
+	// is being modified prior to being used, and the other one is not modified.
+	while (++n < m) {}
+	for (new i = 0; ++i < m; ) {}
+
+	// Case 9: Two variables are used in a loop condition, but one of them
+	// is static and it's modified inside the loop body, and the other one
+	// is a stack variable and it's left unmodified.
+	while (st < m) { st++; }
+
+	// Case 10: Warnings 250 and 251 may be inaccurate when an array is indexed
+	// inside a loop condition. The problem is that we can't memoize the array
+	// that is being indexed by a variable, to unset the "uLOOPVAR" flag for the
+	// array symbol later when the variable is modified.
+	// This is why I had to completely disable those diagnostics for arrays.
+	{
+		new a[3] = { 0, 0, 1 };
+		n = random(sizeof(a));
+		while (a[n] == 0) // Shouldn't warn about "n" not being modified
+			a[n] = random(3);
+		for (new i = 0; a[i++] == 0; ) {} // shouldn't warn about "a" not being modified
+		for (a[0] = 0, m = 10; a[0] < m; ++a[0]) {} // shouldn't warn about "m" not being modified
+	}
+
+	// Case 11: Just as with arrays, warnings 250 and 251 are disabled when
+	// there's a function call inside a loop condition, as those diagnostics
+	// may be inaccurate otherwise.
+	new File:f = fopen("test.txt", io_read);
+	new line[128];
+	while(fread(f,line,sizeof(line),false) < m) {} // shouldn't warn about "f" or "m" not being modified
+	fclose(f);
+
+	// Case 12: Warnings 250 and 251 don't work for global variables (yet?)
+	while (glbvar < 10) {}
+	for (glbvar = 0; glbvar < 10; ) {}
+}
+

From bff030799b4f6a89d935f9246a2bcf3ab87bca64 Mon Sep 17 00:00:00 2001
From: Stanislav Gromov <gromovstanislav@yandex.ru>
Date: Sat, 30 Jan 2021 21:20:25 +0700
Subject: [PATCH 06/11] Disable warnings 250 and 251 when a global variable is
 used inside a loop condition

---
 source/compiler/sc2.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/source/compiler/sc2.c b/source/compiler/sc2.c
index 07bd7bb..75e8b60 100644
--- a/source/compiler/sc2.c
+++ b/source/compiler/sc2.c
@@ -3309,8 +3309,7 @@ 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))
+  if ((usage & (uREAD | uWRITTEN))!=0 && (sym->ident==iVARIABLE || sym->ident==iREFERENCE))
     markloopvariable(sym,usage);
   /* check if (global) reference must be added to the symbol */
   if ((usage & (uREAD | uWRITTEN))!=0) {
@@ -3421,11 +3420,20 @@ SC_FUNC void restoreassignments(int fromlevel,symstate *assignments)
 
 static void markloopvariable(symbol *sym,int usage)
 {
+  if (sc_status!=statWRITE)
+    return;
   while (sym->parent!=NULL)
     sym=sym->parent;
   /* check if the variable used inside a loop condition */
   if (pc_loopcond) {
-    if ((usage & uWRITTEN)!=0) {
+    if (sym->vclass==sGLOBAL) {
+      /* stop counting variables that were used in loop condition, otherwise
+       * warnings 250 and 251 may be inaccurate (global variables can be
+       * modified from another function(s) called from the loop body, and
+       * currently there's no reasonable way to track this) */
+      pc_loopcond=FALSE;
+      pc_numloopvars=0;
+    } else 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 */

From 59c6de121841ab9623fbeb658570e9d5d0e0ace5 Mon Sep 17 00:00:00 2001
From: Stanislav Gromov <gromovstanislav@yandex.ru>
Date: Sat, 30 Jan 2021 21:20:33 +0700
Subject: [PATCH 07/11] Update tests

---
 source/compiler/tests/warning_250_251.pwn | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/source/compiler/tests/warning_250_251.pwn b/source/compiler/tests/warning_250_251.pwn
index 1083e3f..4eb83fb 100644
--- a/source/compiler/tests/warning_250_251.pwn
+++ b/source/compiler/tests/warning_250_251.pwn
@@ -85,8 +85,11 @@ main()
 	while(fread(f,line,sizeof(line),false) < m) {} // shouldn't warn about "f" or "m" not being modified
 	fclose(f);
 
-	// Case 12: Warnings 250 and 251 don't work for global variables (yet?)
-	while (glbvar < 10) {}
-	for (glbvar = 0; glbvar < 10; ) {}
+	// Case 12: Warnings 250 and 251 shouldn't trigger when at least one global
+	// variable is used inside the loop condition, as globals can be modified
+	// from a function called from the loop body and currently there's no easy
+	// way to track this.
+	while (n < glbvar) {}
+	for (new i = 0; i < glbvar; ) {}
 }
 

From 48c10791dbde8ccd3b9d05dc823035272976d461 Mon Sep 17 00:00:00 2001
From: Stanislav Gromov <gromovstanislav@yandex.ru>
Date: Sun, 31 Jan 2021 21:45:26 +0700
Subject: [PATCH 08/11] Don't count function arguments passed by const
 reference as modified

---
 source/compiler/sc3.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/source/compiler/sc3.c b/source/compiler/sc3.c
index c6f2506..60d67c9 100644
--- a/source/compiler/sc3.c
+++ b/source/compiler/sc3.c
@@ -2402,9 +2402,9 @@ static int nesting=0;
           check_tagmismatch_multiple(arg[argidx].tags,arg[argidx].numtags,lval.tag,-1);
           if (lval.tag!=0)
             append_constval(&taglst,arg[argidx].name,lval.tag,0);
-          argidx++;               /* argument done */
-          if (lval.sym!=NULL)
+          if (lval.sym!=NULL && (arg[argidx].usage & uCONST)==0)
             markusage(lval.sym,uWRITTEN);
+          argidx++;               /* argument done */
           break;
         case iREFARRAY:
           if (lval.ident!=iARRAY && lval.ident!=iREFARRAY

From e137b1c4dc75f7aadf2c35a0360244edea99d453 Mon Sep 17 00:00:00 2001
From: Stanislav Gromov <gromovstanislav@yandex.ru>
Date: Sun, 31 Jan 2021 18:02:32 +0700
Subject: [PATCH 09/11] Add two more test cases to make sure
 passed-by-reference function arguments don't conflict with the new warnings

---
 source/compiler/tests/warning_250_251.meta | 14 ++++++-----
 source/compiler/tests/warning_250_251.pwn  | 27 ++++++++++++++++++----
 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/source/compiler/tests/warning_250_251.meta b/source/compiler/tests/warning_250_251.meta
index fa8369f..57a6b24 100644
--- a/source/compiler/tests/warning_250_251.meta
+++ b/source/compiler/tests/warning_250_251.meta
@@ -1,11 +1,13 @@
 {
   'test_type': 'output_check',
   'errors': """
-warning_250_251.pwn(12) : warning 250: variable "n" used in loop condition not modified in loop body
-warning_250_251.pwn(13) : warning 250: variable "i" used in loop condition not modified in loop body
-warning_250_251.pwn(31) : warning 250: variable "n" used in loop condition not modified in loop body
-warning_250_251.pwn(32) : warning 250: variable "i" used in loop condition not modified in loop body
-warning_250_251.pwn(46) : warning 251: none of the variables used in loop condition are modified in loop body
-warning_250_251.pwn(47) : warning 251: none of the variables used in loop condition are modified in loop body
+warning_250_251.pwn(19) : warning 250: variable "n" used in loop condition not modified in loop body
+warning_250_251.pwn(20) : warning 250: variable "i" used in loop condition not modified in loop body
+warning_250_251.pwn(38) : warning 250: variable "n" used in loop condition not modified in loop body
+warning_250_251.pwn(39) : warning 250: variable "i" used in loop condition not modified in loop body
+warning_250_251.pwn(53) : warning 251: none of the variables used in loop condition are modified in loop body
+warning_250_251.pwn(54) : warning 251: none of the variables used in loop condition are modified in loop body
+warning_250_251.pwn(111) : warning 250: variable "n" used in loop condition not modified in loop body
+warning_250_251.pwn(112) : warning 251: none of the variables used in loop condition are modified in loop body
 """
 }
diff --git a/source/compiler/tests/warning_250_251.pwn b/source/compiler/tests/warning_250_251.pwn
index 4eb83fb..9cf76b7 100644
--- a/source/compiler/tests/warning_250_251.pwn
+++ b/source/compiler/tests/warning_250_251.pwn
@@ -3,14 +3,21 @@
 
 new glbvar = 0;
 
+stock UseVarByRef(&arg)
+	return arg;
+
+#pragma warning disable 238 // "meaningless combination of class specifiers (const reference)"
+stock UseVarByConstRef(const &arg)
+	return arg;
+
 main()
 {
 	new n = 0, m = 10;
 	static st = 0;
 
 	// Case 1: Variable is used inside a loop condition without being modified.
-	while (n < 10) {}                      // warning 250: variable used in loop condition not modified in loop body (symbol "n")
-	for (new i = 0, j = 0; i < 10; ++j) {} // warning 250: variable used in loop condition not modified in loop body (symbol "i")
+	while (n < 10) {}                      // warning 250: variable "n" used in loop condition not modified in loop body
+	for (new i = 0, j = 0; i < 10; ++j) {} // warning 250: variable "i" used in loop condition not modified in loop body
 
 	// Case 2: Variable is used inside a loop condition and modified in the loop body.
 	while (n != 0) { n++; }
@@ -28,8 +35,8 @@ main()
 
 	// Case 5: Same variable is used inside a loop condition more than once
 	// and it's not modified.
-	while (n == 0 || n < 10) {}            // warning 250: variable used in loop condition not modified in loop body (symbol "n")
-	for (new i = 0; i == 0 || i < 10; ) {} // warning 250: variable used in loop condition not modified in loop body (symbol "i")
+	while (n == 0 || n < 10) {}            // warning 250: variable "n" used in loop condition not modified in loop body
+	for (new i = 0; i == 0 || i < 10; ) {} // warning 250: variable "i" used in loop condition not modified in loop body
 
 	// Case 6: Same variable is used inside a loop condition more than once,
 	// but it's modified.
@@ -91,5 +98,17 @@ main()
 	// way to track this.
 	while (n < glbvar) {}
 	for (new i = 0; i < glbvar; ) {}
+
+	// Case 13: Warnings 250 and 251 shouldn't trigger when the loop counter
+	// variable is passed to a function by reference.
+	while (n < 10) UseVarByRef(n);
+	while (n < m) UseVarByRef(n);
+
+	// Case 14: While const references for single function arguments are
+	// meaningless and there's warning 238 for this, such references still
+	// shouldn't affect warnings 250 and 251, as variables passed by const
+	// references aren't counted as modified.
+	while (n < 10) UseVarByConstRef(n); // warning 250: variable "n" used in loop condition not modified in loop body
+	while (n < m) UseVarByConstRef(n);  // warning 251: none of the variables used in loop condition are modified in loop body
 }
 

From 1951308a304614b7db10fa600c7d4f190b4f5d85 Mon Sep 17 00:00:00 2001
From: Stanislav Gromov <gromovstanislav@yandex.ru>
Date: Sun, 31 Jan 2021 21:29:25 +0700
Subject: [PATCH 10/11] Make the new diagnostics actually work on `do-while`
 loops

---
 source/compiler/sc.h  |  2 +-
 source/compiler/sc1.c | 70 ++++++++++++++++++++++++++-----------------
 source/compiler/sc2.c |  4 +--
 source/compiler/sc3.c |  8 ++---
 4 files changed, 49 insertions(+), 35 deletions(-)

diff --git a/source/compiler/sc.h b/source/compiler/sc.h
index 2a313c0..3d9d2fd 100644
--- a/source/compiler/sc.h
+++ b/source/compiler/sc.h
@@ -1023,7 +1023,7 @@ 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_loopcond;     /* equals to 'tFOR', 'tWHILE' or 'tDO' if the current expression is a loop condition, zero otherwise */
 SC_VDECL int pc_numloopvars;  /* number of variables used inside a loop condition */
 
 SC_VDECL constvalue_root sc_automaton_tab; /* automaton table */
diff --git a/source/compiler/sc1.c b/source/compiler/sc1.c
index d7ffa55..f38ae61 100644
--- a/source/compiler/sc1.c
+++ b/source/compiler/sc1.c
@@ -116,8 +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 scanloopvariables(symstate **loopvars,int dowhile);
+static void testloopvariables(symstate *loopvars,int dowhile,int line);
 static void destructsymbols(symbol *root,int level);
 static constvalue *find_constval_byval(constvalue_root *table,cell val);
 static symbol *fetchlab(char *name);
@@ -921,7 +921,7 @@ static void resetglobals(void)
   pc_naked=FALSE;
   pc_retexpr=FALSE;
   pc_attributes=0;
-  pc_loopcond=FALSE;
+  pc_loopcond=0;
   emit_flags=0;
   emit_stgbuf_idx=-1;
 }
@@ -5177,7 +5177,7 @@ static int testsymbols(symbol *root,int level,int testlabs,int testconst)
   return entry;
 }
 
-static void scanloopvariables(symstate **loopvars)
+static void scanloopvariables(symstate **loopvars,int dowhile)
 {
   symbol *start,*sym;
   int num;
@@ -5187,9 +5187,10 @@ static void scanloopvariables(symstate **loopvars)
   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)
+  /* if there's no enclosing loop (only one active loop entry, which is the
+   * current loop), and the current loop is not 'do-while', then we don't need
+   * to memoize usage flags for local variables, so we have an early exit */
+  if (wqptr-wqSIZE==wq && !dowhile)
     return;
 
   /* skip labels */
@@ -5211,19 +5212,26 @@ static void scanloopvariables(symstate **loopvars)
     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
+    /* 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->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;
+     * from checks in the current loop, ... */
+    if ((sym->ident==iVARIABLE || sym->ident==iREFERENCE)
+        && (dowhile || (sym->usage & uLOOPVAR)!=0)) {
+      /* ... but it might be already set from an enclosing loop as well, so we
+       * have to temporarily store it in "loopvars[num]" first. Also, if this is
+       * a 'do-while' loop, we need to memoize and unset the 'uWRITTEN' flag, so
+       * later when analyzing the loop condition (which comes after the loop
+       * body) we'll be able to determine if the variable was modified inside
+       * the loop body by checking if the 'uWRITTEN' flag is set. */
+      (*loopvars)[num].usage |= (sym->usage & (uNOLOOPVAR | uWRITTEN));
+      sym->usage &= ~uWRITTEN;
+      if (wqptr-wqSIZE!=wq)
+        sym->usage |= uNOLOOPVAR;
     } /* if */
   } /* if */
 }
 
-static void testloopvariables(symstate *loopvars,int line)
+static void testloopvariables(symstate *loopvars,int dowhile,int line)
 {
   symbol *start,*sym;
   int num,warnnum=0;
@@ -5245,7 +5253,8 @@ static void testloopvariables(symstate *loopvars,int line)
     warnnum=(pc_numloopvars==1) ? 250 : 251;
     for (sym=start; sym!=NULL; sym=sym->next)
       if ((sym->ident==iVARIABLE || sym->ident==iREFERENCE)
-          && (sym->usage & (uLOOPVAR | uNOLOOPVAR))==uLOOPVAR)
+          && (sym->usage & (uLOOPVAR | uNOLOOPVAR))==uLOOPVAR
+          && (!dowhile || (sym->usage & uWRITTEN)==0))
         pc_numloopvars--;
     if (pc_numloopvars==0 && warnnum==251) {
       errorset(sSETPOS,line);
@@ -5268,7 +5277,7 @@ static void testloopvariables(symstate *loopvars,int line)
       } /* if */
       sym->usage &= ~uNOLOOPVAR;
       if (loopvars!=NULL)
-        sym->usage |= (loopvars[num].usage & uNOLOOPVAR);
+        sym->usage |= loopvars[num].usage;
     } /* if */
   } /* for */
   free(loopvars);
@@ -6039,17 +6048,17 @@ static int dowhile(void)
    * tiniest loop, set it below the top of the loop
    */
   setline(TRUE);
-  scanloopvariables(&loopvars);
+  scanloopvariables(&loopvars,FALSE);
   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;
+  pc_loopcond=tWHILE;
   endlessloop=test(wq[wqEXIT],TEST_DO,FALSE);/* branch to wq[wqEXIT] if false */
-  pc_loopcond=FALSE;
+  pc_loopcond=0;
   pc_nestlevel--;
   statement(NULL,FALSE);        /* if so, do a statement */
   clearassignments(pc_nestlevel+1);
-  testloopvariables(loopvars,loopline);
+  testloopvariables(loopvars,FALSE,loopline);
   jumplabel(wq[wqLOOP]);        /* and loop to "while" start */
   setlabel(wq[wqEXIT]);         /* exit label */
   delwhile();                   /* delete queue entry */
@@ -6067,32 +6076,37 @@ static int dowhile(void)
 static int dodo(void)
 {
   int wq[wqSIZE],top;
-  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);           /* see "dowhile" for more info */
   top=getlabel();         /* make a label first */
   setlabel(top);          /* loop label */
+  scanloopvariables(&loopvars,TRUE);
   statement(NULL,FALSE);
   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 */
+  pc_loopcond=tDO;
   endlessloop=test(wq[wqEXIT],TEST_OPT,FALSE);
+  pc_loopcond=0;
   pc_nestlevel--;
   clearassignments(pc_nestlevel+1);
-  testloopvariables(loopvars,loopline);
+  testloopvariables(loopvars,TRUE,loopline);
   jumplabel(top);
   setlabel(wq[wqEXIT]);
   delwhile();
   needtoken(tTERM);
 
   retcode=endlessloop ? tENDLESS : tDO;
+  pc_numloopvars=save_numloopvars;
   endlessloop=save_endlessloop;
   return retcode;
 }
@@ -6144,7 +6158,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);
+  scanloopvariables(&loopvars,FALSE);
   /* 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.
@@ -6159,9 +6173,9 @@ static int dofor(void)
   if (matchtoken(';')) {
     endlessloop=1;
   } else {
-    pc_loopcond=TRUE;
+    pc_loopcond=tFOR;
     endlessloop=test(wq[wqEXIT],TEST_PLAIN,FALSE);/* expression 2 (jump to wq[wqEXIT] if false) */
-    pc_loopcond=FALSE;
+    pc_loopcond=0;
     needtoken(';');
   } /* if */
   stgmark((char)(sEXPRSTART+1));    /* mark start of 3th expression in stage */
@@ -6174,7 +6188,7 @@ static int dofor(void)
   stgset(FALSE);                    /* stop staging */
   statement(NULL,FALSE);
   clearassignments(save_nestlevel+1);
-  testloopvariables(loopvars,loopline);
+  testloopvariables(loopvars,FALSE,loopline);
   jumplabel(wq[wqLOOP]);
   setlabel(wq[wqEXIT]);
   delwhile();
diff --git a/source/compiler/sc2.c b/source/compiler/sc2.c
index 75e8b60..5bd9e1d 100644
--- a/source/compiler/sc2.c
+++ b/source/compiler/sc2.c
@@ -3425,13 +3425,13 @@ 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 (pc_loopcond!=0) {
     if (sym->vclass==sGLOBAL) {
       /* stop counting variables that were used in loop condition, otherwise
        * warnings 250 and 251 may be inaccurate (global variables can be
        * modified from another function(s) called from the loop body, and
        * currently there's no reasonable way to track this) */
-      pc_loopcond=FALSE;
+      pc_loopcond=0;
       pc_numloopvars=0;
     } else if ((usage & uWRITTEN)!=0) {
       /* the symbol is being modified inside a loop condition before being read;
diff --git a/source/compiler/sc3.c b/source/compiler/sc3.c
index 60d67c9..e6234d5 100644
--- a/source/compiler/sc3.c
+++ b/source/compiler/sc3.c
@@ -1770,10 +1770,10 @@ restart:
         error(51);      /* invalid subscript, must use [ ] */
         invsubscript=TRUE;
       } /* if */
-      if (pc_loopcond) {
+      if (pc_loopcond!=0) {
         /* stop counting variables that were used in loop condition,
          * otherwise warnings 250 and 251 may be inaccurate */
-        pc_loopcond=FALSE;
+        pc_loopcond=0;
         pc_numloopvars=0;
       } /* if */
       if (invsubscript) {
@@ -2202,10 +2202,10 @@ static int nesting=0;
     error(29); /* invalid expression, assumed zero */
     return;
   } /* if */
-  if (pc_loopcond) {
+  if (pc_loopcond!=0) {
     /* stop counting variables that were used in loop condition,
      * otherwise warnings 249 and 250 may be inaccurate */
-    pc_loopcond=FALSE;
+    pc_loopcond=0;
     pc_numloopvars=0;
   } /* if */
   /* check whether this is a function that returns an array */

From 64b2cda36ce6638f0d4e2b8b64c0bf5850644a88 Mon Sep 17 00:00:00 2001
From: Stanislav Gromov <gromovstanislav@yandex.ru>
Date: Sun, 31 Jan 2021 22:52:51 +0700
Subject: [PATCH 11/11] Update tests

---
 source/compiler/tests/warning_250_251.meta | 17 ++++++++++-------
 source/compiler/tests/warning_250_251.pwn  | 14 ++++++++++++--
 2 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/source/compiler/tests/warning_250_251.meta b/source/compiler/tests/warning_250_251.meta
index 57a6b24..42bbdb4 100644
--- a/source/compiler/tests/warning_250_251.meta
+++ b/source/compiler/tests/warning_250_251.meta
@@ -2,12 +2,15 @@
   'test_type': 'output_check',
   'errors': """
 warning_250_251.pwn(19) : warning 250: variable "n" used in loop condition not modified in loop body
-warning_250_251.pwn(20) : warning 250: variable "i" used in loop condition not modified in loop body
-warning_250_251.pwn(38) : warning 250: variable "n" used in loop condition not modified in loop body
-warning_250_251.pwn(39) : warning 250: variable "i" used in loop condition not modified in loop body
-warning_250_251.pwn(53) : warning 251: none of the variables used in loop condition are modified in loop body
-warning_250_251.pwn(54) : warning 251: none of the variables used in loop condition are modified in loop body
-warning_250_251.pwn(111) : warning 250: variable "n" used in loop condition not modified in loop body
-warning_250_251.pwn(112) : warning 251: none of the variables used in loop condition are modified in loop body
+warning_250_251.pwn(20) : warning 250: variable "n" used in loop condition not modified in loop body
+warning_250_251.pwn(21) : warning 250: variable "i" used in loop condition not modified in loop body
+warning_250_251.pwn(42) : warning 250: variable "n" used in loop condition not modified in loop body
+warning_250_251.pwn(43) : warning 250: variable "n" used in loop condition not modified in loop body
+warning_250_251.pwn(44) : warning 250: variable "i" used in loop condition not modified in loop body
+warning_250_251.pwn(59) : warning 251: none of the variables used in loop condition are modified in loop body
+warning_250_251.pwn(60) : warning 251: none of the variables used in loop condition are modified in loop body
+warning_250_251.pwn(61) : warning 251: none of the variables used in loop condition are modified in loop body
+warning_250_251.pwn(122) : warning 250: variable "n" used in loop condition not modified in loop body
+warning_250_251.pwn(123) : warning 251: none of the variables used in loop condition are modified in loop body
 """
 }
diff --git a/source/compiler/tests/warning_250_251.pwn b/source/compiler/tests/warning_250_251.pwn
index 9cf76b7..7cd1b4f 100644
--- a/source/compiler/tests/warning_250_251.pwn
+++ b/source/compiler/tests/warning_250_251.pwn
@@ -17,10 +17,12 @@ main()
 
 	// Case 1: Variable is used inside a loop condition without being modified.
 	while (n < 10) {}                      // warning 250: variable "n" used in loop condition not modified in loop body
+	do {} while (n < 10);                  // warning 250: variable "n" used in loop condition not modified in loop body
 	for (new i = 0, j = 0; i < 10; ++j) {} // warning 250: variable "i" used in loop condition not modified in loop body
 
 	// Case 2: Variable is used inside a loop condition and modified in the loop body.
 	while (n != 0) { n++; }
+	do { n++; } while (n < 10);
 	for (new i = 0; i < 10; ) { i++; }
 
 	// Case 3: Variable is used inside a loop condition and modified in the
@@ -30,17 +32,21 @@ main()
 	// Case 4: Variable is used and modified inside a loop condition.
 	while (n++ != 0) {}
 	while (++n != 0) {}
+	do {} while (n++ != 0);
+	do {} while (++n != 0);
 	for (new i = 0; i++ < 10; ) {}
 	for (new i = 0; ++i < 10; ) {}
 
 	// Case 5: Same variable is used inside a loop condition more than once
 	// and it's not modified.
 	while (n == 0 || n < 10) {}            // warning 250: variable "n" used in loop condition not modified in loop body
+	do {} while (n == 0 || n < 10);        // warning 250: variable "n" used in loop condition not modified in loop body
 	for (new i = 0; i == 0 || i < 10; ) {} // warning 250: variable "i" used in loop condition not modified in loop body
 
 	// Case 6: Same variable is used inside a loop condition more than once,
 	// but it's modified.
 	while (n == 0 || n < 10) { n++; }
+	do { n++; } while (n == 0 || n < 10);
 	for (new i = 0; i == 0 || i < 10; i++) {}
 
 	// Case 7: Two variables are used inside a loop condition, both aren't modified.
@@ -51,18 +57,21 @@ main()
 	// Solution: introduce a warning that simply says that none of the variables
 	// were modified, and let the user decide which variable should be modified.
 	while (n < m) {}            // warning 251: none of the variables used in loop condition are modified in loop body
+	do {} while (n < m);        // warning 251: none of the variables used in loop condition are modified in loop body
 	for (new i = 0; i < m; ) {} // warning 251: none of the variables used in loop condition are modified in loop body
 
 	// Case 7: Two variables are used in a loop condition, but one of them
 	// is modified inside the loop body (or the loop counter increment/decrement
 	// section of a "for" loop), and the other one is not modified.
 	while (n < m) { ++n; }
+	do { --m; } while (n < m);
 	for (new i = 0; i < m; ) { i++; }
 	for (new i = 0; i < m; i++) {}
 
 	// Case 8: Two variables are used in a loop condition, but one of them
 	// is being modified prior to being used, and the other one is not modified.
 	while (++n < m) {}
+	do {} while (++n < m);
 	for (new i = 0; ++i < m; ) {}
 
 	// Case 9: Two variables are used in a loop condition, but one of them
@@ -89,7 +98,8 @@ main()
 	// may be inaccurate otherwise.
 	new File:f = fopen("test.txt", io_read);
 	new line[128];
-	while(fread(f,line,sizeof(line),false) < m) {} // shouldn't warn about "f" or "m" not being modified
+	while (fread(f,line,sizeof(line),false) < m) {}     // shouldn't warn about "f" or "m" not being modified
+	do {} while (fread(f,line,sizeof(line),false) < m); // shouldn't warn about "f" or "m" not being modified
 	fclose(f);
 
 	// Case 12: Warnings 250 and 251 shouldn't trigger when at least one global
@@ -97,6 +107,7 @@ main()
 	// from a function called from the loop body and currently there's no easy
 	// way to track this.
 	while (n < glbvar) {}
+	do {} while (n < glbvar);
 	for (new i = 0; i < glbvar; ) {}
 
 	// Case 13: Warnings 250 and 251 shouldn't trigger when the loop counter
@@ -111,4 +122,3 @@ main()
 	while (n < 10) UseVarByConstRef(n); // warning 250: variable "n" used in loop condition not modified in loop body
 	while (n < m) UseVarByConstRef(n);  // warning 251: none of the variables used in loop condition are modified in loop body
 }
-