mirgen: improve scope-only block support (#928)

## Summary

Don't emit `mnkBlock` MIR nodes for `nkBlockStmt`/`nkBlockExpr` nodes
(`block` statements/expression) are not used for control-flow. This
reduces the size of the produced MIR code and means that neither
compiler-generated scope-only blocks nor `block` statements/
expression not targeted by `break` statements reach the code
generators.

Most notably, the code generated by the JavaScript code generator
improves, in terms of size.

## Details

Detection of blocks-used-for-control-flow is done via testing for the
`sfUsed` flag: if present, the `nkBlockStmt`/`nkBlockExpr` is treated
as used for control-flow, otherwise it's treated as only opening a
scope. This is only a conservative approximation, however, and blocks
where the label is marked as used through other means (or by appearing
in a `break` within a `compiles`) will be mis-detected.

So that the detection also works for `transf`-injected breaks and
blocks, all  `nkBreakStmt`  AST in  `transf` / `closureiters` /
`lambdalifting` 
is now generated via the new `newBreakStmt` procedure in `lowerings`,
which marks the provided label as used.

When `mirgen.genBlock` gets passed an `nkBlockExpr`/`nkBlockStmt` node
where the label symbol is not marked as used, it only emits the
`mnkScope` block necessary for correct variable lifetimes. Since
`cnkBlockStmt` are only generated for `mnkBlock` nodes by `cgirgen`,
this means that the block statements are omitted in the final generated
code too.

Finally, `transf.transformBlock` is improved, and a necessary fix is
applied:
- instead of introducing a new label with `newLabel` during inlining,
  the original symbol is copied and adjusted. This ensures that the
  symbol flags (e.g., whether the symbol is used) are kept
- the label symbol is no longer pushed to the `breakSyms` stack in the
  *inlining* case. This was/is unnecessary, as inlined AST is already
  transformed, and so none of the `nkBreakStmt` nodes within are
  processed
- the *non-inlining* case doesn't use `transformSons`, but transforms
  the body AST directly. While having no practical effect at the
  moment, this means that the label slot is no longer treated as a
  symbol-in-a-usage-position

No `mnkBlock` nodes being emitted for scope-only blocks is also visible
in the `--expandArc` output, so the tests depending on the output are
adjusted.

compiler/mir/mirgen.nim

@@ -1245,6 +1245,12 @@ proc genWhile(c: var TCtx, n: PNode) =
 
 proc genBlock(c: var TCtx, n: PNode, dest: Destination) =
   ## Generates and emits the MIR code for a ``block`` expression or statement
+  if sfUsed notin n[0].sym.flags:
+    # if the label is never used, it means that the block is only used for
+    # scoping. Omit emitting an ``mnkBlock`` and just use a scope
+    scope(c.stmts): c.genWithDest(n[1], dest)
+    return
+
   let id = nextLabel(c)
 
   # push the block to the stack:

compiler/sem/closureiters.nim

@@ -1029,10 +1029,7 @@ proc transformStateAssignments(ctx: var Ctx, n: PNode): PNode =
   of nkGotoState:
     result = newNodeI(nkStmtList, n.info)
     result.add(ctx.newStateAssgn(stateFromGotoState(n)))
-
-    let breakState = newNodeI(nkBreakStmt, n.info)
-    breakState.add(newSymNode(ctx.stateLoopLabel))
-    result.add(breakState)
+    result.add(newBreakStmt(n.info, ctx.stateLoopLabel))
 
   else:
     for i in 0..<n.len:

compiler/sem/lambdalifting.nim

@@ -936,9 +936,7 @@ proc liftForLoop*(g: ModuleGraph; body: PNode; idgen: IdGenerator;
   let elifBranch = newNodeI(nkElifBranch, body.info)
   elifBranch.add(bs)
 
-  let br = newTreeI(nkBreakStmt, body.info, newSymNode(breakLabel, body.info))
-
-  elifBranch.add(br)
+  elifBranch.add(newBreakStmt(body.info, breakLabel))
   ibs.add(elifBranch)
 
   loopBody[1] = ibs

compiler/sem/lowerings.nim

@@ -75,6 +75,12 @@ proc newFastMoveStmt*(g: ModuleGraph, le, ri: PNode): PNode =
      newTreeIT(nkCall, ri.info, ri.typ,
        newSymNode(getSysMagic(g, ri.info, "move", mMove)), ri)]
 
+proc newBreakStmt*(info: TLineInfo, label: PSym): PNode =
+  ## Generates the AST for labeled break and marks `label` as used.
+  result = newTreeI(nkBreakStmt, info, newSymNode(label, info))
+  # mark the label as used:
+  label.flags.incl sfUsed
+
 proc lowerTupleUnpacking*(g: ModuleGraph; n: PNode; idgen: IdGenerator; owner: PSym): PNode =
   assert n.kind == nkVarTuple
   let value = n.lastSon

compiler/sem/transf.nim

@@ -319,20 +319,30 @@ proc newLabel(c: PTransf, n: PNode): PSym =
   result.name = getIdent(c.graph.cache, genPrefix)
 
 proc transformBlock(c: PTransf, n: PNode): PNode =
-  var labl: PSym
+  result = shallowCopy(n)
   if c.inlining > 0:
-    labl = newLabel(c, n[0])
+    # all blocks that reach here are labeled. We still need to ensure that
+    # unique symbols are used, so introduce both a copy and associated
+    # mapping
+    let labl = copySym(n[0].sym, nextSymId c.idgen)
+    labl.info = n[0].sym.info
+    labl.owner = getCurrOwner(c)
     idNodeTablePut(c.transCon.mapping, n[0].sym, newSymNode(labl))
+
+    result[0] = newSymNode(labl, n[0].info)
+    # the breaks in the AST were all already transformed
+    result[1] = transform(c, n[1])
   else:
-    labl =
+    let labl =
       if n[0].kind != nkEmpty:
         n[0].sym  # already named block? -> Push symbol on the stack
       else:
         newLabel(c, n)
-  c.breakSyms.add(labl)
-  result = transformSons(c, n)
-  discard c.breakSyms.pop
-  result[0] = newSymNode(labl)
+
+    result[0] = newSymNode(labl, n[0].info)
+    c.breakSyms.add(labl)
+    result[1] = transform(c, n[1])
+    discard c.breakSyms.pop
 
 proc transformLoopBody(c: PTransf, n: PNode): PNode =
   # What if it contains "continue" and "break"? "break" needs
@@ -393,7 +403,7 @@ proc transformWhile(c: PTransf; n: PNode): PNode =
             newTreeI(nkCall, info,
               newSymNode(c.graph.getSysMagic(info, "not", mNot)),
               cond),
-            newTreeI(nkBreakStmt, info, newSymNode(labl, info))))
+            newBreakStmt(info, labl)))
 
       var body = transformLoopBody(c, n[1])
       # use a nested scope for the body. This is important for the clean-up
@@ -412,11 +422,12 @@ proc transformWhile(c: PTransf; n: PNode): PNode =
     discard c.breakSyms.pop
 
 proc transformBreak(c: PTransf, n: PNode): PNode =
-  result = transformSons(c, n)
   if n[0].kind == nkEmpty:
-    assert c.breakSyms.len > 0
-    let labl = c.breakSyms[^1]
-    result[0] = newSymNode(labl)
+    # turn into a labeled break, using the break label stack
+    result = newBreakStmt(n.info, c.breakSyms[^1])
+  else:
+    # already a labeled break
+    result = transformSons(c, n)
 
 proc introduceNewLocalVars(c: PTransf, n: PNode): PNode =
   case n.kind
@@ -1145,8 +1156,8 @@ proc transform(c: PTransf, n: PNode): PNode =
       # disable the original 'defer' statement:
       n.kind = nkEmpty
   of nkContinueStmt:
-    let labl = c.contSyms[^1]
-    result = newTreeI(nkBreakStmt, n.info): newSymNode(labl)
+    # transform into a break out of the loop's inner block
+    result = newBreakStmt(n.info, c.contSyms[^1])
   of nkBreakStmt: result = transformBreak(c, n)
   of nkCallKinds:
     result = transformCall(c, n)

tests/arc/topt_cursor.nim

@@ -20,24 +20,22 @@ finally:
 -- end of expandArc ------------------------
 --expandArc: sio
 
-block label:
-  var filename_cursor = "debug.txt"
-  var f = open(filename_cursor, 0, 8000)
+var filename_cursor = "debug.txt"
+var f = open(filename_cursor, 0, 8000)
+try:
+  var res
   try:
-    var res
-    try:
-      res = newStringOfCap(80)
-      block label_1:
-        while true:
-          if not(readLine(f, res)):
-            break label_1
-          block label_2:
-            var x_cursor = res
-            echo([x_cursor])
-    finally:
-      =destroy(res)
+    res = newStringOfCap(80)
+    block label:
+      while true:
+        if not(readLine(f, res)):
+          break label
+        var x_cursor = res
+        echo([x_cursor])
   finally:
-    close(f)
+    =destroy(res)
+finally:
+  close(f)
 -- end of expandArc ------------------------'''
 """
 

tests/arc/topt_no_cursor.nim

@@ -88,27 +88,25 @@ finally:
 var lan_ip
 try:
   lan_ip = ""
+  var a_cursor = txt
+  var i = 0
+  var L = len(a_cursor)
   block label:
-    var a_cursor = txt
-    var i = 0
-    var L = len(a_cursor)
-    block label_1:
-      while true:
-        if not(<(i, L)):
-          break label_1
-        block label_2:
-          var splitted
-          try:
-            var line = a_cursor[i]
-            splitted = split(line, " ", -1)
-            if ==(splitted[0], "opt"):
-              var :aux_7 = splitted[1]
-              =copy(lan_ip, :aux_7)
-            echo([lan_ip])
-            echo([splitted[1]])
-          finally:
-            =destroy(splitted)
-        inc(i, 1)
+    while true:
+      if not(<(i, L)):
+        break label
+      var splitted
+      try:
+        var line = a_cursor[i]
+        splitted = split(line, " ", -1)
+        if ==(splitted[0], "opt"):
+          var :aux_7 = splitted[1]
+          =copy(lan_ip, :aux_7)
+        echo([lan_ip])
+        echo([splitted[1]])
+      finally:
+        =destroy(splitted)
+      inc(i, 1)
 finally:
   =destroy_1(lan_ip)
 --expandArc: mergeShadowScope
@@ -118,23 +116,21 @@ try:
   var :aux_3 = c[].currentScope
   =copy(shadowScope, :aux_3)
   rawCloseScope(c)
+  var a_cursor = shadowScope[].symbols
+  var i = 0
+  var L = len(a_cursor)
   block label:
-    var a_cursor = shadowScope[].symbols
-    var i = 0
-    var L = len(a_cursor)
-    block label_1:
-      while true:
-        if not(<(i, L)):
-          break label_1
-        block label_2:
-          var :aux_9
-          var sym = a_cursor[i]
-          addInterfaceDecl(c,
-            var :aux_8 = sym
-            :aux_9 = default()
-            =copy_1(:aux_9, :aux_8)
-            :aux_9)
-        inc(i, 1)
+    while true:
+      if not(<(i, L)):
+        break label
+      var :aux_9
+      var sym = a_cursor[i]
+      addInterfaceDecl(c,
+        var :aux_8 = sym
+        :aux_9 = default()
+        =copy_1(:aux_9, :aux_8)
+        :aux_9)
+      inc(i, 1)
 finally:
   =destroy(shadowScope)
 -- end of expandArc ------------------------

tests/arc/topt_refcursors.nim

@@ -8,18 +8,16 @@ block label:
   while true:
     if not(not(==(it_cursor, nil))):
       break label
-    block label_1:
-      echo([it_cursor[].s])
-      it_cursor = it_cursor[].ri
+    echo([it_cursor[].s])
+    it_cursor = it_cursor[].ri
 var jt_cursor = root
-block label_2:
+block label_1:
   while true:
     if not(not(==(jt_cursor, nil))):
-      break label_2
-    block label_3:
-      var ri_1_cursor = jt_cursor[].ri
-      echo([jt_cursor[].s])
-      jt_cursor = ri_1_cursor
+      break label_1
+    var ri_1_cursor = jt_cursor[].ri
+    echo([jt_cursor[].s])
+    jt_cursor = ri_1_cursor
 -- end of expandArc ------------------------'''
 """
 

tests/arc/topt_wasmoved_destroy_pairs.nim

@@ -22,33 +22,30 @@ var b
 var x
 try:
   x = f()
+  var a_1 = 0
+  var b_1 = 4
+  var i = a_1
   block label:
-    var a_1 = 0
-    var b_1 = 4
-    var i = a_1
-    block label_1:
-      while true:
-        if not(<(i, b_1)):
-          break label_1
-        block label_2:
-          block label_3:
-            var :aux_9
-            var i_1_cursor = i
-            if ==(i_1_cursor, 2):
-              return
-            add(a,
-              :aux_9 = default()
-              =copy(:aux_9, x)
-              :aux_9)
-          inc(i, 1)
-  block label_4:
+    while true:
+      if not(<(i, b_1)):
+        break label
+      var :aux_9
+      var i_1_cursor = i
+      if ==(i_1_cursor, 2):
+        return
+      add(a,
+        :aux_9 = default()
+        =copy(:aux_9, x)
+        :aux_9)
+      inc(i, 1)
+  block label_1:
     if cond:
       var :aux_10
       add(a,
         :aux_10 = x
         wasMoved(x)
         :aux_10)
-      break label_4
+      break label_1
     var :aux_11
     add(b,
       :aux_11 = x