Incremental compilation (IC): Improvements (#11881)

* IC: C codegen is aware of IC
* manual: minor change to make VSCode's RST plugin render it properly
* IC: minor refactoring
* testament: code refactorings
* rodutils: removed dead code
* IC: always build the compiler with the IC feature
* IC: C codegen improvements
* IC: implement the undocumented -d:nimMustCache option for testing purposes
* IC: added first basic tests
* IC: extensive testing of the deserialization feature
* testament: refactoring; better IC tests
* IC: removes 'nimMustCache' flag; readonly does the same
* testament: minor refactoring
* update Nimble version
* testament: removed dead code and imports; IC: added simple test
* IC: progress

5 files changed, 172 insertions, 192 deletions

diff --git a/testament/categories.nim b/testament/categories.nim
index 3db90d15f..745abebe3 100644
--- a/testament/categories.nim
+++ b/testament/categories.nim
@@ -10,8 +10,9 @@
 ## Include for the tester that contains test suites that test special features
 ## of the compiler.
 
+# included from tester.nim
+
 import important_packages
-import sequtils
 
 const
   specialCategories = [
@@ -24,6 +25,7 @@ const
     "gc",
     "io",
     "js",
+    "ic",
     "lib",
     "longgc",
     "manyloc",
@@ -41,58 +43,52 @@ const
     "dir with space"
   ]
 
-# included from tester.nim
-# ---------------- ROD file tests ---------------------------------------------
+proc isTestFile*(file: string): bool =
+  let (_, name, ext) = splitFile(file)
+  result = ext == ".nim" and name.startsWith("t")
 
-const
-  rodfilesDir = "tests/rodfiles"
-
-proc delNimCache(filename, options: string) =
-  for target in low(TTarget)..high(TTarget):
-    let dir = nimcacheDir(filename, options, target)
-    try:
-      removeDir(dir)
-    except OSError:
-      echo "[Warning] could not delete: ", dir
-
-proc runRodFiles(r: var TResults, cat: Category, options: string) =
-  template test(filename: string, clearCacheFirst=false) =
-    if clearCacheFirst: delNimCache(filename, options)
-    testSpec r, makeTest(rodfilesDir / filename, options, cat)
-
-
-  # test basic recompilation scheme:
-  test "hallo", true
-  test "hallo"
-  when false:
-    # test incremental type information:
-    test "hallo2"
-
-  # test type converters:
-  test "aconv", true
-  test "bconv"
-
-  # test G, A, B example from the documentation; test init sections:
-  test "deada", true
-  test "deada2"
-
-  when false:
-    # test method generation:
-    test "bmethods", true
-    test "bmethods2"
-
-    # test generics:
-    test "tgeneric1", true
-    test "tgeneric2"
-
-proc compileRodFiles(r: var TResults, cat: Category, options: string) =
-  template test(filename: untyped, clearCacheFirst=true) =
-    if clearCacheFirst: delNimCache(filename, options)
-    testSpec r, makeTest(rodfilesDir / filename, options, cat)
-
-  # test DLL interfacing:
-  test "gtkex1", true
-  test "gtkex2"
+# ---------------- IC tests ---------------------------------------------
+
+proc icTests(r: var TResults; testsDir: string, cat: Category, options: string) =
+  const
+    tooltests = ["compiler/nim.nim", "tools/nimgrep.nim"]
+    writeOnly = " --incremental:writeonly "
+    readOnly = " --incremental:readonly "
+    incrementalOn = " --incremental:on "
+
+  template test(x: untyped) =
+    testSpecWithNimcache(r, makeRawTest(file, x & options, cat), nimcache)
+
+  template editedTest(x: untyped) =
+    var test = makeTest(file, x & options, cat)
+    test.spec.targets = {getTestSpecTarget()}
+    testSpecWithNimcache(r, test, nimcache)
+
+  const tempExt = "_temp.nim"
+  for it in walkDirRec(testsDir / "ic"):
+    if isTestFile(it) and not it.endsWith(tempExt):
+      let nimcache = nimcacheDir(it, options, getTestSpecTarget())
+      removeDir(nimcache)
+
+      let content = readFile(it)
+      for fragment in content.split("#!EDIT!#"):
+        let file = it.replace(".nim", tempExt)
+        writeFile(file, fragment)
+        let oldPassed = r.passed
+        editedTest incrementalOn
+        if r.passed != oldPassed+1: break
+
+  for file in tooltests:
+    let nimcache = nimcacheDir(file, options, getTestSpecTarget())
+    removeDir(nimcache)
+
+    let oldPassed = r.passed
+    test writeOnly
+
+    if r.passed == oldPassed+1:
+      test readOnly
+      if r.passed == oldPassed+2:
+        test readOnly & "-d:nimBackendAssumesChange "
 
 # --------------------- flags tests -------------------------------------------
 
@@ -116,12 +112,6 @@ proc flagTests(r: var TResults, cat: Category, options: string) =
 
 # --------------------- DLL generation tests ----------------------------------
 
-proc safeCopyFile(src, dest: string) =
-  try:
-    copyFile(src, dest)
-  except OSError:
-    echo "[Warning] could not copy: ", src, " to ", dest
-
 proc runBasicDLLTest(c, r: var TResults, cat: Category, options: string) =
   const rpath = when defined(macosx):
       " --passL:-rpath --passL:@loader_path"
@@ -153,11 +143,12 @@ proc runBasicDLLTest(c, r: var TResults, cat: Category, options: string) =
 
   if "boehm" notin options:
     # force build required - see the comments in the .nim file for more details
-    var hcr_integration = makeTest("tests/dll/nimhcr_integration.nim",
+    var hcri = makeTest("tests/dll/nimhcr_integration.nim",
                                    options & " --forceBuild --hotCodeReloading:on" & rpath, cat)
-    hcr_integration.args = prepareTestArgs(hcr_integration.spec.getCmd, hcr_integration.name,
-                                           hcr_integration.options, getTestSpecTarget())
-    testSpec r, hcr_integration
+    let nimcache = nimcacheDir(hcri.name, hcri.options, getTestSpecTarget())
+    hcri.args = prepareTestArgs(hcri.spec.getCmd, hcri.name,
+                                hcri.options, nimcache, getTestSpecTarget())
+    testSpec r, hcri
 
 proc dllTests(r: var TResults, cat: Category, options: string) =
   # dummy compile result:
@@ -363,7 +354,8 @@ proc testNimInAction(r: var TResults, cat: Category, options: string) =
   for i, test in tests:
     let filename = testsDir / test.addFileExt("nim")
     let testHash = getMD5(readFile(filename).string)
-    doAssert testHash == refHashes[i], "Nim in Action test " & filename & " was changed: " & $(i: i, testHash: testHash, refHash: refHashes[i])
+    doAssert testHash == refHashes[i], "Nim in Action test " & filename &
+        " was changed: " & $(i: i, testHash: testHash, refHash: refHashes[i])
   # Run the tests.
   for testfile in tests:
     test "tests/" & testfile & ".nim"
@@ -449,25 +441,6 @@ let
   nimbleExe = findExe("nimble")
   packageIndex = nimbleDir / "packages_official.json"
 
-proc waitForExitEx(p: Process): int =
-  var outp = outputStream(p)
-  var line = newStringOfCap(120).TaintedString
-  while true:
-    if outp.readLine(line):
-      discard
-    else:
-      result = peekExitCode(p)
-      if result != -1: break
-  close(p)
-
-proc getPackageDir(package: string): string =
-  ## TODO - Replace this with dom's version comparison magic.
-  let commandOutput = execCmdEx("nimble path $#" % package)
-  if commandOutput.exitCode != QuitSuccess:
-    return ""
-  else:
-    result = commandOutput[0].string
-
 iterator listPackages(): tuple[name, url, cmd: string, hasDeps: bool] =
   let defaultCmd = "nimble test"
   let packageList = parseFile(packageIndex)
@@ -481,8 +454,8 @@ iterator listPackages(): tuple[name, url, cmd: string, hasDeps: bool] =
         let name = package["name"].str
         if name == n:
           found = true
-          let p_url = package["url"].str
-          yield (name, p_url, cmd, hasDeps)
+          let pUrl = package["url"].str
+          yield (name, pUrl, cmd, hasDeps)
           break
       if not found:
         raise newException(ValueError, "Cannot find package '$#'." % n)
@@ -551,7 +524,7 @@ proc testNimblePackages(r: var TResults, cat: Category) =
 
 # ----------------------------------------------------------------------------
 
-const AdditionalCategories = ["debugger", "examples", "lib"]
+const AdditionalCategories = ["debugger", "examples", "lib", "ic"]
 const MegaTestCat = "megatest"
 
 proc `&.?`(a, b: string): string =
@@ -584,17 +557,12 @@ proc isJoinableSpec(spec: TSpec): bool =
     (spec.targets == {} or spec.targets == {targetC})
 
 proc norm(s: var string) =
-  # equivalent of s/\n+/\n/g (could use a single pass over input if needed)
   while true:
     let tmp = s.replace("\n\n", "\n")
     if tmp == s: break
     s = tmp
   s = s.strip
 
-proc isTestFile*(file: string): bool =
-  let (_, name, ext) = splitFile(file)
-  result = ext == ".nim" and name.startsWith("t")
-
 proc quoted(a: string): string =
   # todo: consider moving to system.nim
   result.addQuoted(a)
@@ -607,10 +575,10 @@ proc runJoinedTest(r: var TResults, cat: Category, testsDir: string) =
     let cat = dir[testsDir.len .. ^1]
     if kind == pcDir and cat notin specialCategories:
       for file in walkDirRec(testsDir / cat):
-        if not isTestFile(file): continue
-        let spec = parseSpec(file)
-        if isJoinableSpec(spec):
-          specs.add spec
+        if isTestFile(file):
+          let spec = parseSpec(file)
+          if isJoinableSpec(spec):
+            specs.add spec
 
   proc cmp(a: TSpec, b:TSpec): auto = cmp(a.file, b.file)
   sort(specs, cmp=cmp) # reproducible order
@@ -646,14 +614,13 @@ proc runJoinedTest(r: var TResults, cat: Category, testsDir: string) =
 
   let args = ["c", "--nimCache:" & outDir, "-d:testing", "--listCmd",
               "--listFullPaths:off", "--excessiveStackTrace:off", "megatest.nim"]
-  proc onStdout(line: string) = echo line
+
   var (cmdLine, buf, exitCode) = execCmdEx2(command = compilerPrefix, args = args, input = "")
   if exitCode != 0:
     echo "$ ", cmdLine
     echo buf.string
     quit("megatest compilation failed")
 
-  # Could also use onStdout here.
   (buf, exitCode) = execCmdEx("./megatest")
   if exitCode != 0:
     echo buf.string
@@ -690,6 +657,8 @@ proc processCategory(r: var TResults, cat: Category,
     when false:
       compileRodFiles(r, cat, options)
       runRodFiles(r, cat, options)
+  of "ic":
+    icTests(r, testsDir, cat, options)
   of "js":
     # only run the JS tests on Windows or Linux because Travis is bad
     # and other OSes like Haiku might lack nodejs:
diff --git a/testament/htmlgen.nim b/testament/htmlgen.nim
index 641c1c32c..67e44a2f8 100644
--- a/testament/htmlgen.nim
+++ b/testament/htmlgen.nim
@@ -9,7 +9,7 @@
 
 ## HTML generator for the tester.
 
-import cgi, backend, strutils, json, os, tables, times
+import strutils, json, os, times
 
 import "testamenthtml.nimf"
 
diff --git a/testament/important_packages.nim b/testament/important_packages.nim
index defd748ab..93aeb1bbc 100644
--- a/testament/important_packages.nim
+++ b/testament/important_packages.nim
@@ -1,4 +1,3 @@
-import strutils
 
 template pkg(name: string; cmd = "nimble test"; hasDeps = false; url = ""): untyped =
   packages.add((name, cmd, hasDeps, url))
diff --git a/testament/specs.nim b/testament/specs.nim
index 4d09f438f..cf3455027 100644
--- a/testament/specs.nim
+++ b/testament/specs.nim
@@ -7,7 +7,7 @@
 #    distribution, for details about the copyright.
 #
 
-import sequtils, parseutils, strutils, os, osproc, streams, parsecfg
+import sequtils, parseutils, strutils, os, streams, parsecfg
 
 var compilerPrefix* = findExe("nim")
 
@@ -122,6 +122,9 @@ proc addLine*(self: var string; a,b: string) =
   self.add b
   self.add "\n"
 
+proc initSpec*(filename: string): TSpec =
+  result.file = filename
+
 proc parseSpec*(filename: string): TSpec =
   result.file = filename
   let specStr = extractSpec(filename)
diff --git a/testament/tester.nim b/testament/tester.nim
index 1e5bc875a..c32228ca8 100644
--- a/testament/tester.nim
+++ b/testament/tester.nim
@@ -10,9 +10,9 @@
 ## This program verifies Nim against the testcases.
 
 import
-  parseutils, strutils, pegs, os, osproc, streams, parsecfg, json,
-  marshal, backend, parseopt, specs, htmlgen, browsers, terminal,
-  algorithm, times, sets, md5, sequtils
+  strutils, pegs, os, osproc, streams, json,
+  backend, parseopt, specs, htmlgen, browsers, terminal,
+  algorithm, times, md5, sequtils
 
 include compiler/nodejs
 
@@ -124,19 +124,20 @@ proc execCmdEx2(command: string, args: openarray[string]; workingDir, input: str
 proc nimcacheDir(filename, options: string, target: TTarget): string =
   ## Give each test a private nimcache dir so they don't clobber each other's.
   let hashInput = options & $target
-  return "nimcache" / (filename & '_' & hashInput.getMD5)
+  result = "nimcache" / (filename & '_' & hashInput.getMD5)
 
-proc prepareTestArgs(cmdTemplate, filename, options: string,
+proc prepareTestArgs(cmdTemplate, filename, options, nimcache: string,
                      target: TTarget, extraOptions=""): seq[string] =
-  let nimcache = nimcacheDir(filename, options, target)
   let options = options & " " & quoteShell("--nimCache:" & nimcache) & extraOptions
-  return parseCmdLine(cmdTemplate % ["target", targetToCmd[target],
+  result = parseCmdLine(cmdTemplate % ["target", targetToCmd[target],
                       "options", options, "file", filename.quoteShell,
                       "filedir", filename.getFileDir()])
 
-proc callCompiler(cmdTemplate, filename, options: string,
-                  target: TTarget, extraOptions=""): TSpec =
-  let c = prepareTestArgs(cmdTemplate, filename, options, target, extraOptions)
+proc callCompiler(cmdTemplate, filename, options, nimcache: string,
+                  target: TTarget,
+                  extraOptions=""): TSpec =
+  let c = prepareTestArgs(cmdTemplate, filename, options, nimcache, target,
+                          extraOptions)
   result.cmd = quoteShellCommand(c)
   var p = startProcess(command=c[0], args=c[1 .. ^1],
                        options={poStdErrToStdOut, poUsePath})
@@ -370,7 +371,7 @@ proc nimoutCheck(test: TTest; expectedNimout: string; given: var TSpec) =
     currentPos = giv.find(line.strip, currentPos)
     if currentPos < 0:
       given.err = reMsgsDiffer
-      return
+      break
 
 proc compilerOutputTests(test: TTest, target: TTarget, given: var TSpec,
                          expected: TSpec; r: var TResults) =
@@ -391,9 +392,9 @@ proc compilerOutputTests(test: TTest, target: TTarget, given: var TSpec,
 
 proc getTestSpecTarget(): TTarget =
   if getEnv("NIM_COMPILE_TO_CPP", "false").string == "true":
-    return targetCpp
+    result = targetCpp
   else:
-    return targetC
+    result = targetC
 
 proc checkDisabled(r: var TResults, test: TTest): bool =
   if test.spec.err in {reDisabled, reJoined}:
@@ -401,13 +402,72 @@ proc checkDisabled(r: var TResults, test: TTest): bool =
     r.addResult(test, targetC, "", "", test.spec.err)
     inc(r.skipped)
     inc(r.total)
-    return
-  true
+    result = false
+  else:
+    result = true
+
+var count = 0
+
+proc testSpecHelper(r: var TResults, test: TTest, expected: TSpec, target: TTarget, nimcache: string) =
+  case expected.action
+  of actionCompile:
+    var given = callCompiler(expected.getCmd, test.name, test.options, nimcache, target,
+          extraOptions = " --stdout --hint[Path]:off --hint[Processing]:off")
+    compilerOutputTests(test, target, given, expected, r)
+  of actionRun:
+    var given = callCompiler(expected.getCmd, test.name, test.options, nimcache, target)
+    if given.err != reSuccess:
+      r.addResult(test, target, "", "$ " & given.cmd & "\n" & given.nimout, given.err)
+    else:
+      let isJsTarget = target == targetJS
+      var exeFile = changeFileExt(test.name, if isJsTarget: "js" else: ExeExt)
+      if not existsFile(exeFile):
+        r.addResult(test, target, expected.output,
+                    "executable not found: " & exeFile, reExeNotFound)
+      else:
+        let nodejs = if isJsTarget: findNodeJs() else: ""
+        if isJsTarget and nodejs == "":
+          r.addResult(test, target, expected.output, "nodejs binary not in PATH",
+                      reExeNotFound)
+        else:
+          var exeCmd: string
+          var args = test.args
+          if isJsTarget:
+            exeCmd = nodejs
+            args = concat(@[exeFile], args)
+          else:
+            exeCmd = exeFile
+          var (_, buf, exitCode) = execCmdEx2(exeCmd, args, input = expected.input)
+          # Treat all failure codes from nodejs as 1. Older versions of nodejs used
+          # to return other codes, but for us it is sufficient to know that it's not 0.
+          if exitCode != 0: exitCode = 1
+          let bufB =
+            if expected.sortoutput:
+              var x = splitLines(strip(buf.string))
+              sort(x, system.cmp)
+              join(x, "\n")
+            else:
+              strip(buf.string)
+          if exitCode != expected.exitCode:
+            r.addResult(test, target, "exitcode: " & $expected.exitCode,
+                              "exitcode: " & $exitCode & "\n\nOutput:\n" &
+                              bufB, reExitCodesDiffer)
+          elif (expected.outputCheck == ocEqual and expected.output != bufB) or
+              (expected.outputCheck == ocSubstr and expected.output notin bufB):
+            given.err = reOutputsDiffer
+            r.addResult(test, target, expected.output, bufB, reOutputsDiffer)
+          else:
+            compilerOutputTests(test, target, given, expected, r)
+  of actionReject:
+    var given = callCompiler(expected.getCmd, test.name, test.options,
+                              nimcache, target)
+    cmpMsgs(r, expected, given, test, target)
+
 
 proc testSpec(r: var TResults, test: TTest, targets: set[TTarget] = {}) =
   var expected = test.spec
   if expected.parseErrors.len > 0:
-    # targetC is a lie, but parameter is required
+    # targetC is a lie, but a parameter is required
     r.addResult(test, targetC, "", expected.parseErrors, reInvalidSpec)
     inc(r.total)
     return
@@ -422,73 +482,18 @@ proc testSpec(r: var TResults, test: TTest, targets: set[TTarget] = {}) =
     if target notin gTargets:
       r.addResult(test, target, "", "", reDisabled)
       inc(r.skipped)
-      continue
-
-    if simulate:
-      var count {.global.} = 0
-      count.inc
+    elif simulate:
+      inc count
       echo "testSpec count: ", count, " expected: ", expected
-      continue
-
-    case expected.action
-    of actionCompile:
-      var given = callCompiler(expected.getCmd, test.name, test.options, target,
-        extraOptions=" --stdout --hint[Path]:off --hint[Processing]:off")
-      compilerOutputTests(test, target, given, expected, r)
-    of actionRun:
-      # In this branch of code "early return" pattern is clearer than deep
-      # nested conditionals - the empty rows in between to clarify the "danger"
-      var given = callCompiler(expected.getCmd, test.name, test.options, target)
-      if given.err != reSuccess:
-        r.addResult(test, target, "", "$ " & given.cmd & "\n" & given.nimout, given.err)
-        continue
-      let isJsTarget = target == targetJS
-      var exeFile = changeFileExt(test.name, if isJsTarget: "js" else: ExeExt)
-      if not existsFile(exeFile):
-        r.addResult(test, target, expected.output,
-                    "executable not found: " & exeFile, reExeNotFound)
-        continue
-
-      let nodejs = if isJsTarget: findNodeJs() else: ""
-      if isJsTarget and nodejs == "":
-        r.addResult(test, target, expected.output, "nodejs binary not in PATH",
-                    reExeNotFound)
-        continue
-      var exeCmd: string
-      var args = test.args
-      if isJsTarget:
-        exeCmd = nodejs
-        args = concat(@[exeFile], args)
-      else:
-        exeCmd = exeFile
-      var (cmdLine, buf, exitCode) = execCmdEx2(exeCmd, args, input = expected.input)
-      # Treat all failure codes from nodejs as 1. Older versions of nodejs used
-      # to return other codes, but for us it is sufficient to know that it's not 0.
-      if exitCode != 0: exitCode = 1
-      let bufB =
-        if expected.sortoutput:
-          var x = splitLines(strip(buf.string))
-          sort(x, system.cmp)
-          join(x, "\n")
-        else:
-          strip(buf.string)
-      if exitCode != expected.exitCode:
-        r.addResult(test, target, "exitcode: " & $expected.exitCode,
-                          "exitcode: " & $exitCode & "\n\nOutput:\n" &
-                          bufB, reExitCodesDiffer)
-        continue
-      if (expected.outputCheck == ocEqual and expected.output != bufB) or
-         (expected.outputCheck == ocSubstr and expected.output notin bufB):
-        given.err = reOutputsDiffer
-        r.addResult(test, target, expected.output, bufB, reOutputsDiffer)
-        continue
-      compilerOutputTests(test, target, given, expected, r)
-      continue
-    of actionReject:
-      var given = callCompiler(expected.getCmd, test.name, test.options,
-                               target)
-      cmpMsgs(r, expected, given, test, target)
-      continue
+    else:
+      let nimcache = nimcacheDir(test.name, test.options, target)
+      testSpecHelper(r, test, expected, target, nimcache)
+
+proc testSpecWithNimcache(r: var TResults, test: TTest; nimcache: string) =
+  if not checkDisabled(r, test): return
+  for target in test.spec.targets:
+    inc(r.total)
+    testSpecHelper(r, test, test.spec, target, nimcache)
 
 proc testC(r: var TResults, test: TTest, action: TTestAction) =
   # runs C code. Doesn't support any specs, just goes by exit code.
@@ -529,6 +534,15 @@ proc makeTest(test, options: string, cat: Category): TTest =
   result.spec = parseSpec(addFileExt(test, ".nim"))
   result.startTime = epochTime()
 
+proc makeRawTest(test, options: string, cat: Category): TTest =
+  result.cat = cat
+  result.name = test
+  result.options = options
+  result.spec = initSpec(addFileExt(test, ".nim"))
+  result.startTime = epochTime()
+  result.spec.action = actionCompile
+  result.spec.targets = {getTestSpecTarget()}
+
 # TODO: fix these files
 const disabledFilesDefault = @[
   "LockFreeHash.nim",
@@ -553,23 +567,18 @@ when defined(windows):
 else:
   const
     # array of modules disabled from compilation test of stdlib.
-    # TODO: why the ["-"]? (previous code should've prob used seq[string] = @[] instead)
-    disabledFiles = disabledFilesDefault & @["-"]
+    disabledFiles = disabledFilesDefault
 
 include categories
 
 proc loadSkipFrom(name: string): seq[string] =
-  if name.len() == 0: return
-
+  if name.len == 0: return
   # One skip per line, comments start with #
   # used by `nlvm` (at least)
-  try:
-    for line in lines(name):
-      let sline = line.strip()
-      if sline.len > 0 and not sline.startsWith("#"):
-        result.add sline
-  except:
-    echo "Could not load " & name & ", ignoring"
+  for line in lines(name):
+    let sline = line.strip()
+    if sline.len > 0 and not sline.startsWith("#"):
+      result.add sline
 
 proc main() =
   os.putenv "NIMTEST_COLOR", "never"