wrap fields iterations in if true scope [backport] (#24343)

fixes #24338

When unrolling each iteration of a `fields` iterator, the compiler only
opens a new scope for semchecking, but doesn't generate a node that
signals to the codegen that a new scope should be created. This causes
issues for reused template instantiations that reuse variable symbols
between each iteration, which causes the codegen to generate multiple
declarations for them in the same scope (regardless of `inject` or
`gensym`). To fix this, we wrap the unrolled iterations in an `if true:
body` node, which both opens a new scope and doesn't interfere with
`break`.

(cherry picked from commit ca5df9a)

compiler/semfields.nim

@@ -18,6 +18,15 @@ type
     replaceByFieldName: bool
     c: PContext
 
+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))
+
 proc instFieldLoopBody(c: TFieldInstCtx, n: PNode, forLoop: PNode): PNode =
   if c.field != nil and isEmptyType(c.field.typ):
     result = newNode(nkEmpty)
@@ -70,7 +79,9 @@ proc semForObjectFields(c: TFieldsCtx, typ, forLoop, father: PNode) =
     fc.replaceByFieldName = c.m == mFieldPairs
     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 that codegen should know about:
+    body = wrapNewScope(c.c, body)
     father.add(semStmt(c.c, body, {}))
     dec c.c.inUnrolledContext
     closeScope(c.c)
@@ -145,6 +156,8 @@ proc semForFields(c: PContext, n: PNode, m: TMagic): PNode =
       fc.c = c
       fc.replaceByFieldName = m == mFieldPairs
       var body = instFieldLoopBody(fc, loopBody, n)
+      # new scope for each field that codegen should know about:
+      body = wrapNewScope(c, body)
       inc c.inUnrolledContext
       stmts.add(semStmt(c, body, {}))
       dec c.inUnrolledContext

tests/system/tfields.nim

@@ -105,4 +105,48 @@ block timplicit_with_partial:
       echo x
     echo x
 
-  foo(FooTask())
+  foo(FooTask())
+
+block: # issue #24338
+  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
+
+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