Revert "disable closure iterator changes in nim-lang#23787 unless `-d…

…:nimOptIters` is enabled (nim-lang#24108)"

This reverts commit 22d2cf2.

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"))
-
-  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)
+  var ctx = Ctx(g: g, fn: fn, idgen: 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: "

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 =
@@ -228,13 +228,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 +274,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 +322,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)
@@ -460,17 +443,6 @@ proc detectCapturedVars(n: PNode; owner: PSym; c: var DetectionPass) =
       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 """
@@ -774,8 +746,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 +863,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:

compiler/transf.nim

@@ -98,10 +98,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): PNode
 
@@ -177,13 +174,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)
-    newVar.owner = owner
-    result = newSymNode(newVar)
+  var newVar = copySym(v, c.idgen)
+  incl(newVar.flags, sfFromGeneric)
+  newVar.owner = owner
+  result = newSymNode(newVar)
 
 proc transformVarSection(c: PTransf, v: PNode): PNode =
   result = newTransNode(v)

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
 """
 

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