don't traverse inner procs to lift locals in closure iters (#24876)

fixes #24863, refs #23787 and #24316

Working off the minimized example, my understanding of the issue is: `n`
captures `r` as `:envP.r1` where `:envP` is the environment of `b`, then
`proc () = n()` does the lambda lifting of `n` again (which isn't done
if the `proc ()` is marked `{.closure.}`, hence the workaround) which
then captures the `:envP` as another field inside the `:envP`, so it
generates `:envP.:envP_2.r1` but the `.:envP_2` field is `nil`, so it
causes a segfault.

The problem is that the capture of `r` in `n` is done inside
`detectCapturedVars` for the surrounding closure iterator: inner procs
are not special cased and traversed as regular nodes, so it thinks it's
inside the iterator and generates a field access of `:envP` freely. The
lambda lifting version of `detectCapturedVars` ignores inner procs and
works off of symbol uses (anonymous iterator and lambda declarations
pretend their symbol is used).

As a naive solution, closure iterators now also ignore inner proc
declarations same as `lambdalifting.detectCapturedVars`, but unlike it
they also don't do anything for the inner proc symbols. Lambdalifting
seems to properly handle the lifted variables but in the worst case we
can also make sure `closureiters.detectCapturedVars` traverses inner
procs by marking every local of the closure iter used in them as needing
lifting (but not doing the lifting). This does not seem necessary for
now so it's not done (was done and reverted in [this
commit](9bb39a9259)),
but regressions are still possible

(cherry picked from commit c06bb6cc03)
This commit is contained in:
metagn
2025-04-15 20:29:46 +03:00
committed by narimiran
parent e7244c0d28
commit a8d87c041c
4 changed files with 65 additions and 1 deletions

View File

@@ -1451,6 +1451,14 @@ proc detectCapturedVars(c: var Ctx, n: PNode, stateIdx: int) =
detectCapturedVars(c, n[0][1], stateIdx)
else:
detectCapturedVars(c, n[0], stateIdx)
of nkEmpty..pred(nkSym), succ(nkSym)..nkNilLit,
nkTemplateDef, nkTypeSection, nkProcDef, nkMethodDef,
nkConverterDef, nkMacroDef, nkFuncDef, nkCommentStmt,
nkTypeOfExpr, nkMixinStmt, nkBindStmt:
discard
of nkLambdaKinds, nkIteratorDef:
if n.typ != nil:
detectCapturedVars(c, n[namePos], stateIdx)
else:
for i in 0 ..< n.safeLen:
detectCapturedVars(c, n[i], stateIdx)
@@ -1481,6 +1489,12 @@ proc liftLocals(c: var Ctx, n: PNode): PNode =
n[0][1] = liftLocals(c, n[0][1])
else:
n[0] = liftLocals(c, n[0])
of nkEmpty..pred(nkSym), succ(nkSym)..nkNilLit,
nkTemplateDef, nkTypeSection, nkProcDef, nkMethodDef,
nkConverterDef, nkMacroDef, nkFuncDef, nkCommentStmt,
nkTypeOfExpr, nkMixinStmt, nkBindStmt,
nkLambdaKinds, nkIteratorDef:
discard
else:
for i in 0 ..< n.safeLen:
n[i] = liftLocals(c, n[i])

View File

@@ -199,7 +199,7 @@ proc interestingVar(s: PSym): bool {.inline.} =
proc illegalCapture(s: PSym): bool {.inline.} =
result = classifyViewType(s.typ) != noView or s.kind == skResult
proc isInnerProc(s: PSym): bool =
proc isInnerProc*(s: PSym): bool =
if s.kind in {skProc, skFunc, skMethod, skConverter, skIterator} and s.magic == mNone:
result = s.skipGenericOwner.kind in routineKinds
else:

30
tests/iter/t24863.nim Normal file
View File

@@ -0,0 +1,30 @@
# issue #24863
type M = object
p: iterator(): M {.gcsafe.}
template h(f: M): int =
yield f
456
proc s(): M =
iterator g(): M {.closure.} = discard
let v = M(p: g)
doAssert(not isNil(v.p))
discard v.p()
v
proc c(): M =
iterator b(): M {.closure.} =
let r = h(s())
doAssert r == 456
proc n(): M =
iterator y(): M {.closure.} =
let _ = r
let _ = y
let _ = proc () = discard n()
let j = M(p: b)
doAssert(not isNil(j.p))
discard j.p()
let _ = c()

View File

@@ -25,6 +25,9 @@ Test 7:
0
1
2
Test 8:
123
456
'''
"""
@@ -156,3 +159,20 @@ block: # issue #12487
doAssert s == @["something"]
main()
block: # minimized issue #24863
echo "Test 8:"
proc c() =
iterator b(): int {.closure.} =
let r = 456
yield 123
proc n() =
echo r
let a = proc () = n()
a()
let j = b
echo j()
discard j()
c()