fix(unittest): check not capturing operands (#950)

## Summary

Fix the value of some operand expressions to `not`, `==`, etc. within
`check` calls not being captured, which would result in incorrect
`check` failure/success.

## Details

### Issue description

For displaying the value of the operands in case of a test failure,
the `check` macro "captures" the values of the operands prior to
evaluating the condition expression. This is implemented in
`inspectArgs` by assigning the operand expressions to temporaries and
then accessing the temporaries.

Type expressions cannot be assigned to run-time variables, so they were
guarded against, by testing for whether the nodes have `ntyTypeDesc`
as the type kind. However, since the AST passed to `check` is
untyped, this cannot work, but so far it did, due to an implementation
bug of `typeKind`, where the value returned for `typeKind` called on a
`NimNode` that has no type results in undefined behaviour.

In this specific case, the bug resulted in the `notin {ntyTypeDesc}`
expressions always to always evaluate to `false`, which in turn
disabled capturing for some expressions.

### The solution

The decision of whether to assign the expression can only happen when
the expressions are typed. A two-phase macro could be used, but that
would require larger changes to `check`, so a different solution is
chosen: instead of an assignment, `inspectArgs` emits a call to the new
`materialize` template, passing along a generated `nksTemplate` symbol.
To access the materialized (or not) value, `inspectArgs` emits calls
with the generated template symbol.

When the `materialize` calls are later expanded, a template with the
passed-long symbol is produced. Using two `materialize` overloads, one
for `typedesc`s and for everything else, doesn't work at the moment: if
the value expression is erroneous, no `materialize` template is
invoked, which would leave the gensym'ed symbol in an improper state,
later causing the compiler to crash when it attempts to use it during
overload resolution.

In addition, for improved clarity, `inspectArgs` is changed to use a
`case` statement.

lib/pure/unittest.nim

@@ -683,6 +683,15 @@ proc untype(arg: NimNode): NimNode =
     for n in arg:
       result.add untype(n)
 
+template materialize(val: untyped, name: untyped) =
+  ## Helper template for ``check``. Captures `val` and makes it accessible
+  ## through a template named `name`.
+  when val is typedesc:
+    template name(): untyped = val
+  else:
+    let tmp = val
+    template name(): untyped = tmp
+
 macro check*(conditions: untyped): untyped =
   ## Verify if a statement or a list of statements is true.
   ## A helpful error message and set checkpoints are printed out on
@@ -711,30 +720,27 @@ macro check*(conditions: untyped): untyped =
                     ">=", "<", ">", "!=", "is", "isnot"]:
 
       for i in 1 ..< exp.len:
-        if exp[i].kind notin nnkLiterals:
-          let argStr = exp[i].toStrLit
-          let paramAst = exp[i]
-          if exp[i].kind == nnkIdent:
-            result.printOuts.add newCall(bindSym"print", argStr, paramAst)
-          if exp[i].kind in nnkCallKinds + {nnkDotExpr, nnkBracketExpr, nnkPar} and
-                  (exp[i].typeKind notin {ntyTypeDesc} or $exp[0] notin ["is", "isnot"]):
-            let callVar = genSym(nskVar, "c")
-            result.assigns.add newVarStmt(callVar, paramAst)
-            result.check[i] = callVar
-            result.printOuts.add newCall(bindSym"print", argStr, callVar)
-          if exp[i].kind == nnkExprEqExpr:
-            # ExprEqExpr
-            #   Ident "v"
-            #   IntLit 2
-            result.check[i] = exp[i][1]
-          if exp[i].typeKind notin {ntyTypeDesc}:
-            let arg = genSym(nskVar, "p")
-            result.assigns.add newVarStmt(arg, paramAst)
-            result.printOuts.add newCall(bindSym"print", argStr, arg)
-            if exp[i].kind != nnkExprEqExpr:
-              result.check[i] = arg
-            else:
-              result.check[i][1] = arg
+        let argAst = exp[i]
+        case exp[i].kind
+        of nnkLiterals:
+          discard "literals are pure; they can be used as is"
+        of nnkExprEqExpr:
+          # named argument:
+          # ExprEqExpr
+          #   Ident "v"
+          #   IntLit 2
+          result.check[i] = exp[i][1]
+        else:
+          # the argument expression might have side-effects or be impure,
+          # meaning that it cannot be duplicated without changing -- it has to
+          # be materialized into a temporary
+          let access = genSym(nskTemplate, "c")
+          result.assigns.add newCall(bindSym"materialize", argAst, access)
+          # ``materialize`` injects a template with the given `access` symbol.
+          # Invoking the template then yields the value
+          result.check[i] = newCall(access)
+          result.printOuts.add:
+            newCall(bindSym"print", toStrLit(argAst), newCall(access))
 
   case checked.kind
   of nnkCallKinds:

tests/stdlib/metaprogramming/tunittest_impure.nim

@@ -0,0 +1,54 @@
+discard """
+  targets: "c js vm"
+  knownIssue.vm: "std/streams is not supported"
+  description: '''
+    Ensure that the values of impure expressions used as operands in
+    ``check`` expression are properly captured (assigned to
+    temporaries) and, in case of failure, displayed
+  '''
+"""
+
+import std/[unittest, sequtils, strutils, exitprocs]
+
+var
+  output: seq[string] ## set after a ``check`` failure
+  wasFailure: bool
+
+type MockFormatter = ref object of OutputFormatter
+
+method failureOccurred(f: MockFormatter, checkpoints: seq[string], _: string) {.gcsafe.} =
+  # only include the printed values
+  output = filterIt(checkpoints, "Check failed:" notin it)
+  wasFailure = true
+
+# we use our own formatter:
+resetOutputFormatters()
+addOutputFormatter(MockFormatter())
+
+template verify(o: varargs[string]) =
+  ## Compares the output of the previous ``check`` failure against `o`.
+  doAssert wasFailure, "check didn't fail"
+  doAssert output == o, "got: " & $output
+  wasFailure = false
+
+proc modify(x: var int): var int =
+  inc x
+  x
+
+# --- the actual test cases
+
+var a = 0
+check a == modify(a)
+verify "a was 0", "modify(a) was 1"
+
+check (inc a; a) == (inc a; a)
+verify "\ninc a\na was 2", "\ninc a\na was 3"
+
+check [modify(a)] == [modify(a)]
+verify "[modify(a)] was [4]", "[modify(a)] was [5]"
+
+check (modify(a),) == (modify(a),)
+verify "(modify(a),) was (6,)", "(modify(a),) was (7,)"
+
+# reset the program result so that no test failure is reported
+setProgramResult(0)