stricter skip for conversions in array indices in transf (#24424)

fixes #17958

In `transf`, conversions in subscript expressions are skipped (with
`skipConv`'s rules). This is because array indexing can produce
conversions to the range type that is the array's index type, which
causes a `RangeDefect` rather than an `IndexDefect` (and also
`--rangeChecks` and `--indexChecks` are both considered). However this
causes problems when explicit conversions are used, between types of
different bitsizes, because those also get skipped.

To fix this, we only skip the conversion if:

* it's a hidden (implicit) conversion
* it's a range check conversion (produces `nkChckRange`)
* the subscript is on an array type and the result type of the
conversion has the same bounds as the array index type

And `skipConv` rules also still apply (int/float classification).

Another idea would be to prevent the implicit conversion to the array
index type from being generated. But there is no good way to do this:
matching to the base type instead prevents types like `uint32` from
implicitly converting (i.e. it can convert to `range[0..3]` but not
`int`), and analyzing whether this is an array bound check is easier in
`transf`, since `sigmatch` just produces a type conversion.

The rules for skipping the conversion could also receive some other
tweaks: We could add a rule that changing bitsizes also doesn't skip the
conversion, but this breaks the `uint32` case. We could simplify it to
only removing implicit skips to specifically fix #17958, but this is
wrong in general.

We could also add something like `nkChckIndex` that generates index
errors instead but this is weird when it doesn't have access to the
collection type and it might be overkill.

compiler/transf.nim

@@ -865,9 +865,18 @@ proc transformArrayAccess(c: PTransf, n: PNode): PNode =
   if n[0].kind == nkSym and n[0].sym.kind == skType:
     result = n
   else:
-    result = newTransNode(n)
-    for i in 0..<n.len:
-      result[i] = transform(c, skipConv(n[i]))
+    result = transformSons(c, n)
+    if n.len >= 2 and result[1].kind in {nkChckRange, nkChckRange64} and
+        n[1].kind in {nkHiddenStdConv, nkHiddenSubConv}:
+      # implicit conversion, was transformed into range check
+      # remove in favor of index check if conversion to array index type
+      # has to be done here because the array index type needs to be relaxed
+      # i.e. a uint32 index can implicitly convert to range[0..3] but not int
+      let arr = skipTypes(n[0].typ, abstractVarRange)
+      if arr.kind == tyArray and
+          firstOrd(c.graph.config, arr) == getOrdValue(result[1][1]) and
+          lastOrd(c.graph.config, arr) == getOrdValue(result[1][2]):
+        result[1] = result[1].skipConv
 
 proc getMergeOp(n: PNode): PSym =
   case n.kind

tests/array/tindexconv.nim

@@ -0,0 +1,4 @@
+block: # issue #17958
+  var mem: array[uint8, uint8]
+  let val = 0xffff'u16
+  discard mem[uint8 val]   # Error: unhandled exception: index 65535 not in 0 .. 255 [IndexDefect]

tests/array/tinvalidarrayaccess.nim

@@ -1,5 +1,5 @@
 discard """
-  errormsg: "index 2 not in 0 .. 1"
+  errormsg: "conversion from int literal(2) to range 0..1(int) is invalid"
   line: 18
 """
 block:

tests/array/tinvalidarrayaccess2.nim

@@ -1,5 +1,5 @@
 discard """
-  errormsg: "index 3 not in 0 .. 1"
+  errormsg: "conversion from int literal(3) to range 0..1(int) is invalid"
   line: 9
 """
 

tests/js/tindexdefect.nim

@@ -5,5 +5,6 @@ discard """
 """
 
 var s = ['a']
-let z = s[10000] == 'a'
-echo z
+let i = 10000
+let z = s[i] == 'a'
+echo z