Refactor injectdestructors (#12295)

One improvement over #devel is visible in the transformation of getEnv. With this approach we move to result whenever possible.

7 files changed, 258 insertions, 578 deletions

diff --git a/compiler/ast.nim b/compiler/ast.nim
index a2598dae4..f24008b30 100644
--- a/compiler/ast.nim
+++ b/compiler/ast.nim
@@ -291,10 +291,6 @@ type
 const
   sfNoInit* = sfMainModule       # don't generate code to init the variable
 
-  sfCursor* = sfDispatcher
-    # local variable has been computed to be a "cursor".
-    # see cursors.nim for details about what that means.
-
   sfAllUntyped* = sfVolatile # macro or template is immediately expanded \
     # in a generic context
 
diff --git a/compiler/cursors.nim b/compiler/cursors.nim
deleted file mode 100644
index 9577102fb..000000000
--- a/compiler/cursors.nim
+++ /dev/null
@@ -1,72 +0,0 @@
-#
-#
-#           The Nim Compiler
-#        (c) Copyright 2019 Andreas Rumpf
-#
-#    See the file "copying.txt", included in this
-#    distribution, for details about the copyright.
-#
-
-import
-  intsets, ast, astalgo, msgs, renderer, magicsys, types, idents, trees,
-  strutils, options, dfa, lowerings, tables, modulegraphs, msgs,
-  lineinfos, parampatterns
-
-##[
-This module implements "cursor" detection. A cursor is a local variable
-that is used for navigation in a datastructure, it does not "own" the
-data it aliases but it might update the underlying datastructure.
-
-Two primary examples for cursors that I have in mind and that are critical
-for optimization:
-
-1. Local string variable introduced by ``for x in a``::
-
-  var i = 0
-  while i < a.len:
-    let cursor = a[i]
-    use cursor
-    inc i
-
-2. Local ``ref`` variable for navigation::
-
-  var cursor = listHead
-  while cursor != nil:
-    use cursor
-    cursor = cursor.next
-
-Cursors are very interesting for the optimizer because they can be copyMem'ed
-and don't need a destructor.
-
-More formally, a cursor is a variable that is set on all paths to
-a *location* or a proc call that produced a ``lent/var`` type. All statements
-that come after these assignments MUST not mutate what the cursor aliases.
-
-Mutations *through* the cursor are allowed if the cursor has ref semantics.
-
-Look at this complex real world example taken from the compiler itself:
-
-.. code-block:: Nim
-
-  proc getTypeName(m: BModule; typ: PType; sig: SigHash): Rope =
-    var t = typ
-    while true:
-      if t.sym != nil and {sfImportc, sfExportc} * t.sym.flags != {}:
-        return t.sym.loc.r
-
-      if t.kind in irrelevantForBackend:
-        t = t.lastSon
-      else:
-        break
-    let typ = if typ.kind in {tyAlias, tySink, tyOwned}: typ.lastSon else: typ
-    if typ.loc.r == nil:
-      typ.loc.r = typ.typeName & $sig
-    result = typ.loc.r
-    if result == nil: internalError(m.config, "getTypeName: " & $typ.kind)
-
-Here `t` is a cursor but without a control flow based analysis we are unlikely
-to detect it.
-
-]##
-
-# Araq: I owe you an implementation. For now use the .cursor pragma. :-/
diff --git a/compiler/injectdestructors.nim b/compiler/injectdestructors.nim
index 5354fd740..95de5777e 100644
--- a/compiler/injectdestructors.nim
+++ b/compiler/injectdestructors.nim
@@ -11,136 +11,14 @@
 ## an optimizer that optimizes copies to moves. This is implemented as an
 ## AST to AST transformation so that every backend benefits from it.
 
-## Rules for destructor injections:
-##
-## foo(bar(X(), Y()))
-## X and Y get destroyed after bar completes:
-##
-## foo( (tmpX = X(); tmpY = Y(); tmpBar = bar(tmpX, tmpY);
-##       destroy(tmpX); destroy(tmpY);
-##       tmpBar))
-## destroy(tmpBar)
-##
-## var x = f()
-## body
-##
-## is the same as:
-##
-##  var x;
-##  try:
-##    move(x, f())
-##  finally:
-##    destroy(x)
-##
-## But this really just an optimization that tries to avoid to
-## introduce too many temporaries, the 'destroy' is caused by
-## the 'f()' call. No! That is not true for 'result = f()'!
-##
-## x = y where y is read only once
-## is the same as:  move(x, y)
-##
-## Actually the more general rule is: The *last* read of ``y``
-## can become a move if ``y`` is the result of a construction.
-##
-## We also need to keep in mind here that the number of reads is
-## control flow dependent:
-## let x = foo()
-## while true:
-##   y = x  # only one read, but the 2nd iteration will fail!
-## This also affects recursions! Only usages that do not cross
-## a loop boundary (scope) and are not used in function calls
-## are safe.
-##
-##
-## x = f() is the same as:  move(x, f())
-##
-## x = y
-## is the same as:  copy(x, y)
-##
-## Reassignment works under this scheme:
-## var x = f()
-## x = y
-##
-## is the same as:
-##
-##  var x;
-##  try:
-##    move(x, f())
-##    copy(x, y)
-##  finally:
-##    destroy(x)
-##
-##  result = f()  must not destroy 'result'!
-##
-## The produced temporaries clutter up the code and might lead to
-## inefficiencies. A better strategy is to collect all the temporaries
-## in a single object that we put into a single try-finally that
-## surrounds the proc body. This means the code stays quite efficient
-## when compiled to C. In fact, we do the same for variables, so
-## destructors are called when the proc returns, not at scope exit!
-## This makes certains idioms easier to support. (Taking the slice
-## of a temporary object.)
-##
-## foo(bar(X(), Y()))
-## X and Y get destroyed after bar completes:
-##
-## var tmp: object
-## foo( (move tmp.x, X(); move tmp.y, Y(); tmp.bar = bar(tmpX, tmpY);
-##       tmp.bar))
-## destroy(tmp.bar)
-## destroy(tmp.x); destroy(tmp.y)
-##
-
-#[
-From https://github.com/nim-lang/Nim/wiki/Destructors
-
-Rule      Pattern                 Transformed into
-----      -------                 ----------------
-1.1	      var x: T; stmts	        var x: T; try stmts
-                                  finally: `=destroy`(x)
-2         x = f()                 `=sink`(x, f())
-3         x = lastReadOf z        `=sink`(x, z); wasMoved(z)
-3.2       x = path z; body        ``x = bitwiseCopy(path z);``
-                                  do not emit `=destroy(x)`. Note: body
-                                  must not mutate ``z`` nor ``x``. All
-                                  assignments to ``x`` must be of the form
-                                  ``path z`` but the ``z`` can differ.
-                                  Neither ``z`` nor ``x`` can have the
-                                  flag ``sfAddrTaken`` to ensure no other
-                                  aliasing is going on.
-4.1       y = sinkParam           `=sink`(y, sinkParam)
-4.2       x = y                   `=`(x, y) # a copy
-5.1       f_sink(g())             f_sink(g())
-5.2       f_sink(y)               f_sink(copy y); # copy unless we can see it's the last read
-5.3       f_sink(move y)          f_sink(y); wasMoved(y) # explicit moves empties 'y'
-5.4       f_noSink(g())           var tmp = bitwiseCopy(g()); f(tmp); `=destroy`(tmp)
-
-Rule 3.2 describes a "cursor" variable, a variable that is only used as a
-view into some data structure. See ``compiler/cursors.nim`` for details.
-
-Note: In order to avoid the very common combination ``reset(x); =sink(x, y)`` for
-variable definitions we must turn "the first sink/assignment" operation into a
-copyMem. This is harder than it looks:
-
-  while true:
-    try:
-      if cond: break # problem if we run destroy(x) here :-/
-      var x = f()
-    finally:
-      destroy(x)
-
-And the C++ optimizers don't sweat to optimize it for us, so we don't have
-to do it.
-]#
+## See doc/destructors.rst for a spec of the implemented rewrite rules
+
 
 import
   intsets, ast, astalgo, msgs, renderer, magicsys, types, idents,
   strutils, options, dfa, lowerings, tables, modulegraphs, msgs,
   lineinfos, parampatterns, sighashes
 
-const
-  InterestingSyms = {skVar, skResult, skLet, skForVar, skTemp}
-
 type
   Con = object
     owner: PSym
@@ -217,43 +95,6 @@ proc isLastRead(n: PNode; c: var Con): bool =
   dbg:
     echo "ugh ", c.otherRead.isNil, " ", result
 
-  when false:
-    let s = n.sym
-    var pcs: seq[int] = @[instr+1]
-    var takenGotos: IntSet
-    var takenForks = initIntSet()
-    while pcs.len > 0:
-      var pc = pcs.pop
-
-      takenGotos = initIntSet()
-      while pc < c.g.len:
-        case c.g[pc].kind
-        of def:
-          if c.g[pc].sym == s:
-            # the path lead to a redefinition of 's' --> abandon it.
-            break
-          inc pc
-        of use:
-          if c.g[pc].sym == s:
-            c.otherRead = c.g[pc].n
-            return false
-          inc pc
-        of goto:
-          # we must leave endless loops eventually:
-          if not takenGotos.containsOrIncl(pc):
-            pc = pc + c.g[pc].dest
-          else:
-            inc pc
-        of fork:
-          # we follow the next instruction but push the dest onto our "work" stack:
-          if not takenForks.containsOrIncl(pc):
-            pcs.add pc + c.g[pc].dest
-          inc pc
-        of InstrKind.join:
-          inc pc
-    #echo c.graph.config $ n.info, " last read here!"
-    return true
-
 proc initialized(code: ControlFlowGraph; pc: int,
                  init, uninit: var IntSet; comesFrom: int): int =
   ## Computes the set of definitely initialized variables across all code paths
@@ -290,9 +131,6 @@ proc initialized(code: ControlFlowGraph; pc: int,
       inc pc
   return pc
 
-template interestingSym(s: PSym): bool =
-  s.owner == c.owner and s.kind in InterestingSyms and hasDestructor(s.typ)
-
 template isUnpackedTuple(s: PSym): bool =
   ## we move out all elements of unpacked tuples,
   ## hence unpacked tuples themselves don't need to be destroyed
@@ -353,8 +191,8 @@ proc canBeMoved(t: PType): bool {.inline.} =
   let t = t.skipTypes({tyGenericInst, tyAlias, tySink})
   result = t.kind != tyRef and t.attachedOps[attachedSink] != nil
 
-proc genSink(c: Con; t: PType; dest, ri: PNode): PNode =
-  let t = t.skipTypes({tyGenericInst, tyAlias, tySink})
+proc genSink(c: Con; dest, ri: PNode): PNode =
+  let t = dest.typ.skipTypes({tyGenericInst, tyAlias, tySink})
   let k = if t.attachedOps[attachedSink] != nil: attachedSink
           else: attachedAsgn
   if t.attachedOps[k] != nil:
@@ -365,20 +203,20 @@ proc genSink(c: Con; t: PType; dest, ri: PNode): PNode =
     # we generate a fast assignment in this case:
     result = newTree(nkFastAsgn, dest)
 
-proc genCopy(c: var Con; t: PType; dest, ri: PNode): PNode =
+proc genCopyNoCheck(c: Con; dest, ri: PNode): PNode =
+  let t = dest.typ.skipTypes({tyGenericInst, tyAlias, tySink})
+  result = genOp(c, t, attachedAsgn, dest, ri)
+
+proc genCopy(c: var Con; dest, ri: PNode): PNode =
+  let t = dest.typ
   if tfHasOwned in t.flags:
     # try to improve the error message here:
     if c.otherRead == nil: discard isLastRead(ri, c)
     checkForErrorPragma(c, t, ri, "=")
-  let t = t.skipTypes({tyGenericInst, tyAlias, tySink})
-  result = genOp(c, t, attachedAsgn, dest, ri)
-
-proc genCopyNoCheck(c: Con; t: PType; dest, ri: PNode): PNode =
-  let t = t.skipTypes({tyGenericInst, tyAlias, tySink})
-  result = genOp(c, t, attachedAsgn, dest, ri)
+  genCopyNoCheck(c, dest, ri)
 
-proc genDestroy(c: Con; t: PType; dest: PNode): PNode =
-  let t = t.skipTypes({tyGenericInst, tyAlias, tySink})
+proc genDestroy(c: Con; dest: PNode): PNode =
+  let t = dest.typ.skipTypes({tyGenericInst, tyAlias, tySink})
   result = genOp(c, t, attachedDestructor, dest, nil)
 
 proc addTopVar(c: var Con; v: PNode) =
@@ -390,20 +228,10 @@ proc getTemp(c: var Con; typ: PType; info: TLineInfo): PNode =
   result = newSymNode(sym)
   c.addTopVar(result)
 
-proc p(n: PNode; c: var Con): PNode
-
-template recurse(n, dest) =
-  for i in 0..<n.len:
-    dest.add p(n[i], c)
-
-proc genMagicCall(n: PNode; c: var Con; magicname: string; m: TMagic): PNode =
-  result = newNodeI(nkCall, n.info)
-  result.add(newSymNode(createMagic(c.graph, magicname, m)))
-  result.add n
-
 proc genWasMoved(n: PNode; c: var Con): PNode =
-  # The mWasMoved builtin does not take the address.
-  result = genMagicCall(n, c, "wasMoved", mWasMoved)
+  result = newNodeI(nkCall, n.info)
+  result.add(newSymNode(createMagic(c.graph, "wasMoved", mWasMoved)))
+  result.add n #mWasMoved does not take the address
 
 proc genDefaultCall(t: PType; c: Con; info: TLineInfo): PNode =
   result = newNodeI(nkCall, info)
@@ -422,9 +250,9 @@ proc destructiveMoveVar(n: PNode; c: var Con): PNode =
   let tempAsNode = newSymNode(temp)
 
   var vpart = newNodeI(nkIdentDefs, tempAsNode.info, 3)
-  vpart.sons[0] = tempAsNode
-  vpart.sons[1] = c.emptyNode
-  vpart.sons[2] = n
+  vpart[0] = tempAsNode
+  vpart[1] = c.emptyNode
+  vpart[2] = n
   add(v, vpart)
 
   result.add v
@@ -437,6 +265,10 @@ proc sinkParamIsLastReadCheck(c: var Con, s: PNode) =
      localError(c.graph.config, c.otherRead.info, "sink parameter `" & $s.sym.name.s &
          "` is already consumed at " & toFileLineCol(c. graph.config, s.info))
 
+proc p(n: PNode; c: var Con): PNode
+proc pArg(arg: PNode; c: var Con; isSink: bool): PNode
+proc moveOrCopy(dest, ri: PNode; c: var Con): PNode
+
 proc passCopyToSink(n: PNode; c: var Con): PNode =
   result = newNodeIT(nkStmtListExpr, n.info, n.typ)
   let tmp = getTemp(c, n.typ, n.info)
@@ -444,7 +276,7 @@ proc passCopyToSink(n: PNode; c: var Con): PNode =
   # out of loops we need to mark it as 'wasMoved'.
   result.add genWasMoved(tmp, c)
   if hasDestructor(n.typ):
-    var m = genCopy(c, n.typ, tmp, n)
+    var m = genCopy(c, tmp, n)
     m.add p(n, c)
     result.add m
     if isLValue(n):
@@ -457,7 +289,7 @@ proc passCopyToSink(n: PNode; c: var Con): PNode =
 
 proc isDangerousSeq(t: PType): bool {.inline.} =
   let t = t.skipTypes(abstractInst)
-  result = t.kind == tySequence and tfHasOwned notin t.sons[0].flags
+  result = t.kind == tySequence and tfHasOwned notin t[0].flags
 
 proc containsConstSeq(n: PNode): bool =
   if n.kind == nkBracket and n.len > 0 and n.typ != nil and isDangerousSeq(n.typ):
@@ -467,19 +299,66 @@ proc containsConstSeq(n: PNode): bool =
   of nkExprEqExpr, nkExprColonExpr, nkHiddenStdConv, nkHiddenSubConv:
     result = containsConstSeq(n[1])
   of nkObjConstr, nkClosure:
-    for i in 1 ..< n.len:
+    for i in 1..<n.len:
       if containsConstSeq(n[i]): return true
   of nkCurly, nkBracket, nkPar, nkTupleConstr:
-    for i in 0 ..< n.len:
-      if containsConstSeq(n[i]): return true
+    for son in n:
+      if containsConstSeq(son): return true
   else: discard
 
-proc pArg(arg: PNode; c: var Con; isSink: bool): PNode =
-  template pArgIfTyped(argPart: PNode): PNode =
-    # typ is nil if we are in if/case expr branch with noreturn
-    if argPart.typ == nil: p(argPart, c)
-    else: pArg(argPart, c, isSink)
+template handleNested(n: untyped, processCall: untyped) =
+  case n.kind
+  of nkStmtList, nkStmtListExpr:
+    if n.len == 0: return n
+    result = copyNode(n)
+    for i in 0..<n.len-1:
+      result.add p(n[i], c)
+    template node: untyped = n[^1]
+    result.add processCall
+  of nkBlockStmt, nkBlockExpr:
+    result = copyNode(n)
+    result.add n[0]
+    template node: untyped = n[1]
+    result.add processCall
+  of nkIfStmt, nkIfExpr:
+    result = copyNode(n)
+    for son in n:
+      var branch = copyNode(son)
+      if son.kind in {nkElifBranch, nkElifExpr}:
+        template node: untyped = son[1]
+        branch.add p(son[0], c) #The condition
+        branch.add if node.typ == nil: p(node, c) #noreturn
+                   else: processCall
+      else:
+        template node: untyped = son[0]
+        branch.add if node.typ == nil: p(node, c) #noreturn
+                   else: processCall
+      result.add branch
+  of nkCaseStmt:
+    result = copyNode(n)
+    result.add p(n[0], c)
+    for i in 1..<n.len:
+      var branch: PNode
+      if n[i].kind == nkOfBranch:
+        branch = n[i] # of branch conditions are constants
+        template node: untyped = n[i][^1]
+        branch[^1] = if node.typ == nil: p(node, c) #noreturn
+                     else: processCall
+      elif n[i].kind in {nkElifBranch, nkElifExpr}:
+        branch = copyNode(n[i])
+        branch.add p(n[i][0], c) #The condition
+        template node: untyped = n[i][1]
+        branch.add if node.typ == nil: p(node, c) #noreturn
+                   else: processCall
+      else:
+        branch = copyNode(n[i])
+        template node: untyped = n[i][0]
+        branch.add if node.typ == nil: p(node, c) #noreturn
+                   else: processCall
+      result.add branch
+  else: assert(false)
 
+proc pArg(arg: PNode; c: var Con; isSink: bool): PNode =
   if isSink:
     if arg.kind in nkCallKinds:
       # recurse but skip the call expression in order to prevent
@@ -495,8 +374,8 @@ proc pArg(arg: PNode; c: var Con; isSink: bool): PNode =
       # sink parameter (bug #11524). Note that the string implementation is
       # different and can deal with 'const string sunk into var'.
       result = passCopyToSink(arg, c)
-    elif arg.kind in {nkBracket, nkObjConstr, nkTupleConstr, nkCharLit..nkTripleStrLit}:
-      discard "object construction to sink parameter: nothing to do"
+    elif arg.kind in {nkBracket, nkObjConstr, nkTupleConstr} + nkLiterals:
+      # object construction to sink parameter: nothing to do
       result = arg
     elif arg.kind == nkSym and isSinkParam(arg.sym):
       # Sinked params can be consumed only once. We need to reset the memory
@@ -507,202 +386,216 @@ proc pArg(arg: PNode; c: var Con; isSink: bool): PNode =
       # it is the last read, can be sinked. We need to reset the memory
       # to disable the destructor which we have not elided
       result = destructiveMoveVar(arg, c)
-    elif arg.kind in {nkBlockExpr, nkBlockStmt}:
-      result = copyNode(arg)
-      result.add arg[0]
-      result.add pArg(arg[1], c, isSink)
-    elif arg.kind == nkStmtListExpr:
-      result = copyNode(arg)
-      for i in 0..arg.len-2:
-        result.add p(arg[i], c)
-      result.add pArg(arg[^1], c, isSink)
-    elif arg.kind in {nkIfExpr, nkIfStmt}:
-      result = copyNode(arg)
-      for i in 0..<arg.len:
-        var branch = copyNode(arg[i])
-        if arg[i].kind in {nkElifBranch, nkElifExpr}:
-          branch.add p(arg[i][0], c)
-          branch.add pArgIfTyped(arg[i][1])
-        else:
-          branch.add pArgIfTyped(arg[i][0])
-        result.add branch
-    elif arg.kind == nkCaseStmt:
-      result = copyNode(arg)
-      result.add p(arg[0], c)
-      for i in 1..<arg.len:
-        var branch: PNode
-        if arg[i].kind == nkOfBranch:
-          branch = arg[i] # of branch conditions are constants
-          branch[^1] = pArgIfTyped(arg[i][^1])
-        elif arg[i].kind in {nkElifBranch, nkElifExpr}:
-          branch = copyNode(arg[i])
-          branch.add p(arg[i][0], c)
-          branch.add pArgIfTyped(arg[i][1])
-        else:
-          branch = copyNode(arg[i])
-          branch.add pArgIfTyped(arg[i][0])
-        result.add branch
+    elif arg.kind in {nkStmtListExpr, nkBlockExpr, nkBlockStmt, nkIfExpr, nkIfStmt, nkCaseStmt}:
+      handleNested(arg): pArg(node, c, isSink)
     else:
       # an object that is not temporary but passed to a 'sink' parameter
       # results in a copy.
       result = passCopyToSink(arg, c)
+  elif arg.kind == nkBracket:
+    # Treat `f([...])` like `f(...)`
+    result = copyNode(arg)
+    for son in arg:
+      result.add pArg(son, c, isSinkTypeForParam(son.typ))
+  elif arg.kind in nkCallKinds and arg.typ != nil and hasDestructor(arg.typ):
+    # produce temp creation
+    result = newNodeIT(nkStmtListExpr, arg.info, arg.typ)
+    let tmp = getTemp(c, arg.typ, arg.info)
+    let res = p(arg, c)
+    var sinkExpr = genSink(c, tmp, res)
+    sinkExpr.add res
+    result.add sinkExpr
+    result.add tmp
+    c.destroys.add genDestroy(c, tmp)
   else:
     result = p(arg, c)
 
+proc p(n: PNode; c: var Con): PNode =
+  case n.kind
+  of nkCallKinds:
+    let parameters = n[0].typ
+    let L = if parameters != nil: parameters.len else: 0
+    for i in 1..<n.len:
+      n[i] = pArg(n[i], c, i < L and isSinkTypeForParam(parameters[i]))
+    result = n
+  of nkDiscardStmt: #Small optimization
+    if n[0].kind != nkEmpty:
+      n[0] = pArg(n[0], c, false)
+    result = n
+  of nkBracket:
+    result = copyTree(n)
+    for i in 0..<n.len:
+      # everything that is passed to an array constructor is consumed,
+      # so these all act like 'sink' parameters:
+      result[i] = pArg(n[i], c, isSink = true)
+  of nkObjConstr:
+    result = copyTree(n)
+    for i in 1..<n.len:
+      # everything that is passed to an object constructor is consumed,
+      # so these all act like 'sink' parameters:
+      result[i][1] = pArg(n[i][1], c, isSink = true)
+  of nkTupleConstr, nkClosure:
+    result = copyTree(n)
+    for i in ord(n.kind == nkClosure)..<n.len:
+      # everything that is passed to an tuple constructor is consumed,
+      # so these all act like 'sink' parameters:
+      if n[i].kind == nkExprColonExpr:
+        result[i][1] = pArg(n[i][1], c, isSink = true)
+      else:
+        result[i] = pArg(n[i], c, isSink = true)
+  of nkVarSection, nkLetSection:
+    # transform; var x = y to  var x; x op y  where op is a move or copy
+    result = newNodeI(nkStmtList, n.info)
+    for it in n:
+      var ri = it[^1]
+      if it.kind == nkVarTuple and hasDestructor(ri.typ):
+        let x = lowerTupleUnpacking(c.graph, it, c.owner)
+        result.add p(x, c)
+      elif it.kind == nkIdentDefs and hasDestructor(it[0].typ):
+        for j in 0..<it.len-2:
+          let v = it[j]
+          if v.kind == nkSym:
+            if sfCompileTime in v.sym.flags: continue
+            # move the variable declaration to the top of the frame:
+            c.addTopVar v
+            # make sure it's destroyed at the end of the proc:
+            if not isUnpackedTuple(it[0].sym):
+              c.destroys.add genDestroy(c, v)
+          if ri.kind == nkEmpty and c.inLoop > 0:
+            ri = genDefaultCall(v.typ, c, v.info)
+          if ri.kind != nkEmpty:
+            let r = moveOrCopy(v, ri, c)
+            result.add r
+      else: # keep the var but transform 'ri':
+        var v = copyNode(n)
+        var itCopy = copyNode(it)
+        for j in 0..<it.len-1:
+          itCopy.add it[j]
+        itCopy.add p(it[^1], c)
+        v.add itCopy
+        result.add v
+  of nkAsgn, nkFastAsgn:
+    if hasDestructor(n[0].typ) and n[1].kind notin {nkProcDef, nkDo, nkLambda}:
+      # rule (self-assignment-removal):
+      if n[1].kind == nkSym and n[0].kind == nkSym and n[0].sym == n[1].sym:
+        result = newNodeI(nkEmpty, n.info)
+      else:
+        result = moveOrCopy(n[0], n[1], c)
+    else:
+      result = copyNode(n)
+      result.add n[0]
+      result.add p(n[1], c)
+  of nkRaiseStmt:
+    if optNimV2 in c.graph.config.globalOptions and n[0].kind != nkEmpty:
+      if n[0].kind in nkCallKinds:
+        let call = p(n[0], c)
+        result = copyNode(n)
+        result.add call
+      else:
+        let tmp = getTemp(c, n[0].typ, n.info)
+        var m = genCopyNoCheck(c, tmp, n[0])
+        m.add p(n[0], c)
+        result = newTree(nkStmtList, genWasMoved(tmp, c), m)
+        var toDisarm = n[0]
+        if toDisarm.kind == nkStmtListExpr: toDisarm = toDisarm.lastSon
+        if toDisarm.kind == nkSym and toDisarm.sym.owner == c.owner:
+          result.add genWasMoved(toDisarm, c)
+        result.add newTree(nkRaiseStmt, tmp)
+    else:
+      result = copyNode(n)
+      result.add p(n[0], c)
+  of nkNone..nkNilLit, nkTypeSection, nkProcDef, nkConverterDef, nkMethodDef,
+      nkIteratorDef, nkMacroDef, nkTemplateDef, nkLambda, nkDo, nkFuncDef,
+      nkConstSection, nkConstDef, nkIncludeStmt, nkImportStmt, nkExportStmt,
+      nkPragma, nkCommentStmt, nkBreakStmt:
+    result = n
+  of nkWhileStmt:
+    result = copyNode(n)
+    inc c.inLoop
+    result.add p(n[0], c)
+    result.add p(n[1], c)
+    dec c.inLoop
+  of nkWhen: # This should be a "when nimvm" node.
+    result = copyTree(n)
+    result[1][0] = p(result[1][0], c)
+  of nkStmtList, nkStmtListExpr, nkBlockStmt, nkBlockExpr, nkIfStmt, nkIfExpr, nkCaseStmt:
+    handleNested(n): p(node, c)
+  else:
+    result = shallowCopy(n)
+    for i in 0..<n.len:
+      result[i] = p(n[i], c)
+
 proc moveOrCopy(dest, ri: PNode; c: var Con): PNode =
   # unfortunately, this needs to be kept consistent with the cases
   # we handle in the 'case of' statement below:
   const movableNodeKinds = (nkCallKinds + {nkSym, nkTupleConstr, nkObjConstr,
                                            nkBracket, nkBracketExpr, nkNilLit})
 
-  template moveOrCopyIfTyped(riPart: PNode): PNode =
-    # typ is nil if we are in if/case expr branch with noreturn
-    if riPart.typ == nil: p(riPart, c)
-    else: moveOrCopy(dest, riPart, c)
-
   case ri.kind
   of nkCallKinds:
-    result = genSink(c, dest.typ, dest, ri)
-    # watch out and no not transform 'ri' twice if it's a call:
-    let ri2 = copyNode(ri)
-    let parameters = ri[0].typ
-    let L = if parameters != nil: parameters.len else: 0
-    ri2.add ri[0]
-    for i in 1..<ri.len:
-      ri2.add pArg(ri[i], c, i < L and isSinkTypeForParam(parameters[i]))
-    #recurse(ri, ri2)
-    result.add ri2
+    result = genSink(c, dest, ri)
+    result.add p(ri, c)
   of nkBracketExpr:
     if ri[0].kind == nkSym and isUnpackedTuple(ri[0].sym):
       # unpacking of tuple: move out the elements
-      result = genSink(c, dest.typ, dest, ri)
+      result = genSink(c, dest, ri)
       result.add p(ri, c)
     elif isAnalysableFieldAccess(ri, c.owner) and isLastRead(ri, c):
       # Rule 3: `=sink`(x, z); wasMoved(z)
-      var snk = genSink(c, dest.typ, dest, ri)
+      var snk = genSink(c, dest, ri)
       snk.add ri
       result = newTree(nkStmtList, snk, genWasMoved(ri, c))
     else:
-      result = genCopy(c, dest.typ, dest, ri)
+      result = genCopy(c, dest, ri)
       result.add p(ri, c)
-  of nkStmtListExpr:
-    result = newNodeI(nkStmtList, ri.info)
-    for i in 0..ri.len-2:
-      result.add p(ri[i], c)
-    result.add moveOrCopy(dest, ri[^1], c)
-  of nkBlockExpr, nkBlockStmt:
-    result = newNodeI(nkBlockStmt, ri.info)
-    result.add ri[0] ## add label
-    result.add moveOrCopy(dest, ri[1], c)
-  of nkIfExpr, nkIfStmt:
-    result = newNodeI(nkIfStmt, ri.info)
-    for i in 0..<ri.len:
-      var branch = copyNode(ri[i])
-      if ri[i].kind in {nkElifBranch, nkElifExpr}:
-        branch.add p(ri[i][0], c)
-        branch.add moveOrCopyIfTyped(ri[i][1])
-      else:
-        branch.add moveOrCopyIfTyped(ri[i][0])
-      result.add branch
-  of nkCaseStmt:
-    result = newNodeI(nkCaseStmt, ri.info)
-    result.add p(ri[0], c)
-    for i in 1..<ri.len:
-      var branch: PNode
-      if ri[i].kind == nkOfBranch:
-        branch = ri[i] # of branch conditions are constants
-        branch[^1] = moveOrCopyIfTyped(ri[i][^1])
-      elif ri[i].kind in {nkElifBranch, nkElifExpr}:
-        branch = copyNode(ri[i])
-        branch.add p(ri[i][0], c)
-        branch.add moveOrCopyIfTyped(ri[i][1])
-      else:
-        branch = copyNode(ri[i])
-        branch.add moveOrCopyIfTyped(ri[i][0])
-      result.add branch
   of nkBracket:
     # array constructor
     if ri.len > 0 and isDangerousSeq(ri.typ):
-      result = genCopy(c, dest.typ, dest, ri)
+      result = genCopy(c, dest, ri)
     else:
-      result = genSink(c, dest.typ, dest, ri)
-    let ri2 = copyTree(ri)
-    for i in 0..<ri.len:
-      # everything that is passed to an array constructor is consumed,
-      # so these all act like 'sink' parameters:
-      ri2[i] = pArg(ri[i], c, isSink = true)
-    result.add ri2
-  of nkObjConstr:
-    result = genSink(c, dest.typ, dest, ri)
-    let ri2 = copyTree(ri)
-    for i in 1..<ri.len:
-      # everything that is passed to an object constructor is consumed,
-      # so these all act like 'sink' parameters:
-      ri2[i].sons[1] = pArg(ri[i][1], c, isSink = true)
-    result.add ri2
-  of nkTupleConstr, nkClosure:
-    result = genSink(c, dest.typ, dest, ri)
-    let ri2 = copyTree(ri)
-    for i in ord(ri.kind == nkClosure)..<ri.len:
-      # everything that is passed to an tuple constructor is consumed,
-      # so these all act like 'sink' parameters:
-      if ri[i].kind == nkExprColonExpr:
-        ri2[i].sons[1] = pArg(ri[i][1], c, isSink = true)
-      else:
-        ri2[i] = pArg(ri[i], c, isSink = true)
-    result.add ri2
-  of nkNilLit:
-    result = genSink(c, dest.typ, dest, ri)
-    result.add ri
+      result = genSink(c, dest, ri)
+    result.add p(ri, c)
+  of nkObjConstr, nkTupleConstr, nkClosure, nkCharLit..nkNilLit:
+    result = genSink(c, dest, ri)
+    result.add p(ri, c)
   of nkSym:
     if isSinkParam(ri.sym):
       # Rule 3: `=sink`(x, z); wasMoved(z)
       sinkParamIsLastReadCheck(c, ri)
-      var snk = genSink(c, dest.typ, dest, ri)
+      var snk = genSink(c, dest, ri)
       snk.add ri
       result = newTree(nkStmtList, snk, genWasMoved(ri, c))
     elif ri.sym.kind != skParam and ri.sym.owner == c.owner and
         isLastRead(ri, c) and canBeMoved(dest.typ):
       # Rule 3: `=sink`(x, z); wasMoved(z)
-      var snk = genSink(c, dest.typ, dest, ri)
+      var snk = genSink(c, dest, ri)
       snk.add ri
       result = newTree(nkStmtList, snk, genWasMoved(ri, c))
     else:
-      result = genCopy(c, dest.typ, dest, ri)
-      result.add p(ri, c)
-  of nkHiddenSubConv, nkHiddenStdConv:
-    if sameType(ri.typ, ri[1].typ):
-      result = moveOrCopy(dest, ri[1], c)
-    elif ri[1].kind in movableNodeKinds:
-      result = moveOrCopy(dest, ri[1], c)
-      var b = newNodeIT(ri.kind, ri.info, ri.typ)
-      b.add ri[0] # add empty node
-      let L = result.len-1
-      b.add result[L]
-      result[L] = b
-    else:
-      result = genCopy(c, dest.typ, dest, ri)
+      result = genCopy(c, dest, ri)
       result.add p(ri, c)
+  of nkHiddenSubConv, nkHiddenStdConv, nkConv:
+    result = moveOrCopy(dest, ri[1], c)
+    if not sameType(ri.typ, ri[1].typ):
+      let copyRi = copyTree(ri)
+      copyRi[1] = result[^1]
+      result[^1] = copyRi
   of nkObjDownConv, nkObjUpConv:
-    if ri[0].kind in movableNodeKinds:
-      result = moveOrCopy(dest, ri[0], c)
-      var b = newNodeIT(ri.kind, ri.info, ri.typ)
-      let L = result.len-1
-      b.add result[L]
-      result[L] = b
-    else:
-      result = genCopy(c, dest.typ, dest, ri)
-      result.add p(ri, c)
+    result = moveOrCopy(dest, ri[0], c)
+    let copyRi = copyTree(ri)
+    copyRi[0] = result[^1]
+    result[^1] = copyRi
+  of nkStmtListExpr, nkBlockExpr, nkIfExpr, nkCaseStmt:
+    handleNested(ri): moveOrCopy(dest, node, c)
   else:
     if isAnalysableFieldAccess(ri, c.owner) and isLastRead(ri, c) and
         canBeMoved(dest.typ):
       # Rule 3: `=sink`(x, z); wasMoved(z)
-      var snk = genSink(c, dest.typ, dest, ri)
+      var snk = genSink(c, dest, ri)
       snk.add ri
       result = newTree(nkStmtList, snk, genWasMoved(ri, c))
     else:
-      # XXX At least string literals can be moved?
-      result = genCopy(c, dest.typ, dest, ri)
+      result = genCopy(c, dest, ri)
       result.add p(ri, c)
 
 proc computeUninit(c: var Con) =
@@ -715,17 +608,14 @@ proc computeUninit(c: var Con) =
 proc injectDefaultCalls(n: PNode, c: var Con) =
   case n.kind
   of nkVarSection, nkLetSection:
-    for i in 0..<n.len:
-      let it = n[i]
-      let L = it.len-1
-      let ri = it[L]
-      if it.kind == nkIdentDefs and ri.kind == nkEmpty:
+    for it in n:
+      if it.kind == nkIdentDefs and it[^1].kind == nkEmpty:
         computeUninit(c)
-        for j in 0..L-2:
+        for j in 0..<it.len-2:
           let v = it[j]
           doAssert v.kind == nkSym
           if c.uninit.contains(v.sym.id):
-            it[L] = genDefaultCall(v.sym.typ, c, v.info)
+            it[^1] = genDefaultCall(v.sym.typ, c, v.info)
             break
   of nkNone..nkNilLit, nkTypeSection, nkProcDef, nkConverterDef, nkMethodDef,
       nkIteratorDef, nkMacroDef, nkTemplateDef, nkLambda, nkDo, nkFuncDef:
@@ -734,130 +624,16 @@ proc injectDefaultCalls(n: PNode, c: var Con) =
     for i in 0..<safeLen(n):
       injectDefaultCalls(n[i], c)
 
-proc isCursor(n: PNode): bool {.inline.} =
-  result = n.kind == nkSym and sfCursor in n.sym.flags
-
-proc keepVar(n, it: PNode, c: var Con): PNode =
-  # keep the var but transform 'ri':
-  result = copyNode(n)
-  var itCopy = copyNode(it)
-  for j in 0..it.len-2:
-    itCopy.add it[j]
-  itCopy.add p(it[it.len-1], c)
-  result.add itCopy
-
-proc p(n: PNode; c: var Con): PNode =
-  case n.kind
-  of nkVarSection, nkLetSection:
-    discard "transform; var x = y to  var x; x op y  where op is a move or copy"
-    result = newNodeI(nkStmtList, n.info)
-
-    for i in 0..<n.len:
-      let it = n[i]
-      let L = it.len
-      var ri = it[L-1]
-      if it.kind == nkVarTuple and hasDestructor(ri.typ):
-        let x = lowerTupleUnpacking(c.graph, it, c.owner)
-        result.add p(x, c)
-      elif it.kind == nkIdentDefs and hasDestructor(it[0].typ) and not isCursor(it[0]):
-        for j in 0..L-3:
-          let v = it[j]
-          if v.kind == nkSym:
-            if sfCompileTime in v.sym.flags: continue
-            # move the variable declaration to the top of the frame:
-            c.addTopVar v
-            # make sure it's destroyed at the end of the proc:
-            if not isUnpackedTuple(it[0].sym):
-              c.destroys.add genDestroy(c, v.typ, v)
-          if ri.kind == nkEmpty and c.inLoop > 0:
-            ri = genDefaultCall(v.typ, c, v.info)
-          if ri.kind != nkEmpty:
-            let r = moveOrCopy(v, ri, c)
-            result.add r
-      else:
-        result.add keepVar(n, it, c)
-  of nkCallKinds:
-    let parameters = n[0].typ
-    let L = if parameters != nil: parameters.len else: 0
-    for i in 1 ..< n.len:
-      n.sons[i] = pArg(n[i], c, i < L and isSinkTypeForParam(parameters[i]))
-    if n.typ != nil and hasDestructor(n.typ):
-      discard "produce temp creation"
-      result = newNodeIT(nkStmtListExpr, n.info, n.typ)
-      let tmp = getTemp(c, n.typ, n.info)
-      var sinkExpr = genSink(c, n.typ, tmp, n)
-      sinkExpr.add n
-      result.add sinkExpr
-      result.add tmp
-      c.destroys.add genDestroy(c, n.typ, tmp)
-    else:
-      result = n
-  of nkAsgn, nkFastAsgn:
-    if hasDestructor(n[0].typ) and n[1].kind notin {nkProcDef, nkDo, nkLambda}:
-      # rule (self-assignment-removal):
-      if n[1].kind == nkSym and n[0].kind == nkSym and n[0].sym == n[1].sym:
-        result = newNodeI(nkEmpty, n.info)
-      else:
-        result = moveOrCopy(n[0], n[1], c)
-    else:
-      result = copyNode(n)
-      recurse(n, result)
-  of nkNone..nkNilLit, nkTypeSection, nkProcDef, nkConverterDef, nkMethodDef,
-      nkIteratorDef, nkMacroDef, nkTemplateDef, nkLambda, nkDo, nkFuncDef:
-    result = n
-  of nkCast, nkHiddenStdConv, nkHiddenSubConv, nkConv:
-    result = copyNode(n)
-    # Destination type
-    result.add n[0]
-    # Analyse the inner expression
-    result.add p(n[1], c)
-  of nkWhen:
-    # This should be a "when nimvm" node.
-    result = copyTree(n)
-    result[1][0] = p(result[1][0], c)
-  of nkRaiseStmt:
-    if optNimV2 in c.graph.config.globalOptions and n[0].kind != nkEmpty:
-      if n[0].kind in nkCallKinds:
-        let call = copyNode(n[0])
-        recurse(n[0], call)
-        result = copyNode(n)
-        result.add call
-      else:
-        let t = n[0].typ
-        let tmp = getTemp(c, t, n.info)
-        var m = genCopyNoCheck(c, t, tmp, n[0])
-
-        m.add p(n[0], c)
-        result = newTree(nkStmtList, genWasMoved(tmp, c), m)
-        var toDisarm = n[0]
-        if toDisarm.kind == nkStmtListExpr: toDisarm = toDisarm.lastSon
-        if toDisarm.kind == nkSym and toDisarm.sym.owner == c.owner:
-          result.add genWasMoved(toDisarm, c)
-        result.add newTree(nkRaiseStmt, tmp)
-    else:
-      result = copyNode(n)
-      recurse(n, result)
-  of nkForStmt, nkParForStmt, nkWhileStmt:
-    inc c.inLoop
-    result = copyNode(n)
-    recurse(n, result)
-    dec c.inLoop
-  else:
-    result = copyNode(n)
-    recurse(n, result)
-
 proc extractDestroysForTemporaries(c: Con, destroys: PNode): PNode =
   result = newNodeI(nkStmtList, destroys.info)
-  for i in 0 ..< destroys.len:
+  for i in 0..<destroys.len:
     if destroys[i][1][0].sym.kind == skTemp:
       result.add destroys[i]
       destroys[i] = c.emptyNode
 
-proc reverseDestroys(destroys: PNode) =
-  var reversed: seq[PNode]
+proc reverseDestroys(destroys: seq[PNode]): seq[PNode] =
   for i in countdown(destroys.len - 1, 0):
-    reversed.add(destroys[i])
-  destroys.sons = reversed
+    result.add destroys[i]
 
 proc injectDestructorCalls*(g: ModuleGraph; owner: PSym; n: PNode): PNode =
   if sfGeneratedOp in owner.flags or isInlineIterator(owner): return n
@@ -874,14 +650,15 @@ proc injectDestructorCalls*(g: ModuleGraph; owner: PSym; n: PNode): PNode =
     if c.g[i].kind in {goto, fork}:
       c.jumpTargets.incl(i+c.g[i].dest)
   dbg:
-    echo "injecting into ", n
+    echo "\n### ", owner.name.s, ":\nCFG:"
     echoCfg(c.g)
+    echo n
   if owner.kind in {skProc, skFunc, skMethod, skIterator, skConverter}:
     let params = owner.typ.n
-    for i in 1 ..< params.len:
-      let param = params[i].sym
-      if isSinkTypeForParam(param.typ) and hasDestructor(param.typ.skipTypes({tySink})):
-        c.destroys.add genDestroy(c, param.typ.skipTypes({tyGenericInst, tyAlias, tySink}), params[i])
+    for i in 1..<params.len:
+      let t = params[i].sym.typ
+      if isSinkTypeForParam(t) and hasDestructor(t.skipTypes({tySink})):
+        c.destroys.add genDestroy(c, params[i])
 
   #if optNimV2 in c.graph.config.globalOptions:
   #  injectDefaultCalls(n, c)
@@ -890,7 +667,7 @@ proc injectDestructorCalls*(g: ModuleGraph; owner: PSym; n: PNode): PNode =
   if c.topLevelVars.len > 0:
     result.add c.topLevelVars
   if c.destroys.len > 0:
-    reverseDestroys(c.destroys)
+    c.destroys.sons = reverseDestroys(c.destroys.sons)
     if owner.kind == skModule:
       result.add newTryFinally(body, extractDestroysForTemporaries(c, c.destroys))
       g.globalDestructors.add c.destroys
@@ -898,8 +675,6 @@ proc injectDestructorCalls*(g: ModuleGraph; owner: PSym; n: PNode): PNode =
       result.add newTryFinally(body, c.destroys)
   else:
     result.add body
-
   dbg:
-    echo "------------------------------------"
-    echo owner.name.s, " transformed to: "
+    echo ">---------transformed-to--------->"
     echo result
diff --git a/compiler/pragmas.nim b/compiler/pragmas.nim
index f8f6a1a23..208f6dae3 100644
--- a/compiler/pragmas.nim
+++ b/compiler/pragmas.nim
@@ -66,7 +66,7 @@ const
   varPragmas* = declPragmas + {wVolatile, wRegister, wThreadVar,
     wMagic, wHeader, wCompilerProc, wCore, wDynlib,
     wNoInit, wCompileTime, wGlobal,
-    wGensym, wInject, wCodegenDecl, wGuard, wGoto, wCursor}
+    wGensym, wInject, wCodegenDecl, wGuard, wGoto}
   constPragmas* = declPragmas + {wHeader, wMagic,
     wGensym, wInject,
     wIntDefine, wStrDefine, wBoolDefine, wCompilerProc, wCore}
@@ -1103,11 +1103,6 @@ proc singlePragma(c: PContext, sym: PSym, n: PNode, i: var int,
           invalidPragma(c, it)
         else:
           sym.flags.incl sfGoto
-      of wCursor:
-        if sym == nil or sym.kind notin {skVar, skLet}:
-          invalidPragma(c, it)
-        else:
-          sym.flags.incl sfCursor
       of wExportNims:
         if sym == nil: invalidPragma(c, it)
         else: magicsys.registerNimScriptSymbol(c.graph, sym)
diff --git a/compiler/transf.nim b/compiler/transf.nim
index 7521fe169..afbe41950 100644
--- a/compiler/transf.nim
+++ b/compiler/transf.nim
@@ -663,7 +663,6 @@ proc transformFor(c: PTransf, n: PNode): PTransNode =
         t = arg.typ
       # generate a temporary and produce an assignment statement:
       var temp = newTemp(c, t, formal.info)
-      #temp.sym.flags.incl sfCursor
       addVar(v, temp)
       add(stmtList, newAsgnStmt(c, nkFastAsgn, temp, arg.PTransNode))
       idNodeTablePut(newC.mapping, formal, temp)
diff --git a/compiler/wordrecg.nim b/compiler/wordrecg.nim
index 22715958f..4650f22ec 100644
--- a/compiler/wordrecg.nim
+++ b/compiler/wordrecg.nim
@@ -37,8 +37,6 @@ type
     wMagic, wThread, wFinal, wProfiler, wMemTracker, wObjChecks,
     wIntDefine, wStrDefine, wBoolDefine
 
-    wCursor,
-
     wImmediate, wConstructor, wDestructor, wDelegator, wOverride,
     wImportCpp, wImportObjC,
     wImportCompilerProc,
@@ -125,8 +123,6 @@ const
     "magic", "thread", "final", "profiler", "memtracker", "objchecks",
     "intdefine", "strdefine", "booldefine",
 
-    "cursor",
-
     "immediate", "constructor", "destructor", "delegator", "override",
     "importcpp", "importobjc",
     "importcompilerproc", "importc", "importjs", "exportc", "exportcpp", "exportnims",
diff --git a/doc/nimc.rst b/doc/nimc.rst
index ebdd00063..d53e3f310 100644
--- a/doc/nimc.rst
+++ b/doc/nimc.rst
@@ -483,7 +483,6 @@ number information are given if the program crashes or an uncaught exception
 is raised.
 
 
-
 DynlibOverride
 ==============
 
@@ -497,14 +496,6 @@ on Linux::
   nim c --dynlibOverride:lua --passL:liblua.lib program.nim
 
 
-Cursor pragma
-=============
-
-The ``.cursor`` pragma is a temporary tool for optimization purposes
-and this property will be computed by Nim's optimizer eventually. Thus it
-remains undocumented.
-
-
 Backend language options
 ========================