Merge nim-lang#318

318: vm: add back support for non-ordinal `case` selectors r=saem a=zerbina

`case` statements using _slicelists_ in their `of` branches now compile
and run again when the selector is of `float` or `string` type

Co-authored-by: zerbina <100542850+zerbina@users.noreply.github.com>

compiler/vm/vm.nim

@@ -2190,23 +2190,38 @@ proc rawExecute(c: var TCtx, pc: var int, tos: var StackFrameIndex): RegisterInd
 
       checkHandle(regs[ra])
 
+      func contains[T](list: openArray[Slice[T]], v: T): bool =
+        for s in list.items:
+          if v in s: return true
+
       var cond = false
       case value.kind
       of cnstInt:      cond = regs[ra].intVal == value.intVal
       of cnstString:   cond = regs[ra].strVal == deref(value.strVal).strVal
-      of cnstBranchLit:
-        let a = regs[ra].intVal
-        let L = value.ranges.len
-        for i in countup(0, L-1, 2):
-          let l = value.ranges[i+0]
-          let h = value.ranges[i+1]
-
-          if a in l..h:
-            cond = true
-            break
+      of cnstFloat:    cond = regs[ra].floatVal == value.floatVal
+      of cnstSliceListInt:   cond = regs[ra].intVal in value.intSlices
+      of cnstSliceListFloat: cond = regs[ra].floatVal in value.floatSlices
+      of cnstSliceListStr:
+        # string slice-lists don't store the strings directly, but the ID of
+        # a constant instead
+        let str = regs[ra].strVal
+        for s in value.strSlices.items:
+          let a = deref(c.constants[s.a].strVal).strVal
+          let r = cmp(a, str)
+          if s.a == s.b:
+            # no need to compare the string with both slice elements if
+            # they're the same
+            if r == 0:
+              cond = true
+              break
+          else:
+            let b = deref(c.constants[s.b].strVal).strVal
+            if r <= 0 and cmp(str, b) <= 0:
+              cond = true
+              break
 
       else:
-        assert false, $value.kind
+        unreachable($value.kind)
 
       assert c.code[pc+1].opcode == opcFJmp
       inc pc
@@ -2354,8 +2369,8 @@ proc rawExecute(c: var TCtx, pc: var int, tos: var StackFrameIndex): RegisterInd
         #      cases where non-NimNode PNodes need to be stored in registers
         #      seems unnecessary however.
         regs[ra] = TFullReg(kind: rkNimNode, nimNode: cnst.node)
-      of cnstBranchLit:
-        # A branch literal must not be used with `LdConst`
+      of cnstSliceListInt..cnstSliceListStr:
+        # A slice-list must not be used with `LdConst`
         assert false
 
     of opcAsgnConst:

compiler/vm/vmdef.nim

@@ -327,7 +327,14 @@ type
     cnstFloat
     cnstString
     cnstNode ## AST, type literals
-    cnstBranchLit
+
+    # slice-lists are used for implementing `opcBranch` (branch for case stmt)
+    cnstSliceListInt
+    cnstSliceListFloat
+    cnstSliceListStr
+
+  ConstantId* = int ## The ID of a `VmConstant`. Currently just an index into
+                    ## `PCtx.constants`
 
   VmConstant* = object
     ## `VmConstant`s are used for passing constant data from `vmgen` to the
@@ -347,11 +354,16 @@ type
       #      `opcMatConst` mechanism and `strVal` should be a simple `string`
     of cnstNode:
       node*: PNode
-    of cnstBranchLit:
-      # Used for implementing multi-value 'of' branches
+
+    of cnstSliceListInt:
       # XXX: always using `BiggestInt` is inredibly wasteful for when the
       #      values are small (e.g. `char`)
-      ranges*: seq[BiggestInt]
+      intSlices*: seq[Slice[BiggestInt]]
+    of cnstSliceListFloat:
+      floatSlices*: seq[Slice[BiggestFloat]]
+    of cnstSliceListStr:
+      strSlices*: seq[Slice[ConstantId]] ## Stores the ids of string constants
+                                         ## as a storage optimization
 
   RegInfo* = object
     refCount*: uint16

compiler/vm/vmgen.nim

@@ -183,7 +183,7 @@ proc codeListing*(c: TCtx, prc: PSym, ast: PNode, start=0; last = -1) =
             newStrNode(nkStrLit, $deref(cnst.strVal).strVal)
           of cnstNode:
             cnst.node
-          of cnstBranchLit:
+          of cnstSliceListInt..cnstSliceListStr:
             # XXX: translate into an `nkOfBranch`?
             newNode(nkEmpty)
 
@@ -661,44 +661,65 @@ proc genLiteral(c: var TCtx, n: PNode): int =
     c.config.internalError(n.info, $n.kind)
     0
 
+template fillSliceList[T](sl: var seq[Slice[T]], nodes: openArray[PNode],
+                          get: untyped) =
+  sl.newSeq(nodes.len)
 
-proc genBranchLit(c: var TCtx, n: PNode): int =
-  ## Turn the of-branch `n` into a constant, add it to `c.constants` and
-  ## return it's index.
-  ##
-  ## Single values are turned into either an int or string constant, while
-  ## range values and multiple values are stored as an array of ranges (int
-  ## pairs).
-  ##
-  ## The of-branch:
-  ##
-  ## .. code-block:: nim
-  ##  of 1..2, 5, 6..10
+  template getIt(n): untyped =
+    block:
+      let it {.cursor, inject.} = n
+      get
+
+  for (i, n) in nodes.pairs:
+    sl[i] =
+      if n.kind == nkRange:
+        getIt(n[0]) .. getIt(n[1])
+      else:
+        let e = getIt(n)
+        e .. e
+
+proc genBranchLit(c: var TCtx, n: PNode, t: PType): int =
+  ## Turns the slice-list or single literal of the given `nkOfBranch` into
+  ## a constant and returns it's index in `c.constant`.
   ##
-  ## would be stored as [1, 2, 5, 5, 6, 10]
+  ## slice-lists are always added as a new constant while single literals
+  ## are reused
+
+  # XXX: slice-list constants (maybe `VmConstant`s in general) should be
+  #      stored in a `BiTable` so that it can be easily detected if they
+  #      already exist
+  assert t.kind in IntegralTypes+{tyString}
 
-  if n.len == 2 and n[0].kind in {nkCharLit..nkUInt64Lit, nkStrLit..nkTripleStrLit}:
+  if n.len == 2 and n[0].kind in nkLiterals:
     # It's an 'of' branch with a single value
     result = c.genLiteral(n[0])
   else:
     # It's an 'of' branch with multiple and/or range values
-    var lit = VmConstant(kind: cnstBranchLit)
-    # XXX: this breaks IC! See comment in `makeCnstFunc`
-    lit.ranges.newSeq((n.len-1) * 2)
-    for i in 0..<n.len - 1:
-      let it = n[i]
-      var l, h: BiggestInt
-      if it.kind == nkRange:
-        l = it[0].intVal
-        h = it[1].intVal
-      else:
-        assert it.kind in nkCharLit..nkUInt64Lit
-        l = it.intVal
-        h = l
-      lit.ranges[i * 2 + 0] = l
-      lit.ranges[i * 2 + 1] = h
+    var cnst: VmConstant
+
+    template values: untyped =
+      n.sons.toOpenArray(0, n.sons.high - 1) # -1 for the branch body
+
+    case t.kind
+    of IntegralTypes-{tyFloat..tyFloat128}:
+      cnst = VmConstant(kind: cnstSliceListInt)
+      cnst.intSlices.fillSliceList(values):
+        it.intVal
+
+    of tyFloat..tyFloat128:
+      cnst = VmConstant(kind: cnstSliceListFloat)
+      cnst.floatSlices.fillSliceList(values):
+        it.floatVal
+
+    of tyString:
+      cnst = VmConstant(kind: cnstSliceListStr)
+      cnst.strSlices.fillSliceList(values):
+        c.toStringCnst(it.strVal)
+
+    else:
+      unreachable($t.kind)
 
-    result = c.rawGenLiteral(lit)
+    result = c.rawGenLiteral(cnst)
 
 
 proc unused(c: TCtx; n: PNode; x: TDest) {.inline.} =
@@ -740,7 +761,7 @@ proc genCase(c: var TCtx; n: PNode; dest: var TDest) =
         # elif branches were eliminated during transformation
         doAssert branch.kind == nkOfBranch
 
-        let b = genBranchLit(c, branch)
+        let b = genBranchLit(c, branch, selType)
         c.gABx(branch, opcBranch, tmp, b)
         let elsePos = c.xjmp(branch.lastSon, opcFJmp, tmp)
         c.gen(branch.lastSon, dest)
@@ -1940,7 +1961,7 @@ proc genDiscrVal(c: var TCtx, discr, n: PNode, oty: PType): TRegister =
         c.gABx(n, opcLdImmInt, bIReg, bI)
       else:
         # of branch
-        let b = genBranchLit(c, branch)
+        let b = genBranchLit(c, branch, dt)
         c.gABx(branch, opcBranch, tmp, b)
         let elsePos = c.xjmp(branch.lastSon, opcFJmp, tmp)
         c.gABx(n, opcLdImmInt, bIReg, bI)

compiler/vm/vmobjects.nim

@@ -422,7 +422,7 @@ func asgnVmString*(dest: var VmString, src: VmString, a: var VmAllocator) =
   else:
     dest = newVmString(nilMemPtr, 0)
 
-func cmp(a, b: VmString): int =
+func cmp*(a, b: VmString): int =
   let minLen = min(a.len, b.len)
   result = cmpMem(a.data.rawPointer, b.data.rawPointer, minLen)
   if result == 0:

tests/vm/tcasestmt.nim

@@ -0,0 +1,51 @@
+discard """
+  description: "Tests for case statement with selector of non-ordinal type"
+  action: compile
+"""
+
+## XXX: merge into `casestmt/tcasestmt.nim` once testament has a VM target
+
+# case stmt with string slice-lists
+static:
+  proc p(str: string): int =
+    case str
+    of "a": 1
+    of "b".."e", "l".."o", "p": 2 # slice-list with gap
+    of "f".."k": 3
+    of "q".."z": 4
+    #of "9".."1": 5 # empty range; rejected by sem
+    else: 6
+
+  doAssert p("a") == 1
+  doAssert p("d") == 2
+  doAssert p("h") == 3
+  doAssert p("p") == 2
+  doAssert p("z") == 4
+  doAssert p("z1") == 6
+  doAssert p("2") == 6
+  doAssert p("other") == 6
+
+# case stmt with float slice-lists
+static:
+  proc p(f: float): int =
+    case f
+    of 5.0, -2.0 .. 1.2, 6.0: 0 # slice-list with gaps
+    of 1.5: 1
+    of 20.0 .. 32.0: 2
+    else: 3
+
+  # direct matches
+  doAssert p(5.0) == 0
+  doAssert p(-2.0) == 0
+  doAssert p(1.2) == 0
+  doAssert p(6.0) == 0
+  doAssert p(1.5) == 1
+  doAssert p(20.0) == 2
+  doAssert p(32.0) == 2
+
+  # in ranges
+  doAssert p(0.1) == 0
+  doAssert p(-1.0) == 0
+  doAssert p(1.1) == 0
+  doAssert p(1.6) == 3
+  doAssert p(25.0) == 2