arc optimizations (#13325)

* scope based destructors
* handle 'or' and 'and' expressions properly, see the new test arc/tcontrolflow.nim
* make this branch mergable, logic is disabled for now

11 files changed, 312 insertions, 95 deletions

diff --git a/compiler/ccgexprs.nim b/compiler/ccgexprs.nim
index 4aad1a4f8..4cf6f36db 100644
--- a/compiler/ccgexprs.nim
+++ b/compiler/ccgexprs.nim
@@ -2573,6 +2573,10 @@ proc expr(p: BProc, n: PNode, d: var TLoc) =
       else:
         putLocIntoDest(p, d, sym.loc)
     of skTemp:
+      if sym.loc.r == nil:
+        # we now support undeclared 'skTemp' variables for easier
+        # transformations in other parts of the compiler:
+        assignLocalVar(p, n)
       if sym.loc.r == nil or sym.loc.t == nil:
         #echo "FAILED FOR PRCO ", p.prc.name.s
         #echo renderTree(p.prc.ast, {renderIds})
diff --git a/compiler/cgmeth.nim b/compiler/cgmeth.nim
index a71ac6ea7..edf9db383 100644
--- a/compiler/cgmeth.nim
+++ b/compiler/cgmeth.nim
@@ -7,7 +7,7 @@
 #    distribution, for details about the copyright.
 #
 
-## This module implements code generation for multi methods.
+## This module implements code generation for methods.
 
 import
   intsets, options, ast, msgs, idents, renderer, types, magicsys,
diff --git a/compiler/closureiters.nim b/compiler/closureiters.nim
index ef08a7bae..3422a8a46 100644
--- a/compiler/closureiters.nim
+++ b/compiler/closureiters.nim
@@ -154,8 +154,8 @@ type
                     # is their finally. For finally it is parent finally. Otherwise -1
 
 const
-  nkSkip = { nkEmpty..nkNilLit, nkTemplateDef, nkTypeSection, nkStaticStmt,
-            nkCommentStmt } + procDefs
+  nkSkip = {nkEmpty..nkNilLit, nkTemplateDef, nkTypeSection, nkStaticStmt,
+            nkCommentStmt} + procDefs
 
 proc newStateAccess(ctx: var Ctx): PNode =
   if ctx.stateVarSym.isNil:
diff --git a/compiler/injectdestructors.nim b/compiler/injectdestructors.nim
index 5e51aeb9d..55886540c 100644
--- a/compiler/injectdestructors.nim
+++ b/compiler/injectdestructors.nim
@@ -13,6 +13,13 @@
 
 ## See doc/destructors.rst for a spec of the implemented rewrite rules
 
+## XXX Optimization to implement: if a local variable is only assigned
+## string literals as in ``let x = conf: "foo" else: "bar"`` do not
+## produce a destructor call for ``x``. The address of ``x`` must also
+## not have been taken. ``x = "abc"; x.add(...)``
+
+# Todo:
+# - eliminate 'wasMoved(x); destroy(x)' pairs as a post processing step.
 
 import
   intsets, ast, msgs, renderer, magicsys, types, idents,
@@ -20,6 +27,10 @@ import
   lineinfos, parampatterns, sighashes
 
 from trees import exprStructuralEquivalent
+from algorithm import reverse
+
+const
+  scopeBasedDestruction = false
 
 type
   Con = object
@@ -27,10 +38,13 @@ type
     g: ControlFlowGraph
     jumpTargets: IntSet
     destroys, topLevelVars: PNode
+    scopeDestroys: seq[PNode] # used as a stack that pop from
+                              # at strategic places which try to
+                              # mimic the natural scope.
     graph: ModuleGraph
     emptyNode: PNode
     otherRead: PNode
-    inLoop: int
+    inLoop, hasUnstructuredCf, inDangerousBranch: int
     declaredVars: IntSet # variables we already moved to the top level
     uninit: IntSet # set of uninit'ed vars
     uninitComputed: bool
@@ -285,7 +299,6 @@ proc getTemp(c: var Con; typ: PType; info: TLineInfo): PNode =
   let sym = newSym(skTemp, getIdent(c.graph.cache, ":tmpD"), c.owner, info)
   sym.typ = typ
   result = newSymNode(sym)
-  c.addTopVar(result)
 
 proc genWasMoved(n: PNode; c: var Con): PNode =
   result = newNodeI(nkCall, n.info)
@@ -339,10 +352,10 @@ proc isClosureEnv(n: PNode): bool = n.kind == nkSym and n.sym.name.s[0] == ':'
 proc passCopyToSink(n: PNode; c: var Con): PNode =
   result = newNodeIT(nkStmtListExpr, n.info, n.typ)
   let tmp = getTemp(c, n.typ, n.info)
-  # XXX This is only required if we are in a loop. Since we move temporaries
-  # out of loops we need to mark it as 'wasMoved'.
-  result.add genWasMoved(tmp, c)
+  when not scopeBasedDestruction:
+    c.addTopVar(tmp)
   if hasDestructor(n.typ):
+    result.add genWasMoved(tmp, c)
     var m = genCopy(c, tmp, n)
     m.add p(n, c, normal)
     result.add m
@@ -354,6 +367,8 @@ proc passCopyToSink(n: PNode; c: var Con): PNode =
     if c.graph.config.selectedGC in {gcArc, gcOrc}:
       assert(not containsGarbageCollectedRef(n.typ))
     result.add newTree(nkAsgn, tmp, p(n, c, normal))
+  # Since we know somebody will take over the produced copy, there is
+  # no need to destroy it.
   result.add tmp
 
 proc isDangerousSeq(t: PType): bool {.inline.} =
@@ -375,60 +390,137 @@ proc containsConstSeq(n: PNode): bool =
       if containsConstSeq(son): return true
   else: discard
 
-template handleNested(n: untyped, processCall: untyped) =
+proc handleTmpDestroys(c: var Con; body: PNode; t: PType;
+                       oldHasUnstructuredCf, oldTmpDestroysLen: int) =
+  if c.hasUnstructuredCf == oldHasUnstructuredCf:
+    # no need for a try-finally statement:
+    if body.kind == nkStmtList:
+      for i in countdown(c.scopeDestroys.high, oldTmpDestroysLen):
+        body.add c.scopeDestroys[i]
+    elif isEmptyType(t):
+      var n = newNodeI(nkStmtList, body.info)
+      n.add body[^1]
+      for i in countdown(c.scopeDestroys.high, oldTmpDestroysLen):
+        n.add c.scopeDestroys[i]
+      body[^1] = n
+    elif body.kind == nkStmtListExpr and body.len > 0 and body[^1].kind == nkSym:
+      # special case: Do not translate (x; y; sym) into
+      # (x; y; tmp = sym; destroy(x); destroy(y); tmp )
+      # but into
+      # (x; y; destroy(x); destroy(y); sym )
+      let sym = body[^1]
+      body[^1] = c.scopeDestroys[^1]
+      for i in countdown(c.scopeDestroys.high - 1, oldTmpDestroysLen):
+        body.add c.scopeDestroys[i]
+      body.add sym
+    else:
+      # fun ahead: We have to transform (x; y; E()) into
+      # (x; y; tmp = E(); destroy(x); destroy(y); tmp )
+      let t2 = body[^1].typ
+      let tmp = getTemp(c, t2, body.info)
+      when not scopeBasedDestruction:
+        c.addTopVar(tmp)
+      # the tmp does not have to be initialized
+      var n = newNodeIT(nkStmtListExpr, body.info, t2)
+      n.add newTree(nkFastAsgn, tmp, body[^1])
+      for i in countdown(c.scopeDestroys.high, oldTmpDestroysLen):
+        n.add c.scopeDestroys[i]
+      n.add tmp
+      body[^1] = n
+      #c.scopeDestroys.add genDestroy(c, tmp)
+  else:
+    # unstructured control flow was used, use a 'try finally' to ensure
+    # destruction:
+    if isEmptyType(t):
+      var n = newNodeI(nkStmtList, body.info)
+      for i in countdown(c.scopeDestroys.high, oldTmpDestroysLen):
+        n.add c.scopeDestroys[i]
+      body[^1] = newTryFinally(body[^1], n)
+    else:
+      # fun ahead: We have to transform (x; y; E()) into
+      # ((try: tmp = (x; y; E()); finally: destroy(x); destroy(y)); tmp )
+      let t2 = body[^1].typ
+      let tmp = getTemp(c, t2, body.info)
+      when not scopeBasedDestruction:
+        c.addTopVar(tmp)
+      # the tmp does not have to be initialized
+      var fin = newNodeI(nkStmtList, body.info)
+      for i in countdown(c.scopeDestroys.high, oldTmpDestroysLen):
+        fin.add c.scopeDestroys[i]
+      var n = newNodeIT(nkStmtListExpr, body.info, t2)
+      n.add newTryFinally(newTree(nkFastAsgn, tmp, body[^1]), fin)
+      n.add tmp
+      body[^1] = n
+      #c.scopeDestroys.add genDestroy(c, tmp)
+
+  c.scopeDestroys.setLen oldTmpDestroysLen
+
+proc handleNested(n, dest: PNode; c: var Con; mode: ProcessMode): PNode =
+  template processCall(node: PNode): PNode =
+    if node.typ == nil or dest == nil:
+      p(node, c, mode)
+    else:
+      moveOrCopy(dest, node, c)
+
+  proc handleScope(n, dest: PNode; t: PType;
+                   takeOver: Natural; c: var Con; mode: ProcessMode): PNode =
+    let oldHasUnstructuredCf = c.hasUnstructuredCf
+    let oldTmpDestroysLen = c.scopeDestroys.len
+    result = shallowCopy(n)
+    for i in 0..<takeOver:
+      result[i] = n[i]
+    let last = n.len - 1
+    for i in takeOver..<last:
+      result[i] = p(n[i], c, normal)
+
+    # if we have an expression producing a temporary, we must
+    # not destroy it too early:
+    if isEmptyType(t):
+      result[last] = processCall(n[last])
+      if c.scopeDestroys.len > oldTmpDestroysLen:
+        handleTmpDestroys(c, result, t, oldHasUnstructuredCf, oldTmpDestroysLen)
+    else:
+      setLen(result.sons, last)
+      if c.scopeDestroys.len > oldTmpDestroysLen:
+        handleTmpDestroys(c, result, t, oldHasUnstructuredCf, oldTmpDestroysLen)
+      if result.kind != nkFinally:
+        result.add processCall(n[last])
+      else:
+        result = newTree(nkStmtListExpr, result, processCall(n[last]))
+        result.typ = t
+
   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, normal)
-    template node: untyped = n[^1]
-    result.add processCall
+    result = shallowCopy(n)
+    let last = n.len - 1
+    for i in 0..<last:
+      result[i] = p(n[i], c, normal)
+    result[last] = processCall(n[last])
+    # A statement list does not introduce a scope, the AST can
+    # contain silly nested statement lists.
+    #result = handleScope(n, dest, n.typ, 0, c, mode)
   of nkBlockStmt, nkBlockExpr:
-    result = copyNode(n)
-    result.add n[0]
-    template node: untyped = n[1]
-    result.add processCall
+    result = handleScope(n, dest, n.typ, 1, c, mode)
   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, normal) #The condition
-        branch.add if node.typ == nil: p(node, c, normal) #noreturn
-                   else: processCall
-      else:
-        template node: untyped = son[0]
-        branch.add if node.typ == nil: p(node, c, normal) #noreturn
-                   else: processCall
-      result.add branch
+      result.add handleScope(son, dest, son[^1].typ, 0, c, mode)
   of nkCaseStmt:
     result = copyNode(n)
     result.add p(n[0], c, normal)
     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, normal) #noreturn
-                     else: processCall
-      elif n[i].kind in {nkElifBranch, nkElifExpr}:
-        branch = copyNode(n[i])
-        branch.add p(n[i][0], c, normal) #The condition
-        template node: untyped = n[i][1]
-        branch.add if node.typ == nil: p(node, c, normal) #noreturn
-                   else: processCall
-      else:
-        branch = copyNode(n[i])
-        template node: untyped = n[i][0]
-        branch.add if node.typ == nil: p(node, c, normal) #noreturn
-                   else: processCall
-      result.add branch
+      result.add handleScope(n[i], dest, n[i][^1].typ, n[i].len - 1, c, mode)
   of nkWhen: # This should be a "when nimvm" node.
     result = copyTree(n)
-    template node: untyped = n[1][0]
-    result[1][0] = processCall
+    result[1][0] = handleScope(n[1][0], dest, n[1][0][^1].typ, 0, c, mode)
+  of nkWhileStmt:
+    #result = copyNode(n)
+    inc c.inLoop
+    result = handleScope(n, dest, nil, 0, c, mode)
+    #result.add p(n[0], c, normal)
+    #result.add p(n[1], c, normal)
+    dec c.inLoop
   else: assert(false)
 
 proc ensureDestruction(arg: PNode; c: var Con): PNode =
@@ -439,9 +531,23 @@ proc ensureDestruction(arg: PNode; c: var Con): PNode =
     # This was already done in the sink parameter handling logic.
     result = newNodeIT(nkStmtListExpr, arg.info, arg.typ)
     let tmp = getTemp(c, arg.typ, arg.info)
-    result.add genSink(c, tmp, arg)
-    result.add tmp
-    c.destroys.add genDestroy(c, tmp)
+    when not scopeBasedDestruction:
+      c.addTopVar(tmp)
+      result.add genSink(c, tmp, arg)
+      result.add tmp
+      c.destroys.add genDestroy(c, tmp)
+    else:
+      # if we're inside a dangerous 'or' or 'and' expression, we
+      # do need to initialize it. 'elif' is not among this problem
+      # as we have a separate scope for 'elif' to attach the destructors to.
+      if c.inDangerousBranch == 0 and c.hasUnstructuredCf == 0:
+        tmp.sym.flags.incl sfNoInit
+      c.addTopVar(tmp)
+      # since we do not initialize these temporaries anymore, we
+      # use raw assignments instead of =sink:
+      result.add newTree(nkFastAsgn, tmp, arg)
+      result.add tmp
+      c.scopeDestroys.add genDestroy(c, tmp)
   else:
     result = arg
 
@@ -486,10 +592,49 @@ proc cycleCheck(n: PNode; c: var Con) =
       message(c.graph.config, n.info, warnCycleCreated, msg)
       break
 
+proc pVarTopLevel(v: PNode; c: var Con; ri, res: PNode) =
+  # move the variable declaration to the top of the frame:
+  if not containsOrIncl(c.declaredVars, v.sym.id):
+    c.addTopVar v
+  if isUnpackedTuple(v):
+    if c.inLoop > 0:
+      # unpacked tuple needs reset at every loop iteration
+      res.add newTree(nkFastAsgn, v, genDefaultCall(v.typ, c, v.info))
+  elif sfThread notin v.sym.flags:
+    # do not destroy thread vars for now at all for consistency.
+    c.destroys.add genDestroy(c, v)
+  if ri.kind == nkEmpty and c.inLoop > 0:
+    res.add moveOrCopy(v, genDefaultCall(v.typ, c, v.info), c)
+  elif ri.kind != nkEmpty:
+    res.add moveOrCopy(v, ri, c)
+
+proc pVarScoped(v: PNode; c: var Con; ri, res: PNode) =
+  if not containsOrIncl(c.declaredVars, v.sym.id):
+    c.addTopVar(v)
+  if isUnpackedTuple(v):
+    if c.inLoop > 0:
+      # unpacked tuple needs reset at every loop iteration
+      res.add newTree(nkFastAsgn, v, genDefaultCall(v.typ, c, v.info))
+  elif {sfGlobal, sfThread} * v.sym.flags == {sfGlobal}:
+    c.destroys.add genDestroy(c, v)
+  else:
+    # We always translate 'var v = f()' into bitcopies. If 'v' is in a loop,
+    # the destruction at the loop end will free the resources. Other assignments
+    # will destroy the old value inside 'v'. If we have 'var v' without an initial
+    # default value we translate it into 'var v = default()'. We translate
+    # 'var x = someGlobal' into 'var v = default(); `=`(v, someGlobal). The
+    # lack of copy constructors is really beginning to hurt us. :-(
+    #if c.inDangerousBranch == 0: v.sym.flags.incl sfNoInit
+    c.scopeDestroys.add genDestroy(c, v)
+  if ri.kind == nkEmpty and c.inLoop > 0:
+    res.add moveOrCopy(v, genDefaultCall(v.typ, c, v.info), c)
+  elif ri.kind != nkEmpty:
+    res.add moveOrCopy(v, ri, c)
+
 proc p(n: PNode; c: var Con; mode: ProcessMode): PNode =
   if n.kind in {nkStmtList, nkStmtListExpr, nkBlockStmt, nkBlockExpr, nkIfStmt,
-                nkIfExpr, nkCaseStmt, nkWhen}:
-    handleNested(n): p(node, c, mode)
+                nkIfExpr, nkCaseStmt, nkWhen, nkWhileStmt}:
+    result = handleNested(n, nil, c, mode)
   elif mode == sinkArg:
     if n.containsConstSeq:
       # const sequences are not mutable and so we need to pass a copy to the
@@ -522,6 +667,9 @@ proc p(n: PNode; c: var Con; mode: ProcessMode): PNode =
     elif n.kind in {nkObjDownConv, nkObjUpConv}:
       result = copyTree(n)
       result[0] = p(n[0], c, sinkArg)
+    elif n.typ == nil:
+      # 'raise X' can be part of a 'case' expression. Deal with it here:
+      result = p(n, c, normal)
     else:
       # copy objects that are not temporary but passed to a 'sink' parameter
       result = passCopyToSink(n, c)
@@ -554,12 +702,22 @@ proc p(n: PNode; c: var Con; mode: ProcessMode): PNode =
     of nkCallKinds:
       let parameters = n[0].typ
       let L = if parameters != nil: parameters.len else: 0
+
+      var isDangerous = false
+      if n[0].kind == nkSym and n[0].sym.magic in {mOr, mAnd}:
+        inc c.inDangerousBranch
+        isDangerous = true
+
       result = shallowCopy(n)
       for i in 1..<n.len:
         if i < L and isSinkTypeForParam(parameters[i]):
           result[i] = p(n[i], c, sinkArg)
         else:
           result[i] = p(n[i], c, normal)
+
+      if isDangerous:
+        dec c.inDangerousBranch
+
       if n[0].kind == nkSym and n[0].sym.magic in {mNew, mNewFinalize}:
         result[0] = copyTree(n[0])
         if c.graph.config.selectedGC in {gcHooks, gcArc, gcOrc}:
@@ -567,7 +725,8 @@ proc p(n: PNode; c: var Con; mode: ProcessMode): PNode =
           result = newTree(nkStmtList, destroyOld, result)
       else:
         result[0] = p(n[0], c, normal)
-
+      when scopeBasedDestruction:
+        if canRaise(n[0]): inc c.hasUnstructuredCf
       if mode == normal:
         result = ensureDestruction(result, c)
     of nkDiscardStmt: # Small optimization
@@ -589,20 +748,15 @@ proc p(n: PNode; c: var Con; mode: ProcessMode): PNode =
             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:
-              if not containsOrIncl(c.declaredVars, v.sym.id):
-                c.addTopVar v
-              # make sure it's destroyed at the end of the proc:
-              if not isUnpackedTuple(v) and sfThread notin v.sym.flags:
-                # do not destroy thread vars for now at all for consistency.
-                c.destroys.add genDestroy(c, v)
-              elif c.inLoop > 0:
-                # unpacked tuple needs reset at every loop iteration
-                result.add newTree(nkFastAsgn, v, genDefaultCall(v.typ, c, v.info))
-            if ri.kind == nkEmpty and c.inLoop > 0:
-              ri = genDefaultCall(v.typ, c, v.info)
-            if ri.kind != nkEmpty:
-              result.add moveOrCopy(v, ri, c)
+              when not scopeBasedDestruction:
+                pVarTopLevel(v, c, ri, result)
+              else:
+                pVarScoped(v, c, ri, result)
+            else:
+              if ri.kind == nkEmpty and c.inLoop > 0:
+                ri = genDefaultCall(v.typ, c, v.info)
+              if ri.kind != nkEmpty:
+                result.add moveOrCopy(v, ri, c)
         else: # keep the var but transform 'ri':
           var v = copyNode(n)
           var itCopy = copyNode(it)
@@ -634,6 +788,7 @@ proc p(n: PNode; c: var Con; mode: ProcessMode): PNode =
           result.add call
         else:
           let tmp = getTemp(c, n[0].typ, n.info)
+          c.addTopVar(tmp)
           var m = genCopyNoCheck(c, tmp, n[0])
           m.add p(n[0], c, normal)
           result = newTree(nkStmtList, genWasMoved(tmp, c), m)
@@ -648,17 +803,22 @@ proc p(n: PNode; c: var Con; mode: ProcessMode): PNode =
           result.add p(n[0], c, sinkArg)
         else:
           result.add copyNode(n[0])
+      inc c.hasUnstructuredCf
     of nkWhileStmt:
-      result = copyNode(n)
-      inc c.inLoop
-      result.add p(n[0], c, normal)
-      result.add p(n[1], c, normal)
-      dec c.inLoop
+      result = handleNested(n, nil, c, mode)
     of nkNone..nkNilLit, nkTypeSection, nkProcDef, nkConverterDef,
        nkMethodDef, nkIteratorDef, nkMacroDef, nkTemplateDef, nkLambda, nkDo,
        nkFuncDef, nkConstSection, nkConstDef, nkIncludeStmt, nkImportStmt,
-       nkExportStmt, nkPragma, nkCommentStmt, nkBreakStmt, nkBreakState:
+       nkExportStmt, nkPragma, nkCommentStmt, nkBreakState:
+      result = n
+    of nkBreakStmt:
+      inc c.hasUnstructuredCf
       result = n
+    of nkReturnStmt:
+      result = shallowCopy(n)
+      for i in 0..<n.len:
+        result[i] = p(n[i], c, mode)
+      inc c.hasUnstructuredCf
     else:
       result = shallowCopy(n)
       for i in 0..<n.len:
@@ -721,7 +881,7 @@ proc moveOrCopy(dest, ri: PNode; c: var Con): PNode =
     else:
       result = genSink(c, dest, p(ri, c, sinkArg))
   of nkStmtListExpr, nkBlockExpr, nkIfExpr, nkCaseStmt:
-    handleNested(ri): moveOrCopy(dest, node, c)
+    result = handleNested(ri, dest, c, normal)
   else:
     if isAnalysableFieldAccess(ri, c.owner) and isLastRead(ri, c) and
         canBeMoved(c, dest.typ):
@@ -765,10 +925,6 @@ proc extractDestroysForTemporaries(c: Con, destroys: PNode): PNode =
       result.add destroys[i]
       destroys[i] = c.emptyNode
 
-proc reverseDestroys(destroys: seq[PNode]): seq[PNode] =
-  for i in countdown(destroys.len - 1, 0):
-    result.add destroys[i]
-
 proc injectDestructorCalls*(g: ModuleGraph; owner: PSym; n: PNode): PNode =
   if sfGeneratedOp in owner.flags or (owner.kind == skIterator and isInlineIterator(owner.typ)):
     return n
@@ -801,13 +957,16 @@ proc injectDestructorCalls*(g: ModuleGraph; owner: PSym; n: PNode): PNode =
   result = newNodeI(nkStmtList, n.info)
   if c.topLevelVars.len > 0:
     result.add c.topLevelVars
-  if c.destroys.len > 0:
-    c.destroys.sons = reverseDestroys(c.destroys.sons)
+  if c.destroys.len > 0 or c.scopeDestroys.len > 0:
+    reverse c.destroys.sons
+    var fin: PNode
     if owner.kind == skModule:
-      result.add newTryFinally(body, extractDestroysForTemporaries(c, c.destroys))
+      fin = newTryFinally(body, extractDestroysForTemporaries(c, c.destroys))
       g.globalDestructors.add c.destroys
     else:
-      result.add newTryFinally(body, c.destroys)
+      fin = newTryFinally(body, c.destroys)
+    for i in countdown(c.scopeDestroys.high, 0): fin[1][0].add c.scopeDestroys[i]
+    result.add fin
   else:
     result.add body
   dbg:
diff --git a/doc/destructors.rst b/doc/destructors.rst
index 6d3ed00dd..0d4a6c985 100644
--- a/doc/destructors.rst
+++ b/doc/destructors.rst
@@ -451,6 +451,7 @@ for expressions of type ``lent T`` or of type ``var T``.
     result = Tree(kids: kids)
     # converted into:
     `=sink`(result.kids, kids); wasMoved(kids)
+    `=destroy`(kids)
 
   proc `[]`*(x: Tree; i: int): lent Tree =
     result = x.kids[i]
diff --git a/lib/system.nim b/lib/system.nim
index 602fbc1f1..cd941b19f 100644
--- a/lib/system.nim
+++ b/lib/system.nim
@@ -1954,14 +1954,7 @@ template newException*(exceptn: typedesc, message: string;
                        parentException: ref Exception = nil): untyped =
   ## Creates an exception object of type ``exceptn`` and sets its ``msg`` field
   ## to `message`. Returns the new exception object.
-  when declared(owned):
-    var e: owned(ref exceptn)
-  else:
-    var e: ref exceptn
-  new(e)
-  e.msg = message
-  e.parent = parentException
-  e
+  (ref exceptn)(msg: message, parent: parentException)
 
 when hostOS == "standalone" and defined(nogc):
   proc nimToCStringConv(s: NimString): cstring {.compilerproc, inline.} =
diff --git a/tests/arc/tcontrolflow.nim b/tests/arc/tcontrolflow.nim
new file mode 100644
index 000000000..8fc9bf61d
--- /dev/null
+++ b/tests/arc/tcontrolflow.nim
@@ -0,0 +1,55 @@
+discard """
+  output: '''begin A
+elif
+destroyed
+end A
+begin false
+if
+destroyed
+end false
+begin true
+if
+end true
+'''
+  cmd: "nim c --gc:arc -d:danger $file"
+  disabled: "true"
+"""
+# we use the -d:danger switch to detect uninitialized stack
+# slots more reliably (there shouldn't be any, of course).
+
+# XXX Enable once scope based destruction works!
+
+type
+  Foo = object
+    id: int
+
+proc `=destroy`(x: var Foo) =
+  if x.id != 0:
+    echo "destroyed"
+    x.id = 0
+
+proc construct(): Foo = Foo(id: 3)
+
+proc elifIsEasy(cond: bool) =
+  echo "begin A"
+  if cond:
+    echo "if"
+  elif construct().id == 3:
+    echo "elif"
+  else:
+    echo "else"
+  echo "end A"
+
+elifIsEasy(false)
+
+
+proc orIsHard(cond: bool) =
+  echo "begin ", cond
+  if cond or construct().id == 3:
+    echo "if"
+  else:
+    echo "else"
+  echo "end ", cond
+
+orIsHard(false)
+orIsHard(true)
diff --git a/tests/destructor/tasync_prototype.nim b/tests/destructor/tasync_prototype.nim
index bd80adf0c..81fd824e9 100644
--- a/tests/destructor/tasync_prototype.nim
+++ b/tests/destructor/tasync_prototype.nim
@@ -32,7 +32,12 @@ proc serve(server: PAsyncHttpServer): PFutureBase =
       yield acceptAddrFut
       var fut = acceptAddrFut.value
 
+      # with the new scope based destruction, this cannot
+      # possibly work:
       var f {.cursor.} = processClient()
+      # It also seems to be the wrong way how to avoid the
+      # cycle. The cycle is caused by capturing the 'env'
+      # part from 'env.f'.
       when true:
         f.callback =
           proc () =
diff --git a/tests/destructor/tdestructor.nim b/tests/destructor/tdestructor.nim
index 5cfecea4e..b6d60323c 100644
--- a/tests/destructor/tdestructor.nim
+++ b/tests/destructor/tdestructor.nim
@@ -33,7 +33,7 @@ type
     p: pointer
 
 proc `=destroy`(o: var TMyObj) =
-  if o.p != nil: 
+  if o.p != nil:
     dealloc o.p
     o.p = nil
     echo "myobj destroyed"
diff --git a/tests/destructor/tmisc_destructors.nim b/tests/destructor/tmisc_destructors.nim
index 53c67e34b..73c54eab3 100644
--- a/tests/destructor/tmisc_destructors.nim
+++ b/tests/destructor/tmisc_destructors.nim
@@ -22,7 +22,7 @@ proc `=`(dest: var Foo, src: Foo) =
   assign_counter.inc
 
 proc test(): auto =
-  var a,b : Foo
+  var a, b: Foo
   return (a, b, Foo(boo: 5))
 
 var (a, b, _) = test()
diff --git a/tests/destructor/tv2_raise.nim b/tests/destructor/tv2_raise.nim
index 828d0a396..4ea94a864 100644
--- a/tests/destructor/tv2_raise.nim
+++ b/tests/destructor/tv2_raise.nim
@@ -2,7 +2,7 @@ discard """
   valgrind: true
   cmd: '''nim c -d:nimAllocStats --newruntime $file'''
   output: '''OK 3
-(allocCount: 8, deallocCount: 3)'''
+(allocCount: 8, deallocCount: 5)'''
 """
 
 import strutils, math