Merge remote-tracking branch 'upstream/devel' into nimlsp5

Author: bung87

.github/workflows/ci.yml

@@ -123,7 +123,7 @@ jobs:
         run: echo "::add-matcher::.github/nim-problem-matcher.json"
 
       - name: Build release binaries
-        run: ./koch.py all
+        run: ./koch.py all-strict
 
       - name: Upload workspace to artifacts
         uses: ./.github/actions/upload-compiler

compiler/ast/ast_query.nim

@@ -617,8 +617,7 @@ proc isSinkType*(t: PType): bool {.inline.} =
   t.kind == tySink
 
 const magicsThatCanRaise* = {
-  mNone, mSlurp, mParseExprToAst, mParseStmtToAst, mEcho,
-  mChckRange }
+  mNone, mParseExprToAst, mParseStmtToAst, mEcho, mChckRange }
 
 proc canRaiseConservative*(fn: PNode): bool =
   if fn.kind == nkSym and fn.sym.magic notin magicsThatCanRaise:

compiler/ast/ast_types.nim

@@ -742,7 +742,7 @@ type
     mDefined, mDeclared, mDeclaredInScope, mCompiles, mArrGet, mArrPut, mAsgn,
     mLow, mHigh, mSizeOf, mAlignOf, mOffsetOf, mTypeTrait,
     mIs, mOf, mAddr, mType, mTypeOf,
-    mPlugin, mEcho, mShallowCopy, mSlurp,
+    mPlugin, mEcho, mShallowCopy,
     mStatic,
     mParseExprToAst, mParseStmtToAst, mExpandToAst, mQuoteAst,
     mInc, mDec, mOrd,

compiler/ast/renderer.nim

@@ -364,9 +364,7 @@ proc atom(g: TSrcGen; n: PNode): string =
     if (n.typ != nil) and (n.typ.sym != nil): result = n.typ.sym.name.s
     else: result = "[type node]"
   else:
-    g.config.localReport InternalReport(
-      kind: rintUnreachable, msg: "renderer.atom " & $n.kind)
-    result = ""
+    g.config.internalError("renderer.atom " & $n.kind)
 
 proc lcomma(g: TSrcGen; n: PNode, start: int = 0, theEnd: int = - 1): int =
   assert(theEnd < 0)
@@ -1638,8 +1636,7 @@ proc gsub(g: var TSrcGen, n: PNode, c: TContext, fromStmtList = false) =
     gsub(g, n.sons[0], c)
   else:
     #nkNone, nkExplicitTypeListCall:
-    g.config.localReport InternalReport(
-      kind: rintUnreachable, msg: "renderer.gsub(" & $n.kind & ')')
+    g.config.internalError("renderer.gsub(" & $n.kind & ')')
 
 proc renderTree*(n: PNode, renderFlags: TRenderFlags = {}): string =
   if n == nil: return "<nil tree>"

compiler/ast/report_enums.nim

@@ -71,15 +71,10 @@ type
     repNone
 
     #--------------------------  Internal reports  ---------------------------#
-    # Internal reports being
+    # Internal reports begin
     # fatal errors begin
-    rintUnknown ## Unknown internal report kind
-    rintFatal ## Explicitly fatal compiler error
-
     rintUnreachable ## State in the compiler code that must not be reached
     rintAssert ## Failed internal assert in the compiler
-
-    rintIce ## Internal compilation error
     # fatal end
 
     # errors being
@@ -909,7 +904,7 @@ type
 
   ExternalReportKind* = range[rextCmdRequiresFile .. rextPath]
 
-  InternalReportKind* = range[rintUnknown .. rintEchoMessage]
+  InternalReportKind* = range[rintUnreachable .. rintEchoMessage]
 
 
 const
@@ -972,8 +967,8 @@ const
   repInternalKinds*: ReportKinds = {
     low(InternalReportKind) .. high(InternalReportKind)}
 
-  rintFatalKinds* = {rintUnknown .. rintIce} ## Fatal internal compilation
-                                             ## reports
+  rintFatalKinds* = {rintUnreachable .. rintAssert} ## Fatal internal compilation
+                                                ## reports
   rintErrorKinds* = {rintCannotOpenFile .. rintNotImplemented}
   rintWarningKinds* = {rintWarnCannotOpenFile .. rintWarnFileChanged}
   rintHintKinds* = {rintSource .. rintSuccessX}

compiler/ast/types.nim

@@ -35,9 +35,6 @@ import
     modulegraphs,
   ]
 
-# TODO: switch to internalError/Assert or better yet emit the appropriate
-#       diagnostic/event/telemetry data instead, then drop this dependency
-from compiler/ast/reports_internal import InternalReport
 from compiler/ast/report_enums import ReportKind
 
 from compiler/ast/reports_sem import SemReport,
@@ -565,10 +562,7 @@ proc firstFloat*(t: PType): BiggestFloat =
      tyStatic, tyInferred, tyUserTypeClasses:
     firstFloat(lastSon(t))
   else:
-    newPartialConfigRef().localReport InternalReport(
-      kind: rintUnreachable,
-      msg: "invalid kind for firstFloat(" & $t.kind & ')')
-    NaN
+    unreachable("invalid kind for firstFloat(" & $t.kind & ')')
 
 proc lastFloat*(t: PType): BiggestFloat =
   case t.kind
@@ -582,10 +576,7 @@ proc lastFloat*(t: PType): BiggestFloat =
      tyStatic, tyInferred, tyUserTypeClasses:
     lastFloat(lastSon(t))
   else:
-    newPartialConfigRef().localReport InternalReport(
-      kind: rintUnreachable,
-      msg: "invalid kind for firstFloat(" & $t.kind & ')')
-    NaN
+    unreachable("invalid kind for firstFloat(" & $t.kind & ')')
 
 proc floatRangeCheck*(x: BiggestFloat, t: PType): bool =
   case t.kind
@@ -601,10 +592,7 @@ proc floatRangeCheck*(x: BiggestFloat, t: PType): bool =
      tyStatic, tyInferred, tyUserTypeClasses:
     floatRangeCheck(x, lastSon(t))
   else:
-    newPartialConfigRef().localReport InternalReport(
-      kind: rintUnreachable,
-      msg: "invalid kind for floatRangeCheck(" & $t.kind & ')')
-    false
+    unreachable("invalid kind for floatRangeCheck(" & $t.kind & ')')
 
 # -------------- type equality -----------------------------------------------
 

compiler/backend/ccgexprs.nim

@@ -1839,7 +1839,7 @@ proc genMagicExpr(p: BProc, e: CgNode, d: var TLoc, op: TMagic) =
   of mDefault: genDefault(p, e, d)
   of mEcho: genEcho(p, e[1].skipConv)
   of mArrToSeq: genArrToSeq(p, e, d)
-  of mNLen..mNError, mSlurp..mQuoteAst:
+  of mNLen..mNError, mStatic..mQuoteAst:
     localReport(p.config, e.info, reportSym(
       rsemConstExpressionExpected, e[0].sym))
 

compiler/backend/jsgen.nim

@@ -2009,7 +2009,7 @@ proc genMagic(p: PProc, n: CgNode, r: var TCompRes) =
   of mDefault: genDefault(p, n, r)
   of mWasMoved: genReset(p, n)
   of mEcho: genEcho(p, n, r)
-  of mNLen..mNError, mSlurp:
+  of mNLen..mNError:
     localReport(p.config, n.info, reportSym(
       rsemConstExpressionExpected, n[0].sym))
 

compiler/front/cli_reporter.nim

@@ -2632,15 +2632,6 @@ To create a stacktrace, rerun compilation with './koch temp $1 <file>'
     of rintCannotOpenFile, rintWarnCannotOpenFile:
       result = "cannot open file: $1" % r.file
 
-    of rintUnknown:
-      result = "unknown"
-
-    of rintFatal:
-      result = "fatal"
-
-    of rintIce:
-      result = r.msg
-
     of rintNotUsingNimcore:
       result = "Nim tooling must be built using -d:nimcore"
 
@@ -3990,17 +3981,8 @@ proc reportHook*(conf: ConfigRef, r: Report): TErrorHandling =
   ## `reportBody` for report, which then calls respective (for each report
   ## category) `reportBody` overloads defined above
   assertKind r
-  let wkind = conf.writabilityKind(r)
-  # debug reports can be both enabled and force enabled. So adding a case here
-  # is not really useful, since report writability kind does not necessarily
-  # dictate how it is written, just whether it can/must/cannot be written.
-  # xxx: the above convoluted comment brought to you by _glaring_ design flaws!
-  if wkind == writeDisabled:
-    return
-  else:
-    if wkind == writeForceEnabled:
-      echo conf.reportFull(r)
-    elif r.kind == rsemProcessing and conf.hintProcessingDots:
+  if true:
+    if r.kind == rsemProcessing and conf.hintProcessingDots:
       # xxx: the report hook is handling processing dots output, why? this whole
       #      infrastructure is overwrought. seriously, they're not hints, they're
       #      progress indicators.

compiler/front/msgs.nim

@@ -609,7 +609,6 @@ proc report*(conf: ConfigRef, node: PNode): TErrorHandling =
 
 proc fillReportAndHandleVmTrace(c: ConfigRef, r: var Report,
                                 reportFrom: InstantiationInfo) =
-  r.reportFrom = toReportLineInfo(reportFrom)
   if r.category in { repSem, repVM } and r.location.isSome():
     r.context = c.getContext(r.location.get())
 
@@ -621,24 +620,63 @@ proc handleReport*(
     r: Report,
     reportFrom: InstantiationInfo,
     eh: TErrorHandling = doNothing) {.noinline.} =
+  ## Takes the report `r` and handles it. If the report is "enabled" according
+  ## to the active configuration, it is passed to the active report hook and,
+  ## if the report corresponds to an error, error handling is performed.
+  ## `eh` is currently only a suggestion, and it is sometimes ignored depending
+  ## on the currently active configuration.
   var rep = r
-  fillReportAndHandleVmTrace(conf, rep, reportFrom)
+  rep.reportFrom = toReportLineInfo(reportFrom)
+
+  if not conf.isEnabled(rep):
+    # the report is disabled -> neither invoke the report hook nor perform
+    # error handling
+    return
+
+  var userAction = doNothing
+  case writabilityKind(conf, rep)
+  of writeDisabled:
+    discard "don't invoke the hook"
+  of writeEnabled:
+    # go through the report hook
+    fillReportAndHandleVmTrace(conf, rep, reportFrom)
+    userAction = conf.report(rep)
+  of writeForceEnabled:
+    # also go through the report hook, but temporarily override ``writeln``
+    # with something that always echoes something
+    fillReportAndHandleVmTrace(conf, rep, reportFrom)
+    let oldHook = conf.writelnHook
+    conf.writelnHook = proc (conf: ConfigRef, msg: string, flags: MsgFlags) =
+      echo msg
 
-  let
     userAction = conf.report(rep)
-    (action, trace) =
-      case userAction
-      of doDefault:
-        errorActions(conf, rep, eh)
-      else:
-        (userAction, false)
+    conf.writelnHook = oldHook
 
+  # ``errorActions`` also increments the error counter, so make sure to always
+  # call it
+  var (action, trace) = errorActions(conf, rep, eh)
+
+  # decide what to do, based on the hook-provided action and the computed
+  # action. The more severe handling out of the two wins
+  case userAction
+  of doAbort:
+    # a hook-requested abort always overrides the computed handling
+    (action, trace) = (doAbort, false)
+  of doRaise:
+    case action
+    of doRaise, doAbort:
+      discard "a hook-requested raise doesn't override an abort"
+    of doNothing, doDefault:
+      (action, trace) = (doRaise, false)
+  of doNothing, doDefault:
+    discard "use the computed strategy"
+
+  # now perform the selected action:
   case action
   of doAbort:   quit(conf, trace)
   of doRaise:   raiseRecoverableError("report")
   of doNothing: discard
-  of doDefault: assert(
-    false,
+  of doDefault: unreachable(
     "Default error handing action must be turned into ignore/raise/abort")
 
 template globalAssert*(
@@ -665,34 +703,6 @@ template globalReport*(conf: ConfigRef, report: ReportTypes) =
   handleReport(
     conf, wrap(report, instLoc()), instLoc(), doRaise)
 
-proc reportAndFail*(
-  conf: ConfigRef, r: Report, reportFrom: InstantiationInfo) =
-  ## Similar to `handleReport`, but, unless overridden with aborting
-  ## (`doAbort`) by the structured report hook, always raises a recoverable
-  ## error.
-  var rep = r
-  fillReportAndHandleVmTrace(conf, rep, reportFrom)
-
-  case conf.report(rep)
-  of doAbort:
-    quit(conf, false)
-  of doDefault:
-    let (action, trace) = errorActions(conf, rep, doRaise)
-    case action
-    of doAbort:
-      quit(conf, trace)
-    of doRaise, doNothing:
-      raiseRecoverableError("report")
-    of doDefault:
-      unreachable()
-  of doRaise, doNothing:
-    raiseRecoverableError("report")
-
-template reportAndFail*(
-  conf: ConfigRef; info: TLineInfo, report: ReportTypes) =
-  reportAndFail(
-    conf, wrap(report, instLoc(), info), instLoc())
-
 template localReport*(conf: ConfigRef; info: TLineInfo, report: ReportTypes) =
   {.line.}:
     handleReport(
@@ -717,27 +727,9 @@ template localReport*(conf: ConfigRef, report: Report) =
 
 # xxx: `internalError` and `internalAssert` in conjunction with `handleReport`,
 #      and the whole concept of "reports" indicating error handling action at a
-#      callsite, is *terrible*. Since neither will necessarily raise/end
-#      execution of the current routine, which may lead to NPEs and the like.
-
-template internalError*(
-    conf: ConfigRef, repKind: InternalReportKind, fail: string): untyped =
-  ## Causes an internal error; but does not necessarily raise/end the currently
-  ## executing routine.
-  conf.handleReport(
-    wrap(InternalReport(kind: repKind, msg: fail), instLoc()),
-    instLoc(),
-    doAbort)
-
-template internalError*(
-    conf: ConfigRef, info: TLineInfo,
-    repKind: InternalReportKind, fail: string): untyped =
-  ## Causes an internal error; but does not necessarily raise/end the currently
-  ## executing routine.
-  conf.handleReport(
-    wrap(InternalReport(kind: repKind, msg: fail), instLoc(), info),
-    instLoc(),
-    doAbort)
+#      callsite, is *terrible*. While it will result in the compiler exiting,
+#      it is currently implemented very indirectly, through
+#      ``isCompilerFatal``.
 
 proc doInternalUnreachable*(conf: ConfigRef, info: TLineInfo, msg: string,
                             instLoc: InstantiationInfo) {.noreturn, inline.} =
@@ -752,19 +744,18 @@ proc doInternalUnreachable*(conf: ConfigRef, info: TLineInfo, msg: string,
         wrap(intRep, instLoc, info)
 
   conf.handleReport(rep, instLoc, doAbort)
+  unreachable("not aborted")
 
 template internalError*(
     conf: ConfigRef,
     info: TLineInfo,
     fail: string,
   ): untyped =
-  ## Causes an internal error; but does not necessarily raise/end the currently
-  ## executing routine.
+  ## Causes an internal error. Always ends the currently executing routine.
   doInternalUnreachable(conf, info, fail, instLoc())
 
 template internalError*(conf: ConfigRef, fail: string): untyped =
-  ## Causes an internal error; but does not necessarily raise/end the currently
-  ## executing routine.
+  ## Causes an internal error. Always ends the currently executing routine.
   doInternalUnreachable(conf, unknownLineInfo, fail, instLoc())
 
 proc doInternalAssert*(conf: ConfigRef,
@@ -782,17 +773,18 @@ proc doInternalAssert*(conf: ConfigRef,
         wrap(intRep, instLoc, info)
 
   conf.handleReport(rep, instLoc, doAbort)
+  unreachable("not aborted")
 
 template internalAssert*(
     conf: ConfigRef, condition: bool, info: TLineInfo, failMsg: string = "") =
-  ## Causes an internal error if the provided condition evaluates to false; but
-  ## does not necessarily raise/end the currently executing routine.
+  ## Causes an internal error if the provided condition evaluates to false.
+  ## Always ends the currently executing routine.
   if not condition:
     doInternalAssert(conf, instLoc(), failMsg, info)
 
 template internalAssert*(conf: ConfigRef, condition: bool, failMsg = "") =
-  ## Causes an internal error if the provided condition evaluates to false; but
-  ## does not necessarily raise/end the currently executing routine.
+  ## Causes an internal error if the provided condition evaluates to false.
+  ## Always ends the currently executing routine.
   if not condition:
     doInternalAssert(conf, instLoc(), failMsg)