Skip to content

Commit

Permalink
shallow fold prevention for addr, nkHiddenAddr (#24322)
Browse files Browse the repository at this point in the history
fixes #24305, refs #23807

Since #23014 `nkHiddenAddr` is produced to fast assign array elements in
iterators. However the array access inside this `nkHiddenAddr` can get
folded at compile time, generating invalid code. In #23807, compile time
folding of regular `addr` expressions was changed to be prevented in
`transf` but `nkHiddenAddr` was not updated alongside it.

The method for preventing folding in `addr` in #23807 was also faulty,
it should only trigger on the immediate child node of the address rather
than all nodes nested inside it. This caused a regression as outlined in
[this
comment](#24322 (comment)).

To fix both issues, `addr` and `nkHiddenAddr` now both shallowly prevent
constant folding for their immediate children.

(cherry picked from commit 52cf7df)
  • Loading branch information
metagn authored and narimiran committed Oct 25, 2024
1 parent 372c6de commit 5dbdb49
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 13 deletions.
22 changes: 9 additions & 13 deletions compiler/transf.nim
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ type
isIntroducingNewLocalVars: bool # true if we are in `introducingNewLocalVars` (don't transform yields)
contSyms, breakSyms: seq[PSym] # to transform 'continue' and 'break'
deferDetected, tooEarly: bool
inAddr: bool
graph: ModuleGraph
idgen: IdGenerator

Expand Down Expand Up @@ -101,12 +100,12 @@ proc newTemp(c: PTransf, typ: PType, info: TLineInfo): PNode =
else:
result = newSymNode(r)

proc transform(c: PTransf, n: PNode): PNode
proc transform(c: PTransf, n: PNode, noConstFold = false): PNode

proc transformSons(c: PTransf, n: PNode): PNode =
proc transformSons(c: PTransf, n: PNode, noConstFold = false): PNode =
result = newTransNode(n)
for i in 0..<n.len:
result[i] = transform(c, n[i])
result[i] = transform(c, n[i], noConstFold)

proc newAsgnStmt(c: PTransf, kind: TNodeKind, le: PNode, ri: PNode; isFirstWrite: bool): PNode =
result = newTransNode(kind, ri.info, 2)
Expand Down Expand Up @@ -471,8 +470,8 @@ proc transformYield(c: PTransf, n: PNode): PNode =
result.add(introduceNewLocalVars(c, c.transCon.forLoopBody))
c.isIntroducingNewLocalVars = false

proc transformAddrDeref(c: PTransf, n: PNode, kinds: TNodeKinds): PNode =
result = transformSons(c, n)
proc transformAddrDeref(c: PTransf, n: PNode, kinds: TNodeKinds, isAddr = false): PNode =
result = transformSons(c, n, noConstFold = isAddr)
# inlining of 'var openarray' iterators; bug #19977
if n.typ.kind != tyOpenArray and (c.graph.config.backend == backendCpp or sfCompileToCpp in c.module.flags): return
var n = result
Expand Down Expand Up @@ -974,7 +973,7 @@ proc transformDerefBlock(c: PTransf, n: PNode): PNode =
result[i] = e0[i]
result[e0.len-1] = newTreeIT(nkHiddenDeref, n.info, n.typ, e0[e0.len-1])

proc transform(c: PTransf, n: PNode): PNode =
proc transform(c: PTransf, n: PNode, noConstFold = false): PNode =
when false:
var oldDeferAnchor: PNode
if n.kind in {nkElifBranch, nkOfBranch, nkExceptBranch, nkElifExpr,
Expand Down Expand Up @@ -1041,12 +1040,9 @@ proc transform(c: PTransf, n: PNode): PNode =
of nkCallKinds:
result = transformCall(c, n)
of nkHiddenAddr:
result = transformAddrDeref(c, n, {nkHiddenDeref})
result = transformAddrDeref(c, n, {nkHiddenDeref}, isAddr = true)
of nkAddr:
let oldInAddr = c.inAddr
c.inAddr = true
result = transformAddrDeref(c, n, {nkDerefExpr, nkHiddenDeref})
c.inAddr = oldInAddr
result = transformAddrDeref(c, n, {nkDerefExpr, nkHiddenDeref}, isAddr = true)
of nkDerefExpr:
result = transformAddrDeref(c, n, {nkAddr, nkHiddenAddr})
of nkHiddenDeref:
Expand Down Expand Up @@ -1126,7 +1122,7 @@ proc transform(c: PTransf, n: PNode): PNode =
let exprIsPointerCast = n.kind in {nkCast, nkConv, nkHiddenStdConv} and
n.typ != nil and
n.typ.kind == tyPointer
if not exprIsPointerCast and not c.inAddr:
if not exprIsPointerCast and not noConstFold:
var cnst = getConstExpr(c.module, result, c.idgen, c.graph)
# we inline constants if they are not complex constants:
if cnst != nil and not dontInlineConstant(n, cnst):
Expand Down
32 changes: 32 additions & 0 deletions tests/iter/tfoldedaddr.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
discard """
output: '''
23
23
23
23
23
23
'''
"""

block: # issue #24305
iterator demo(a: openArray[int]): int =
for k in countUp(a[0], 19):
yield 23

for k in demo(@[17]):
echo k

block: # issue #24305 with array
iterator demo(a: array[1, int]): int =
for k in countUp(a[0], 19):
yield 23

for k in demo([17]):
echo k

block: # related regression
proc main =
let a = [0, 1, 2]
let x = addr a[low(a)]
main()

0 comments on commit 5dbdb49

Please sign in to comment.