Merge nim-lang#237

237: Enable debugging unreported errors r=Clyybber a=Clyybber

## Summary
Enabled with -d:nimDebugUnreportedErrors
Doesn't keep track of where the nodes were created yet.
Also fix semIf not handling nkError nodes correctly.
Also cleanup buildErrorList/walkErrors roundabout way
of doing post-visit DFS.
Also improve rsemTypeKindMismatch error message.

Co-authored-by: Clyybber <darkmine956@gmail.com>

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:

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)

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

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",

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"

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

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]
 

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(

doc/debug.rst

@@ -21,13 +21,14 @@ 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**
 

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