Warn about non-volatile spinlocks (see #95)

src/main/scala/millfork/compiler/AbstractStatementPreprocessor.scala

@@ -135,6 +135,16 @@ abstract class AbstractStatementPreprocessor(protected val ctx: CompilationConte
           ctx.log.warn("Pointless expression.", stmt.position)
         }
 
+      case WhileStatement(condition, Nil, Nil, _) =>
+        if (!env.isGoodEmptyLoopCondition(condition)) {
+          ctx.log.warn("Empty loop with a non-volatile condition may be optimized into a no-op or an infinite loop.", stmt.position)
+        }
+
+      case DoWhileStatement(Nil, Nil, condition, _) =>
+        if (!env.isGoodEmptyLoopCondition(condition)) {
+          ctx.log.warn("Empty loop with a non-volatile condition may be optimized into a no-op or an infinite loop.", stmt.position)
+        }
+
       case _ =>
     }
     stmt match {

src/main/scala/millfork/env/Environment.scala

@@ -2689,11 +2689,42 @@ class Environment(val parent: Option[Environment], val prefix: String, val cpuFa
         case _ => true // TODO: ?
       }
       case e: FunctionCallExpression => e.expressions.exists(isVolatile)
+      case e: SumExpression => e.expressions.exists(e => isVolatile(e._2))
       case e: IndexedExpression => isVolatile(VariableExpression(e.name)) || isVolatile(e.index)
       case _ => true
     }
   }
 
+  def isGoodEmptyLoopCondition(target: Expression): Boolean = {
+    if (eval(target).isDefined) {
+      // the user means an infinite loop or an empty loop
+      return true
+    }
+    target match {
+      case _: LiteralExpression => false
+      case _: GeneratedConstantExpression => false
+      case e: VariableExpression => maybeGet[Thing](e.name) match {
+        case Some(v: Variable) => v.isVolatile
+        case Some(v: MfArray) => true // TODO: all arrays assumed volatile for now
+        case Some(_: Constant) => false
+        case Some(_: Type) => false
+        case _ => true // TODO: ?
+      }
+      case e: FunctionCallExpression =>
+        e.functionName match {
+          case "==" | "!=" | ">" | "<" | ">=" | "<=" |
+               "*" | "*'" | "/" | "%%" |
+               "<<" | ">>" | "<<'" | ">>'" | ">>>>"
+               | "&" | "^" | "|" | "&&" | "^^" | "||" | "not" | "hi" | "lo" =>
+            e.expressions.exists(isVolatile)
+          case _ => true
+        }
+      case e: SumExpression => e.expressions.exists(e => isGoodEmptyLoopCondition(e._2))
+      case e: IndexedExpression => isGoodEmptyLoopCondition(VariableExpression(e.name)) || isGoodEmptyLoopCondition(e.index)
+      case _ => true
+    }
+  }
+
   def overlapsVariable(variable: String, expr: Expression): Boolean = {
     if (eval(expr).isDefined) return false
     if (expr.containsVariable(variable)) return true

src/test/scala/millfork/test/WarningSuite.scala

@@ -27,4 +27,18 @@ class WarningSuite extends FunSuite with Matchers {
       m.readByte(0xc000) should equal(4)
     }
   }
+  test("Loop over non-volatile variables") {
+    EmuUnoptimizedCrossPlatformRun(Cpu.Mos, Cpu.Z80, Cpu.Motorola6809)(
+      """
+        | byte flag
+        | volatile byte vflag
+        | void main () {
+        |  flag = 0
+        |  vflag = 0
+        |  while (flag != 0) {}
+        |  while (vflag != 0) {}
+        | }
+      """.stripMargin) { m =>
+    }
+  }
 }