Merge #94

94: Structured reports r=haxscramper a=haxscramper

Full rework of the compiler message handling pipeline. Remove old-style message generation that was based purely on strings that were formatted in-place, and instead implement structured logging where main compiler code only instantiates objects with required information.

This would allow to
- decouple error presentation logic from the semantic checking and other parts of the compiler. Added bonus: user-facing error messages can be improved much faster, since changes don't have to be propagated through the test suite each time.
- allow compilation reports to be printed out as a stream of S-expression or JSON lines
- Added bonus: user-facing error messages can be improved much faster, since changes don't have to be propagated through the test suite each time.

In addition to architectural improvements this PR also reorganizes and documents various error kinds. Whole thing has to be rewritten from the ground up, so RFCs like 
nim-lang/RFCs#323 nim-lang/RFCs#324 nim-lang/RFCs#325 will be implemented as a collateral

Decoupling of the data and presentation also allows to properly test compilation errors, warnings and hints generated by the compiler and finally untie compiler unit tests from the hardcoded string formatting in the semantic checking phase.

This PR is an orthogonal to the `nkError` project, and is only concerned with a *content* of the error report, while `nkError` is all about *presence* of errors during compilation. 

Co-authored-by: haxscramper <[email protected]>

compiler/astalgo.nim

@@ -627,6 +627,13 @@ proc value(this: var DebugPrinter; value: PNode) =
     if this.renderSymType and value.typ != nil:
       this.key "typ"
       this.value value.typ
+
+    if value.kind == nkError:
+      this.key "file"
+      this.value $value[compilerInfoPos].strVal
+      this.key "line"
+      this.value $value[compilerInfoPos].info.line
+
     if value.len > 0:
       this.key "sons"
       this.openBracket

compiler/ccgcalls.nim

@@ -53,7 +53,7 @@ proc preventNrvo(p: BProc; le, ri: PNode): bool =
     # annoying warnings, see #14514
     if canRaise(ri[0]) and
         locationEscapes(p, le, p.nestedTryStmts.len > 0):
-      message(p.config, le.info, warnObservableStores, $le)
+      localReport(p.config, le, reportSem rsemObservableStores)
 
 proc hasNoInit(call: PNode): bool {.inline.} =
   result = call[0].kind == nkSym and sfNoInit in call[0].sym.flags
@@ -500,7 +500,9 @@ proc genOtherArg(p: BProc; ri: PNode; i: int; typ: PType): Rope =
       result = genArgNoParam(p, ri[i]) #, typ.n[i].sym)
   else:
     if tfVarargs notin typ.flags:
-      localError(p.config, ri.info, "wrong argument count")
+      localReport(p.config, ri.info, semReportCountMismatch(
+        rsemWrongNumberOfArguments, expected = 1, got = 0, node = ri))
+
       result = nil
     else:
       result = genArgNoParam(p, ri[i])
@@ -624,7 +626,7 @@ proc genPatternCall(p: BProc; ri: PNode; pat: string; typ: PType): Rope =
             result.add genOtherArg(p, ri, k, typ)
           result.add(~")")
         else:
-          localError(p.config, ri.info, "call expression expected for C++ pattern")
+          localReport(p.config, ri, reportSem rsemExpectedCallForCxxPattern)
         inc i
       elif i+1 < pat.len and pat[i+1] == '.':
         result.add genThisArg(p, ri, j, typ)

compiler/ccgexprs.nim

@@ -951,16 +951,26 @@ proc genArrayElem(p: BProc, n, x, y: PNode, d: var TLoc) =
     if not isConstExpr(y):
       # semantic pass has already checked for const index expressions
       if firstOrd(p.config, ty) == 0 and lastOrd(p.config, ty) >= 0:
-        if (firstOrd(p.config, b.t) < firstOrd(p.config, ty)) or (lastOrd(p.config, b.t) > lastOrd(p.config, ty)):
+        if (firstOrd(p.config, b.t) < firstOrd(p.config, ty)) or
+           (lastOrd(p.config, b.t) > lastOrd(p.config, ty)):
+
           linefmt(p, cpsStmts, "if ((NU)($1) > (NU)($2)){ #raiseIndexError2($1, $2); $3}$n",
                   [rdCharLoc(b), intLiteral(lastOrd(p.config, ty)), raiseInstr(p)])
       else:
         linefmt(p, cpsStmts, "if ($1 < $2 || $1 > $3){ #raiseIndexError3($1, $2, $3); $4}$n",
                 [rdCharLoc(b), first, intLiteral(lastOrd(p.config, ty)), raiseInstr(p)])
     else:
       let idx = getOrdValue(y)
-      if idx < firstOrd(p.config, ty) or idx > lastOrd(p.config, ty):
-        localError(p.config, x.info, formatErrorIndexBound(idx, firstOrd(p.config, ty), lastOrd(p.config, ty)))
+      if idx < firstOrd(p.config, ty) or lastOrd(p.config, ty) < idx:
+        localReport(
+          p.config, x.info, SemReport(
+            kind: rsemStaticOutOfBounds,
+            indexSpec: (
+              usedIdx: idx,
+              minIdx: firstOrd(p.config, ty),
+              maxIdx: lastOrd(p.config, ty)),
+            ast: y))
+
   d.inheritLocation(a)
   putIntoDest(p, d, n,
               ropecg(p.module, "$1[($2)- $3]", [rdLoc(a), rdCharLoc(b), first]), a.storage)
@@ -1165,7 +1175,8 @@ proc genEcho(p: BProc, n: PNode) =
       linefmt(p, cpsStmts, "fflush(stdout);$n", [])
 
 proc gcUsage(conf: ConfigRef; n: PNode) =
-  if conf.selectedGC == gcNone: message(conf, n.info, warnGcMem, n.renderTree)
+  if conf.selectedGC == gcNone:
+    localReport(conf, n, reportSem rsemUseOfGc)
 
 proc strLoc(p: BProc; d: TLoc): Rope =
   if optSeqDestructors in p.config.globalOptions:
@@ -1330,9 +1341,7 @@ proc rawGenNew(p: BProc, a: var TLoc, sizeExpr: Rope; needsInit: bool) =
       # finalizer is: ``proc (x: ref T) {.nimcall.}``. We need to check the calling
       # convention at least:
       if op.typ == nil or op.typ.callConv != ccNimCall:
-        localError(p.module.config, a.lode.info,
-          "the destructor that is turned into a finalizer needs " &
-          "to have the 'nimcall' calling convention")
+        localReport(p.module.config, a.lode, reportSem rsemExpectedNimcallProc)
       var f: TLoc
       initLocExpr(p, newSymNode(op), f)
       p.module.s[cfsTypeInit3].addf("$1->finalizer = (void*)$2;$n", [ti, rdLoc(f)])
@@ -1644,9 +1653,10 @@ proc genOf(p: BProc, x: PNode, typ: PType, d: var TLoc) =
     while t.kind == tyObject and t[0] != nil:
       r.add(~".Sup")
       t = skipTypes(t[0], skipPtrs)
+
   if isObjLackingTypeField(t):
-    globalError(p.config, x.info,
-      "no 'of' operator available for pure objects")
+    localReport(p.config, x, reportSem rsemDisallowedOfForPureObjects)
+
   if nilCheck != nil:
     r = ropecg(p.module, "(($1) && ($2))", [nilCheck, genOfHelper(p, dest, r, x.info)])
   else:
@@ -1658,7 +1668,7 @@ proc genOf(p: BProc, n: PNode, d: var TLoc) =
 
 proc genRepr(p: BProc, e: PNode, d: var TLoc) =
   if optTinyRtti in p.config.globalOptions:
-    localError(p.config, e.info, "'repr' is not available for --newruntime")
+    localReport(p.config, e, reportSem rsemDisallowedReprForNewruntime)
   var a: TLoc
   initLocExpr(p, e[1], a)
   var t = skipTypes(e[1].typ, abstractVarRange)
@@ -1701,7 +1711,7 @@ proc genRepr(p: BProc, e: PNode, d: var TLoc) =
                 ropecg(p.module, "#reprAny($1, $2)", [
                 rdLoc(a), genTypeInfoV1(p.module, t, e.info)]), a.storage)
   of tyEmpty, tyVoid:
-    localError(p.config, e.info, "'repr' doesn't support 'void' type")
+    localReport(p.config, e, reportSem rsemUnexpectedVoidType)
   else:
     putIntoDest(p, d, e, ropecg(p.module, "#reprAny($1, $2)",
                               [addrLoc(p.config, a), genTypeInfoV1(p.module, t, e.info)]),
@@ -2259,7 +2269,7 @@ proc genSlice(p: BProc; e: PNode; d: var TLoc) =
   if d.k == locNone: getTemp(p, e.typ, d)
   linefmt(p, cpsStmts, "$1.Field0 = $2; $1.Field1 = $3;$n", [rdLoc(d), x, y])
   when false:
-    localError(p.config, e.info, "invalid context for 'toOpenArray'; " &
+    localReport(p.config, e.info, "invalid context for 'toOpenArray'; " &
       "'toOpenArray' is only valid within a call expression")
 
 proc genEnumToStr(p: BProc, e: PNode, d: var TLoc) =
@@ -2423,12 +2433,15 @@ proc genMagicExpr(p: BProc, e: PNode, d: var TLoc, op: TMagic) =
   of mEcho: genEcho(p, e[1].skipConv)
   of mArrToSeq: genArrToSeq(p, e, d)
   of mNLen..mNError, mSlurp..mQuoteAst:
-    localError(p.config, e.info, strutils.`%`(errXMustBeCompileTime, e[0].sym.name.s))
+    localReport(p.config, e.info, reportSym(
+      rsemConstExpressionExpected, e[0].sym))
+
   of mSpawn:
     when defined(leanCompiler):
       p.config.quitOrRaise "compiler built without support for the 'spawn' statement"
     else:
-      let n = spawn.wrapProcForSpawn(p.module.g.graph, p.module.idgen, p.module.module, e, e.typ, nil, nil)
+      let n = spawn.wrapProcForSpawn(
+        p.module.g.graph, p.module.idgen, p.module.module, e, e.typ, nil, nil)
       expr(p, n, d)
   of mParallel:
     when defined(leanCompiler):
@@ -2438,8 +2451,7 @@ proc genMagicExpr(p: BProc, e: PNode, d: var TLoc, op: TMagic) =
       expr(p, n, d)
   of mDeepCopy:
     if p.config.selectedGC in {gcArc, gcOrc} and optEnableDeepCopy notin p.config.globalOptions:
-      localError(p.config, e.info,
-        "for --gc:arc|orc 'deepcopy' support has to be enabled with --deepcopy:on")
+      localReport(p.config, e, reportSem rsemRequiresDeepCopyEnabled)
 
     var a, b: TLoc
     let x = if e[1].kind in {nkAddr, nkHiddenAddr}: e[1][0] else: e[1]
@@ -2746,8 +2758,9 @@ proc expr(p: BProc, n: PNode, d: var TLoc) =
       #if sym.kind == skIterator:
       #  echo renderTree(sym.getBody, {renderIds})
       if sfCompileTime in sym.flags:
-        localError(p.config, n.info, "request to generate code for .compileTime proc: " &
-           sym.name.s)
+        localReport(p.config, n.info, reportSym(
+          rsemCannotCodegenCompiletimeProc, sym))
+
       if useAliveDataFromDce in p.module.flags and sym.typ.callConv != ccInline:
         fillProcLoc(p.module, n)
         genProcPrototype(p.module, sym)
@@ -3000,7 +3013,8 @@ proc getDefaultValue(p: BProc; typ: PType; info: TLineInfo): Rope =
     if mapSetType(p.config, t) == ctArray: result = rope"{}"
     else: result = rope"0"
   else:
-    globalError(p.config, info, "cannot create null element for: " & $t.kind)
+    internalError(
+      p.config, info, "cannot create null element for: " & $t.kind)
 
 proc caseObjDefaultBranch(obj: PNode; branch: Int128): int =
   for i in 1 ..< obj.len:
@@ -3070,7 +3084,7 @@ proc getNullValueAux(p: BProc; t: PType; obj, constOrNil: PNode,
     # not found, produce default value:
     result.add getDefaultValue(p, field.typ, info)
   else:
-    localError(p.config, info, "cannot create null element for: " & $obj)
+    internalError(p.config, info, "cannot create null element for: " & $obj)
 
 proc getNullValueAuxT(p: BProc; orig, t: PType; obj, constOrNil: PNode,
                       result: var Rope; count: var int;
@@ -3215,7 +3229,8 @@ proc genBracedInit(p: BProc, n: PNode; isConst: bool; optionalType: PType): Rope
       result = genConstTuple(p, n, isConst, typ)
     of tyOpenArray:
       if n.kind != nkBracket:
-        internalError(p.config, n.info, "const openArray expression is not an array construction")
+        internalError(
+          p.config, n.info, "const openArray expression is not an array construction")
 
       let data = genConstSimpleList(p, n, isConst)
 

compiler/ccgliterals.nim

@@ -100,7 +100,8 @@ proc genStringLiteralDataOnly(m: BModule; s: string; info: TLineInfo;
     result = getTempName(m)
     genStringLiteralDataOnlyV2(m, s, result, isConst)
   else:
-    localError(m.config, info, "cannot determine how to produce code for string literal")
+    internalError(
+      m.config, info, "cannot determine how to produce code for string literal")
 
 proc genNilStringLiteral(m: BModule; info: TLineInfo): Rope =
   result = ropecg(m, "((#NimStringDesc*) NIM_NIL)", [])
@@ -110,4 +111,5 @@ proc genStringLiteral(m: BModule; n: PNode): Rope =
   of 0, 1: result = genStringLiteralV1(m, n)
   of 2: result = genStringLiteralV2(m, n, isConst = true)
   else:
-    localError(m.config, n.info, "cannot determine how to produce code for string literal")
+    internalError(
+      m.config, n.info, "cannot determine how to produce code for string literal")

compiler/ccgreset.nim

@@ -26,7 +26,7 @@ proc specializeResetN(p: BProc, accessor: Rope, n: PNode;
     let disc = n[0].sym
     if disc.loc.r == nil: fillObjectFields(p.module, typ)
     if disc.loc.t == nil:
-      internalError(p.config, n.info, "specializeResetN()")
+     internalError(p.config, n.info, "specializeResetN()")
     lineF(p, cpsStmts, "switch ($1.$2) {$n", [accessor, disc.loc.r])
     for i in 1..<n.len:
       let branch = n[i]

compiler/ccgstmts.nim

@@ -268,7 +268,8 @@ proc genBreakState(p: BProc, n: PNode, d: var TLoc) =
 
 proc genGotoVar(p: BProc; value: PNode) =
   if value.kind notin {nkCharLit..nkUInt64Lit}:
-    localError(p.config, value.info, "'goto' target must be a literal value")
+    localReport(p.config, value, reportSem rsemExpectedLiteralForGoto)
+
   else:
     lineF(p, cpsStmts, "goto NIMSTATE_$#;$n", [value.intVal.rope])
 
@@ -481,7 +482,7 @@ proc genGotoForCase(p: BProc; caseStmt: PNode) =
     let it = caseStmt[i]
     for j in 0..<it.len-1:
       if it[j].kind == nkRange:
-        localError(p.config, it.info, "range notation not available for computed goto")
+        localReport(p.config, it, reportSem rsemDisallowedRangeForComputedGoto)
         return
       let val = getOrdValue(it[j])
       lineF(p, cpsStmts, "NIMSTATE_$#:$n", [val.rope])
@@ -510,22 +511,28 @@ proc genComputedGoto(p: BProc; n: PNode) =
     let it = n[i]
     if it.kind == nkCaseStmt:
       if lastSon(it).kind != nkOfBranch:
-        localError(p.config, it.info,
-            "case statement must be exhaustive for computed goto"); return
+        localReport(p.config, it, reportSem rsemExpectedExhaustiveCaseForComputedGoto)
+        return
+
       casePos = i
       if enumHasHoles(it[0].typ):
-        localError(p.config, it.info,
-            "case statement cannot work on enums with holes for computed goto"); return
+        localReport(p.config, it, reportSem rsemExpectedUnholyEnumForComputedGoto)
+        return
+
       let aSize = lengthOrd(p.config, it[0].typ)
       if aSize > 10_000:
-        localError(p.config, it.info,
-            "case statement has too many cases for computed goto"); return
+        localReport(p.config, it, reportSem rsemTooManyEntriesForComputedGoto)
+        return
+
       arraySize = toInt(aSize)
       if firstOrd(p.config, it[0].typ) != 0:
-        localError(p.config, it.info,
-            "case statement has to start at 0 for computed goto"); return
+        localReport(p.config, it, reportSem rsemExpectedLow0ForComputedGoto)
+        return
+
   if casePos < 0:
-    localError(p.config, n.info, "no case statement found for computed goto"); return
+    localReport(p.config, n, reportSem rsemExpectedCaseForComputedGoto)
+    return
+
   var id = p.labels+1
   inc p.labels, arraySize+1
   let tmp = "TMP$1_" % [id.rope]
@@ -549,7 +556,7 @@ proc genComputedGoto(p: BProc; n: PNode) =
     let it = caseStmt[i]
     for j in 0..<it.len-1:
       if it[j].kind == nkRange:
-        localError(p.config, it.info, "range notation not available for computed goto")
+        localReport(p.config, it, reportSem rsemDisallowedRangeForComputedGoto)
         return
 
       let val = getOrdValue(it[j])
@@ -1551,7 +1558,7 @@ proc asgnFieldDiscriminant(p: BProc, e: PNode) =
   if optTinyRtti notin p.config.globalOptions:
     let field = dotExpr[1].sym
     genDiscriminantCheck(p, a, tmp, dotExpr[0].typ, field)
-    message(p.config, e.info, warnCaseTransition)
+    localReport(p.config, e, reportSem rsemCaseTransition)
   genAssignment(p, a, tmp, {})
 
 proc genAsgn(p: BProc, e: PNode, fastAsgn: bool) =
@@ -1579,7 +1586,7 @@ proc genAsgn(p: BProc, e: PNode, fastAsgn: bool) =
 proc genStmts(p: BProc, t: PNode) =
   var a: TLoc
 
-  let isPush = p.config.hasHint(hintExtendedContext)
+  let isPush = p.config.hasHint(rsemExtendedContext)
   if isPush: pushInfoContext(p.config, t.info)
   expr(p, t, a)
   if isPush: popInfoContext(p.config)

compiler/ccgtypes.nim

@@ -1148,8 +1148,9 @@ proc genObjectFields(m: BModule, typ, origType: PType, n: PNode, expr: Rope;
 proc genObjectInfo(m: BModule, typ, origType: PType, name: Rope; info: TLineInfo) =
   if typ.kind == tyObject:
     if incompleteType(typ):
-      localError(m.config, info, "request for RTTI generation for incomplete object: " &
-                        typeToString(typ))
+      localReport(m.config, info, reportTyp(
+        rsemRttiRequestForIncompleteObject, typ))
+
     genTypeInfoAux(m, typ, origType, name, info)
   else:
     genTypeInfoAuxBase(m, typ, origType, name, rope("0"), info)
@@ -1293,8 +1294,8 @@ proc genHook(m: BModule; t: PType; info: TLineInfo; op: TTypeAttachedOp): Rope =
     # finalizer is: ``proc (x: ref T) {.nimcall.}``. We need to check the calling
     # convention at least:
     if theProc.typ == nil or theProc.typ.callConv != ccNimCall:
-      localError(m.config, info,
-        theProc.name.s & " needs to have the 'nimcall' calling convention")
+      localReport(m.config, info, reportSym(
+        rsemExpectedNimcallProc, theProc))
 
     genProc(m, theProc)
     result = theProc.loc.r
@@ -1315,8 +1316,9 @@ proc genTypeInfoV2Impl(m: BModule, t, origType: PType, name: Rope; info: TLineIn
   var typeName: Rope
   if t.kind in {tyObject, tyDistinct}:
     if incompleteType(t):
-      localError(m.config, info, "request for RTTI generation for incomplete object: " &
-                 typeToString(t))
+      localReport(m.config, info, reportTyp(
+        rsemRttiRequestForIncompleteObject, t))
+
     typeName = genTypeInfo2Name(m, t)
   else:
     typeName = rope("NIM_NIL")
@@ -1461,7 +1463,7 @@ proc genTypeInfoV1(m: BModule, t: PType; info: TLineInfo): Rope =
     if t.n != nil: result = genTypeInfoV1(m, lastSon t, info)
     else: internalError(m.config, "genTypeInfoV1(" & $t.kind & ')')
   of tyUserTypeClasses:
-    internalAssert m.config, t.isResolvedUserTypeClass
+    internalAssert(m.config, t.isResolvedUserTypeClass, "")
     return genTypeInfoV1(m, t.lastSon, info)
   of tyProc:
     if t.callConv != ccClosure: