System cleanup, part 1 (#13069)

* system.nim: mark what every .pop does

* system.nim: un-nest when statements

lib/system.nim

@@ -658,6 +658,8 @@ include "system/inclrtl"
 const NoFakeVars* = defined(nimscript) ## `true` if the backend doesn't support \
   ## "fake variables" like `var EBADF {.importc.}: cint`.
 
+const notJSnotNims = not defined(JS) and not defined(nimscript)
+
 when not defined(JS) and not defined(nimSeqsV2):
   type
     TGenericSeq {.compilerproc, pure, inheritable.} = object
@@ -670,11 +672,11 @@ when not defined(JS) and not defined(nimSeqsV2):
       data: UncheckedArray[char]
     NimString = ptr NimStringDesc
 
-when not defined(JS) and not defined(nimscript):
-  when not defined(nimSeqsV2):
+when notJSnotNims and not defined(nimSeqsV2):
   template space(s: PGenericSeq): int {.dirty.} =
     s.reserved and not (seqShallowFlag or strlitFlag)
-  when not defined(nimV2):
+
+when notJSnotNims and not defined(nimV2):
   include "system/hti"
 
 type
@@ -833,7 +835,7 @@ type
     ##
     ## This is only raised if the `segfaults module <segfaults.html>`_ was imported!
 
-when defined(js) or defined(nimdoc):
+when defined(JS) or defined(nimdoc):
   type
     JsRoot* = ref object of RootObj
       ## Root type of the JavaScript object hierarchy
@@ -1113,7 +1115,6 @@ when defined(nimNoZeroExtendMagic):
     ## zero extends a smaller integer type to ``int``. This treats `x` as
     ## unsigned.
     ## **Deprecated since version 0.19.9**: Use unsigned integers instead.
-
     cast[int](uint(cast[uint8](x)))
 
   proc ze*(x: int16): int {.deprecated.} =
@@ -1806,18 +1807,17 @@ when defined(nimHasDefault):
     ##
     ##   echo @a # => @[1, 3, 5]
     ##   echo @b # => @['f', 'o', 'o']
-else:
-  proc `@`* [IDX, T](a: array[IDX, T]): seq[T] {.
-    magic: "ArrToSeq", noSideEffect.}
 
-when defined(nimHasDefault):
   proc default*(T: typedesc): T {.magic: "Default", noSideEffect.}
     ## returns the default value of the type ``T``.
 
   proc reset*[T](obj: var T) {.noSideEffect.} =
     ## Resets an object `obj` to its default value.
     obj = default(typeof(obj))
+
 else:
+  proc `@`* [IDX, T](a: array[IDX, T]): seq[T] {.
+    magic: "ArrToSeq", noSideEffect.}
   when defined(nimV2):
     proc reset*[T](obj: var T) {.magic: "Destroy", noSideEffect.}
   else:
@@ -2035,7 +2035,7 @@ const
     ## is the value that should be passed to `quit <#quit,int>`_ to indicate
     ## failure.
 
-when defined(js) and defined(nodejs) and not defined(nimscript):
+when defined(JS) and defined(nodejs) and not defined(nimscript):
   var programResult* {.importc: "process.exitCode".}: int
   programResult = 0
 elif hostOS != "standalone":
@@ -2077,9 +2077,7 @@ elif defined(genode):
   proc quit*(errorcode: int = QuitSuccess) =
     systemEnv.quit(errorcode)
 
-
-
-elif defined(js) and defined(nodejs) and not defined(nimscript):
+elif defined(JS) and defined(nodejs) and not defined(nimscript):
   proc quit*(errorcode: int = QuitSuccess) {.magic: "Exit",
     importc: "process.exit", noreturn.}
 
@@ -2087,6 +2085,7 @@ else:
   proc quit*(errorcode: int = QuitSuccess) {.
     magic: "Exit", importc: "exit", header: "<stdlib.h>", noreturn.}
 
+
 template sysAssert(cond: bool, msg: string) =
   when defined(useSysAssert):
     if not cond:
@@ -2097,12 +2096,12 @@ template sysAssert(cond: bool, msg: string) =
 
 const hasAlloc = (hostOS != "standalone" or not defined(nogc)) and not defined(nimscript)
 
-when not defined(JS) and not defined(nimscript) and hostOS != "standalone":
+when notJSnotNims and hostOS != "standalone":
   include "system/cgprocs"
-when not defined(JS) and not defined(nimscript) and hasAlloc and not defined(nimSeqsV2):
+when notJSnotNims and hasAlloc and not defined(nimSeqsV2):
   proc addChar(s: NimString, c: char): NimString {.compilerproc, benign.}
 
-when not defined(nimSeqsV2) or defined(nimscript):
+when defined(nimscript) or not defined(nimSeqsV2):
   proc add*[T](x: var seq[T], y: T) {.magic: "AppendSeqElem", noSideEffect.}
     ## Generic proc for adding a data item `y` to a container `x`.
     ##
@@ -2170,7 +2169,7 @@ proc delete*[T](x: var seq[T], i: Natural) {.noSideEffect.} =
   when nimvm:
     defaultImpl()
   else:
-    when defined(js):
+    when defined(JS):
       {.emit: "`x`.splice(`i`, 1);".}
     else:
       defaultImpl()
@@ -2191,7 +2190,7 @@ proc insert*[T](x: var seq[T], item: T, i = 0.Natural) {.noSideEffect.} =
   when nimvm:
     defaultImpl()
   else:
-    when defined(js):
+    when defined(JS):
       var it : T
       {.emit: "`x` = `x` || []; `x`.splice(`i`, 0, `it`);".}
     else:
@@ -2344,7 +2343,7 @@ proc addQuitProc*(quitProc: proc() {.noconv.}) {.
 # In case of an unhandled exception the exit handlers should
 # not be called explicitly! The user may decide to do this manually though.
 
-when not defined(nimscript) and not defined(JS):
+when notJSnotNims:
   proc zeroMem*(p: pointer, size: Natural) {.inline, noSideEffect,
     tags: [], locks: 0, raises: [].}
     ## Overwrites the contents of the memory at ``p`` with the value 0.
@@ -2379,7 +2378,6 @@ when not defined(nimscript) and not defined(JS):
     ## otherwise. Like any procedure dealing with raw memory this is
     ## **unsafe**.
 
-when not defined(nimscript):
 when hasAlloc:
   proc alloc*(size: Natural): pointer {.noconv, rtl, tags: [], benign, raises: [].}
     ## Allocates a new memory block with at least ``size`` bytes.
@@ -2577,7 +2575,7 @@ proc swap*[T](a, b: var T) {.magic: "Swap", noSideEffect.}
   ##   assert a == 9
   ##   assert b == 5
 
-when not defined(js) and not defined(booting) and defined(nimTrMacros):
+when not defined(JS) and not defined(booting) and defined(nimTrMacros):
   template swapRefsInArray*{swap(arr[a], arr[b])}(arr: openArray[ref], a, b: int) =
     # Optimize swapping of array elements if they are refs. Default swap
     # implementation will cause unsureAsgnRef to be emitted which causes
@@ -2598,7 +2596,7 @@ const
 
 # GC interface:
 
-when not defined(nimscript) and hasAlloc:
+when hasAlloc:
   proc getOccupiedMem*(): int {.rtl.}
     ## Returns the number of bytes that are owned by the process and hold data.
 
@@ -2609,7 +2607,8 @@ when not defined(nimscript) and hasAlloc:
   proc getTotalMem*(): int {.rtl.}
     ## Returns the number of bytes that are owned by the process.
 
-  when hasThreadSupport:
+
+when hasAlloc and hasThreadSupport:
   proc getOccupiedSharedMem*(): int {.rtl.}
     ## Returns the number of bytes that are owned by the process
     ## on the shared heap and hold data. This is only available when
@@ -2624,6 +2623,7 @@ when not defined(nimscript) and hasAlloc:
     ## Returns the number of bytes on the shared heap that are owned by the
     ## process. This is only available when threads are enabled.
 
+
 proc `|`*(a, b: typedesc): typedesc = discard
 
 when sizeof(int) <= 2:
@@ -2750,7 +2750,7 @@ when defined(nimNewRoof):
   dotdotLessImpl(uint64)
   dotdotLessImpl(uint32)
 
-else:
+else: # not defined(nimNewRoof)
   iterator countup*[S, T](a: S, b: T, step = 1): T {.inline.} =
     ## Counts from ordinal value `a` up to `b` (inclusive) with the given
     ## step count.
@@ -2894,7 +2894,7 @@ proc min*[T: not SomeFloat](x, y: T): T {.inline.} =
   if x <= y: x else: y
 proc max*[T: not SomeFloat](x, y: T): T {.inline.} =
   if y <= x: x else: y
-{.pop.}
+{.pop.} # stackTrace: off
 
 proc high*(T: typedesc[SomeFloat]): T = Inf
 proc low*(T: typedesc[SomeFloat]): T = NegInf
@@ -3090,19 +3090,19 @@ proc compiles*(x: untyped): bool {.magic: "Compiles", noSideEffect, compileTime.
   ##     echo "'+' for integers is available"
   discard
 
-when not defined(js) and not defined(nimscript):
+when notJSnotNims:
   import "system/ansi_c"
   import "system/memory"
 
-when not defined(js):
+
 {.push stackTrace: off.}
 
-  when hasThreadSupport and hostOS != "standalone":
+when not defined(JS) and hasThreadSupport and hostOS != "standalone":
   const insideRLocksModule = false
   include "system/syslocks"
   include "system/threadlocalstorage"
 
-  when defined(nimV2):
+when not defined(JS) and defined(nimV2):
   type
     TNimNode {.compilerproc.} = object # to keep the code generator simple
     DestructorProc = proc (p: pointer) {.nimcall, benign, raises: [].}
@@ -3114,13 +3114,14 @@ when not defined(js):
       disposeImpl: pointer
     PNimType = ptr TNimType
 
-  when defined(nimSeqsV2) and not defined(nimscript):
+when notJSnotNims and defined(nimSeqsV2):
   include "system/strs_v2"
   include "system/seqs_v2"
 
 {.pop.}
 
-when not defined(js) and not defined(nimscript):
+
+when notJSnotNims:
   proc writeStackTrace*() {.tags: [], gcsafe, raises: [].}
     ## Writes the current stack trace to ``stderr``. This is only works
     ## for debug builds. Since it's usually used for debugging, this
@@ -3129,7 +3130,7 @@ when not defined(js) and not defined(nimscript):
 when not declared(sysFatal):
   include "system/fatal"
 
-when not defined(JS) and not defined(nimscript):
+when notJSnotNims:
   {.push stackTrace: off, profiler: off.}
 
   proc atomicInc*(memLoc: var int, x: int = 1): int {.inline,
@@ -3144,6 +3145,7 @@ when not defined(JS) and not defined(nimscript):
 
   {.pop.}
 
+
 when defined(nimV2):
   include system/refs_v2
 
@@ -3227,7 +3229,7 @@ const
 when not usesDestructors:
   {.pragma: nodestroy.}
 
-when not defined(nimscript) and hasAlloc:
+when hasAlloc:
   type
     GC_Strategy* = enum  ## The strategy the GC should use for the application.
       gcThroughput,      ## optimize for throughput
@@ -3235,7 +3237,7 @@ when not defined(nimscript) and hasAlloc:
       gcOptimizeTime,    ## optimize for speed
       gcOptimizeSpace    ## optimize for memory footprint
 
-  when not defined(JS) and not usesDestructors:
+when hasAlloc and not defined(JS) and not usesDestructors:
   proc GC_disable*() {.rtl, inl, benign.}
     ## Disables the GC. If called `n` times, `n` calls to `GC_enable`
     ## are needed to reactivate the GC.
@@ -3276,12 +3278,11 @@ when not defined(nimscript) and hasAlloc:
   proc GC_unref*(x: string) {.magic: "GCunref", benign.}
     ## See the documentation of `GC_ref <#GC_ref,string>`_.
 
-    when not defined(JS) and not defined(nimscript) and hasAlloc:
   proc nimGC_setStackBottom*(theStackBottom: pointer) {.compilerRtl, noinline, benign.}
     ## Expands operating GC stack range to `theStackBottom`. Does nothing
       ## if current stack bottom is already lower than `theStackBottom`.
 
-  elif defined(JS):
+when hasAlloc and defined(JS):
   template GC_disable* =
     {.warning: "GC_disable is a no-op in JavaScript".}
 
@@ -3418,7 +3419,7 @@ when defined(JS):
   proc add*(x: var cstring, y: cstring) {.magic: "AppendStrStr".}
 
 elif hasAlloc:
-  {.push stack_trace:off, profiler:off.}
+  {.push stackTrace: off, profiler: off.}
   proc add*(x: var string, y: cstring) =
     var i = 0
     if y != nil:
@@ -3578,8 +3579,8 @@ type
     fspEnd            ## Seek relative to end
 
 
-when not defined(JS): #and not defined(nimscript):
-  {.push stack_trace: off, profiler:off.}
+when not defined(JS):
+  {.push stackTrace: off, profiler: off.}
 
   when hasAlloc:
     when not defined(gcRegions) and not usesDestructors:
@@ -3606,7 +3607,15 @@ when not defined(JS): #and not defined(nimscript):
         strDesc = TNimType(size: sizeof(string), kind: tyString, flags: {ntfAcyclic})
       {.pop.}
 
-  when not defined(nimscript):
+  {.pop.}
+
+
+when not defined(JS):
+  # ugly hack, see the accompanying .pop for
+  # the mysterious error message
+  {.push stackTrace: off, profiler: off.}
+
+when notJSnotNims:
   proc zeroMem(p: pointer, size: Natural) =
     nimZeroMem(p, size)
     when declared(memTrackerOp):
@@ -3622,6 +3631,7 @@ when not defined(JS): #and not defined(nimscript):
   proc equalMem(a, b: pointer, size: Natural): bool =
     nimCmpMem(a, b, size) == 0
 
+when not defined(JS):
   proc cmp(x, y: string): int =
     when defined(nimscript):
       if x < y: result = -1
@@ -3652,9 +3662,8 @@ when not defined(JS): #and not defined(nimscript):
       while a[L] != nil: inc(L)
       result = cstringArrayToSeq(a, L)
 
-  # -------------------------------------------------------------------------
 
-  when declared(alloc0) and declared(dealloc):
+when not defined(JS) and declared(alloc0) and declared(dealloc):
   proc allocCStringArray*(a: openArray[string]): cstringArray =
     ## Creates a NULL terminated cstringArray from `a`. The result has to
     ## be freed with `deallocCStringArray` after it's not needed anymore.
@@ -3673,7 +3682,7 @@ when not defined(JS): #and not defined(nimscript):
       inc(i)
     dealloc(a)
 
-  when not defined(nimscript):
+when notJSnotNims:
   type
     PSafePoint = ptr TSafePoint
     TSafePoint {.compilerproc, final.} = object
@@ -3682,6 +3691,7 @@ when not defined(JS): #and not defined(nimscript):
       context: C_JmpBuf
     SafePoint = TSafePoint
 
+when not defined(JS):
   when declared(initAllocator):
     initAllocator()
   when hasThreadSupport:
@@ -3690,7 +3700,7 @@ when not defined(JS): #and not defined(nimscript):
     when not defined(useNimRtl) and not defined(createNimRtl): initStackBottom()
     when declared(initGC): initGC()
 
-  when not defined(nimscript):
+when notJSnotNims:
   proc setControlCHook*(hook: proc () {.noconv.})
     ## Allows you to override the behaviour of your application when CTRL+C
     ## is pressed. Only one such hook is supported.
@@ -3720,12 +3730,20 @@ when not defined(JS): #and not defined(nimscript):
   # we cannot compile this with stack tracing on
   # as it would recurse endlessly!
   include "system/arithm"
-    {.pop.} # stack trace
-  {.pop.} # stack trace
+  {.pop.}
 
-  when hostOS != "standalone" and not defined(nimscript):
+
+when not defined(JS):
+  # this is a hack: without this when statement, you would get:
+  # Error: system module needs: nimGCvisit
+  {.pop.} # stackTrace: off, profiler: off
+
+
+
+when notJSnotNims:
+  when hostOS != "standalone":
     include "system/dyncalls"
-  when not defined(nimscript):
+
   include "system/sets"
 
   when defined(gogc):
@@ -3755,26 +3773,26 @@ when not defined(JS): #and not defined(nimscript):
       else:
         result = n.sons[n.len]
 
+when notJSnotNims and hasAlloc:
   {.push profiler: off.}
-    when hasAlloc: include "system/mmdisp"
+  include "system/mmdisp"
   {.pop.}
-    {.push stack_trace: off, profiler:off.}
-    when hasAlloc:
+  {.push stackTrace: off, profiler: off.}
   when not defined(nimSeqsV2):
     include "system/sysstr"
   {.pop.}
-    when hasAlloc: include "system/strmantle"
 
-    when hasThreadSupport:
-      when hostOS != "standalone": include "system/channels"
-
-  when not defined(nimscript) and hasAlloc:
+  include "system/strmantle"
   when not usesDestructors:
     include "system/assign"
   when not defined(nimV2):
     include "system/repr"
 
-  when hostOS != "standalone" and not defined(nimscript):
+when notJSnotNims and hasThreadSupport and hostOS != "standalone":
+  include "system/channels"
+
+
+when notJSnotNims and hostOS != "standalone":
   proc getCurrentException*(): ref Exception {.compilerRtl, inl, benign.} =
     ## Retrieves the current exception; if there is none, `nil` is returned.
     result = currException
@@ -3795,12 +3813,13 @@ when not defined(JS): #and not defined(nimscript):
     ## **Warning**: Only use this if you know what you are doing.
     currException = exc
 
-  {.push stack_trace: off, profiler:off.}
-  when (defined(profiler) or defined(memProfiler)) and not defined(nimscript):
-    include "system/profiler"
-  {.pop.} # stacktrace
 
-  when not defined(nimscript):
+when notJSnotNims:
+  {.push stackTrace: off, profiler: off.}
+  when (defined(profiler) or defined(memProfiler)):
+    include "system/profiler"
+  {.pop.}
+
   proc rawProc*[T: proc](x: T): pointer {.noSideEffect, inline.} =
     ## Retrieves the raw proc pointer of the closure `x`. This is
     ## useful for interfacing closures with C.
@@ -3821,7 +3840,7 @@ when not defined(JS): #and not defined(nimscript):
     `result` = ((NI*) `x`.ClE_0)[1] < 0;
     """.}
 
-elif defined(JS):
+when defined(JS):
   # Stubs:
   proc getOccupiedMem(): int = return -1
   proc getFreeMem(): int = return -1
@@ -3837,11 +3856,10 @@ elif defined(JS):
   proc deallocShared(p: pointer) = discard
   proc reallocShared(p: pointer, newsize: Natural): pointer = discard
 
-
-  when defined(JS) and not defined(nimscript):
+  when not defined(nimscript):
     include "system/jssys"
     include "system/reprjs"
-  elif defined(nimscript):
+  else:
     proc cmp(x, y: string): int =
       if x == y: return 0
       if x < y: return -1
@@ -3856,7 +3874,7 @@ when defined(JS) or defined(nimscript):
 
 proc quit*(errormsg: string, errorcode = QuitFailure) {.noreturn.} =
   ## A shorthand for ``echo(errormsg); quit(errorcode)``.
-  when defined(nimscript) or defined(js) or (hostOS == "standalone"):
+  when defined(nimscript) or defined(JS) or (hostOS == "standalone"):
     echo errormsg
   else:
     when nimvm:
@@ -3866,8 +3884,8 @@ proc quit*(errormsg: string, errorcode = QuitFailure) {.noreturn.} =
       cstderr.rawWrite("\n")
   quit(errorcode)
 
-{.pop.} # checks
-{.pop.} # hints
+{.pop.} # checks: off
+{.pop.} # hints: off
 
 proc `/`*(x, y: int): float {.inline, noSideEffect.} =
   ## Division of integers that results in a float.
@@ -4350,8 +4368,7 @@ proc locals*(): RootObj {.magic: "Plugin", noSideEffect.} =
   ##   # -> B is 1
   discard
 
-when hasAlloc and not defined(nimscript) and not defined(JS) and
-    not usesDestructors:
+when hasAlloc and notJSnotNims and not usesDestructors:
   # XXX how to implement 'deepCopy' is an open problem.
   proc deepCopy*[T](x: var T, y: T) {.noSideEffect, magic: "DeepCopy".} =
     ## Performs a deep copy of `y` and copies it into `x`.
@@ -4385,8 +4402,7 @@ proc `==`*(x, y: cstring): bool {.magic: "EqCString", noSideEffect,
   elif x.isNil or y.isNil: result = false
   else: result = strcmp(x, y) == 0
 
-when defined(nimNoNilSeqs2):
-  when not compileOption("nilseqs"):
+when defined(nimNoNilSeqs2) and not compileOption("nilseqs"):
   when defined(nimHasUserErrors):
     # bug #9149; ensure that 'type(nil)' does not match *too* well by using 'type(nil) | type(nil)'.
     # Eventually (in 0.20?) we will be able to remove this hack completely.
@@ -4443,7 +4459,7 @@ template once*(body: untyped): untyped =
     alreadyExecuted = true
     body
 
-{.pop.} #{.push warning[GcMem]: off, warning[Uninit]: off.}
+{.pop.} # warning[GcMem]: off, warning[Uninit]: off
 
 proc substr*(s: string, first, last: int): string =
   ## Copies a slice of `s` into a new string and returns this new
@@ -4472,7 +4488,7 @@ proc substr*(s: string, first = 0): string =
 when defined(nimconfig):
   include "system/nimscript"
 
-when not defined(js):
+when not defined(JS):
   proc toOpenArray*[T](x: ptr UncheckedArray[T]; first, last: int): openArray[T] {.
     magic: "Slice".}
   when defined(nimToOpenArrayCString):