Skip to content

Commit

Permalink
lambdalifting: ignore AST literals for var capture (#697)
Browse files Browse the repository at this point in the history
## Summary

`nkNimNodeLit` are no longer scanned during var capture detection,
ensuring that literals do not alter capture analysis of a routine
holding typed literal AST with symbols from the outer routine.

## Details

Capture detection is done in two modules `lambdalifting` and `sempass2`
in the `detectCapturedVars` and `detectCapture` procedures,
respectively. With this change both procedures do not traverse into
`nkNimNodeLit` (treating them as more opaque literal data) for literal
analysis. A regression tests has been added a test for this (thanks to
Zerbina).

---------

Co-authored-by: zerbina <[email protected]>
  • Loading branch information
saem and zerbina authored May 11, 2023
1 parent 7f2a42c commit 3b80b24
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 0 deletions.
2 changes: 2 additions & 0 deletions compiler/sem/lambdalifting.nim
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,8 @@ proc detectCapturedVars(n: PNode; owner: PSym; c: var DetectionPass) =
detectCapturedVars(n[namePos], owner, c)
of nkReturnStmt:
detectCapturedVars(n[0], owner, c)
of nkNimNodeLit:
discard "skip node literals as they're data not code"
else:
for i in 0..<n.len:
detectCapturedVars(n[i], owner, c)
Expand Down
2 changes: 2 additions & 0 deletions compiler/sem/sempass2.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1602,6 +1602,8 @@ proc detectCapture(owner, top: PSym, n: PNode, marker: var IntSet): PNode =
result = detectCapture(owner, top, n[1], marker)
of nkLambdaKinds:
result = detectCapture(owner, top, n[namePos], marker)
of nkNimNodeLit:
discard "ignore node literals as they're data not code"
else:
for it in n.items:
result = detectCapture(owner, top, it, marker)
Expand Down
29 changes: 29 additions & 0 deletions tests/lang_callable/macros/tnodeliteral.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
discard """
description: "Tests for passing node literals (AST) around"
"""

import std/macros

block node_literals_are_opaque_in_static_analysis:
block regression_ignore_node_literals_in_lambdalifting:
proc sym() =
discard

proc outer() {.compileTime.} =
var x = 0

proc sym() {.compileTime.} =
# close over an outer local in order to make `sym` a closure procedure:
x = 0

proc inner() {.compileTime.} =
let x = bindSym"sym" # create a symbol choice that includes the closure
# procedure

# `inner` doesn't close over a local and also doesn't access a closure
# procedure, so it must not be a closure procedure
doAssert inner isnot "closure"

static:
# call the procedure to make sure it passes the lambda-lifting pass
outer()

0 comments on commit 3b80b24

Please sign in to comment.