From bffd2e0330db9969519787044ba2b66e4c1b7ba8 Mon Sep 17 00:00:00 2001
From: metagn <metagngn@gmail.com>
Date: Sat, 12 Oct 2024 23:39:59 +0300
Subject: [PATCH] don't evaluate "cannot eval" errors with nim check (#24289)

fixes #24288, refs #23625

Since #23625 "cannot evaluate" errors during VM code generation are
"soft" errors with `nim check`, i.e. the code generation isn't halted
(except for the current proc which `return`s which can cause wrong
codegen) and the expression is still attempted to be evaluated. Now,
these errors signal to the VM that the current generated VM code cannot
be evaluated, and so instead of evaluating, an error node is returned.
This keeps the benefit of the "soft" errors without potentially crashing
the compiler on improperly generated VM code. Although maybe the
compiler might not be able to handle the generated error node in some
cases.

This fixes the chame example in #24288 but this is not tested in CI.
Presumably it or the compiler was doing something like `compiles()` on
code that can't run in the VM.

I would accept nicer ways of tracking non-evaluability than
`c.cannotEval = true` but I tried to keep it as harmless as possible.

(cherry picked from commit def1fea43a79b28dd669942de6c2193e297a48e8)
---
 compiler/vm.nim    | 22 +++++++++++++++++-----
 compiler/vmdef.nim |  1 +
 compiler/vmgen.nim |  1 +
 3 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/compiler/vm.nim b/compiler/vm.nim
index 161b025a65..93d2f9daed 100644
--- a/compiler/vm.nim
+++ b/compiler/vm.nim
@@ -2332,9 +2332,17 @@ proc execProc*(c: PCtx; sym: PSym; args: openArray[PNode]): PNode =
     localError(c.config, sym.info,
       "NimScript: attempt to call non-routine: " & sym.name.s)
 
+proc errorNode(idgen: IdGenerator; owner: PSym, n: PNode): PNode =
+  result = newNodeI(nkEmpty, n.info)
+  result.typ = newType(tyError, idgen, owner)
+  result.typ.flags.incl tfCheckedForDestructor
+
 proc evalStmt*(c: PCtx, n: PNode) =
   let n = transformExpr(c.graph, c.idgen, c.module, n)
   let start = genStmt(c, n)
+  if c.cannotEval:
+    c.cannotEval = false
+    return
   # execute new instructions; this redundant opcEof check saves us lots
   # of allocations in 'execute':
   if c.code[start].opcode != opcEof:
@@ -2345,7 +2353,10 @@ proc evalExpr*(c: PCtx, n: PNode): PNode =
   # `nim --eval:"expr"` might've used it at some point for idetools; could
   # be revived for nimsuggest
   let n = transformExpr(c.graph, c.idgen, c.module, n)
+  c.cannotEval = false
   let start = genExpr(c, n)
+  if c.cannotEval:
+    return errorNode(c.idgen, c.module, n)
   assert c.code[start].opcode != opcEof
   result = execute(c, start)
 
@@ -2398,7 +2409,10 @@ proc evalConstExprAux(module: PSym; idgen: IdGenerator;
   var c = PCtx g.vm
   let oldMode = c.mode
   c.mode = mode
+  c.cannotEval = false
   let start = genExpr(c, n, requiresValue = mode!=emStaticStmt)
+  if c.cannotEval:
+    return errorNode(idgen, prc, n)
   if c.code[start].opcode == opcEof: return newNodeI(nkEmpty, n.info)
   assert c.code[start].opcode != opcEof
   when debugEchoCode: c.echoCode start
@@ -2469,11 +2483,6 @@ iterator genericParamsInMacroCall*(macroSym: PSym, call: PNode): (PSym, PNode) =
 # to prevent endless recursion in macro instantiation
 const evalMacroLimit = 1000
 
-#proc errorNode(idgen: IdGenerator; owner: PSym, n: PNode): PNode =
-#  result = newNodeI(nkEmpty, n.info)
-#  result.typ = newType(tyError, idgen, owner)
-#  result.typ.flags.incl tfCheckedForDestructor
-
 proc evalMacroCall*(module: PSym; idgen: IdGenerator; g: ModuleGraph; templInstCounter: ref int;
                     n, nOrig: PNode, sym: PSym): PNode =
   #if g.config.errorCounter > 0: return errorNode(idgen, module, n)
@@ -2497,7 +2506,10 @@ proc evalMacroCall*(module: PSym; idgen: IdGenerator; g: ModuleGraph; templInstC
   c.comesFromHeuristic.line = 0'u16
   c.callsite = nOrig
   c.templInstCounter = templInstCounter
+  c.cannotEval = false
   let start = genProc(c, sym)
+  if c.cannotEval:
+    return errorNode(idgen, module, n)
 
   var tos = PStackFrame(prc: sym, comesFrom: 0, next: nil)
   let maxSlots = sym.offset
diff --git a/compiler/vmdef.nim b/compiler/vmdef.nim
index bdb0aeed15..42e58c63b8 100644
--- a/compiler/vmdef.nim
+++ b/compiler/vmdef.nim
@@ -270,6 +270,7 @@ type
     templInstCounter*: ref int # gives every template instantiation a unique ID, needed here for getAst
     vmstateDiff*: seq[(PSym, PNode)] # we remember the "diff" to global state here (feature for IC)
     procToCodePos*: Table[int, int]
+    cannotEval*: bool
 
   PStackFrame* = ref TStackFrame
   TStackFrame* {.acyclic.} = object
diff --git a/compiler/vmgen.nim b/compiler/vmgen.nim
index 0c7a49984f..ddb877d93a 100644
--- a/compiler/vmgen.nim
+++ b/compiler/vmgen.nim
@@ -1545,6 +1545,7 @@ template cannotEval(c: PCtx; n: PNode) =
   if c.config.cmd == cmdCheck:
     localError(c.config, n.info, "cannot evaluate at compile time: " & 
     n.renderTree)
+    c.cannotEval = true
     return
   globalError(c.config, n.info, "cannot evaluate at compile time: " &
     n.renderTree)