From 192634445abc69503bdb87718a8df8ea2e69a8f6 Mon Sep 17 00:00:00 2001
From: Jasper Jenkins <jasper.vs.jenkins@gmail.com>
Date: Tue, 17 Dec 2019 19:27:08 -0800
Subject: [PATCH] better case coverage error message for alias and range enum

---
 compiler/semobjconstr.nim                  | 56 +++----------------
 compiler/semstmts.nim                      |  6 +-
 compiler/semtypes.nim                      | 65 ++++++++++++++++------
 tests/casestmt/tincompletecaseobject2.nim  | 23 ++++++++
 tests/objvariant/tnon_zero_discrim_err.nim | 14 +++++
 tests/objvariant/trt_discrim_err2.nim      |  6 +-
 6 files changed, 100 insertions(+), 70 deletions(-)
 create mode 100644 tests/casestmt/tincompletecaseobject2.nim
 create mode 100644 tests/objvariant/tnon_zero_discrim_err.nim

diff --git a/compiler/semobjconstr.nim b/compiler/semobjconstr.nim
index 0062af24bfb4a..0ddec1706b050 100644
--- a/compiler/semobjconstr.nim
+++ b/compiler/semobjconstr.nim
@@ -84,55 +84,17 @@ proc caseBranchMatchesExpr(branch, matched: PNode): bool =
 
   return false
 
-template processBranchVals(b, op) =
-  if b.kind != nkElifBranch:
-    for i in 0..<b.len-1:
-      if b[i].kind == nkIntLit:
-        result.op(b[i].intVal.int)
-      elif b[i].kind == nkRange:
-        for i in b[i][0].intVal..b[i][1].intVal:
-          result.op(i.int)
-
-proc allPossibleValues(c: PContext, t: PType): IntSet =
-  result = initIntSet()
-  if t.enumHasHoles:
-    let t = t.skipTypes(abstractRange)
-    for field in t.n.sons:
-      result.incl(field.sym.position)
-  else:
-    for i in toInt64(firstOrd(c.config, t))..toInt64(lastOrd(c.config, t)):
-      result.incl(i.int)
-
 proc branchVals(c: PContext, caseNode: PNode, caseIdx: int,
                 isStmtBranch: bool): IntSet =
   if caseNode[caseIdx].kind == nkOfBranch:
     result = initIntSet()
-    processBranchVals(caseNode[caseIdx], incl)
+    for val in processBranchVals(caseNode[caseIdx]):
+      result.incl(val)
   else:
-    result = allPossibleValues(c, caseNode[0].typ)
+    result = c.getIntSetOfType(caseNode[0].typ)
     for i in 1..<caseNode.len-1:
-      processBranchVals(caseNode[i], excl)
-
-proc rangeTypVals(rangeTyp: PType): IntSet =
-  assert rangeTyp.kind == tyRange
-  let (a, b) = (rangeTyp.n[0].intVal, rangeTyp.n[1].intVal)
-  result = initIntSet()
-  for it in a..b:
-    result.incl(it.int)
-
-proc formatUnsafeBranchVals(t: PType, diffVals: IntSet): string =
-  if diffVals.len <= 32:
-    var strs: seq[string]
-    let t = t.skipTypes(abstractRange)
-    if t.kind in {tyEnum, tyBool}:
-      var i = 0
-      for val in diffVals:
-        while t.n[i].sym.position < val: inc(i)
-        strs.add(t.n[i].sym.name.s)
-    else:
-      for val in diffVals:
-        strs.add($val)
-    result = "{" & strs.join(", ") & "} "
+      for val in processBranchVals(caseNode[i]):
+        result.excl(val)
 
 proc findUsefulCaseContext(c: PContext, discrimator: PNode): (PNode, int) =
   for i in countdown(c.p.caseContext.high, 0):
@@ -250,9 +212,9 @@ proc semConstructFields(c: PContext, recNode: PNode,
 
       template valuesInConflictError(valsDiff) =
         localError(c.config, discriminatorVal.info, ("possible values " &
-          "$2are in conflict with discriminator values for " &
+          "$2 are in conflict with discriminator values for " &
           "selected object branch $1.") % [$selectedBranch,
-          formatUnsafeBranchVals(recNode[0].typ, valsDiff)])
+          valsDiff.renderAsType(recNode[0].typ)])
 
       let branchNode = recNode[selectedBranch]
       let flags = flags*{efAllowDestructor} + {efPreferStatic,
@@ -275,7 +237,7 @@ proc semConstructFields(c: PContext, recNode: PNode,
         let (ctorCase, ctorIdx) = findUsefulCaseContext(c, discriminatorVal)
         if ctorCase == nil:
           if discriminatorVal.typ.kind == tyRange:
-            let rangeVals = rangeTypVals(discriminatorVal.typ)
+            let rangeVals = c.getIntSetOfType(discriminatorVal.typ)
             let recBranchVals = branchVals(c, recNode, selectedBranch, false)
             let diff = rangeVals - recBranchVals
             if diff.len != 0:
@@ -311,7 +273,7 @@ proc semConstructFields(c: PContext, recNode: PNode,
               break
         if failedBranch != -1:
           if discriminatorVal.typ.kind == tyRange:
-            let rangeVals = rangeTypVals(discriminatorVal.typ)
+            let rangeVals = c.getIntSetOfType(discriminatorVal.typ)
             let recBranchVals = branchVals(c, recNode, selectedBranch, false)
             let diff = rangeVals - recBranchVals
             if diff.len != 0:
diff --git a/compiler/semstmts.nim b/compiler/semstmts.nim
index cf1063fe6b11f..0e87c205ad56d 100644
--- a/compiler/semstmts.nim
+++ b/compiler/semstmts.nim
@@ -953,9 +953,9 @@ proc semCase(c: PContext, n: PNode; flags: TExprFlags): PNode =
   if chckCovered:
     if covered == toCover(c, n[0].typ):
       hasElse = true
-    elif n[0].typ.kind == tyEnum:
-      localError(c.config, n.info, "not all cases are covered; missing: {$1}" %
-                 formatMissingEnums(n))
+    elif n[0].typ.skipTypes(abstractRange).kind == tyEnum:
+      localError(c.config, n.info, "not all cases are covered; missing: $1" %
+                 formatMissingEnums(c, n))
     else:
       localError(c.config, n.info, "not all cases are covered")
   popCaseContext(c)
diff --git a/compiler/semtypes.nim b/compiler/semtypes.nim
index 4d749638db420..cb6cb04f7bc8c 100644
--- a/compiler/semtypes.nim
+++ b/compiler/semtypes.nim
@@ -592,22 +592,53 @@ proc toCover(c: PContext, t: PType): Int128 =
 proc semRecordNodeAux(c: PContext, n: PNode, check: var IntSet, pos: var int,
                       father: PNode, rectype: PType, hasCaseFields = false)
 
-proc formatMissingEnums(n: PNode): string =
+proc getIntSetOfType(c: PContext, t: PType): IntSet =
+  result = initIntSet()
+  if t.enumHasHoles:
+    let t = t.skipTypes(abstractRange)
+    for field in t.n.sons:
+      result.incl(field.sym.position)
+  else:
+    assert(lengthOrd(c.config, t) <= BiggestInt(MaxSetElements))
+    for i in toInt64(firstOrd(c.config, t))..toInt64(lastOrd(c.config, t)):
+      result.incl(i.int)
+
+iterator processBranchVals(b: PNode): int =
+  assert b.kind in {nkOfBranch, nkElifBranch, nkElse}
+  if b.kind == nkOfBranch:
+    for i in 0..<b.len-1:
+      if b[i].kind == nkIntLit:
+        yield b[i].intVal.int
+      elif b[i].kind == nkRange:
+        for i in b[i][0].intVal..b[i][1].intVal:
+          yield i.int
+
+proc renderAsType(vals: IntSet, t: PType): string =
+  result = "{"
+  let t = t.skipTypes(abstractRange)
+  var enumSymOffset = 0
+  var i = 0
+  for val in vals:
+    if result.len > 1:
+      result &= ", "
+    if t.kind in {tyEnum, tyBool}:
+      while t.n[enumSymOffset].sym.position < val: inc(enumSymOffset)
+      result &= t.n[enumSymOffset].sym.name.s
+    else:
+      if i == 64:
+        result &= "omitted $1 values..." % $(vals.len - i)
+        break
+      else:
+        result &= $val
+    inc(i)
+  result &= "}"
+
+proc formatMissingEnums(c: PContext, n: PNode): string =
   var coveredCases = initIntSet()
   for i in 1..<n.len:
-    let ofBranch = n[i]
-    for j in 0..<ofBranch.len - 1:
-      let child = ofBranch[j]
-      if child.kind == nkIntLit:
-        coveredCases.incl(child.intVal.int)
-      elif child.kind == nkRange:
-        for k in child[0].intVal.int..child[1].intVal.int:
-          coveredCases.incl k
-  for child in n[0].typ.n.sons:
-    if child.sym.position notin coveredCases:
-      if result.len > 0:
-        result.add ", "
-      result.add child.sym.name.s
+    for val in processBranchVals(n[i]):
+      coveredCases.incl val
+  result = (c.getIntSetOfType(n[0].typ) - coveredCases).renderAsType(n[0].typ)
 
 proc semRecordCase(c: PContext, n: PNode, check: var IntSet, pos: var int,
                    father: PNode, rectype: PType) =
@@ -656,9 +687,9 @@ proc semRecordCase(c: PContext, n: PNode, check: var IntSet, pos: var int,
     delSon(b, b.len - 1)
     semRecordNodeAux(c, lastSon(n[i]), check, pos, b, rectype, hasCaseFields = true)
   if chckCovered and covered != toCover(c, a[0].typ):
-    if a[0].typ.kind == tyEnum:
-      localError(c.config, a.info, "not all cases are covered; missing: {$1}" %
-        formatMissingEnums(a))
+    if a[0].typ.skipTypes(abstractRange).kind == tyEnum:
+      localError(c.config, a.info, "not all cases are covered; missing: $1" %
+                 formatMissingEnums(c, a))
     else:
       localError(c.config, a.info, "not all cases are covered")
   father.add a
diff --git a/tests/casestmt/tincompletecaseobject2.nim b/tests/casestmt/tincompletecaseobject2.nim
new file mode 100644
index 0000000000000..ccf6a3a6c2aac
--- /dev/null
+++ b/tests/casestmt/tincompletecaseobject2.nim
@@ -0,0 +1,23 @@
+discard """
+cmd: "nim check $file"
+errormsg: "not all cases are covered; missing: {A, B}"
+nimout: '''
+tincompletecaseobject2.nim(16, 1) Error: not all cases are covered; missing: {B, C, D}
+tincompletecaseobject2.nim(19, 1) Error: not all cases are covered; missing: {A, C}
+tincompletecaseobject2.nim(22, 1) Error: not all cases are covered; missing: {A, B}
+'''
+"""
+type
+  ABCD = enum A, B, C, D
+  AliasABCD = ABCD
+  RangeABC = range[A .. C]
+  AliasRangeABC = RangeABC
+
+case AliasABCD A:
+of A: discard
+
+case RangeABC A:
+of B: discard
+
+case AliasRangeABC A:
+of C: discard
diff --git a/tests/objvariant/tnon_zero_discrim_err.nim b/tests/objvariant/tnon_zero_discrim_err.nim
new file mode 100644
index 0000000000000..c595bba1bde31
--- /dev/null
+++ b/tests/objvariant/tnon_zero_discrim_err.nim
@@ -0,0 +1,14 @@
+discard """
+  errormsg: "low(kind) must be 0 for discriminant"
+  line: 7
+"""
+type
+  HoledObj = object
+    case kind: int
+    of 0: a: int
+    else: discard
+
+let someInt = low(int)
+case someInt
+of 938: echo HoledObj(kind: someInt, a: 1)
+else: discard
diff --git a/tests/objvariant/trt_discrim_err2.nim b/tests/objvariant/trt_discrim_err2.nim
index c595bba1bde31..4f2790bc6c799 100644
--- a/tests/objvariant/trt_discrim_err2.nim
+++ b/tests/objvariant/trt_discrim_err2.nim
@@ -1,10 +1,10 @@
 discard """
-  errormsg: "low(kind) must be 0 for discriminant"
-  line: 7
+  errormsg: " branch initialization with a runtime discriminator only supports ordinal types with 2^16 elements or less."
+  line: 13
 """
 type
   HoledObj = object
-    case kind: int
+    case kind: range[0 .. 20000]
     of 0: a: int
     else: discard