From 5fa96ef270a5137510d9a21e91013cea70f4c63f Mon Sep 17 00:00:00 2001
From: Yuriy Glukhov <yglukhov@users.noreply.github.com>
Date: Fri, 18 Oct 2024 10:36:41 +0200
Subject: [PATCH] Fixes #3824, fixes #19154, and hopefully #24094. Re-applies
 #23787. (#24316)

The first commit reverts the revert of #23787.
The second fixes lambdalifting in convolutedly nested
closures/closureiters. This is considered to be the reason of #24094,
though I can't tell for sure, as I was not able to reproduce #24094 for
complicated but irrelevant reasons. Therefore I ask @jmgomez, @metagn or
anyone who could reproduce it to try it again with this PR.

I would suggest this PR to not be squashed if possible, as the history
is already messy enough.

Some theory behind the lambdalifting fix:
- A closureiter that captures anything outside its body will always have
`:up` in its env. This property is now used as a trigger to lift any
proc that captures such a closureiter.
- Instantiating a closureiter involves filling out its `:up`, which was
previously done incorrectly. The fixed algorithm is to use "current" env
if it is the owner of the iter declaration, or traverse through `:up`s
of env param until the common ancestor is found.

---------

Co-authored-by: Andreas Rumpf <rumpf_a@web.de>
---
 compiler/closureiters.nim                     | 126 +++-------------
 compiler/lambdalifting.nim                    |  76 +++++-----
 compiler/transf.nim                           |  16 +-
 tests/destructor/tuse_ownedref_after_move.nim |   2 +-
 tests/iter/tnestedclosures.nim                | 139 ++++++++++++++++++
 tests/iter/tyieldintry.nim                    |   4 +-
 6 files changed, 204 insertions(+), 159 deletions(-)
 create mode 100644 tests/iter/tnestedclosures.nim

diff --git a/compiler/closureiters.nim b/compiler/closureiters.nim
index 8bdd04ca78..9ee394111b 100644
--- a/compiler/closureiters.nim
+++ b/compiler/closureiters.nim
@@ -161,10 +161,6 @@ type
                     # is their finally. For finally it is parent finally. Otherwise -1
     idgen: IdGenerator
     varStates: Table[ItemId, int] # Used to detect if local variable belongs to multiple states
-    stateVarSym: PSym # :state variable. nil if env already introduced by lambdalifting
-      # remove if -d:nimOptIters is default, treating it as always nil
-    nimOptItersEnabled: bool # tracks if -d:nimOptIters is enabled
-      # should be default when issues are fixed, see #24094
 
 const
   nkSkip = {nkEmpty..nkNilLit, nkTemplateDef, nkTypeSection, nkStaticStmt,
@@ -174,11 +170,8 @@ const
   localRequiresLifting = -2
 
 proc newStateAccess(ctx: var Ctx): PNode =
-  if ctx.stateVarSym.isNil:
-    result = rawIndirectAccess(newSymNode(getEnvParam(ctx.fn)),
+  result = rawIndirectAccess(newSymNode(getEnvParam(ctx.fn)),
       getStateField(ctx.g, ctx.fn), ctx.fn.info)
-  else:
-    result = newSymNode(ctx.stateVarSym)
 
 proc newStateAssgn(ctx: var Ctx, toValue: PNode): PNode =
   # Creates state assignment:
@@ -196,22 +189,12 @@ proc newEnvVar(ctx: var Ctx, name: string, typ: PType): PSym =
   result.flags.incl sfNoInit
   assert(not typ.isNil, "Env var needs a type")
 
-  if not ctx.stateVarSym.isNil:
-    # We haven't gone through labmda lifting yet, so just create a local var,
-    # it will be lifted later
-    if ctx.tempVars.isNil:
-      ctx.tempVars = newNodeI(nkVarSection, ctx.fn.info)
-      addVar(ctx.tempVars, newSymNode(result))
-  else:
-    let envParam = getEnvParam(ctx.fn)
-    # let obj = envParam.typ.lastSon
-    result = addUniqueField(envParam.typ.elementType, result, ctx.g.cache, ctx.idgen)
+  let envParam = getEnvParam(ctx.fn)
+  # let obj = envParam.typ.lastSon
+  result = addUniqueField(envParam.typ.elementType, result, ctx.g.cache, ctx.idgen)
 
 proc newEnvVarAccess(ctx: Ctx, s: PSym): PNode =
-  if ctx.stateVarSym.isNil:
-    result = rawIndirectAccess(newSymNode(getEnvParam(ctx.fn)), s, ctx.fn.info)
-  else:
-    result = newSymNode(s)
+  result = rawIndirectAccess(newSymNode(getEnvParam(ctx.fn)), s, ctx.fn.info)
 
 proc newTempVarAccess(ctx: Ctx, s: PSym): PNode =
   result = newSymNode(s, ctx.fn.info)
@@ -263,20 +246,12 @@ proc newTempVarDef(ctx: Ctx, s: PSym, initialValue: PNode): PNode =
     v = ctx.g.emptyNode
   newTree(nkVarSection, newTree(nkIdentDefs, newSymNode(s), ctx.g.emptyNode, v))
 
-proc newEnvVarAsgn(ctx: Ctx, s: PSym, v: PNode): PNode
-
 proc newTempVar(ctx: var Ctx, typ: PType, parent: PNode, initialValue: PNode = nil): PSym =
-  if ctx.nimOptItersEnabled:
-    result = newSym(skVar, getIdent(ctx.g.cache, ":tmpSlLower" & $ctx.tempVarId), ctx.idgen, ctx.fn, ctx.fn.info)
-  else:
-    result = ctx.newEnvVar(":tmpSlLower" & $ctx.tempVarId, typ)
+  result = newSym(skVar, getIdent(ctx.g.cache, ":tmpSlLower" & $ctx.tempVarId), ctx.idgen, ctx.fn, ctx.fn.info)
   inc ctx.tempVarId
   result.typ = typ
   assert(not typ.isNil, "Temp var needs a type")
-  if ctx.nimOptItersEnabled:
-    parent.add(ctx.newTempVarDef(result, initialValue))
-  elif initialValue != nil:
-    parent.add(ctx.newEnvVarAsgn(result, initialValue))
+  parent.add(ctx.newTempVarDef(result, initialValue))
 
 proc hasYields(n: PNode): bool =
   # TODO: This is very inefficient. It traverses the node, looking for nkYieldStmt.
@@ -455,24 +430,13 @@ proc newTempVarAsgn(ctx: Ctx, s: PSym, v: PNode): PNode =
     result = newTree(nkFastAsgn, ctx.newTempVarAccess(s), v)
     result.info = v.info
 
-proc newEnvVarAsgn(ctx: Ctx, s: PSym, v: PNode): PNode =
-  # unused with -d:nimOptIters
-  if isEmptyType(v.typ):
-    result = v
-  else:
-    result = newTree(nkFastAsgn, ctx.newEnvVarAccess(s), v)
-    result.info = v.info
-
 proc addExprAssgn(ctx: Ctx, output, input: PNode, sym: PSym) =
-  var input = input
   if input.kind == nkStmtListExpr:
     let (st, res) = exprToStmtList(input)
     output.add(st)
-    input = res
-  if ctx.nimOptItersEnabled:
-    output.add(ctx.newTempVarAsgn(sym, input))
+    output.add(ctx.newTempVarAsgn(sym, res))
   else:
-    output.add(ctx.newEnvVarAsgn(sym, input))
+    output.add(ctx.newTempVarAsgn(sym, input))
 
 proc convertExprBodyToAsgn(ctx: Ctx, exprBody: PNode, res: PSym): PNode =
   result = newNodeI(nkStmtList, exprBody.info)
@@ -601,11 +565,7 @@ proc lowerStmtListExprs(ctx: var Ctx, n: PNode, needsSplit: var bool): PNode =
         else:
           internalError(ctx.g.config, "lowerStmtListExpr(nkIf): " & $branch.kind)
 
-      if isExpr:
-        if ctx.nimOptItersEnabled:
-          result.add(ctx.newTempVarAccess(tmp))
-        else:
-          result.add(ctx.newEnvVarAccess(tmp))
+      if isExpr: result.add(ctx.newTempVarAccess(tmp))
 
   of nkTryStmt, nkHiddenTryStmt:
     var ns = false
@@ -635,10 +595,7 @@ proc lowerStmtListExprs(ctx: var Ctx, n: PNode, needsSplit: var bool): PNode =
           else:
             internalError(ctx.g.config, "lowerStmtListExpr(nkTryStmt): " & $branch.kind)
         result.add(n)
-        if ctx.nimOptItersEnabled:
-          result.add(ctx.newTempVarAccess(tmp))
-        else:
-          result.add(ctx.newEnvVarAccess(tmp))
+        result.add(ctx.newTempVarAccess(tmp))
 
   of nkCaseStmt:
     var ns = false
@@ -670,10 +627,7 @@ proc lowerStmtListExprs(ctx: var Ctx, n: PNode, needsSplit: var bool): PNode =
           else:
             internalError(ctx.g.config, "lowerStmtListExpr(nkCaseStmt): " & $branch.kind)
         result.add(n)
-        if ctx.nimOptItersEnabled:
-          result.add(ctx.newTempVarAccess(tmp))
-        else:
-          result.add(ctx.newEnvVarAccess(tmp))
+        result.add(ctx.newTempVarAccess(tmp))
       elif n[0].kind == nkStmtListExpr:
         result = newNodeI(nkStmtList, n.info)
         let (st, ex) = exprToStmtList(n[0])
@@ -706,11 +660,7 @@ proc lowerStmtListExprs(ctx: var Ctx, n: PNode, needsSplit: var bool): PNode =
         let tmp = ctx.newTempVar(cond.typ, result, cond)
         # result.add(ctx.newTempVarAsgn(tmp, cond))
 
-        var check: PNode
-        if ctx.nimOptItersEnabled:
-          check = ctx.newTempVarAccess(tmp)
-        else:
-          check = ctx.newEnvVarAccess(tmp)
+        var check = ctx.newTempVarAccess(tmp)
         if n[0].sym.magic == mOr:
           check = ctx.g.newNotCall(check)
 
@@ -720,18 +670,12 @@ proc lowerStmtListExprs(ctx: var Ctx, n: PNode, needsSplit: var bool): PNode =
           let (st, ex) = exprToStmtList(cond)
           ifBody.add(st)
           cond = ex
-        if ctx.nimOptItersEnabled:
-          ifBody.add(ctx.newTempVarAsgn(tmp, cond))
-        else:
-          ifBody.add(ctx.newEnvVarAsgn(tmp, cond))
+        ifBody.add(ctx.newTempVarAsgn(tmp, cond))
 
         let ifBranch = newTree(nkElifBranch, check, ifBody)
         let ifNode = newTree(nkIfStmt, ifBranch)
         result.add(ifNode)
-        if ctx.nimOptItersEnabled:
-          result.add(ctx.newTempVarAccess(tmp))
-        else:
-          result.add(ctx.newEnvVarAccess(tmp))
+        result.add(ctx.newTempVarAccess(tmp))
       else:
         for i in 0..<n.len:
           if n[i].kind == nkStmtListExpr:
@@ -742,10 +686,7 @@ proc lowerStmtListExprs(ctx: var Ctx, n: PNode, needsSplit: var bool): PNode =
           if n[i].kind in nkCallKinds: # XXX: This should better be some sort of side effect tracking
             let tmp = ctx.newTempVar(n[i].typ, result, n[i])
             # result.add(ctx.newTempVarAsgn(tmp, n[i]))
-            if ctx.nimOptItersEnabled:
-              n[i] = ctx.newTempVarAccess(tmp)
-            else:
-              n[i] = ctx.newEnvVarAccess(tmp)
+            n[i] = ctx.newTempVarAccess(tmp)
 
         result.add(n)
 
@@ -1343,13 +1284,6 @@ proc wrapIntoStateLoop(ctx: var Ctx, n: PNode): PNode =
   result.info = n.info
 
   let localVars = newNodeI(nkStmtList, n.info)
-  if not ctx.stateVarSym.isNil:
-    let varSect = newNodeI(nkVarSection, n.info)
-    addVar(varSect, newSymNode(ctx.stateVarSym))
-    localVars.add(varSect)
-
-    if not ctx.tempVars.isNil:
-      localVars.add(ctx.tempVars)
 
   let blockStmt = newNodeI(nkBlockStmt, n.info)
   blockStmt.add(newSymNode(ctx.stateLoopLabel))
@@ -1552,21 +1486,11 @@ proc liftLocals(c: var Ctx, n: PNode): PNode =
       n[i] = liftLocals(c, n[i])
 
 proc transformClosureIterator*(g: ModuleGraph; idgen: IdGenerator; fn: PSym, n: PNode): PNode =
-  var ctx = Ctx(g: g, fn: fn, idgen: idgen,
-    # should be default when issues are fixed, see #24094:
-    nimOptItersEnabled: isDefined(g.config, "nimOptIters"))
+  var ctx = Ctx(g: g, fn: fn, idgen: idgen)
 
-  if getEnvParam(fn).isNil:
-    if ctx.nimOptItersEnabled:
-      # The transformation should always happen after at least partial lambdalifting
-      # is performed, so that the closure iter environment is always created upfront.
-      doAssert(false, "Env param not created before iter transformation")
-    else:
-      # Lambda lifting was not done yet. Use temporary :state sym, which will
-      # be handled specially by lambda lifting. Local temp vars (if needed)
-      # should follow the same logic.
-      ctx.stateVarSym = newSym(skVar, getIdent(ctx.g.cache, ":state"), idgen, fn, fn.info)
-      ctx.stateVarSym.typ = g.createClosureIterStateType(fn, idgen)
+  # The transformation should always happen after at least partial lambdalifting
+  # is performed, so that the closure iter environment is always created upfront.
+  doAssert(getEnvParam(fn) != nil, "Env param not created before iter transformation")
 
   ctx.stateLoopLabel = newSym(skLabel, getIdent(ctx.g.cache, ":stateLoop"), idgen, fn, fn.info)
   var pc = PreprocessContext(finallys: @[], config: g.config, idgen: idgen)
@@ -1592,10 +1516,9 @@ proc transformClosureIterator*(g: ModuleGraph; idgen: IdGenerator; fn: PSym, n:
   let caseDispatcher = newTreeI(nkCaseStmt, n.info,
       ctx.newStateAccess())
 
-  if ctx.nimOptItersEnabled:
-    # Lamdalifting will not touch our locals, it is our responsibility to lift those that
-    # need it.
-    detectCapturedVars(ctx)
+  # Lamdalifting will not touch our locals, it is our responsibility to lift those that
+  # need it.
+  detectCapturedVars(ctx)
 
   for s in ctx.states:
     let body = ctx.transformStateAssignments(s.body)
@@ -1604,8 +1527,7 @@ proc transformClosureIterator*(g: ModuleGraph; idgen: IdGenerator; fn: PSym, n:
   caseDispatcher.add newTreeI(nkElse, n.info, newTreeI(nkReturnStmt, n.info, g.emptyNode))
 
   result = wrapIntoStateLoop(ctx, caseDispatcher)
-  if ctx.nimOptItersEnabled:
-    result = liftLocals(ctx, result)
+  result = liftLocals(ctx, result)
 
   when false:
     echo "TRANSFORM TO STATES: "
diff --git a/compiler/lambdalifting.nim b/compiler/lambdalifting.nim
index 54cdfc5bc0..3d3a50089b 100644
--- a/compiler/lambdalifting.nim
+++ b/compiler/lambdalifting.nim
@@ -150,7 +150,7 @@ template isIterator*(owner: PSym): bool =
 
 proc createEnvObj(g: ModuleGraph; idgen: IdGenerator; owner: PSym; info: TLineInfo): PType =
   result = createObj(g, idgen, owner, info, final=false)
-  if owner.isIterator or not isDefined(g.config, "nimOptIters"):
+  if owner.isIterator:
     rawAddField(result, createStateField(g, owner, idgen))
 
 proc getClosureIterResult*(g: ModuleGraph; iter: PSym; idgen: IdGenerator): PSym =
@@ -175,6 +175,7 @@ proc addHiddenParam(routine: PSym, param: PSym) =
   #echo "produced environment: ", param.id, " for ", routine.id
 
 proc getEnvParam*(routine: PSym): PSym =
+  if routine.ast.isNil: return nil
   let params = routine.ast[paramsPos]
   let hidden = lastSon(params)
   if hidden.kind == nkSym and hidden.sym.kind == skParam and hidden.sym.name.s == paramName:
@@ -228,13 +229,6 @@ proc makeClosure*(g: ModuleGraph; idgen: IdGenerator; prc: PSym; env: PNode; inf
   if tfHasAsgn in result.typ.flags or optSeqDestructors in g.config.globalOptions:
     prc.flags.incl sfInjectDestructors
 
-proc interestingIterVar(s: PSym): bool {.inline.} =
-  # unused with -d:nimOptIters
-  # XXX optimization: Only lift the variable if it lives across
-  # yield/return boundaries! This can potentially speed up
-  # closure iterators quite a bit.
-  result = s.kind in {skResult, skVar, skLet, skTemp, skForVar} and sfGlobal notin s.flags
-
 template liftingHarmful(conf: ConfigRef; owner: PSym): bool =
   ## lambda lifting can be harmful for JS-like code generators.
   let isCompileTime = sfCompileTime in owner.flags or owner.kind == skMacro
@@ -281,16 +275,6 @@ proc liftIterSym*(g: ModuleGraph; n: PNode; idgen: IdGenerator; owner: PSym): PN
   createTypeBoundOpsLL(g, env.typ, n.info, idgen, owner)
   result.add makeClosure(g, idgen, iter, env, n.info)
 
-proc freshVarForClosureIter*(g: ModuleGraph; s: PSym; idgen: IdGenerator; owner: PSym): PNode =
-  # unused with -d:nimOptIters
-  let envParam = getHiddenParam(g, owner)
-  let obj = envParam.typ.skipTypes({tyOwned, tyRef, tyPtr})
-  let field = addField(obj, s, g.cache, idgen)
-
-  var access = newSymNode(envParam)
-  assert obj.kind == tyObject
-  result = rawIndirectAccess(access, field, s.info)
-
 # ------------------ new stuff -------------------------------------------
 
 proc markAsClosure(g: ModuleGraph; owner: PSym; n: PNode) =
@@ -339,7 +323,7 @@ proc getEnvTypeForOwner(c: var DetectionPass; owner: PSym;
   result = c.ownerToType.getOrDefault(owner.id)
   if result.isNil:
     let env = getEnvParam(owner)
-    if env.isNil or not owner.isIterator or not isDefined(c.graph.config, "nimOptIters"):
+    if env.isNil or not owner.isIterator:
       result = newType(tyRef, c.idgen, owner)
       let obj = createEnvObj(c.graph, c.idgen, owner, info)
       rawAddSon(result, obj)
@@ -436,6 +420,12 @@ proc addClosureParam(c: var DetectionPass; fn: PSym; info: TLineInfo) =
     localError(c.graph.config, fn.info, "internal error: inconsistent environment type")
   #echo "adding closure to ", fn.name.s
 
+proc iterEnvHasUpField(g: ModuleGraph, iter: PSym): bool =
+  let cp = getEnvParam(iter)
+  doAssert(cp != nil, "Env param not present in iter")
+  let upField = lookupInRecord(cp.typ.skipTypes({tyOwned, tyRef, tyPtr}).n, getIdent(g.cache, upName))
+  upField != nil
+
 proc detectCapturedVars(n: PNode; owner: PSym; c: var DetectionPass) =
   case n.kind
   of nkSym:
@@ -454,23 +444,12 @@ proc detectCapturedVars(n: PNode; owner: PSym; c: var DetectionPass) =
         let body = transformBody(c.graph, c.idgen, s, {useCache})
         detectCapturedVars(body, s, c)
     let ow = s.skipGenericOwner
-    let innerClosure = innerProc and s.typ.callConv == ccClosure and not s.isIterator
+    let innerClosure = innerProc and s.typ.callConv == ccClosure and (not s.isIterator or iterEnvHasUpField(c.graph, s))
     let interested = interestingVar(s)
     if ow == owner:
       if owner.isIterator:
         c.somethingToDo = true
         addClosureParam(c, owner, n.info)
-        if not isDefined(c.graph.config, "nimOptIters") and interestingIterVar(s):
-          if not c.capturedVars.contains(s.id):
-            if not c.inTypeOf: c.capturedVars.incl(s.id)
-            let obj = getHiddenParam(c.graph, owner).typ.skipTypes({tyOwned, tyRef, tyPtr})
-            #let obj = c.getEnvTypeForOwner(s.owner).skipTypes({tyOwned, tyRef, tyPtr})
-
-            if s.name.id == getIdent(c.graph.cache, ":state").id:
-              obj.n[0].sym.flags.incl sfNoInit
-              obj.n[0].sym.itemId = ItemId(module: s.itemId.module, item: -s.itemId.item)
-            else:
-              discard addField(obj, s, c.graph.cache, c.idgen)
     # direct or indirect dependency:
     elif innerClosure or interested:
       discard """
@@ -670,16 +649,27 @@ proc finishClosureCreation(owner: PSym; d: var DetectionPass; c: LiftingPass;
     res.add newAsgnStmt(unowned, nilLit, info)
     createTypeBoundOpsLL(d.graph, unowned.typ, info, d.idgen, owner)
 
-proc closureCreationForIter(iter: PNode;
+proc getUpForIter(g: ModuleGraph; owner, iterOwner: PSym, expectedUpTyp: PType): PNode =
+  var p = getHiddenParam(g, owner)
+  var res = p.newSymNode
+  while res.typ.skipTypes({tyOwned, tyRef, tyPtr}) != expectedUpTyp:
+    let upField = lookupInRecord(p.typ.skipTypes({tyOwned, tyRef, tyPtr}).n, getIdent(g.cache, upName))
+    if upField == nil:
+      return nil
+    p = upField
+    res = rawIndirectAccess(res, upField, p.info)
+  res
+
+proc closureCreationForIter(owner: PSym, iter: PNode;
                             d: var DetectionPass; c: var LiftingPass): PNode =
   result = newNodeIT(nkStmtListExpr, iter.info, iter.sym.typ)
-  let owner = iter.sym.skipGenericOwner
-  var v = newSym(skVar, getIdent(d.graph.cache, envName), d.idgen, owner, iter.info)
+  let iterOwner = iter.sym.skipGenericOwner
+  var v = newSym(skVar, getIdent(d.graph.cache, envName), d.idgen, iterOwner, iter.info)
   incl(v.flags, sfShadowed)
   v.typ = asOwnedRef(d, getHiddenParam(d.graph, iter.sym).typ)
   var vnode: PNode
-  if owner.isIterator:
-    let it = getHiddenParam(d.graph, owner)
+  if iterOwner.isIterator:
+    let it = getHiddenParam(d.graph, iterOwner)
     addUniqueField(it.typ.skipTypes({tyOwned, tyRef, tyPtr}), v, d.graph.cache, d.idgen)
     vnode = indirectAccess(newSymNode(it), v, v.info)
   else:
@@ -688,12 +678,14 @@ proc closureCreationForIter(iter: PNode;
     addVar(vs, vnode)
     result.add(vs)
   result.add genCreateEnv(vnode)
-  createTypeBoundOpsLL(d.graph, vnode.typ, iter.info, d.idgen, owner)
+  createTypeBoundOpsLL(d.graph, vnode.typ, iter.info, d.idgen, iterOwner)
 
   let upField = lookupInRecord(v.typ.skipTypes({tyOwned, tyRef, tyPtr}).n, getIdent(d.graph.cache, upName))
   if upField != nil:
-    let u = setupEnvVar(owner, d, c, iter.info)
-    if u.typ.skipTypes({tyOwned, tyRef, tyPtr}) == upField.typ.skipTypes({tyOwned, tyRef, tyPtr}):
+    let expectedUpTyp = upField.typ.skipTypes({tyOwned, tyRef, tyPtr})
+    let u = if iterOwner == owner: setupEnvVar(iterOwner, d, c, iter.info)
+            else: getUpForIter(d.graph, owner, iterOwner, expectedUpTyp)
+    if u != nil and u.typ.skipTypes({tyOwned, tyRef, tyPtr}) == expectedUpTyp:
       result.add(newAsgnStmt(rawIndirectAccess(vnode, upField, iter.info),
                  u, iter.info))
     else:
@@ -727,7 +719,7 @@ proc symToClosure(n: PNode; owner: PSym; d: var DetectionPass;
     let available = getHiddenParam(d.graph, owner)
     result = makeClosure(d.graph, d.idgen, s, available.newSymNode, n.info)
   elif s.isIterator:
-    result = closureCreationForIter(n, d, c)
+    result = closureCreationForIter(owner, n, d, c)
   elif s.skipGenericOwner == owner:
     # direct dependency, so use the outer's env variable:
     result = makeClosure(d.graph, d.idgen, s, setupEnvVar(owner, d, c, n.info), n.info)
@@ -774,8 +766,6 @@ proc liftCapturedVars(n: PNode; owner: PSym; d: var DetectionPass;
     elif s.id in d.capturedVars:
       if s.owner != owner:
         result = accessViaEnvParam(d.graph, n, owner)
-      elif owner.isIterator and not isDefined(d.graph.config, "nimOptIters") and interestingIterVar(s):
-        result = accessViaEnvParam(d.graph, n, owner)
       else:
         result = accessViaEnvVar(n, owner, d, c)
   of nkEmpty..pred(nkSym), succ(nkSym)..nkNilLit, nkComesFrom,
@@ -893,7 +883,7 @@ proc liftLambdas*(g: ModuleGraph; fn: PSym, body: PNode; tooEarly: var bool;
     # ignore forward declaration:
     result = body
     tooEarly = true
-    if fn.isIterator and isDefined(g.config, "nimOptIters"):
+    if fn.isIterator:
       var d = initDetectionPass(g, fn, idgen)
       addClosureParam(d, fn, body.info)
   else:
diff --git a/compiler/transf.nim b/compiler/transf.nim
index 7ee9683743..a302fb52d4 100644
--- a/compiler/transf.nim
+++ b/compiler/transf.nim
@@ -97,10 +97,7 @@ proc newTemp(c: PTransf, typ: PType, info: TLineInfo): PNode =
   r.typ = typ #skipTypes(typ, {tyGenericInst, tyAlias, tySink})
   incl(r.flags, sfFromGeneric)
   let owner = getCurrOwner(c)
-  if owner.isIterator and not c.tooEarly and not isDefined(c.graph.config, "nimOptIters"):
-    result = freshVarForClosureIter(c.graph, r, c.idgen, owner)
-  else:
-    result = newSymNode(r)
+  result = newSymNode(r)
 
 proc transform(c: PTransf, n: PNode, noConstFold = false): PNode
 
@@ -176,13 +173,10 @@ proc transformSym(c: PTransf, n: PNode): PNode =
 
 proc freshVar(c: PTransf; v: PSym): PNode =
   let owner = getCurrOwner(c)
-  if owner.isIterator and not c.tooEarly and not isDefined(c.graph.config, "nimOptIters"):
-    result = freshVarForClosureIter(c.graph, v, c.idgen, owner)
-  else:
-    var newVar = copySym(v, c.idgen)
-    incl(newVar.flags, sfFromGeneric)
-    setOwner(newVar, owner)
-    result = newSymNode(newVar)
+  var newVar = copySym(v, c.idgen)
+  incl(newVar.flags, sfFromGeneric)
+  setOwner(newVar, owner)
+  result = newSymNode(newVar)
 
 proc transformVarSection(c: PTransf, v: PNode): PNode =
   result = newTransNode(v)
diff --git a/tests/destructor/tuse_ownedref_after_move.nim b/tests/destructor/tuse_ownedref_after_move.nim
index 69348d5303..34d5be65a4 100644
--- a/tests/destructor/tuse_ownedref_after_move.nim
+++ b/tests/destructor/tuse_ownedref_after_move.nim
@@ -1,6 +1,6 @@
 discard """
   cmd: '''nim c --newruntime $file'''
-  errormsg: "'=copy' is not available for type <owned Button>; requires a copy because it's not the last read of ':envAlt.b1'; routine: main"
+  errormsg: "'=copy' is not available for type <owned Button>; requires a copy because it's not the last read of ':envAlt.b0'; routine: main"
   line: 48
 """
 
diff --git a/tests/iter/tnestedclosures.nim b/tests/iter/tnestedclosures.nim
new file mode 100644
index 0000000000..273c4aa303
--- /dev/null
+++ b/tests/iter/tnestedclosures.nim
@@ -0,0 +1,139 @@
+discard """
+  targets: "c"
+  output: '''
+Test 1:
+12
+Test 2:
+23
+23
+Test 3:
+34
+34
+Test 4:
+45
+45
+50
+50
+Test 5:
+45
+123
+47
+50
+Test 6:
+<hi>
+Test 7:
+0
+1
+2
+'''
+"""
+
+block: #24094
+  echo "Test 1:"
+  proc foo() =
+    let x = 12
+    iterator bar2(): int {.closure.} =
+      yield x
+    proc bar() =
+      let z = bar2
+      for y in z(): # just doing bar2() gives param not in env: x
+        echo y
+    bar()
+
+  foo()
+
+block: #24094
+  echo "Test 2:"
+  iterator foo(): int {.closure.} =
+    let x = 23
+    iterator bar2(): int {.closure.} =
+      yield x
+    proc bar() =
+      let z = bar2
+      for y in z():
+        echo y
+    bar()
+    yield x
+
+  for x in foo(): echo x
+
+block: #24094
+  echo "Test 3:"
+  iterator foo(): int {.closure.} =
+    let x = 34
+    proc bar() =
+      echo x
+    iterator bar2(): int {.closure.} =
+      bar()
+      yield x
+    for y in bar2():
+      yield y
+
+  for x in foo(): echo x
+
+block:
+  echo "Test 4:"
+  proc foo() =
+    var x = 45
+    iterator bar2(): int {.closure.} =
+      yield x
+      yield x + 3
+
+    let b1 = bar2
+    let b2 = bar2
+    echo b1()
+    echo b2()
+    x = 47
+    echo b1()
+    echo b2()
+  foo()
+
+block:
+  echo "Test 5:"
+  proc foo() =
+    var x = 45
+    iterator bar2(): int {.closure.} =
+      yield x
+      yield x + 3
+
+    proc bar() =
+      var y = 123
+      iterator bar3(): int {.closure.} =
+        yield x
+        yield y
+      let b3 = bar3
+      for z in b3():
+        echo z
+      x = 47
+      let b2 = bar2
+      for z in b2():
+        echo z
+    bar()
+  foo()
+
+block: #19154
+  echo "Test 6:"
+  proc test(s: string): proc(): iterator(): string =
+    iterator it(): string = yield s
+    proc f(): iterator(): string = it
+    return f
+
+  let it = test("hi")()
+  for s in it():
+    echo "<", s, ">"
+
+block: #3824
+  echo "Test 7:"
+  proc main =
+    iterator factory(): int {.closure.} =
+      iterator bar(): int {.closure.} =
+        yield 0
+        yield 1
+        yield 2
+
+      for x in bar(): yield x
+
+    for x in factory():
+      echo x
+
+  main()
diff --git a/tests/iter/tyieldintry.nim b/tests/iter/tyieldintry.nim
index e51ab7f0d5..04409795b0 100644
--- a/tests/iter/tyieldintry.nim
+++ b/tests/iter/tyieldintry.nim
@@ -1,5 +1,5 @@
 discard """
-  matrix: "; --experimental:strictdefs; -d:nimOptIters"
+  matrix: "; --experimental:strictdefs"
   targets: "c cpp"
 """
 
@@ -505,7 +505,7 @@ block: # void iterator
       discard
   var a = it
 
-if defined(nimOptIters): # Locals present in only 1 state should be on the stack
+block: # Locals present in only 1 state should be on the stack
   proc checkOnStack(a: pointer, shouldBeOnStack: bool) =
     # Quick and dirty way to check if a points to stack
     var dummy = 0