injectdestructors: remove obsolete processing (#956)

## Summary

Remove the moving of `def`s from the destructor injection pass. It is
obsolete now that unscoped `def`s are properly handled during the
MIR -> `CgNode` translation.

## Details

Moving the `def`s was necessary back when their semantics weren't
as fleshed out, but with the behaviour of a `def` statements now being
clearer, and `cgirgen` properly translating them, this step is
unnecessary.

Not moving the `def` during destructor injection also improves the
final generated code, as the regions previously used for turning a
`def` statement into an assignment often assigned the input value to
an, in this case, unnecessary temporary first.

compiler/sem/injectdestructors.nim

@@ -1006,37 +1006,6 @@ proc injectDestructors(tree: MirTree, graph: ModuleGraph,
       result = ord(x.pos) - ord(y.pos)
   )
 
-  # second pass: if at least one entity in a scope needs its destructor call
-  # placed in a ``finally`` clause, all others in the same scope do too, as the
-  # order-of-destruction would be violated otherwise
-  for pos, scope in entries.items:
-    if scope in needsFinally:
-      # if the destroy call has to be placed inside a ``finally`` clause, we
-      # first need to move the 'def' from its current position to the start of
-      # the scope, as it'd be otherwise located inside the ``try``'s body
-      # (which would render the entity unavailable inside the ``finally``
-      # clause)
-      assert tree[pos].kind == mnkDef
-      c.seek(pos)
-      if hasInput(tree, Operation pos):
-        # replace the 'def' with an initializing assignment if it has an
-        # input:
-        c.replaceMulti(buf):
-          buf.subTree MirNode(kind: mnkRegion):
-            argBlock(buf):
-              let e = getDefEntity(tree, pos) # the entity (e.g. local)
-              chain(buf): emit(tree[e]) => name()
-              chain(buf): opParam(0, tree[e].typ) => arg()
-            buf.add MirNode(kind: mnkInit)
-
-      else:
-        c.remove()
-
-      # insert the 'def' at the start of the scope:
-      c.seek(scope + 1)
-      c.insert(NodeInstance pos, buf):
-        buf.add toOpenArray(tree, pos.int, pos.int+2)
-
   iterator scopeItems(e: seq[DestroyEntry]): Slice[int] {.inline.} =
     ## Partitions `e` using the `scope` field and yields the slice of each
     ## partition
@@ -1052,7 +1021,7 @@ proc injectDestructors(tree: MirTree, graph: ModuleGraph,
           scopePos = e[i].scope
           start = i
 
-  # third pass: inject the destructors and place them inside a ``finally``
+  # second pass: inject the destructors and place them inside a ``finally``
   # clause if necessary
   for s in scopeItems(entries):
     let
@@ -1062,8 +1031,7 @@ proc injectDestructors(tree: MirTree, graph: ModuleGraph,
         ## the node to inherit the origin information from
 
     if useFinally:
-      # at the start of the scope (after the 'def's previously moved there),
-      # insert the start nodes of a 'try' and a 'stmtList':
+      # start a 'finally' at the beginning of the scope:
       c.seek(scopeStart + 1)
       c.insert(source, buf):
         buf.add MirNode(kind: mnkTry, len: 1)

tests/arc/topt_cursor.nim

@@ -4,7 +4,7 @@ discard """
   nimout: '''--expandArc: main
 
 var x_cursor
-var :aux_2
+var :aux_3
 try:
   x_cursor = ("hi", 5)
   block :label_0:
@@ -13,11 +13,10 @@ try:
       break :label_0
     x_cursor = [type node](("string here", 80))
   echo([
-    var :aux_4 = $(x_cursor)
-    :aux_2 = :aux_4
-    :aux_2])
+    :aux_3 = $(x_cursor)
+    :aux_3])
 finally:
-  =destroy(:aux_2)
+  =destroy(:aux_3)
 -- end of expandArc ------------------------
 --expandArc: sio
 

tests/arc/topt_no_cursor.nim

@@ -60,25 +60,24 @@ result.value = move(lvalue)
 --expandArc: tt
 
 var it_cursor
+var a
+var :aux_4
 var :aux_5
 var :aux_6
-var a
-var :aux_3
 try:
   it_cursor = x
   a = (
+    :aux_4 = default()
+    =copy(:aux_4, it_cursor.key)
+    :aux_4,
     :aux_5 = default()
-    =copy(:aux_5, it_cursor.key)
-    :aux_5,
-    :aux_6 = default()
-    =copy(:aux_6, it_cursor.val)
-    :aux_6)
+    =copy(:aux_5, it_cursor.val)
+    :aux_5)
   echo([
-    var :aux_7 = $(a)
-    :aux_3 = :aux_7
-    :aux_3])
+    :aux_6 = $(a)
+    :aux_6])
 finally:
-  =destroy(:aux_3)
+  =destroy(:aux_6)
   =destroy_1(a)
 -- end of expandArc ------------------------
 --expandArc: extractConfig

tests/lang_objects/destructor/tv2_cast.nim

@@ -9,13 +9,11 @@ var data
 var :aux_2
 var :aux_3
 try:
-  var :aux_5 = encode(
-      var :aux_4 = newString(100)
-      :aux_2 = :aux_4
+  :aux_3 = encode(
+      :aux_2 = newString(100)
       cast[seq[byte]](:aux_2))
-  :aux_3 = :aux_5
-  var :aux_6 = cast[string](:aux_3)
-  =copy(data, :aux_6)
+  var :aux_4 = cast[string](:aux_3)
+  =copy(data, :aux_4)
 finally:
   =destroy(:aux_3)
   =destroy_1(:aux_2)
@@ -27,10 +25,9 @@ var data
 var :aux_3
 try:
   s = newString(100)
-  var :aux_4 = encode(toOpenArrayByte(s, 0, -(len(s), 1)))
-  :aux_3 = :aux_4
-  var :aux_5 = cast[string](:aux_3)
-  =copy(data, :aux_5)
+  :aux_3 = encode(toOpenArrayByte(s, 0, -(len(s), 1)))
+  var :aux_4 = cast[string](:aux_3)
+  =copy(data, :aux_4)
 finally:
   =destroy(:aux_3)
   =destroy_1(data)
@@ -42,10 +39,9 @@ var data
 var :aux_3
 try:
   s = newSeq(100)
-  var :aux_4 = encode(s)
-  :aux_3 = :aux_4
-  var :aux_5 = cast[string](:aux_3)
-  =copy(data, :aux_5)
+  :aux_3 = encode(s)
+  var :aux_4 = cast[string](:aux_3)
+  =copy(data, :aux_4)
 finally:
   =destroy(:aux_3)
   =destroy_1(data)
@@ -56,13 +52,11 @@ var data
 var :aux_2
 var :aux_3
 try:
-  var :aux_5 = encode(
-      var :aux_4 = newSeq(100)
-      :aux_2 = :aux_4
+  :aux_3 = encode(
+      :aux_2 = newSeq(100)
       :aux_2)
-  :aux_3 = :aux_5
-  var :aux_6 = cast[string](:aux_3)
-  =copy(data, :aux_6)
+  var :aux_4 = cast[string](:aux_3)
+  =copy(data, :aux_4)
 finally:
   =destroy(:aux_3)
   =destroy(:aux_2)