From 9af2cf6f9f8b186f895121df4045d83500b5b97c Mon Sep 17 00:00:00 2001
From: metagn <metagngn@gmail.com>
Date: Tue, 22 Oct 2024 14:35:53 +0300
Subject: [PATCH 1/2] wrap `fields` iterations in `if true` scope [backport]

fixes #24338
---
 compiler/semfields.nim   | 12 +++++++++++-
 tests/system/tfields.nim | 23 ++++++++++++++++++++++-
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/compiler/semfields.nim b/compiler/semfields.nim
index 874055cdcde9..8223587e0d6c 100644
--- a/compiler/semfields.nim
+++ b/compiler/semfields.nim
@@ -18,6 +18,12 @@ type
     replaceByFieldName: bool
     c: PContext
 
+proc wrapNewScope(c: PContext, n: PNode): PNode {.inline.} =
+  # use `if true` to not interfere with `break`
+  let trueLit = newIntLit(c.graph, n.info, 1)
+  trueLit.typ() = getSysType(c.graph, n.info, tyBool)
+  result = newTreeI(nkIfStmt, n.info, newTreeI(nkElifBranch, n.info, trueLit, n))
+
 proc instFieldLoopBody(c: TFieldInstCtx, n: PNode, forLoop: PNode): PNode =
   if c.field != nil and isEmptyType(c.field.typ):
     result = newNode(nkEmpty)
@@ -72,7 +78,9 @@ proc semForObjectFields(c: TFieldsCtx, typ, forLoop, father: PNode) =
     )
     openScope(c.c)
     inc c.c.inUnrolledContext
-    let body = instFieldLoopBody(fc, lastSon(forLoop), forLoop)
+    var body = instFieldLoopBody(fc, lastSon(forLoop), forLoop)
+    # new scope for each field
+    body = wrapNewScope(c.c, body)
     father.add(semStmt(c.c, body, {}))
     dec c.c.inUnrolledContext
     closeScope(c.c)
@@ -148,6 +156,8 @@ proc semForFields(c: PContext, n: PNode, m: TMagic): PNode =
           replaceByFieldName: m == mFieldPairs
       )
       var body = instFieldLoopBody(fc, loopBody, n)
+      # new scope for each field
+      body = wrapNewScope(c, body)
       inc c.inUnrolledContext
       stmts.add(semStmt(c, body, {}))
       dec c.inUnrolledContext
diff --git a/tests/system/tfields.nim b/tests/system/tfields.nim
index 0bf3a4e1adf0..b5eb42b558cc 100644
--- a/tests/system/tfields.nim
+++ b/tests/system/tfields.nim
@@ -105,4 +105,25 @@ block timplicit_with_partial:
       echo x
     echo x
 
-  foo(FooTask())
\ No newline at end of file
+  foo(FooTask())
+
+block:
+  var innerCount = 0
+  var outerCount = 0
+  template c(w: int): int =
+    let q = w
+    inc innerCount
+    0
+
+  template t(r: (int, int); x: int) =
+    for _ in r.fields:
+      let w = x
+      doAssert w == 0
+      dec outerCount
+
+  proc k() =
+    t((0, 0), c(0))
+
+  k()
+  doAssert innerCount == 2
+  doAssert outerCount == -2

From eedd7bcfce137d61332afd2453d63b19fd290800 Mon Sep 17 00:00:00 2001
From: metagn <metagngn@gmail.com>
Date: Tue, 22 Oct 2024 14:42:02 +0300
Subject: [PATCH 2/2] more comments, object test

---
 compiler/semfields.nim   |  7 +++++--
 tests/system/tfields.nim | 25 ++++++++++++++++++++++++-
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/compiler/semfields.nim b/compiler/semfields.nim
index 8223587e0d6c..7e2f5d6fdd5b 100644
--- a/compiler/semfields.nim
+++ b/compiler/semfields.nim
@@ -20,6 +20,9 @@ type
 
 proc wrapNewScope(c: PContext, n: PNode): PNode {.inline.} =
   # use `if true` to not interfere with `break`
+  # just opening scope via `openScope(c)` isn't enough,
+  # a scope has to be opened in the codegen as well for reused
+  # template instantiations
   let trueLit = newIntLit(c.graph, n.info, 1)
   trueLit.typ() = getSysType(c.graph, n.info, tyBool)
   result = newTreeI(nkIfStmt, n.info, newTreeI(nkElifBranch, n.info, trueLit, n))
@@ -79,7 +82,7 @@ proc semForObjectFields(c: TFieldsCtx, typ, forLoop, father: PNode) =
     openScope(c.c)
     inc c.c.inUnrolledContext
     var body = instFieldLoopBody(fc, lastSon(forLoop), forLoop)
-    # new scope for each field
+    # new scope for each field that codegen should know about:
     body = wrapNewScope(c.c, body)
     father.add(semStmt(c.c, body, {}))
     dec c.c.inUnrolledContext
@@ -156,7 +159,7 @@ proc semForFields(c: PContext, n: PNode, m: TMagic): PNode =
           replaceByFieldName: m == mFieldPairs
       )
       var body = instFieldLoopBody(fc, loopBody, n)
-      # new scope for each field
+      # new scope for each field that codegen should know about:
       body = wrapNewScope(c, body)
       inc c.inUnrolledContext
       stmts.add(semStmt(c, body, {}))
diff --git a/tests/system/tfields.nim b/tests/system/tfields.nim
index b5eb42b558cc..8e7533baacdf 100644
--- a/tests/system/tfields.nim
+++ b/tests/system/tfields.nim
@@ -107,7 +107,7 @@ block timplicit_with_partial:
 
   foo(FooTask())
 
-block:
+block: # issue #24338
   var innerCount = 0
   var outerCount = 0
   template c(w: int): int =
@@ -127,3 +127,26 @@ block:
   k()
   doAssert innerCount == 2
   doAssert outerCount == -2
+
+block: # issue #24338 with object
+  type Foo = object
+    x, y: int
+  var innerCount = 0
+  var outerCount = 0
+  template c(w: int): int =
+    let q = w
+    inc innerCount
+    0
+
+  template t(r: Foo; x: int) =
+    for _ in r.fields:
+      let w = x
+      doAssert w == 0
+      dec outerCount
+
+  proc k() =
+    t(Foo(x: 0, y: 0), c(0))
+
+  k()
+  doAssert innerCount == 2
+  doAssert outerCount == -2