macros: disallow creating special nodes manually (nim-lang#1235)

## Summary

Attempting to manually create a node (via `newNimNode`) of kind
`nnkError`, `nnkIdent`, `nnkSym`, or `nnkType` now results in a
compiler error. Creating them was previously allowed, but the
compiler could crash when such incomplete nodes appear in macro
output.

## Details

* all node kinds where the initial state is not valid (`nkError`,
  `nkIdent`, `nkSym`, and `nkType`) are forbidden
* the `opcNNewNimNode` implementation guards against the node kinds
* a VM event, diagnostic, and report is added for the error
* to not introduce a new variant, the `msg` variant is re-used for the
  new VM event
* the untyped `nnkType` test case is removed from `tempty_type_nodes`;
  macros cannot create untyped `nnkType` nodes anymore

compiler/ast/ast_types.nim

@@ -987,6 +987,7 @@ type
     adVmFieldNotFound
     adVmNotAField
     adVmFieldUnavailable
+    adVmCannotCreateNode
     adVmCannotSetChild
     adVmCannotAddChild
     adVmCannotGetChild
@@ -1009,7 +1010,8 @@ type
         indexSpec*: tuple[usedIdx, minIdx, maxIdx: Int128]
       of adVmErrInternal, adVmNilAccess, adVmIllegalConv,
           adVmFieldUnavailable, adVmFieldNotFound,
-          adVmCacheKeyAlreadyExists, adVmMissingCacheKey:
+          adVmCacheKeyAlreadyExists, adVmMissingCacheKey,
+          adVmCannotCreateNode:
         msg*: string
       of adVmCannotSetChild, adVmCannotAddChild, adVmCannotGetChild,
           adVmUnhandledException, adVmNoType, adVmNodeNotASymbol:

compiler/ast/report_enums.nim

@@ -247,6 +247,7 @@ type
     rvmCacheKeyAlreadyExists
     rvmFieldNotFound
     rvmFieldInavailable
+    rvmCannotCreateNode
     rvmCannotSetChild
     rvmCannotAddChild
     rvmCannotGetChild

compiler/front/cli_reporter.nim

@@ -2949,6 +2949,9 @@ proc reportBody*(conf: ConfigRef, r: VMReport): string =
   of rvmFieldNotFound:
     result = "node lacks field: " & r.str
 
+  of rvmCannotCreateNode:
+    result = "cannot manually create a node of kind: n" & r.str
+
   of rvmCannotSetChild:
     result = "cannot set child of node kind: n" & $r.ast.kind
 
@@ -3867,7 +3870,8 @@ func astDiagToLegacyReport(conf: ConfigRef, diag: PAstDiag): Report {.inline.} =
         location: some location,
         reportInst: diag.instLoc.toReportLineInfo)
     of rvmErrInternal, rvmNilAccess, rvmIllegalConv, rvmFieldInavailable,
-        rvmFieldNotFound, rvmCacheKeyAlreadyExists, rvmMissingCacheKey:
+        rvmFieldNotFound, rvmCacheKeyAlreadyExists, rvmMissingCacheKey,
+        rvmCannotCreateNode:
       vmRep = VMReport(
         kind: kind,
         str: diag.vmErr.msg,

compiler/front/msgs.nim

@@ -391,6 +391,7 @@ func astDiagVmToLegacyReportKind*(
   of adVmFieldNotFound: rvmFieldNotFound
   of adVmNotAField: rvmNotAField
   of adVmFieldUnavailable: rvmFieldInavailable
+  of adVmCannotCreateNode: rvmCannotCreateNode
   of adVmCannotSetChild: rvmCannotSetChild
   of adVmCannotAddChild: rvmCannotAddChild
   of adVmCannotGetChild: rvmCannotGetChild

compiler/vm/vm.nim

@@ -2752,13 +2752,21 @@ proc rawExecute(c: var TCtx, t: var VmThread, pc: var int): YieldReason =
         raiseVmError(VmEvent(kind: vmEvtFieldNotFound, msg: "strVal"))
     of opcNNewNimNode:
       decodeBC(rkNimNode)
-      var k = regs[rb].intVal
+      let k = regs[rb].intVal
       guestValidate(k in 0..ord(high(TNodeKind)),
         "request to create a NimNode of invalid kind")
 
+      let kind = TNodeKind(int(k))
+      case kind
+      of nkError, nkIdent, nkSym, nkType:
+        # nodes that cannot be created manually
+        raiseVmError(VmEvent(kind: vmEvtCannotCreateNode, msg: $kind))
+      of nkWithSons, nkLiterals, nkCommentStmt, nkEmpty:
+        discard "the uninitialized state is valid"
+
       let cc = regs[rc].nimNode
 
-      let x = newNodeI(TNodeKind(int(k)),
+      let x = newNodeI(kind,
         if cc.kind != nkNilLit:
           cc.info
         elif c.comesFromHeuristic.line != 0'u16:
@@ -2767,8 +2775,7 @@ proc rawExecute(c: var TCtx, t: var VmThread, pc: var int): YieldReason =
           c.callsite[1].info
         else:
           c.debug[pc])
-      # prevent crashes in the compiler resulting from wrong macros:
-      if x.kind == nkIdent: x.ident = c.cache.emptyIdent
+
       regs[ra].nimNode = x
     of opcNCopyNimNode:
       decodeB(rkNimNode)
@@ -3017,6 +3024,7 @@ func vmEventToAstDiagVmError*(evt: VmEvent): AstDiagVmError {.inline.} =
     of vmEvtFieldNotFound: adVmFieldNotFound
     of vmEvtNotAField: adVmNotAField
     of vmEvtFieldUnavailable: adVmFieldUnavailable
+    of vmEvtCannotCreateNode: adVmCannotCreateNode
     of vmEvtCannotSetChild: adVmCannotSetChild
     of vmEvtCannotAddChild: adVmCannotAddChild
     of vmEvtCannotGetChild: adVmCannotGetChild
@@ -3048,7 +3056,8 @@ func vmEventToAstDiagVmError*(evt: VmEvent): AstDiagVmError {.inline.} =
           indexSpec: evt.indexSpec)
       of adVmErrInternal, adVmNilAccess, adVmIllegalConv,
           adVmFieldUnavailable, adVmFieldNotFound,
-          adVmCacheKeyAlreadyExists, adVmMissingCacheKey:
+          adVmCacheKeyAlreadyExists, adVmMissingCacheKey,
+          adVmCannotCreateNode:
         AstDiagVmError(
           kind: kind,
           msg: evt.msg)

compiler/vm/vmdef.nim

@@ -598,6 +598,7 @@ type
     vmEvtFieldNotFound
     vmEvtNotAField
     vmEvtFieldUnavailable
+    vmEvtCannotCreateNode
     vmEvtCannotSetChild
     vmEvtCannotAddChild
     vmEvtCannotGetChild
@@ -623,7 +624,8 @@ type
         indexSpec*: tuple[usedIdx, minIdx, maxIdx: Int128]
       of vmEvtErrInternal, vmEvtNilAccess, vmEvtIllegalConv,
           vmEvtFieldUnavailable, vmEvtFieldNotFound,
-          vmEvtCacheKeyAlreadyExists, vmEvtMissingCacheKey:
+          vmEvtCacheKeyAlreadyExists, vmEvtMissingCacheKey,
+          vmEvtCannotCreateNode:
         msg*: string
       of vmEvtCannotSetChild, vmEvtCannotAddChild, vmEvtCannotGetChild,
          vmEvtNoType, vmEvtNodeNotASymbol:

compiler/vm/vmrunner.nim

@@ -273,6 +273,7 @@ func vmEventToLegacyReportKind(evt: VmEventKind): ReportKind {.inline.} =
   of vmEvtFieldNotFound: rvmFieldNotFound
   of vmEvtNotAField: rvmNotAField
   of vmEvtFieldUnavailable: rvmFieldInavailable
+  of vmEvtCannotCreateNode: rvmCannotCreateNode
   of vmEvtCannotSetChild: rvmCannotSetChild
   of vmEvtCannotAddChild: rvmCannotAddChild
   of vmEvtCannotGetChild: rvmCannotGetChild

tests/lang_callable/macros/tdisallow_create_special_nodes.nim

@@ -0,0 +1,26 @@
+discard """
+  description: '''
+    Ensure that an error is reported when attempting to create special
+    atom nodes with `newNimNode`
+  '''
+  matrix: "--errorMax:4"
+  action: reject
+"""
+
+import std/macros
+
+static:
+  discard newNimNode(nnkError) #[tt.Error
+                    ^ cannot manually create a node of kind: nnkError]#
+
+static:
+  discard newNimNode(nnkIdent) #[tt.Error
+                    ^ cannot manually create a node of kind: nnkIdent]#
+
+static:
+  discard newNimNode(nnkSym) #[tt.Error
+                    ^ cannot manually create a node of kind: nnkSym]#
+
+static:
+  discard newNimNode(nnkType) #[tt.Error
+                    ^ cannot manually create a node of kind: nnkType]#

tests/lang_callable/macros/tempty_type_nodes.nim

@@ -1,6 +1,6 @@
 discard """
   description: '''
-    Ensure that untyped `nnkEmpty` and `nnkType` nodes used in type positions
+    Ensure that untyped `nnkEmpty` nodes used in type positions
     result in a proper error
   '''
   target: native
@@ -17,11 +17,3 @@ macro test1(): untyped =
     newNimNode(nnkEmpty)
 
 test1()
-
-macro test2(): untyped =
-  # test case #2: untyped ``nnkType`` node in type position
-  nnkConv.newTree(newNimNode(nnkType)): #[tt.Error
-                            ^ type expected, but expression has no type]#
-    newNimNode(nnkEmpty)
-
-test2()