Lock semchecked ast for macros (#11883) [bugfix]

* reject to modify type checked AST
* add flag to back out
* Introduce legacy feature set.

compiler/commands.nim

@@ -50,15 +50,16 @@ const
       "Compiled at $4\n" &
       "Copyright (c) 2006-" & copyrightYear & " by Andreas Rumpf\n"
 
-const
+proc genFeatureDesc[T: enum](t: typedesc[T]): string {.compileTime.} =
-  Usage = slurp"../doc/basicopt.txt".replace(" //", " ")
-  FeatureDesc = block:
   var x = ""
-    for f in low(Feature)..high(Feature):
+  for f in low(T)..high(T):
     if x.len > 0: x.add "|"
     x.add $f
   x
-  AdvancedUsage = slurp"../doc/advopt.txt".replace(" //", " ") % FeatureDesc
+
+const
+  Usage = slurp"../doc/basicopt.txt".replace(" //", " ")
+  AdvancedUsage = slurp"../doc/advopt.txt".replace(" //", " ") % [genFeatureDesc(Feature), genFeatureDesc(LegacyFeature)]
 
 proc getCommandLineDesc(conf: ConfigRef): string =
   result = (HelpMessage % [VersionAsString, platform.OS[conf.target.hostOS].name,
@@ -744,6 +745,11 @@ proc processSwitch*(switch, arg: string, pass: TCmdLinePass, info: TLineInfo;
         conf.features.incl parseEnum[Feature](arg)
       except ValueError:
         localError(conf, info, "unknown experimental feature")
+  of "legacy":
+    try:
+      conf.legacyFeatures.incl parseEnum[LegacyFeature](arg)
+    except ValueError:
+      localError(conf, info, "unknown obsolete feature")
   of "nocppexceptions":
     expectNoArg(conf, switch, arg, pass, info)
     incl(conf.globalOptions, optNoCppExceptions)

compiler/options.nim

@@ -135,6 +135,12 @@ type
       ## which itself requires `nimble install libffi`, see #10150
       ## Note: this feature can't be localized with {.push.}
 
+  LegacyFeature* = enum
+    allowSemcheckedAstModification,
+      ## Allows to modify a NimNode where the type has already been
+      ## flaged with nfSem. If you actually do this, it will cause
+      ## bugs.
+
   SymbolFilesOption* = enum
     disabledSf, writeOnlySf, readOnlySf, v2Sf
 
@@ -196,6 +202,7 @@ type
     cppDefines*: HashSet[string] # (*)
     headerFile*: string
     features*: set[Feature]
+    legacyFeatures*: set[LegacyFeature]
     arguments*: string ## the arguments to be passed to the program that
                        ## should be run
     ideCmd*: IdeCmd
@@ -315,7 +322,7 @@ proc newConfigRef*(): ConfigRef =
     m: initMsgConfig(),
     evalExpr: "",
     cppDefines: initHashSet[string](),
-    headerFile: "", features: {}, foreignPackageNotes: {hintProcessing, warnUnknownMagic,
+    headerFile: "", features: {}, legacyFeatures: {}, foreignPackageNotes: {hintProcessing, warnUnknownMagic,
     hintQuitCalled, hintExecuting},
     notes: NotesVerbosity[1], mainPackageNotes: NotesVerbosity[1],
     configVars: newStringTable(modeStyleInsensitive),

compiler/vm.nim

@@ -1392,27 +1392,32 @@ proc rawExecute(c: PCtx, start: int, tos: PStackFrame): TFullReg =
       decodeBC(rkNode)
       let idx = regs[rb].intVal.int
       var dest = regs[ra].node
-      if dest.kind notin {nkEmpty..nkNilLit} and idx <% dest.len:
+      if nfSem in dest.flags and allowSemcheckedAstModification notin c.config.legacyFeatures:
-        dest.sons[idx] = regs[rc].node
+        stackTrace(c, tos, pc, "typechecked nodes may not be modified")
-      else:
+      elif dest.kind in {nkEmpty..nkNilLit} or idx >=% dest.len:
         stackTrace(c, tos, pc, formatErrorIndexBound(idx, dest.len-1))
+      else:
+        dest.sons[idx] = regs[rc].node
     of opcNAdd:
       decodeBC(rkNode)
       var u = regs[rb].node
-      if u.kind notin {nkEmpty..nkNilLit}:
+      if nfSem in u.flags and allowSemcheckedAstModification notin c.config.legacyFeatures:
-        u.add(regs[rc].node)
+        stackTrace(c, tos, pc, "typechecked nodes may not be modified")
-      else:
+      elif u.kind in {nkEmpty..nkNilLit}:
         stackTrace(c, tos, pc, "cannot add to node kind: " & $u.kind)
+      else:
+        u.add(regs[rc].node)
       regs[ra].node = u
     of opcNAddMultiple:
       decodeBC(rkNode)
       let x = regs[rc].node
       var u = regs[rb].node
-      if u.kind notin {nkEmpty..nkNilLit}:
+      if nfSem in u.flags and allowSemcheckedAstModification notin c.config.legacyFeatures:
-        # XXX can be optimized:
+        stackTrace(c, tos, pc, "typechecked nodes may not be modified")
-        for i in 0..<x.len: u.add(x.sons[i])
+      elif u.kind in {nkEmpty..nkNilLit}:
-      else:
         stackTrace(c, tos, pc, "cannot add to node kind: " & $u.kind)
+      else:
+        for i in 0 ..< x.len: u.add(x.sons[i])
       regs[ra].node = u
     of opcNKind:
       decodeB(rkInt)

doc/advopt.txt

@@ -120,6 +120,9 @@ Advanced options:
   --errorMax:N              stop compilation after N errors; 0 means unlimited
   --experimental:$1
                             enable experimental language feature
+  --legacy:$2
+                            enable obsolete/legacy language feature
+                            legacy code.
   --newruntime              use an alternative runtime that uses destructors
                             and that uses a shared heap via -d:useMalloc
   --profiler:on|off         enable profiling; requires `import nimprof`, and

tests/macros/tlocktypednode1.nim

@@ -0,0 +1,12 @@
+discard """
+errormsg: "typechecked nodes may not be modified"
+"""
+
+import macros
+
+macro doSomething(arg: typed): untyped =
+  echo arg.treeREpr
+  result = arg
+  result.add newCall(bindSym"echo", newLit(1))
+
+doSomething((echo(1); echo(2)))

tests/macros/tlocktypednode2.nim

@@ -0,0 +1,12 @@
+discard """
+errormsg: "typechecked nodes may not be modified"
+"""
+
+import macros
+
+macro doSomething(arg: typed): untyped =
+  echo arg.treeREpr
+  result = arg
+  result[0] = newCall(bindSym"echo", newLit(1))
+
+doSomething((echo(1); echo(2)))

tests/macros/tlocktypednode3.nim

@@ -0,0 +1,15 @@
+discard """
+errormsg: "typechecked nodes may not be modified"
+"""
+
+import macros
+
+macro doSomething(arg: typed): untyped =
+  echo arg.treeREpr
+  result = arg
+  result.add(
+    newCall(bindSym"echo", newLit(3)),
+    newCall(bindSym"echo", newLit(1))
+  )
+
+doSomething((echo(1); echo(2)))

tests/macros/tmacro7.nim

@@ -1,19 +1,20 @@
 discard """
-  output: '''calling!stuff
+output: '''
+calling!stuff
 calling!stuff
 '''
-  joinable: false
+disabled: true
 """
 
+# this test modifies an already semchecked ast (bad things happen)
+# this test relies on the bug #4547
 # issue #7792
 
 import macros
 
-
 proc callProc(str: string) =
   echo "calling!" & str
 
-
 macro testMacro(code: typed): untyped =
   let stmtList = newNimNode(nnkStmtList)
 
@@ -27,11 +28,9 @@ macro testMacro(code: typed): untyped =
 
   result = newEmptyNode()
 
-
 proc main() {.testMacro.} =
   echo "test"
   echo "test2"
 
-
 when isMainModule:
   main()