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