vm: convert gorge and staticExec to vmops (#878)

<!--- The Pull Request (=PR) message is what will get automatically used
as
the commit message when the PR is merged. Make sure that no line is
longer
than 72 characters -->

## Summary

Convert `gorge` and `staticExec` in the `system` module from magics to
VM callback operations.

## Details

Removal of the magic requires creating a conditional compilation step
based on the `nimskullReworkStaticExec` conditional symbol (see:
`condsym`). Two definitions of `gorge` and `staticExec` now exist, one
with the magic and one without. Without this, bootstrapping would no
longer work.

Also marked `gorge` as deprecated, `staticExec` is a far better name,
and noted that `staticExec` will have its return type revised to return
the exit code in addition to the output. The reason for the deprecation
is that `gorge` is just not as clear as `staticExec`, same for `slurp`.

This change enables future removal of the `mStaticExec` magic enum, and
more importantly the `opcGorge` VM opcode in the near future.

compiler/ast/ast_types.nim

@@ -742,7 +742,9 @@ type
     mDefined, mDeclared, mDeclaredInScope, mCompiles, mArrGet, mArrPut, mAsgn,
     mLow, mHigh, mSizeOf, mAlignOf, mOffsetOf, mTypeTrait,
     mIs, mOf, mAddr, mType, mTypeOf,
-    mPlugin, mEcho, mShallowCopy, mSlurp, mStaticExec, mStatic,
+    mPlugin, mEcho, mShallowCopy, mSlurp,
+    mStaticExec ## deprecated, remove me
+    mStatic,
     mParseExprToAst, mParseStmtToAst, mExpandToAst, mQuoteAst,
     mInc, mDec, mOrd,
     mNew, mNewSeq, mNewSeqOfCap,

compiler/front/condsyms.nim

@@ -70,3 +70,5 @@ proc initDefines*(symbols: StringTableRef) =
   defineSymbol("nimHasTyConceptRemoved")
   defineSymbol("nimHasNkBreakStateNodeRemoved")
   defineSymbol("nimHasTyOwnedRemoved")
+
+  defineSymbol("nimskullReworkStaticExec")

compiler/vm/vm.nim

@@ -53,7 +53,6 @@ import
   ],
   compiler/vm/[
     vmprofiler,
-    gorgeimpl,
     vmchecks,
     vmcompilerserdes,
     vmdef,
@@ -69,7 +68,6 @@ import
 
 # xxx: reports are a code smell meaning data types are misplaced
 from compiler/ast/reports_sem import SemReport
-from compiler/ast/reports_internal import InternalReport
 from compiler/ast/report_enums import ReportKind
 
 # xxx: `Report` is faaaar too wide a type for what the VM needs, even with all
@@ -2633,33 +2631,7 @@ proc rawExecute(c: var TCtx, pc: var int): YieldReason =
       regs[ra].strVal = opSlurp($regs[rb].strVal, c.debug[pc],
                                      c.module, c.config)
     of opcGorge:
-      decodeBC(akString)
-      inc pc
-      let rd = c.code[pc].regA
-      checkHandle(regs[ra])
-      if defined(nimsuggest) or c.config.cmd == cmdCheck or
-         vmopsDanger notin c.config.features:
-        discard "don't run staticExec for 'nim suggest'"
-        regs[ra].strVal = ""
-      else:
-        when defined(nimcore):
-          checkHandle(regs[rb])
-          checkHandle(regs[rc])
-          checkHandle(regs[rd])
-          regs[ra].strVal = opGorge($regs[rb].strVal,
-                                        $regs[rc].strVal, $regs[rd].strVal,
-                                        c.debug[pc], c.config)[0]
-        else:
-          regs[ra].strVal = ""
-          # TODO: this is neither an internal error nor should ``globalReport``
-          #       be used to report it. As an improvement, it could be
-          #       treated as a normal VM error. ``opcGorge`` implements a part
-          #       of the compiler's compile-time interface, so it should be
-          #       eventually moved out of the VM via either callbacks or a
-          #       better mechanism for "system calls"
-          globalReport(c.config, c.debug[pc], InternalReport(
-            kind: rintNotUsingNimcore,
-            msg: "VM is not built with 'gorge' support"))
+      unreachable("no longer an opcode/magic")
 
     of opcParseExprToAst, opcParseStmtToAst:
       decodeBC(rkNimNode)

compiler/vm/vm_enums.nim

@@ -118,7 +118,7 @@ type
     opcNctPut, opcNctLen, opcNctGet, opcNctHasNext, opcNctNext, opcNodeId,
 
     opcSlurp,
-    opcGorge,
+    opcGorge,                 ## deprecated to be removed
     opcParseExprToAst,
     opcParseStmtToAst,
     opcNGetLineInfo, opcNSetLineInfo,

compiler/vm/vmgen.nim

@@ -1939,7 +1939,7 @@ proc genMagic(c: var TCtx; n: CgNode; dest: var TDest; m: TMagic) =
     c.gABC(n, opcTypeTrait, dest, tmp)
     c.freeTemp(tmp)
   of mSlurp: genUnaryABC(c, n, dest, opcSlurp)
-  of mStaticExec: genBinaryABCD(c, n, dest, opcGorge)
+  of mStaticExec: unreachable("no longer a magic")
   of mNLen: genUnaryABI(c, n, dest, opcLenSeq, nimNodeFlag)
   of mGetImpl: genUnaryABC(c, n, dest, opcGetImpl)
   of mGetImplTransf: genUnaryABC(c, n, dest, opcGetImplTransf)

compiler/vm/vmops.nim

@@ -64,7 +64,6 @@ from std/md5 import getMD5
 from std/times import getTime
 from std/hashes import hash
 from std/osproc import nil
-from std/options as std_options import some
 from system/formatfloat import writeFloatToBufferSprintf
 
 from compiler/modules/modulegraphs import `$`
@@ -395,6 +394,12 @@ iterator compileTimeOps*(): Override =
 
 iterator gorgeOps*(): Override =
   ## Special operations for executing external programs at compile time.
+  for op in ["stdlib.system.gorge", "stdlib.system.staticExec"]:
+    override op, proc (a: VmArgs) {.nimcall.} =
+      let (output, _) = opGorge(getString(a, 0), getString(a, 1),
+                                getString(a, 2), a.currentLineInfo, a.config)
+      writeResult(output)
+
   override "stdlib.system.gorgeEx", proc (a: VmArgs) {.nimcall.} =
     let ret = opGorge(getString(a, 0), getString(a, 1), getString(a, 2),
                       a.currentLineInfo, a.config)

lib/system.nim

@@ -2548,6 +2548,7 @@ proc `[]=`*(s: var string; i: BackwardsIndex; x: char) {.inline.} =
 
 proc slurp*(filename: string): string {.magic: "Slurp".}
   ## This is an alias for `staticRead <#staticRead,string>`_.
+  ## to be deprecated, use `staticRead`
 
 proc staticRead*(filename: string): string {.magic: "Slurp".}
   ## Compile-time `readFile <io.html#readFile,string>`_ proc for easy
@@ -2561,41 +2562,52 @@ proc staticRead*(filename: string): string {.magic: "Slurp".}
   ##
   ## `slurp <#slurp,string>`_ is an alias for `staticRead`.
 
-proc gorge*(command: string, input = "", cache = ""): string {.
-  magic: "StaticExec".} = discard
-  ## This is an alias for `staticExec <#staticExec,string,string,string>`_.
+when defined(nimskullReworkStaticExec):
+  proc gorge*(command: string, input = "", cache = ""): string {.
+    compileTime, deprecated: "use staticExec".} = discard
+    ## This is an alias for `staticExec <#staticExec,string,string,string>`_.
 
-proc staticExec*(command: string, input = "", cache = ""): string {.
-  magic: "StaticExec".} = discard
-  ## Executes an external process at compile-time and returns its text output
-  ## (stdout + stderr).
-  ##
-  ## If `input` is not an empty string, it will be passed as a standard input
-  ## to the executed program.
-  ##
-  ## .. code-block:: Nim
-  ##     const buildInfo = "Revision " & staticExec("git rev-parse HEAD") &
-  ##                       "\nCompiled on " & staticExec("uname -v")
-  ##
-  ## `gorge <#gorge,string,string,string>`_ is an alias for `staticExec`.
-  ##
-  ## Note that you can use this proc inside a pragma like
-  ## `passc <manual.html#implementation-specific-pragmas-passc-pragma>`_ or
-  ## `passl <manual.html#implementation-specific-pragmas-passl-pragma>`_.
-  ##
-  ## If `cache` is not empty, the results of `staticExec` are cached within
-  ## the `nimcache` directory. Use `--forceBuild` to get rid of this caching
-  ## behaviour then. `command & input & cache` (the concatenated string) is
-  ## used to determine whether the entry in the cache is still valid. You can
-  ## use versioning information for `cache`:
-  ##
-  ## .. code-block:: Nim
-  ##     const stateMachine = staticExec("dfaoptimizer", "input", "0.8.0")
+  proc staticExec*(command: string, input = "", cache = ""): string {.
+    compileTime.} = discard
+    ## Executes an external process at compile-time and returns its text output
+    ## (stdout + stderr).
+    ##
+    ## If `input` is not an empty string, it will be passed as a standard input
+    ## to the executed program.
+    ##
+    ## .. code-block:: Nim
+    ##     const buildInfo = "Revision " & staticExec("git rev-parse HEAD") &
+    ##                       "\nCompiled on " & staticExec("uname -v")
+    ##
+    ## `gorge <#gorge,string,string,string>`_ is an alias for `staticExec`.
+    ##
+    ## Note that you can use this proc inside a pragma like
+    ## `passc <manual.html#implementation-specific-pragmas-passc-pragma>`_ or
+    ## `passl <manual.html#implementation-specific-pragmas-passl-pragma>`_.
+    ##
+    ## If `cache` is not empty, the results of `staticExec` are cached within
+    ## the `nimcache` directory. Use `--forceBuild` to get rid of this caching
+    ## behaviour then. `command & input & cache` (the concatenated string) is
+    ## used to determine whether the entry in the cache is still valid. You can
+    ## use versioning information for `cache`:
+    ##
+    ## .. code-block:: Nim
+    ##     const stateMachine = staticExec("dfaoptimizer", "input", "0.8.0")
+    ## 
+    ## Deprecate/Replace with variant that returns the exit code and output
+else:
+  proc gorge*(command: string, input = "", cache = ""): string {.
+    magic: "StaticExec".} = discard
+    ## kept for bootstrapping
+
+  proc staticExec*(command: string, input = "", cache = ""): string {.
+    magic: "StaticExec".} = discard
+    ## kept for bootstrapping
 
 proc gorgeEx*(command: string, input = "", cache = ""): tuple[output: string,
                                                               exitCode: int] =
   ## Similar to `gorge <#gorge,string,string,string>`_ but also returns the
-  ## precious exit code.
+  ## exit code.
   discard
 
 

tests/vm/tgorge.nim

@@ -19,12 +19,12 @@ block gorge:
     relOutput = gorge(execName)
     absOutput = gorge(getScriptDir() / execName)
 
-  doAssert relOutput == "gorge test"
-  doAssert absOutput == "gorge test"
+  doAssert relOutput == "gorge test", relOutput
+  doAssert absOutput == "gorge test", absOutput
 
 block gorgeEx:
   const
     execName = when defined(windows): "tgorgeex.bat" else: "./tgorgeex.sh"
     res = gorgeEx(execName)
-  doAssert res.output == "gorgeex test"
-  doAssert res.exitCode == 1
+  doAssert res.output == "gorgeex test", res.output
+  doAssert res.exitCode == 1, $res.exitCode