stricter checks for negative array indexing

compiler/src/prog8/compiler/astprocessing/AstChecker.kt

@@ -1418,6 +1418,13 @@ internal class AstChecker(private val program: Program,
         if(target is VarDecl) {
             if(target.datatype !in IterableDatatypes && target.datatype!=DataType.UWORD)
                 errors.err("indexing requires an iterable or address uword variable", arrayIndexedExpression.position)
+            val indexVariable = arrayIndexedExpression.indexer.indexExpr as? IdentifierReference
+            if(indexVariable!=null) {
+                if(indexVariable.targetVarDecl(program)?.datatype in SignedDatatypes) {
+                    errors.err("variable array indexing can't be performed with signed variables", indexVariable.position)
+                    return
+                }
+            }
             val arraysize = target.arraysize?.constIndex()
             val index = arrayIndexedExpression.indexer.constIndex()
             if(arraysize!=null) {

compiler/src/prog8/compiler/astprocessing/VariousCleanups.kt

@@ -277,6 +277,10 @@ internal class VariousCleanups(val program: Program, val errors: IErrorReporter,
             val target = arrayIndexedExpression.arrayvar.targetVarDecl(program)
             val arraysize = target?.arraysize?.constIndex()
             if(arraysize!=null) {
+                if(arraysize+index < 0) {
+                    errors.err("index out of bounds", arrayIndexedExpression.position)
+                    return noModifications
+                }
                 // replace the negative index by the normal index
                 val newIndex = NumericLiteral.optimalNumeric(arraysize+index, arrayIndexedExpression.indexer.position)
                 arrayIndexedExpression.indexer.indexExpr = newIndex

compiler/test/ast/TestVariousCompilerAst.kt

@@ -535,7 +535,7 @@ main {
         byte @shared col = 20
         col++
         ubyte @shared ubb = lsb(col as uword)
-        uword @shared vaddr = bottom[col] as uword << 8          ; a mkword will get inserted here
+        uword @shared vaddr = bottom[cx16.r0L] as uword << 8          ; a mkword will get inserted here
     }
 }"""
         val result = compileText(VMTarget(), optimize=true, src, writeAssembly=false)!!

compiler/test/codegeneration/TestArrayThings.kt

@@ -6,6 +6,7 @@ import io.kotest.matchers.shouldNotBe
 import io.kotest.matchers.string.shouldContain
 import prog8.code.target.C64Target
 import prog8.code.target.VMTarget
+import prog8tests.helpers.ErrorReporterForTests
 import prog8tests.helpers.compileText
 import kotlin.io.path.readText
 
@@ -220,5 +221,104 @@ main {
 }"""
         compileText(C64Target(), false, src, writeAssembly = true) shouldNotBe null
     }
+
+    test("negative array index variables are not allowed, but ptr indexing is allowed") {
+        val src="""
+main {
+    sub start() {
+        uword @shared pointer
+        ubyte[10] array
+        str name = "hello"
+        
+        byte sindex
+        
+        array[sindex] = 10
+        cx16.r0L = array[sindex]
+        name[sindex] = 0
+        cx16.r0L = name[sindex]
+        pointer[sindex] = 10
+        cx16.r0L=pointer[sindex]
+    }
+}"""
+        val errors = ErrorReporterForTests()
+        compileText(VMTarget(), false, src, writeAssembly = false, errors = errors) shouldBe null
+        errors.errors.size shouldBe 4
+        errors.errors[0] shouldContain "signed variables"
+        errors.errors[1] shouldContain "signed variables"
+        errors.errors[2] shouldContain "signed variables"
+        errors.errors[3] shouldContain "signed variables"
+    }
+
+    test("bounds checking for both positive and negative indexes, correct cases") {
+        val src="""
+main {
+    sub start() {
+        ubyte[10] array
+        array[0] = 0
+        array[9] = 0
+        array[-1] = 0
+        array[-10] = 0
+    }
+}"""
+        compileText(VMTarget(), false, src, writeAssembly = false) shouldNotBe null
+    }
+
+    test("bounds checking on strings, correct cases") {
+        val src="""
+main {
+    sub start() {
+        str name = "1234567890"
+        name[0] = 0
+        name[9] = 0
+    }
+}"""
+        compileText(VMTarget(), false, src, writeAssembly = false) shouldNotBe null
+    }
+
+    test("bounds checking for positive indexes, invalid case") {
+        val src="""
+main {
+    sub start() {
+        ubyte[10] array
+        array[10] = 0
+    }
+}"""
+        val errors = ErrorReporterForTests()
+        compileText(VMTarget(), false, src, writeAssembly = false, errors = errors) shouldBe null
+        errors.errors.size shouldBe 1
+        errors.errors[0] shouldContain "out of bounds"
+    }
+
+    test("bounds checking for negative indexes, invalid case") {
+        val src="""
+main {
+    sub start() {
+        ubyte[10] array
+        array[-11] = 0
+    }
+}"""
+        val errors = ErrorReporterForTests()
+        compileText(VMTarget(), false, src, writeAssembly = false, errors = errors) shouldBe null
+        errors.errors.size shouldBe 1
+        errors.errors[0] shouldContain "out of bounds"
+    }
+
+    test("bounds checking on strings invalid cases") {
+        val src="""
+main {
+    sub start() {
+        str name = "1234567890"
+        name[10] = 0
+        name[-1] = 0
+        name[-11] = 0
+    }
+}"""
+        val errors = ErrorReporterForTests()
+        compileText(VMTarget(), false, src, writeAssembly = false, errors = errors) shouldBe null
+        errors.errors.size shouldBe 3
+        errors.errors[0] shouldContain "out of bounds"
+        errors.errors[1] shouldContain "out of bounds"
+        errors.errors[2] shouldContain "out of bounds"
+    }
 })
 

docs/source/programming.rst

@@ -337,7 +337,8 @@ An uword variable can be used in limited scenarios as a 'pointer' to a byte in m
 dynamic, location. You can use array indexing on a pointer variable to use it as a byte array at
 a dynamic location in memory: currently this is equivalent to directly referencing the bytes in
 memory at the given index. In contrast to a real array variable, the index value can be the size of a word.
-Unlike array variables, you cannot use a negative index to count from the end, because the size of the array is unknown.
+Unlike array variables, negative indexing for pointer variables does *not* mean it will be counting from the end, because the size of the buffer is unknown.
+Instead, it simply addresses memory that lies *before* the pointer variable.
 See also :ref:`pointervars_programming`
 
 **LSB/MSB split word arrays:**

docs/source/syntaxreference.rst

@@ -525,7 +525,8 @@ Array indexing
 
 Strings and arrays are a sequence of values. You can access the individual values by indexing.
 Negative index means counted from the end of the array rather than the beginning, where -1 means
-the last element in the array, -2 the second-to-last, etc. (Python uses this same scheme)
+the last element in the array, -2 the second-to-last, etc. (Python uses this same scheme.
+Note that this syntax is only valid for arrays, not for strings!  Python does allow the latter, but prog8 does not right now.)
 Use brackets to index into an array:  ``arrayvar[x]`` ::
 
     array[2]        ; the third byte in the array (index is 0-based)

docs/source/todo.rst

@@ -3,11 +3,6 @@ TODO
 
 See open issues on github.
 
-Generate proper index out of bounds error for array[-11] if the array is only size 10. (only array[-1]..array[-10] are valid)
-
-Putting a signed variable as an array index should be a compiler error. (using it for a pointer indexing is fine - but weird).
-Add some more explanation about this to the "array indexing" paragraph in the docs.
-
 Re-generate the skeletons doc files.
 
 optimize signed byte/word division by powers of 2 (and shift right?), it's now using divmod routine.  (also % ?)

examples/queens.p8

@@ -10,8 +10,10 @@ main {
     const ubyte NUMQUEENS=8
     ubyte[NUMQUEENS] board
 
-    sub could_place(byte row, byte col) -> bool {
+    sub could_place(ubyte row, ubyte col) -> bool {
-        byte i
+        if row==0
+            return true
+        ubyte i
         for i in 0 to row-1 {
             if board[i]==col or board[i]-i==col-row or board[i]+i==col+row
                 return false
@@ -44,7 +46,7 @@ main {
         }
         ubyte col
         for col in 0 to NUMQUEENS-1 {
-            if could_place(row as byte, col as byte) {
+            if could_place(row, col) {
                 board[row] = col
                 ; we need to save the local variables row and col.
                 sys.push(row)

examples/test.p8

@@ -5,49 +5,9 @@
 
 main {
     sub start() {
-        uword @shared ptr = $2000
+        ubyte[10] array
-        uword @shared index = 1000
+        array[10] = 0
-
+        ; array[-11] = 0
-        @($2000+1000) = 123
-        @($2000+1001) = 124
-
-        cx16.r0L = @(ptr+index)
-        cx16.r1L = @(ptr+1001)
-
-        txt.print_ub(cx16.r0L)
-        txt.spc()
-        txt.print_ub(cx16.r1L)
-        txt.spc()
-
-        cx16.r2L = ptr[index]
-        cx16.r3L = ptr[1001]
-
-        txt.print_ub(cx16.r2L)
-        txt.spc()
-        txt.print_ub(cx16.r3L)
-        txt.spc()
-
-        cx16.r0L = 200
-        cx16.r1L = 201
-
-        @(ptr+index) = cx16.r0L
-        @(ptr+1001) = cx16.r1L
-
-        txt.print_ub(@($2000+1000))
-        txt.spc()
-        txt.print_ub(@($2000+1001))
-        txt.spc()
-
-        cx16.r0L = 203
-        cx16.r1L = 204
-
-        ptr[index] = cx16.r0L
-        ptr[1001] = cx16.r1L
-
-        txt.print_ub(@($2000+1000))
-        txt.spc()
-        txt.print_ub(@($2000+1001))
-        txt.spc()
 
 ;        txt.print_ub(ptr[index])
 ;        txt.nl()