From 3e6d16a7a8a4bcdecce3323b329c525b49406d68 Mon Sep 17 00:00:00 2001
From: Irmen de Jong <irmen@razorvine.net>
Date: Fri, 11 Aug 2023 02:14:54 +0200
Subject: [PATCH] add error message for invalid step size in range expression

---
 .../prog8/codegen/intermediate/IRCodeGen.kt   | 11 ++---
 .../optimizer/ConstantIdentifierReplacer.kt   |  8 ++-
 compiler/test/TestCompilerOnRanges.kt         | 10 +++-
 examples/test.p8                              | 49 +++++++++++++++++--
 4 files changed, 66 insertions(+), 12 deletions(-)

diff --git a/codeGenIntermediate/src/prog8/codegen/intermediate/IRCodeGen.kt b/codeGenIntermediate/src/prog8/codegen/intermediate/IRCodeGen.kt
index da0782ded..21030ce2a 100644
--- a/codeGenIntermediate/src/prog8/codegen/intermediate/IRCodeGen.kt
+++ b/codeGenIntermediate/src/prog8/codegen/intermediate/IRCodeGen.kt
@@ -589,22 +589,21 @@ class IRCodeGen(
         val iterable = forLoop.iterable as PtRange
         val step = iterable.step.number.toInt()
         val rangeStart = (iterable.from as PtNumber).number.toInt()
-        val rangeEndUntyped = (iterable.to as PtNumber).number.toInt() + step
+        val rangeEndExclusiveUntyped = (iterable.to as PtNumber).number.toInt() + step
         if(step==0)
             throw AssemblyError("step 0")
-        if(step>0 && rangeEndUntyped<rangeStart || step<0 && rangeEndUntyped>rangeStart)
+        if(step>0 && rangeEndExclusiveUntyped<rangeStart || step<0 && rangeEndExclusiveUntyped>rangeStart)
             throw AssemblyError("empty range")
-        val rangeEndWrapped = if(loopvarDtIr==IRDataType.BYTE) rangeEndUntyped and 255 else rangeEndUntyped and 65535
+        val rangeEndExclusiveWrapped = if(loopvarDtIr==IRDataType.BYTE) rangeEndExclusiveUntyped and 255 else rangeEndExclusiveUntyped and 65535
         val result = mutableListOf<IRCodeChunkBase>()
         val chunk = IRCodeChunk(null, null)
         chunk += IRInstruction(Opcode.LOAD, loopvarDtIr, reg1=indexReg, immediate = rangeStart)
         chunk += IRInstruction(Opcode.STOREM, loopvarDtIr, reg1=indexReg, labelSymbol=loopvarSymbol)
         result += chunk
         result += labelFirstChunk(translateNode(forLoop.statements), loopLabel)
-        result += addConstMem(loopvarDtIr, null, loopvarSymbol, step)
-        val chunk2 = IRCodeChunk(null, null)
+        val chunk2 = addConstMem(loopvarDtIr, null, loopvarSymbol, step)
         chunk2 += IRInstruction(Opcode.LOADM, loopvarDtIr, reg1 = indexReg, labelSymbol = loopvarSymbol)
-        chunk2 += IRInstruction(Opcode.BNE, loopvarDtIr, reg1 = indexReg, immediate = rangeEndWrapped, labelSymbol = loopLabel)
+        chunk2 += IRInstruction(Opcode.BNE, loopvarDtIr, reg1 = indexReg, immediate = rangeEndExclusiveWrapped, labelSymbol = loopLabel)
         result += chunk2
         return result
     }
diff --git a/codeOptimizers/src/prog8/optimizer/ConstantIdentifierReplacer.kt b/codeOptimizers/src/prog8/optimizer/ConstantIdentifierReplacer.kt
index 0d9bd7700..3c932cc05 100644
--- a/codeOptimizers/src/prog8/optimizer/ConstantIdentifierReplacer.kt
+++ b/codeOptimizers/src/prog8/optimizer/ConstantIdentifierReplacer.kt
@@ -280,10 +280,16 @@ internal class ConstantIdentifierReplacer(private val program: Program, private
                     val rangeExpr = decl.value as? RangeExpression
                     if(rangeExpr!=null) {
                         // convert the initializer range expression to an actual array
+                        val constRange = rangeExpr.toConstantIntegerRange()
+                        if(constRange?.isEmpty()==true) {
+                            if(constRange.first>constRange.last && constRange.step>=0)
+                                errors.err("descending range with positive step", decl.value?.position!!)
+                            else if(constRange.first<constRange.last && constRange.step<=0)
+                                errors.err("ascending range with negative step", decl.value?.position!!)
+                        }
                         val declArraySize = decl.arraysize?.constIndex()
                         if(declArraySize!=null && declArraySize!=rangeExpr.size())
                             errors.err("range expression size (${rangeExpr.size()}) doesn't match declared array size ($declArraySize)", decl.value?.position!!)
-                        val constRange = rangeExpr.toConstantIntegerRange()
                         if(constRange!=null) {
                             val eltType = rangeExpr.inferType(program).getOr(DataType.UBYTE)
                             val newValue = if(eltType in ByteDatatypes) {
diff --git a/compiler/test/TestCompilerOnRanges.kt b/compiler/test/TestCompilerOnRanges.kt
index e6cce92e4..617d57f56 100644
--- a/compiler/test/TestCompilerOnRanges.kt
+++ b/compiler/test/TestCompilerOnRanges.kt
@@ -264,7 +264,15 @@ class TestCompilerOnRanges: FunSpec({
             NumericLiteral.optimalInteger(2, Position.DUMMY),
             Position.DUMMY)
         expr.size() shouldBe 6
-        expr.toConstantIntegerRange()
+        expr.toConstantIntegerRange() shouldBe (10..20 step 2)
+
+        val expr2 = RangeExpression(
+            NumericLiteral.optimalInteger(20, Position.DUMMY),
+            NumericLiteral.optimalInteger(10, Position.DUMMY),
+            NumericLiteral.optimalInteger(-3, Position.DUMMY),
+            Position.DUMMY)
+        expr2.size() shouldBe 4
+        expr2.toConstantIntegerRange() shouldBe (20 downTo 10 step 3)
     }
 
     test("range with negative step should be constvalue") {
diff --git a/examples/test.p8 b/examples/test.p8
index 2615e9ace..84d35578d 100644
--- a/examples/test.p8
+++ b/examples/test.p8
@@ -3,11 +3,31 @@
 
 main {
     sub start () {
-        ubyte[4] array = 10 to 13
-        ubyte[4] array2 = 10 to 20 step 3
-        ubyte[4] array3 = 20 downto 10 step 3       ; TODO fix error about empty range expression
+        ubyte[4] @shared array = 10 to 20 step 3
+        ubyte[4] @shared array2 = 20 downto 10 step -3
+        byte[7] @shared array3 = 10 downto -10 step -3
 
         ubyte xx
+        byte bb
+
+        for xx in array {
+            txt.print_ub(xx)
+            txt.spc()
+        }
+        txt.nl()
+        for xx in array2 {
+            txt.print_ub(xx)
+            txt.spc()
+        }
+        txt.nl()
+        for bb in array3 {
+            txt.print_b(bb)
+            txt.spc()
+        }
+        txt.nl()
+        txt.nl()
+
+
         for xx in 10 to 20 step 3 {             ; TODO fix IR/VM code that wraps around instead of stopping at 19
             txt.print_ub(xx)
             txt.spc()
@@ -20,10 +40,31 @@ main {
         }
         txt.nl()
 
-        byte bb
         for bb in 10 downto -10 step -3 {        ; TODO fix IR/VM code that wraps around instead of stopping at -8
             txt.print_b(bb)
             txt.spc()
         }
+
+        txt.nl()
+        txt.nl()
+        ubyte ending = 20
+
+        for xx in 10 to ending step 3 {
+            txt.print_ub(xx)
+            txt.spc()
+        }
+        txt.nl()
+        ending =10
+        for xx in 20 downto ending step -3 {
+            txt.print_ub(xx)
+            txt.spc()
+        }
+        txt.nl()
+
+        byte endingb = -10
+        for bb in 10 downto endingb step -3 {
+            txt.print_b(bb)
+            txt.spc()
+        }
     }
 }