From f959a02037849b45c8d842a7a95b95ba9ced3a3e Mon Sep 17 00:00:00 2001
From: ringabout <43030857+ringabout@users.noreply.github.com>
Date: Mon, 8 Jun 2026 14:53:10 +0800
Subject: [PATCH] fixes #25725; environment misses: s with iterator (#25828)

fixes #25725

This pull request makes significant improvements to symbol handling
during transformation passes in the compiler, particularly for routines
(procedures, iterators) and their parameters. The changes ensure that
when routines are copied (for inlining, closure generation, etc.), all
relevant symbols and type headers are also freshly copied and correctly
owned, preventing subtle bugs from symbol reuse. Additionally, new
regression tests are added to cover previously problematic iterator
cases.

**Improvements to symbol copying and ownership:**

* Introduced `freshOwnedSym` to create a fresh copy of a symbol with a
specified owner, ensuring that transformed routines and their parameters
do not share symbols with the originals, which prevents accidental
aliasing and ownership issues.
* Refactored `freshVar` to use `freshOwnedSym`, centralizing fresh
symbol creation logic.
* Added `introduceNewRoutineHeaderSyms` and `copyRoutineTypeHeader` to
ensure that when routines are copied, all parameter/result symbols and
their types are also freshly copied and mapped, avoiding shared state
between original and transformed routines.
* Updated `introduceNewLocalVars` to use `freshOwnedSym` for routine
symbols and to invoke the new header/type copying procedures, ensuring
correctness in routine transformation.

**Testing and regression coverage:**

* Added new blocks to `tests/iter/titer_issues.nim` to test iterator
transformation edge cases, including scenarios that previously led to
symbol reuse bugs (e.g., bugs #25724 and #25725).
---
 compiler/transf.nim         | 61 ++++++++++++++++++++++++++++++++-----
 tests/iter/titer_issues.nim | 22 ++++++++++++-
 2 files changed, 74 insertions(+), 9 deletions(-)

diff --git a/compiler/transf.nim b/compiler/transf.nim
index 47be5b9c61..f131c862f0 100644
--- a/compiler/transf.nim
+++ b/compiler/transf.nim
@@ -90,11 +90,21 @@ proc getCurrOwner(c: PTransf): PSym =
   if c.transCon != nil: result = c.transCon.owner
   else: result = c.module
 
+proc freshOwnedSym(c: PTransf; s, owner: PSym): PNode =
+  # We need to copy the symbol here because we might need to change its owner and
+  # we don't want to mess with the original symbol which might be used in other places.
+  # This can happen for example for iterators which are transformed multiple times when
+  # they are used in different contexts.
+  var fresh = copySym(s, c.idgen)
+  if fresh.kind notin routineKinds:
+    incl(fresh.flagsImpl, sfFromGeneric)
+  setOwner(fresh, owner)
+  result = newSymNode(fresh)
+
 proc newTemp(c: PTransf, typ: PType, info: TLineInfo): PNode =
   let r = newSym(skTemp, getIdent(c.graph.cache, genPrefix), c.idgen, getCurrOwner(c), info)
   r.typ = typ #skipTypes(typ, {tyGenericInst, tyAlias, tySink})
   incl(r.flagsImpl, sfFromGeneric)
-  let owner = getCurrOwner(c)
   result = newSymNode(r)
 
 proc transform(c: PTransf, n: PNode, noConstFold = false): PNode
@@ -185,11 +195,39 @@ proc transformSym(c: PTransf, n: PNode): PNode =
   result = transformSymAux(c, n)
 
 proc freshVar(c: PTransf; v: PSym): PNode =
-  let owner = getCurrOwner(c)
-  var newVar = copySym(v, c.idgen)
-  incl(newVar.flagsImpl, sfFromGeneric)
-  setOwner(newVar, owner)
-  result = newSymNode(newVar)
+  result = freshOwnedSym(c, v, getCurrOwner(c))
+
+proc introduceNewRoutineHeaderSyms(c: PTransf; n: PNode; oldOwner, newOwner: PSym) =
+  # We need to introduce new symbols for the parameters and result of a routine when
+  # we copy it for inlining or closure generation.
+  # Otherwise, we would have multiple nodes referring to the same parameter symbols which
+  # can lead to problems when we need to change the owner of these symbols.
+  case n.kind
+  of nkSym:
+    if n.sym.owner == oldOwner:
+      c.transCon.mapping[n.sym.itemId] = freshOwnedSym(c, n.sym, newOwner)
+  of nkEmpty..pred(nkSym), succ(nkSym)..nkNilLit:
+    discard
+  else:
+    for i in 0..<n.len:
+      introduceNewRoutineHeaderSyms(c, n[i], oldOwner, newOwner)
+
+proc copyRoutineTypeHeader(c: PTransf; oldProc, newProc: PSym) =
+  # We need to copy the routine type header to ensure that
+  # modifications to the newProc do not affect the oldProc.
+  if oldProc.typ != nil and oldProc.typ.kind == tyProc and oldProc.typ.n != nil:
+    newProc.typ = copyType(oldProc.typ, c.idgen, newProc)
+    newProc.typ.n = newNodeI(oldProc.typ.n.kind, oldProc.typ.n.info)
+    if oldProc.typ.n.len > 0:
+      newProc.typ.n.add copyTree(oldProc.typ.n[0])
+      for i in 1..<oldProc.typ.n.len:
+        let oldParam = oldProc.typ.n[i].sym
+        var newParam = getOrDefault(c.transCon.mapping, oldParam.itemId)
+        if newParam == nil:
+          newParam = freshOwnedSym(c, oldParam, newProc)
+          c.transCon.mapping[oldParam.itemId] = newParam
+        doAssert newParam.kind == nkSym
+        newProc.typ.addParam newParam.sym
 
 proc transformVarSection(c: PTransf, v: PNode): PNode =
   result = newTransNode(v)
@@ -338,11 +376,18 @@ proc introduceNewLocalVars(c: PTransf, n: PNode): PNode =
     return n
   of nkLambdaKinds, nkProcDef, nkFuncDef, nkMethodDef, nkConverterDef: # todo optimize nosideeffects?
     result = newTransNode(n)
-    let x = newSymNode(copySym(n[namePos].sym, c.idgen))
-    c.transCon.mapping[n[namePos].sym.itemId] = x
+    let oldProc = n[namePos].sym
+    let x = freshOwnedSym(c, oldProc, oldProc.owner)
+    c.transCon.mapping[oldProc.itemId] = x
+    introduceNewRoutineHeaderSyms(c, n[paramsPos], oldProc, x.sym)
+    if resultPos < n.len and n[resultPos] != nil:
+      introduceNewRoutineHeaderSyms(c, n[resultPos], oldProc, x.sym)
+    copyRoutineTypeHeader(c, oldProc, x.sym)
     result[namePos] = x # we have to copy proc definitions for iters
     for i in 1..<n.len:
       result[i] = introduceNewLocalVars(c, n[i])
+    if x.sym.typ != nil and x.sym.typ.kind == tyProc:
+      result[paramsPos] = x.sym.typ.n
     result[namePos].sym.ast = result
   else:
     result = newTransNode(n)
diff --git a/tests/iter/titer_issues.nim b/tests/iter/titer_issues.nim
index 468c230997..fdc7ab0599 100644
--- a/tests/iter/titer_issues.nim
+++ b/tests/iter/titer_issues.nim
@@ -465,4 +465,24 @@ block: # bug #25724
     else: yield 1
   for w in c():
     let n = w
-    (proc() = discard n)()
\ No newline at end of file
+    (proc() = discard n)()
+
+block:
+  iterator c(): int =
+    yield 1
+    yield 1
+
+  for w in c():
+    proc p(s: int) =
+      let sap = s
+    p(0)
+
+block: # bug #25725
+  iterator c(): int =
+    when nimvm: yield 0
+    else: yield 1
+  for w in c():
+    let n = w
+    proc p(s: int) =
+      let s = s; discard n
+    p(0)