From 9ff0333a4ccffb253f03fc113d086bbc81b566e9 Mon Sep 17 00:00:00 2001
From: ringabout <43030857+ringabout@users.noreply.github.com>
Date: Mon, 9 Sep 2024 17:22:37 +0800
Subject: [PATCH] fixes #21353; fixes default closure in the VM (#24070)

fixes #21353

```nim
  result = newNodeIT(nkTupleConstr, info, t)
  result.add(newNodeIT(nkNilLit, info, t))
  result.add(newNodeIT(nkNilLit, info, t))
```
The old implementation uses `t` which is the type of the closure
function as its type. It is not correct and generates ((nil, nil), (nil,
nil)) for `default(closures)`. This PR creates `(tyPointer, tyPointer)`
for fake closure types just like what cctypes do.
---
 compiler/vm.nim      | 12 ++++++------
 compiler/vmgen.nim   | 34 +++++++++++++++++-----------------
 tests/vm/tvmmisc.nim |  4 ++++
 3 files changed, 27 insertions(+), 23 deletions(-)

diff --git a/compiler/vm.nim b/compiler/vm.nim
index a9a25e4ea5..ce14a9496e 100644
--- a/compiler/vm.nim
+++ b/compiler/vm.nim
@@ -507,7 +507,7 @@ proc setLenSeq(c: PCtx; node: PNode; newLen: int; info: TLineInfo) =
   setLen(node.sons, newLen)
   if oldLen < newLen:
     for i in oldLen..<newLen:
-      node[i] = getNullValue(typ.elementType, info, c.config)
+      node[i] = getNullValue(c, typ.elementType, info, c.config)
 
 const
   errNilAccess = "attempt to access a nil address"
@@ -1424,7 +1424,7 @@ proc rawExecute(c: PCtx, start: int, tos: PStackFrame): TFullReg =
         var newFrame = PStackFrame(prc: prc, comesFrom: pc, next: tos)
         newSeq(newFrame.slots, prc.offset+ord(isClosure))
         if not isEmptyType(prc.typ.returnType):
-          putIntoReg(newFrame.slots[0], getNullValue(prc.typ.returnType, prc.info, c.config))
+          putIntoReg(newFrame.slots[0], getNullValue(c, prc.typ.returnType, prc.info, c.config))
         for i in 1..rc-1:
           newFrame.slots[i] = regs[rb+i]
         if isClosure:
@@ -1557,7 +1557,7 @@ proc rawExecute(c: PCtx, start: int, tos: PStackFrame): TFullReg =
     of opcNew:
       ensureKind(rkNode)
       let typ = c.types[instr.regBx - wordExcess]
-      regs[ra].node = getNullValue(typ, c.debug[pc], c.config)
+      regs[ra].node = getNullValue(c, typ, c.debug[pc], c.config)
       regs[ra].node.flags.incl nfIsRef
     of opcNewSeq:
       let typ = c.types[instr.regBx - wordExcess]
@@ -1569,7 +1569,7 @@ proc rawExecute(c: PCtx, start: int, tos: PStackFrame): TFullReg =
       regs[ra].node.typ = typ
       newSeq(regs[ra].node.sons, count)
       for i in 0..<count:
-        regs[ra].node[i] = getNullValue(typ.elementType, c.debug[pc], c.config)
+        regs[ra].node[i] = getNullValue(c, typ.elementType, c.debug[pc], c.config)
     of opcNewStr:
       decodeB(rkNode)
       regs[ra].node = newNodeI(nkStrLit, c.debug[pc])
@@ -1581,7 +1581,7 @@ proc rawExecute(c: PCtx, start: int, tos: PStackFrame): TFullReg =
     of opcLdNull:
       ensureKind(rkNode)
       let typ = c.types[instr.regBx - wordExcess]
-      regs[ra].node = getNullValue(typ, c.debug[pc], c.config)
+      regs[ra].node = getNullValue(c, typ, c.debug[pc], c.config)
       # opcLdNull really is the gist of the VM's problems: should it load
       # a fresh null to  regs[ra].node  or to regs[ra].node[]? This really
       # depends on whether regs[ra] represents the variable itself or whether
@@ -2310,7 +2310,7 @@ proc execProc*(c: PCtx; sym: PSym; args: openArray[PNode]): PNode =
 
       # setup parameters:
       if not isEmptyType(sym.typ.returnType) or sym.kind == skMacro:
-        putIntoReg(tos.slots[0], getNullValue(sym.typ.returnType, sym.info, c.config))
+        putIntoReg(tos.slots[0], getNullValue(c, sym.typ.returnType, sym.info, c.config))
       # XXX We could perform some type checking here.
       for i in 0..<sym.typ.paramsLen:
         putIntoReg(tos.slots[i+1], args[i])
diff --git a/compiler/vmgen.nim b/compiler/vmgen.nim
index 1879245abc..20249ae734 100644
--- a/compiler/vmgen.nim
+++ b/compiler/vmgen.nim
@@ -1690,10 +1690,10 @@ proc importcSym(c: PCtx; info: TLineInfo; s: PSym) =
     localError(c.config, info,
                "cannot 'importc' variable at compile time; " & s.name.s)
 
-proc getNullValue*(typ: PType, info: TLineInfo; conf: ConfigRef): PNode
+proc getNullValue*(c: PCtx; typ: PType, info: TLineInfo; conf: ConfigRef): PNode
 
 proc genGlobalInit(c: PCtx; n: PNode; s: PSym) =
-  c.globals.add(getNullValue(s.typ, n.info, c.config))
+  c.globals.add(getNullValue(c, s.typ, n.info, c.config))
   s.position = c.globals.len
   # This is rather hard to support, due to the laziness of the VM code
   # generator. See tests/compile/tmacro2 for why this is necessary:
@@ -1872,21 +1872,21 @@ proc genArrAccess(c: PCtx; n: PNode; dest: var TDest; flags: TGenFlags) =
     let opc = if gfNodeAddr in flags: opcLdArrAddr else: opcLdArr
     genArrAccessOpcode(c, n, dest, opc, flags)
 
-proc getNullValueAux(t: PType; obj: PNode, result: PNode; conf: ConfigRef; currPosition: var int) =
+proc getNullValueAux(c: PCtx; t: PType; obj: PNode, result: PNode; conf: ConfigRef; currPosition: var int) =
   if t != nil and t.baseClass != nil:
     let b = skipTypes(t.baseClass, skipPtrs)
-    getNullValueAux(b, b.n, result, conf, currPosition)
+    getNullValueAux(c, b, b.n, result, conf, currPosition)
   case obj.kind
   of nkRecList:
-    for i in 0..<obj.len: getNullValueAux(nil, obj[i], result, conf, currPosition)
+    for i in 0..<obj.len: getNullValueAux(c, nil, obj[i], result, conf, currPosition)
   of nkRecCase:
-    getNullValueAux(nil, obj[0], result, conf, currPosition)
+    getNullValueAux(c, nil, obj[0], result, conf, currPosition)
     for i in 1..<obj.len:
-      getNullValueAux(nil, lastSon(obj[i]), result, conf, currPosition)
+      getNullValueAux(c, nil, lastSon(obj[i]), result, conf, currPosition)
   of nkSym:
     let field = newNodeI(nkExprColonExpr, result.info)
     field.add(obj)
-    let value = getNullValue(obj.sym.typ, result.info, conf)
+    let value = getNullValue(c, obj.sym.typ, result.info, conf)
     value.flags.incl nfSkipFieldChecking
     field.add(value)
     result.add field
@@ -1894,7 +1894,7 @@ proc getNullValueAux(t: PType; obj: PNode, result: PNode; conf: ConfigRef; currP
     inc currPosition
   else: globalError(conf, result.info, "cannot create null element for: " & $obj)
 
-proc getNullValue(typ: PType, info: TLineInfo; conf: ConfigRef): PNode =
+proc getNullValue(c: PCtx; typ: PType, info: TLineInfo; conf: ConfigRef): PNode =
   var t = skipTypes(typ, abstractRange+{tyStatic, tyOwned}-{tyTypeDesc})
   case t.kind
   of tyBool, tyEnum, tyChar, tyInt..tyInt64:
@@ -1914,22 +1914,22 @@ proc getNullValue(typ: PType, info: TLineInfo; conf: ConfigRef): PNode =
       result = newNodeIT(nkNilLit, info, t)
     else:
       result = newNodeIT(nkTupleConstr, info, t)
-      result.add(newNodeIT(nkNilLit, info, t))
-      result.add(newNodeIT(nkNilLit, info, t))
+      result.add(newNodeIT(nkNilLit, info, getSysType(c.graph, info, tyPointer)))
+      result.add(newNodeIT(nkNilLit, info, getSysType(c.graph, info, tyPointer)))
   of tyObject:
     result = newNodeIT(nkObjConstr, info, t)
     result.add(newNodeIT(nkEmpty, info, t))
     # initialize inherited fields, and all in the correct order:
     var currPosition = 0
-    getNullValueAux(t, t.n, result, conf, currPosition)
+    getNullValueAux(c, t, t.n, result, conf, currPosition)
   of tyArray:
     result = newNodeIT(nkBracket, info, t)
     for i in 0..<toInt(lengthOrd(conf, t)):
-      result.add getNullValue(elemType(t), info, conf)
+      result.add getNullValue(c, elemType(t), info, conf)
   of tyTuple:
     result = newNodeIT(nkTupleConstr, info, t)
     for a in t.kids:
-      result.add getNullValue(a, info, conf)
+      result.add getNullValue(c, a, info, conf)
   of tySet:
     result = newNodeIT(nkCurly, info, t)
   of tySequence, tyOpenArray:
@@ -1957,7 +1957,7 @@ proc genVarSection(c: PCtx; n: PNode) =
         if s.position == 0:
           if importcCond(c, s): c.importcSym(a.info, s)
           else:
-            let sa = getNullValue(s.typ, a.info, c.config)
+            let sa = getNullValue(c, s.typ, a.info, c.config)
             #if s.ast.isNil: getNullValue(s.typ, a.info)
             #else: s.ast
             assert sa.kind != nkCall
@@ -1979,7 +1979,7 @@ proc genVarSection(c: PCtx; n: PNode) =
           # the problem is that closure types are tuples in VM, but the types of its children
           # shouldn't have the same type as closure types.
           let tmp = c.genx(a[0], {gfNodeAddr})
-          let sa = getNullValue(s.typ, a.info, c.config)
+          let sa = getNullValue(c, s.typ, a.info, c.config)
           let val = c.genx(sa)
           c.genAdditionalCopy(sa, opcWrDeref, tmp, 0, val)
           c.freeTemp(val)
@@ -2182,7 +2182,7 @@ proc gen(c: PCtx; n: PNode; dest: var TDest; flags: TGenFlags = {}) =
       genLit(c, n, dest)
   of nkUIntLit..pred(nkNilLit): genLit(c, n, dest)
   of nkNilLit:
-    if not n.typ.isEmptyType: genLit(c, getNullValue(n.typ, n.info, c.config), dest)
+    if not n.typ.isEmptyType: genLit(c, getNullValue(c, n.typ, n.info, c.config), dest)
     else: unused(c, n, dest)
   of nkAsgn, nkFastAsgn, nkSinkAsgn:
     unused(c, n, dest)
diff --git a/tests/vm/tvmmisc.nim b/tests/vm/tvmmisc.nim
index 6160702ad2..6aeac5529b 100644
--- a/tests/vm/tvmmisc.nim
+++ b/tests/vm/tvmmisc.nim
@@ -790,3 +790,7 @@ block: # bug #23925
 
   static: bar()
   bar()
+
+static: # bug #21353
+  var s: proc () = default(proc ())
+  doAssert s == nil