From 3b80b24e40e649917052e6c1730395b3c3e180ea Mon Sep 17 00:00:00 2001
From: Saem Ghani <saemghani+github@gmail.com>
Date: Thu, 11 May 2023 11:31:46 -0700
Subject: [PATCH] lambdalifting: ignore AST literals for var capture (#697)

## 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 <100542850+zerbina@users.noreply.github.com>
---
 compiler/sem/lambdalifting.nim              |  2 ++
 compiler/sem/sempass2.nim                   |  2 ++
 tests/lang_callable/macros/tnodeliteral.nim | 29 +++++++++++++++++++++
 3 files changed, 33 insertions(+)
 create mode 100644 tests/lang_callable/macros/tnodeliteral.nim

diff --git a/compiler/sem/lambdalifting.nim b/compiler/sem/lambdalifting.nim
index 780b99972b8..669c8d23cf7 100644
--- a/compiler/sem/lambdalifting.nim
+++ b/compiler/sem/lambdalifting.nim
@@ -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)
diff --git a/compiler/sem/sempass2.nim b/compiler/sem/sempass2.nim
index 02860ef7a3f..c84d14afcb4 100644
--- a/compiler/sem/sempass2.nim
+++ b/compiler/sem/sempass2.nim
@@ -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)
diff --git a/tests/lang_callable/macros/tnodeliteral.nim b/tests/lang_callable/macros/tnodeliteral.nim
new file mode 100644
index 00000000000..d280eaac6cb
--- /dev/null
+++ b/tests/lang_callable/macros/tnodeliteral.nim
@@ -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()