macros: make quote a proper quasi-quoting operator (#1393)

## Summary

`quote` now keeps the quoted block as is, which means that:
* no symbols are bound or mixed in automatically from the enclosing
  scopes
* identifiers in definition positions aren't turned into gensyms; they
  stay as is
* `.gensym` and `.inject` pragmas within the quoted block don't affect
  the AST

Symbols thus need to be bound or created explicitly, via `bindSym` and
`genSym`, respectively. **This is a breaking change.**

## Details

### Motivation For The Change

* symbols from the `quote`'s scope being bound is error-prone, leading
  to confusing compilation errors
* the intention of documentation of `quote` already said it does quasi-
  quoting (even though it didn't)
* the implementation relied on the intricacies of templates and
  template evaluation

### New Behaviour

* quoted AST is not modified. No symbols are bound and no identifiers
  are turned into gensyms

### Implementation

* `semQuoteAst` transforms the `quote` call into a call to the internal
  `quoteImpl` procedure
* the pre-processed quoted AST is passed as the first arguments; the
  extracted unquoted expression are passed as the remaining arguments
* the internal-only `evalToAst` magic procedure is used for evaluating
  the unquoted expressions. `newLit` cannot be used here, as the trees
  it produces for `object` values are only valid when all the type's
  fields are exported
* placing the AST is of the evaluated unqouted expressions is handled
  in-VM, by `quoteImpl`

### Standard Library And Test Changes

* multiple modules from the standard library relied on the previous
  symbol binding and gensym behaviour; they're changed to use `bindSym`
  or `genSym`. Outside-visible behaviour doesn't change
* the `t7875.nim` test relied on the gensym behaviour. The definition
  outside the macro is not relevant to the issue the test guards
  against, so it can just be removed
* the quoted ASTs in `tsizeof.nim` are wrapped in blocks in order to
  prevent the identifiers from colliding

compiler/ast/ast_types.nim

@@ -817,6 +817,8 @@ type
     mException, mBuiltinType, mSymOwner, mUncheckedArray, mGetImplTransf,
     mSymIsInstantiationOf, mNodeId, mPrivateAccess
 
+    mEvalToAst
+
     # magics only used internally:
     mStrToCStr
       ## the backend-dependent string-to-cstring conversion

compiler/sem/semexprs.nim

@@ -2552,9 +2552,6 @@ proc expectString(c: PContext, n: PNode): string =
   else:
     localReport(c.config, n, reportSem rsemStringLiteralExpected)
 
-proc newAnonSym(c: PContext; kind: TSymKind, info: TLineInfo): PSym =
-  result = newSym(kind, c.cache.idAnon, nextSymId c.idgen, getCurrOwner(c), info)
-
 proc semExpandToAst(c: PContext, n: PNode): PNode =
   let macroCall = n[1]
 
@@ -2606,14 +2603,11 @@ proc semExpandToAst(c: PContext, n: PNode, magicSym: PSym,
   else:
     result = semDirectOp(c, n, flags)
 
-proc processQuotations(c: PContext; n: var PNode, op: string,
-                       quotes: var seq[PNode],
-                       ids: var seq[PNode]) =
+proc processQuotations(c: PContext; n: PNode, op: string, call: PNode): PNode =
   template returnQuote(q) =
-    quotes.add q
-    n = newIdentNode(getIdent(c.cache, $quotes.len), n.info)
-    ids.add n
-    return
+    call.add q
+    # return a placeholder node. The integer represents the parameter index
+    return newTreeI(nkAccQuoted, n.info, newIntNode(nkIntLit, call.len - 3))
 
   template handlePrefixOp(prefixed) =
     if prefixed[0].kind == nkIdent:
@@ -2639,14 +2633,22 @@ proc processQuotations(c: PContext; n: var PNode, op: string,
         tempNode[0] = n[0]
         tempNode[1] = n[1]
         handlePrefixOp(tempNode)
-  of nkIdent:
-    if n.ident.s == "result":
-      n = ids[0]
   else:
     discard # xxx: raise an error
 
+  result = n
   for i in 0..<n.safeLen:
-    processQuotations(c, n[i], op, quotes, ids)
+    let x = processQuotations(c, n[i], op, call)
+    if x != n[i]:
+      # copy on write
+      if result == n:
+        result = copyNodeWithKids(n)
+      result[i] = x
+
+  if result.kind == nkAccQuoted:
+    # escape the accquote node by wrapping it in another accquote. This signals
+    # that the node is not a placeholder
+    result = newTree(nkAccQuoted, result)
 
 proc semQuoteAst(c: PContext, n: PNode): PNode =
   if n.len != 2 and n.len != 3:
@@ -2657,57 +2659,38 @@ proc semQuoteAst(c: PContext, n: PNode): PNode =
     #      got = result.len - 1
     return
 
-  # We transform the do block into a template with a param for
-  # each interpolation. We'll pass this template to getAst.
   var
     quotedBlock = n[^1]
     op = if n.len == 3: expectString(c, n[1]) else: "``"
-    quotes = newSeq[PNode](2)
-      # the quotes will be added to a nkCall statement
-      # leave some room for the callee symbol and the result symbol
-    ids = newSeq[PNode](1)
-      # this will store the generated param names
-      # leave some room for the result symbol
 
   if quotedBlock.kind != nkStmtList:
     semReportIllformedAst(c.config, n, {nkStmtList})
 
-  # This adds a default first field to pass the result symbol
-  ids[0] = newAnonSym(c, skParam, n.info).newSymNode
-  processQuotations(c, quotedBlock, op, quotes, ids)
-
-  var dummyTemplate = newProcNode(
-    nkTemplateDef, quotedBlock.info, body = quotedBlock,
-    params = c.graph.emptyNode,
-    name = newAnonSym(c, skTemplate, n.info).newSymNode,
-              pattern = c.graph.emptyNode, genericParams = c.graph.emptyNode,
-              pragmas = c.graph.emptyNode, exceptions = c.graph.emptyNode)
-
-  if ids.len > 0:
-    dummyTemplate[paramsPos] = newNodeI(nkFormalParams, n.info)
-    dummyTemplate[paramsPos].add:
-      getSysSym(c.graph, n.info, "untyped").newSymNode # return type
-    ids.add getSysSym(c.graph, n.info, "untyped").newSymNode # params type
-    ids.add c.graph.emptyNode # no default value
-    dummyTemplate[paramsPos].add newTreeI(nkIdentDefs, n.info, ids)
-
-  var tmpl = semTemplateDef(c, dummyTemplate)
-  quotes[0] = tmpl[namePos]
-  # This adds a call to newIdentNode("result") as the first argument to the
-  # template call
-  let identNodeSym = getCompilerProc(c.graph, "newIdentNode")
-  # so that new Nim compilers can compile old macros.nim versions, we check for
-  # 'nil' here and provide the old fallback solution:
-  let identNode = if identNodeSym == nil:
-                    newIdentNode(getIdent(c.cache, "newIdentNode"), n.info)
-                  else:
-                    identNodeSym.newSymNode
-  quotes[1] = newTreeI(nkCall, n.info, identNode, newStrNode(nkStrLit, "result"))
-  result =
-    c.semExpandToAst:
-      newTreeI(nkCall, n.info,
-        createMagic(c.graph, c.idgen, "getAst", mExpandToAst).newSymNode,
-        newTreeI(nkCall, n.info, quotes))
+  # turn the quasi-quoted block into a call to the internal ``quoteImpl``
+  # procedure
+  # future direction: implement this transformation in user code. The compiler
+  # only needs to provide an AST quoting facility (without quasi-quoting)
+
+  let call = newNodeI(nkCall, n.info, 2)
+  call[0] = newSymNode(c.graph.getCompilerProc("quoteImpl"))
+  # extract the unquoted parts and append them to `call`:
+  let quoted = processQuotations(c, quotedBlock, op, call)
+  # the pre-processed AST of the quoted block is passed as the first argument:
+  call[1] = newTreeI(nkNimNodeLit, n.info, quoted)
+  call[1].typ = sysTypeFromName(c.graph, n.info, "NimNode")
+
+  template ident(name: string): PNode =
+    newIdentNode(c.cache.getIdent(name), unknownLineInfo)
+
+  # the unquoted expressions are wrapped in evalToAst calls. Use a qualified
+  # identifier in order to prevent user-defined evalToAst calls to be picked
+  let callee = newTree(nkDotExpr, ident("macros"), ident("evalToAst"))
+  for i in 2..<call.len:
+    call[i] = newTreeI(nkCall, call[i].info, [callee, call[i]])
+
+  # type the call. The actual work of substituting the placeholders is
+  # done in-VM, by the ``quoteImpl`` procedure
+  result = semDirectOp(c, call, {})
 
 proc tryExpr(c: PContext, n: PNode, flags: TExprFlags = {}): PNode =
   # watch out, hacks ahead:

compiler/sem/sempass2.nim

@@ -1448,6 +1448,8 @@ proc track(tracked: PEffects, n: PNode) =
     reportErrors(tracked.config, n)
   of nkError:
     localReport(tracked.config, n)
+  of nkNimNodeLit:
+    discard "don't analyse literal AST"
   else:
     for i in 0 ..< n.safeLen:
       track(tracked, n[i])

compiler/sem/varpartitions.nim

@@ -670,7 +670,7 @@ const
     nkMethodDef, nkIteratorDef, nkMacroDef, nkTemplateDef, nkLambda, nkDo,
     nkFuncDef, nkConstSection, nkConstDef, nkIncludeStmt, nkImportStmt,
     nkExportStmt, nkPragma, nkTypeOfExpr, nkMixinStmt,
-    nkBindStmt}
+    nkBindStmt, nkNimNodeLit}
 
 proc potentialMutationViaArg(c: var Partitions; n: PNode; callee: PType) =
   if constParameters in c.goals and tfNoSideEffect in callee.flags:

compiler/vm/vmgen.nim

@@ -207,7 +207,7 @@ const
                      cnkObjDownConv, cnkDeref, cnkDerefView, cnkLvalueConv}
 
   MagicsToKeep* = {mIsolate, mNHint, mNWarning, mNError, mMinI, mMaxI,
-                   mAbsI, mDotDot, mNGetType, mNSizeOf, mNLineInfo}
+                   mAbsI, mDotDot, mNGetType, mNSizeOf, mNLineInfo, mEvalToAst}
     ## the set of magics that are kept as normal procedure calls and thus need
     ## an entry in the function table.
     # XXX: mNGetType, mNGetSize, and mNLineInfo *are* real magics, but their
@@ -2171,6 +2171,13 @@ proc genMagic(c: var TCtx; n: CgNode; dest: var TDest; m: TMagic) =
 
     c.gABC(n, opcExpandToAst, dest, x, numArgs)
     c.freeTempRange(x, numArgs)
+  of mEvalToAst:
+    if n[1].typ.isNimNode():
+      # don't use ``DataToAst` if the argument is already a NimNode, only copy
+      # the tree
+      c.genUnaryABC(n, dest, opcNCopyNimTree)
+    else:
+      c.genDataToAst(n[1], dest)
   of mSizeOf, mAlignOf, mOffsetOf:
     fail(n.info, vmGenDiagMissingImportcCompleteStruct, m)
 

lib/core/macros.nim

@@ -522,7 +522,8 @@ proc quote*(bl: typed, op = "``"): NimNode {.magic: "QuoteAst", noSideEffect.} =
   ## A custom operator interpolation needs accent quoted (``) whenever it resolves
   ## to a symbol.
   ##
-  ## See also `genasts <genasts.html>`_ which avoids some issues with `quote`.
+  ## See also:
+  ## * `genasts <genasts.html>`_
   runnableExamples:
     macro check(ex: untyped) =
       # this is a simplified version of the check macro from the
@@ -591,6 +592,30 @@ proc quote*(bl: typed, op = "``"): NimNode {.magic: "QuoteAst", noSideEffect.} =
         doAssert y == 3
     bar2()
 
+proc quoteImpl(n: NimNode, args: varargs[NimNode]): NimNode {.compilerproc,
+    compileTime.} =
+  ## Substitutes the placeholders in `n` with the corresponding AST from
+  ## `args`. Invoked by the compiler for implementating ``quote``.
+  proc aux(n: NimNode, args: openArray[NimNode]): NimNode =
+    case n.kind
+    of nnkAccQuoted:
+      if n[0].kind == nnkAccQuoted:
+        result = n[0] # an escaped accquoted tree
+      else:
+        result = args[n[0].intVal] # a placeholder
+    else:
+      result = n
+      for i in 0..<n.len:
+        result[i] = aux(n[i], args)
+
+  result = aux(n, args)
+  # unwrap single-element statement lists:
+  if n.kind == nnkStmtList and n.len == 1:
+    result = n[0]
+
+proc evalToAst*[T](x: T): NimNode {.magic: "EvalToAst".} =
+  ## Leaked implementation detail. **Do not use**.
+
 proc expectKind*(n: NimNode, k: NimNodeKind) =
   ## Checks that `n` is of kind `k`. If this is not the case,
   ## compilation aborts with an error message. This is useful for writing

lib/experimental/ast_pattern_matching.nim

@@ -38,7 +38,7 @@ type
     WrongIdent
     WrongCustomCondition
 
-  MatchingError = object
+  MatchingError* = object
     node*: NimNode
     expectedKind*: set[NimNodeKind]
     case kind*: MatchingErrorKind

lib/js/asyncjs.nim

@@ -141,12 +141,13 @@ proc generateJsasync(arg: NimNode): NimNode =
   if len(code) > 0:
     # turn |NimSkull| outgoing exceptions into JavaScript errors
     let body = result.body
+    let reraise = bindSym("reraise")
     result.body = quote:
       try:
         `body`
       except CatchableError as e:
         # use .noreturn call to make sure `body` being an expression works
-        reraise(e)
+        `reraise`(e)
 
   let asyncPragma = quote:
     {.codegenDecl: "async function $2($3)".}

lib/js/jsffi.nim

@@ -225,38 +225,36 @@ macro `.`*(obj: JsObject, field: untyped): JsObject =
     let obj = newJsObject()
     obj.a = 20
     assert obj.a.to(int) == 20
+  let helper = genSym(nskProc, "helper")
   if validJsName($field):
     let importString = "#." & $field
     result = quote do:
-      proc helper(o: JsObject): JsObject
-        {.importjs: `importString`, gensym.}
-      helper(`obj`)
+      proc `helper`(o: JsObject): JsObject {.importjs: `importString`.}
+      `helper`(`obj`)
   else:
     if not mangledNames.hasKey($field):
       mangledNames[$field] = $mangleJsName($field)
     let importString = "#." & mangledNames[$field]
     result = quote do:
-      proc helper(o: JsObject): JsObject
-        {.importjs: `importString`, gensym.}
-      helper(`obj`)
+      proc `helper`(o: JsObject): JsObject {.importjs: `importString`.}
+      `helper`(`obj`)
 
 macro `.=`*(obj: JsObject, field, value: untyped): untyped =
   ## Experimental dot accessor (set) for type JsObject.
   ## Sets the value of a property of name `field` in a JsObject `x` to `value`.
+  let helper = genSym(nskProc, "helper")
   if validJsName($field):
     let importString = "#." & $field & " = #"
     result = quote do:
-      proc helper(o: JsObject, v: auto)
-        {.importjs: `importString`, gensym.}
-      helper(`obj`, `value`)
+      proc `helper`(o: JsObject, v: auto) {.importjs: `importString`.}
+      `helper`(`obj`, `value`)
   else:
     if not mangledNames.hasKey($field):
       mangledNames[$field] = $mangleJsName($field)
     let importString = "#." & mangledNames[$field] & " = #"
     result = quote do:
-      proc helper(o: JsObject, v: auto)
-        {.importjs: `importString`, gensym.}
-      helper(`obj`, `value`)
+      proc `helper`(o: JsObject, v: auto) {.importjs: `importString`.}
+      `helper`(`obj`, `value`)
 
 macro `.()`*(obj: JsObject,
              field: untyped,
@@ -284,10 +282,12 @@ macro `.()`*(obj: JsObject,
     if not mangledNames.hasKey($field):
       mangledNames[$field] = $mangleJsName($field)
     importString = "#." & mangledNames[$field] & "(@)"
+
+  let helper = genSym(nskProc, "helper")
   result = quote:
-    proc helper(o: JsObject): JsObject
-      {.importjs: `importString`, gensym, discardable.}
-    helper(`obj`)
+    proc `helper`(o: JsObject): JsObject
+      {.importjs: `importString`, discardable.}
+    `helper`(`obj`)
   for idx in 0 ..< args.len:
     let paramName = newIdentNode("param" & $idx)
     result[0][3].add newIdentDefs(paramName, newIdentNode("JsObject"))
@@ -304,10 +304,11 @@ macro `.`*[K: cstring, V](obj: JsAssoc[K, V],
     if not mangledNames.hasKey($field):
       mangledNames[$field] = $mangleJsName($field)
     importString = "#." & mangledNames[$field]
+
+  let helper = genSym(nskProc, "helper")
   result = quote do:
-    proc helper(o: type(`obj`)): `obj`.V
-      {.importjs: `importString`, gensym.}
-    helper(`obj`)
+    proc `helper`(o: type(`obj`)): `obj`.V {.importjs: `importString`.}
+    `helper`(`obj`)
 
 macro `.=`*[K: cstring, V](obj: JsAssoc[K, V],
                                     field: untyped,
@@ -321,10 +322,11 @@ macro `.=`*[K: cstring, V](obj: JsAssoc[K, V],
     if not mangledNames.hasKey($field):
       mangledNames[$field] = $mangleJsName($field)
     importString = "#." & mangledNames[$field] & " = #"
+
+  let helper = genSym(nskProc, "helper")
   result = quote do:
-    proc helper(o: type(`obj`), v: `obj`.V)
-      {.importjs: `importString`, gensym.}
-    helper(`obj`, `value`)
+    proc `helper`(o: type(`obj`), v: `obj`.V) {.importjs: `importString`.}
+    `helper`(`obj`, `value`)
 
 macro `.()`*[K: cstring, V: proc](obj: JsAssoc[K, V],
                                            field: untyped,
@@ -447,10 +449,11 @@ macro `{}`*(typ: typedesc, xs: varargs[untyped]): auto =
   body.add quote do:
     return `a`
 
+  let inner = genSym(nskProc, "inner")
   result = quote do:
-    proc inner(): `typ` {.gensym.} =
+    proc `inner`(): `typ` =
       `body`
-    inner()
+    `inner`()
 
 # Macro to build a lambda using JavaScript's `this`
 # from a proc, `this` being the first argument.

lib/std/jsonutils.nim

@@ -186,11 +186,11 @@ proc discKeyMatch[T](obj: T, json: JsonNode, key: static string): bool =
 macro discKeysMatchBodyGen(obj: typed, json: JsonNode,
                            keys: static seq[string]): untyped =
   result = newStmtList()
-  let r = ident("result")
+  let match = bindSym("discKeyMatch")
   for key in keys:
     let keyLit = newLit key
     result.add quote do:
-      `r` = `r` and discKeyMatch(`obj`, `json`, `keyLit`)
+      result = result and `match`(`obj`, `json`, `keyLit`)
 
 proc discKeysMatch[T](obj: T, json: JsonNode, keys: static seq[string]): bool =
   result = true

lib/std/tasks.nim

@@ -183,8 +183,9 @@ macro toTask*(e: typed{nkCall | nkInfix | nkPrefix | nkPostfix | nkCommand | nkC
                     )
 
 
+    let cAlloc = bindSym("c_calloc")
     let scratchObjPtrType = quote do:
-      cast[ptr `scratchObjType`](c_calloc(csize_t 1, csize_t sizeof(`scratchObjType`)))
+      cast[ptr `scratchObjType`](`cAlloc`(csize_t 1, csize_t sizeof(`scratchObjType`)))
 
     let scratchLetSection = newLetStmt(
       scratchIdent,