#110 Add a warning for comparisons between bytes and pointers.

docs/api/command-line.md

@@ -293,6 +293,10 @@ You can also enable or disable warnings individually:
     Whether should warn about deprecated aliases.
     Default: enabled.
 
+* `-Wcomparisons`, `-Wno-comparisons` –
+    Whether should warn about comparisons between bytes and pointers.
+    Default: enabled.
+
 * `-Wextra-comparisons`, `-Wno-extra-comparisons` –
     Whether should warn about simplifiable unsigned integer comparisons.
     Default: disabled.

src/main/scala/millfork/CompilationOptions.scala

@@ -424,6 +424,7 @@ object Cpu extends Enumeration {
     UselessCodeWarning,
     BuggyCodeWarning,
     FallbackValueUseWarning,
+    BytePointerComparisonWarning,
     DeprecationWarning,
     NonZeroTerminatedLiteralWarning,
     CallToOverlappingBankWarning,
@@ -588,6 +589,7 @@ object CompilationFlag extends Enumeration {
   BuggyCodeWarning,
   DeprecationWarning,
   FallbackValueUseWarning,
+  BytePointerComparisonWarning,
   ExtraComparisonWarnings,
   RorWarning,
   NonZeroTerminatedLiteralWarning,
@@ -605,6 +607,7 @@ object CompilationFlag extends Enumeration {
     BuggyCodeWarning,
     DeprecationWarning,
     FallbackValueUseWarning,
+    BytePointerComparisonWarning,
     ExtraComparisonWarnings,
     NonZeroTerminatedLiteralWarning,
     CallToOverlappingBankWarning,

src/main/scala/millfork/Main.scala

@@ -794,6 +794,10 @@ object Main {
       c.changeFlag(CompilationFlag.DeprecationWarning, v)
     }.description("Whether should warn about deprecated aliases. Default: enabled.")
 
+    boolean("-Wcomparisons", "-Wno-comparisons").repeatable().action { (c, v) =>
+      c.changeFlag(CompilationFlag.BytePointerComparisonWarning, v)
+    }.description("Whether should warn about comparisons between bytes and pointers. Default: enabled.")
+
     boolean("-Wextra-comparisons", "-Wno-extra-comparisons").repeatable().action { (c, v) =>
       c.changeFlag(CompilationFlag.ExtraComparisonWarnings, v)
     }.description("Whether should warn about simplifiable unsigned integer comparisons. Default: disabled.")

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

@@ -404,6 +404,20 @@ abstract class AbstractStatementPreprocessor(protected val ctx: CompilationConte
         }) ctx.log.warn("Division by zero.", params.head.position)
       case FunctionCallExpression("<<" | ">>" | "<<'" | "<<=" | ">>=" | "<<'=" | ">>>>", List(lhs@_, LiteralExpression(0, _))) =>
         if (ctx.options.flag(CompilationFlag.UselessCodeWarning)) ctx.log.warn("Shift by zero.", lhs.position)
+      case expr@FunctionCallExpression("==" | "!=" | ">" | ">=" | "<" | "<=", params) =>
+        if (ctx.options.flag(CompilationFlag.BytePointerComparisonWarning)) {
+          val types = params.map(p => AbstractExpressionCompiler.getExpressionType(ctx, p))
+          val hasByte = types.exists(t => t.size == 1)
+          val hasPointer = types.exists(t => t.name == "pointer" || t.isPointy)
+          if (hasByte && hasPointer) {
+            ctx.log.warn("Comparison between a byte and a pointer is usually pointless.", expr.position)
+            for (param <- params) param match {
+              case TextLiteralExpression(List(_)) =>
+                ctx.log.info("Did you mean to use a character literal here?", param.position)
+              case _ =>
+            }
+          }
+        }
       case _ =>
     }
     expr match {

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

@@ -41,4 +41,18 @@ class WarningSuite extends FunSuite with Matchers {
       """.stripMargin) { m =>
     }
   }
+
+  test("Warn about byte to pointer comparisons") {
+    EmuUnoptimizedCrossPlatformRun(Cpu.Mos, Cpu.Z80, Cpu.Motorola6809)(
+      """
+        | volatile byte b @$cfff
+        | noinline void f(){}
+        | void main () {
+        |  if b == "h" { f() }
+        |  if b == b.pointer { f() }
+        |  if b == b.addr { f() }
+        | }
+      """.stripMargin) { m =>
+    }
+  }
 }