From ecd83dd48621de783c81b36d3f3d3632272ff012 Mon Sep 17 00:00:00 2001
From: Clyybber <darkmine956@gmail.com>
Date: Tue, 22 Feb 2022 19:27:09 +0100
Subject: [PATCH] Debug unreported errors with -d:nimDebugUnreportedErrors

Also fix semIf not handling nkError nodes correctly.
Also cleanup buildErrorList/walkErrors roundabout way
of doing post-visit DFS.
Also improve rsemTypeKindMismatch error message.
---
 compiler/ast/errorhandling.nim   | 16 ++++++++++++----
 compiler/ast/errorreporting.nim  |  8 +++++++-
 compiler/ast/reports.nim         |  1 +
 compiler/ast/typesrenderer.nim   |  2 +-
 compiler/front/cli_reporter.nim  |  8 ++++++++
 compiler/front/main.nim          | 19 +++++++++++++++++++
 compiler/front/options.nim       |  4 ++++
 compiler/sem/semstmts.nim        | 16 +++++++++++-----
 doc/debug.rst                    | 15 ++++++++-------
 tests/errmsgs/tnested_errors.nim | 18 ++++++++++++++++++
 10 files changed, 89 insertions(+), 18 deletions(-)
 create mode 100644 tests/errmsgs/tnested_errors.nim

diff --git a/compiler/ast/errorhandling.nim b/compiler/ast/errorhandling.nim
index 923ea08a4955a..6be9cce95a1d6 100644
--- a/compiler/ast/errorhandling.nim
+++ b/compiler/ast/errorhandling.nim
@@ -43,6 +43,9 @@ import
   front/[
     msgs,
     options
+  ],
+  std/[
+    tables
   ]
 
 proc errorSubNode*(n: PNode): PNode =
@@ -102,6 +105,10 @@ proc newError*(
   for a in args:
     result.add #[ 3+ ]# a
 
+  when defined(nimDebugUnreportedErrors):
+    if errorKind != rsemWrappedError:
+      conf.unreportedErrors[result.reportId] = result
+
 proc newError*(
     conf: ConfigRef,
     wrongNode: PNode,
@@ -166,15 +173,16 @@ proc wrapIfErrorInSubTree*(conf: ConfigRef, wrongNodeContainer: PNode): PNode
         instLoc())
 
 proc buildErrorList(config: ConfigRef, n: PNode, errs: var seq[PNode]) =
-  ## creates a list (`errs` seq) from least specific to most specific
+  ## creates a list (`errs` seq) from most specific to least specific
+  ## by traversing the the error tree in a depth-first-search.
   case n.kind
   of nkEmpty .. nkNilLit:
     discard
   of nkError:
-    errs.add n
     buildErrorList(config, n[wrongNodePos], errs)
+    errs.add n
   else:
-    for i in countdown(n.len - 1, 0):
+    for i in 0..<n.len:
       buildErrorList(config, n[i], errs)
 
 iterator walkErrors*(config: ConfigRef; n: PNode): PNode =
@@ -186,7 +194,7 @@ iterator walkErrors*(config: ConfigRef; n: PNode): PNode =
   buildErrorList(config, n, errNodes)
 
   # report from last to first (deepest in tree to highest)
-  for i in countdown(errNodes.len - 1, 0):
+  for i in 0..<errNodes.len:
     # reverse index so we go from the innermost to outermost
     let e = errNodes[i]
     if e.errorKind == rsemWrappedError:
diff --git a/compiler/ast/errorreporting.nim b/compiler/ast/errorreporting.nim
index 692f1bf3d766f..9076223fc294e 100644
--- a/compiler/ast/errorreporting.nim
+++ b/compiler/ast/errorreporting.nim
@@ -13,7 +13,7 @@
 ## * write an error reporting proc that handles string conversion and also
 ##   determines which error handling strategy to use doNothing, raise, etc.
 
-import ast, errorhandling, renderer, reports
+import ast, errorhandling, renderer, reports, std/tables
 from front/options import ConfigRef
 from front/msgs import TErrorHandling
 
@@ -34,6 +34,12 @@ proc errorHandling*(err: PNode): TErrorHandling =
 template localReport*(conf: ConfigRef, node: PNode) =
   ## Write out existing sem report that is stored in the nkError node
   assert node.kind == nkError, $node.kind
+
+  when defined(nimDebugUnreportedErrors):
+    conf.unreportedErrors.del node.reportId
+    for err in walkErrors(conf, node):
+      conf.unreportedErrors.del err.reportId
+
   for err in walkErrors(conf, node):
     if true or canReport(conf, err):
       handleReport(conf, err.reportId, instLoc(), node.errorHandling)
diff --git a/compiler/ast/reports.nim b/compiler/ast/reports.nim
index b47b0e73e1ad5..ebbf3d3b0688e 100644
--- a/compiler/ast/reports.nim
+++ b/compiler/ast/reports.nim
@@ -955,4 +955,5 @@ func getReport*(list: ReportList, id: ReportId): Report =
 
 func actualType*(r: SemReport): PType = r.typeMismatch[0].actualType
 func formalType*(r: SemReport): PType = r.typeMismatch[0].formalType
+func formalTypeKind*(r: SemReport): set[TTypeKind] = r.typeMismatch[0].formalTypeKind
 func symstr*(r: SemReport): string = r.sym.name.s
diff --git a/compiler/ast/typesrenderer.nim b/compiler/ast/typesrenderer.nim
index 93b446ce96e06..2a2766938cda6 100644
--- a/compiler/ast/typesrenderer.nim
+++ b/compiler/ast/typesrenderer.nim
@@ -157,7 +157,7 @@ proc rangeToStr(n: PNode): string =
 const
   preferToResolveSymbols = {preferName, preferTypeName, preferModuleInfo,
   preferGenericArg, preferResolved, preferMixed}
-  typeToStr: array[TTypeKind, string] = ["None", "bool", "char", "empty",
+  typeToStr*: array[TTypeKind, string] = ["None", "bool", "char", "empty",
     "Alias", "typeof(nil)", "untyped", "typed", "typeDesc",
     # xxx typeDesc=>typedesc: typedesc is declared as such, and is 10x more common.
     "GenericInvocation", "GenericBody", "GenericInst", "GenericParam",
diff --git a/compiler/front/cli_reporter.nim b/compiler/front/cli_reporter.nim
index 1c4551bd0d139..b32ca000eca67 100644
--- a/compiler/front/cli_reporter.nim
+++ b/compiler/front/cli_reporter.nim
@@ -910,6 +910,14 @@ proc reportBody*(conf: ConfigRef, r: SemReport): string =
 
     of rsemTypeKindMismatch:
       result = r.str
+      result.add  " got '$1'" % typeToString(r.actualType)
+      result.add " but expected "
+      var first = true
+      for typKind in r.formalTypeKind:
+        if not first:
+          result.add " or "
+        result.add "'$1'" % typeToStr[typKind]
+        first = false
 
     of rsemExprHasNoAddress:
       result = "expression has no address"
diff --git a/compiler/front/main.nim b/compiler/front/main.nim
index bb53e4b8c774c..4fcbdf5a072fd 100644
--- a/compiler/front/main.nim
+++ b/compiler/front/main.nim
@@ -59,6 +59,19 @@ from ic / ic import rodViewer
 when not defined(leanCompiler):
   import backend/jsgen, tools/[docgen, docgen2]
 
+when defined(nimDebugUnreportedErrors):
+  import std/exitprocs
+  import utils/astrepr
+
+  proc echoAndResetUnreportedErrors(conf: ConfigRef) =
+    if conf.unreportedErrors.len > 0:
+      echo "Unreported errors:"
+      for reportId, node in conf.unreportedErrors:
+        var reprConf = defaultTReprConf
+        reprConf.flags.incl trfShowNodeErrors
+        echo conf.treeRepr(node)
+      conf.unreportedErrors.clear
+
 proc semanticPasses(g: ModuleGraph) =
   registerPass g, verbosePass
   registerPass g, semPass
@@ -318,6 +331,9 @@ proc mainCommand*(graph: ModuleGraph) =
     if conf.cmd in cmdDocLike + {cmdRst2html, cmdRst2tex}: ret = ret / htmldocsDir
     conf.outDir = ret
 
+  when defined(nimDebugUnreportedErrors):
+    addExitProc proc = echoAndResetUnreportedErrors(conf)
+
   ## process all commands
   case conf.cmd
   of cmdBackends: compileToBackend()
@@ -432,6 +448,9 @@ proc mainCommand*(graph: ModuleGraph) =
       echo conf.dump(conf.vmProfileData)
     genSuccessX(conf)
 
+  when defined(nimDebugUnreportedErrors):
+    echoAndResetUnreportedErrors(conf)
+
   when PrintRopeCacheStats:
     echo "rope cache stats: "
     echo "  tries : ", gCacheTries
diff --git a/compiler/front/options.nim b/compiler/front/options.nim
index 5d773debbf252..803ca532d5c6e 100644
--- a/compiler/front/options.nim
+++ b/compiler/front/options.nim
@@ -19,6 +19,7 @@ export in_options
 from terminal import isatty
 from times import utc, fromUnix, local, getTime, format, DateTime
 from std/private/globs import nativeToUnixPath
+
 const
   hasTinyCBackend* = defined(tinyc)
   useEffectSystem* = true
@@ -244,6 +245,9 @@ type
       debugUtilsStack*: seq[string] ## which proc name to stop trace output
       ## len is also used for output indent level
 
+    when defined(nimDebugUnreportedErrors):
+      unreportedErrors*: OrderedTable[ReportId, PNode]
+
 template `[]`*(conf: ConfigRef, idx: FileIndex): TFileInfo =
   conf.m.fileInfos[idx.uint32]
 
diff --git a/compiler/sem/semstmts.nim b/compiler/sem/semstmts.nim
index 44acfee3806f0..928d5a71dd679 100644
--- a/compiler/sem/semstmts.nim
+++ b/compiler/sem/semstmts.nim
@@ -134,6 +134,7 @@ proc semIf(c: PContext, n: PNode; flags: TExprFlags): PNode =
   result = n
   var typ = commonTypeBegin
   var hasElse = false
+  var hasError = false
   for i in 0..<n.len:
     var it = n[i]
     if it.len == 2:
@@ -142,16 +143,23 @@ proc semIf(c: PContext, n: PNode; flags: TExprFlags): PNode =
       it[1] = semExprBranch(c, it[1], flags)
       typ = commonType(c, typ, it[1])
       closeScope(c)
+      if it[0].isError or it[1].isError:
+        hasError = true
     elif it.len == 1:
       hasElse = true
       it[0] = semExprBranchScope(c, it[0])
       typ = commonType(c, typ, it[0])
+      if it[0].isError:
+        hasError = true
     else:
       semReportIllformedAst(
         c.config, it,
         "Expected one or two subnodes for if statement, but found " & $it.len)
 
-  if isEmptyType(typ) or typ.kind in {tyNil, tyUntyped} or
+  if hasError:
+    result = c.config.wrapErrorInSubTree(result)
+
+  elif isEmptyType(typ) or typ.kind in {tyNil, tyUntyped} or
       (not hasElse and efInTypeof notin flags):
     for it in n: discardCheck(c, it.lastSon, flags)
     result.transitionSonsKind(nkIfStmt)
@@ -532,10 +540,8 @@ proc semVarOrLet(c: PContext, n: PNode, symkind: TSymKind): PNode =
     var tup = skipTypes(typ, {tyGenericInst, tyAlias, tySink})
     if a.kind == nkVarTuple:
       if tup.kind != tyTuple:
-        localReport(c.config, a.info, SemReport(
-          kind: rsemTypeKindMismatch,
-          typeMismatch: @[c.config.typeMismatch(
-            formal = {tyTuple}, actual = tup)]))
+        localReport(c.config, a.info, c.config.semReportTypeMismatch(
+          a, {tyTuple}, tup))
 
       elif a.len - 2 != tup.len:
         localReport(
diff --git a/doc/debug.rst b/doc/debug.rst
index 078e334ab59cd..7a6a84826184e 100644
--- a/doc/debug.rst
+++ b/doc/debug.rst
@@ -20,13 +20,14 @@ overhead, because some debugging tools are not exactly cheap to run.
 
 **Used when compiling temporary compiler**
 
-==================== =======
-Define               Enables
--------------------- -------
-`nimVMDebugExecute`    Print out every instruction executed by the VM
-`nimVMDebugGenerate`   List VM code generated for every procedure called at compile-time
-`nimDebugUtils`        Enable semantic analysis execution tracer
-==================== =======
+=========================== =======
+Define                      Enables
+--------------------------- -------
+`nimVMDebugExecute`         Print out every instruction executed by the VM
+`nimVMDebugGenerate`        List VM code generated for every procedure called at compile-time
+`nimDebugUtils`             Enable semantic analysis execution tracer
+`nimDebugUnreportedErrors`  Enable unreported error debugging
+=========================== =======
 
 **Used when executing temporary compiler**
 
diff --git a/tests/errmsgs/tnested_errors.nim b/tests/errmsgs/tnested_errors.nim
new file mode 100644
index 0000000000000..eb6138d3b2343
--- /dev/null
+++ b/tests/errmsgs/tnested_errors.nim
@@ -0,0 +1,18 @@
+discard """
+description: "Tests that nested errors are not being lost or output in the wrong order"
+cmd: '''nim check $file'''
+action: reject
+nimout: '''
+tnested_errors.nim(15, 12) Error: undeclared identifier: 'b'
+tnested_errors.nim(15, 12) Error: expression has no type: b
+tnested_errors.nim(16, 11) Error: undeclared identifier: 'b'
+tnested_errors.nim(15, 9) Error: expression has no type: if b: b else: 1
+'''
+"""
+
+
+
+var a = if b:
+          b
+        else:
+          1