Macro generated generic proc doesnt work

[SolitudeSF]

I have following macro that generates the procedure out of existing procedure. It essentially just strips a return value and adds var to a first argument. I also had to move some generic related fields, because getImpl for some reason provides illformed AST.

Example

import macros

macro genMutating*(p: typed, name: untyped) =
  result = p.getImpl

  var call = nnkCall.newTree(result[0].strVal.ident)
  for i in 1..<result[3].len:
    for c in result[3][i]:
      if c.kind == nnkIdent: call.add c

  result[0] = nnkPostfix.newTree(ident "*", ident name.strVal)
  result[2] = result[5][1]
  result[3][0] = newEmptyNode()
  result[3][1][1] = nnkVarTy.newTree(result[3][1][1])
  result[5] = newEmptyNode()
  result[6] = nnkStmtList.newTree(nnkAsgn.newTree(result[3][1][0], call))

type
  Color* = array[3, int] | array[4, int]
  Image*[T: Color] = seq[T]

func test*[T: Color](img: Image[T]): Image[T] = discard

test.genMutating res
# func res[T](img: var Image[T]) =
#   img = test(img)

var i = Image[array[3, int]](@[[1, 2, 3]])
i.res

Current Output

Compilation fails with

test.nim(29, 2) template/generic instantiation of `res` from here
test.nim(22, 12) Error: cannot instantiate: 'T'

But if i use --expandMacro or repr result and place the output into the code instead of macro the program compiles and runs fine.

Expected Output

Should compile and run successfully.

Tested on 0.20.2 and latest devel.

[mratsim]

Welcome to the fabulous land of nim-lang/RFCs#44.

Take a chair and a glass and let me provide you with a magic fix:

proc replaceNodes*(ast: NimNode): NimNode =
  # Replace NimIdent and NimSym by a fresh ident node
  proc inspect(node: NimNode): NimNode =
    case node.kind:
    of {nnkIdent, nnkSym}:
      return ident($node)
    of nnkEmpty:
      return node
    of nnkLiterals:
      return node
    else:
      var rTree = node.kind.newTree()
      for child in node:
        rTree.add inspect(child)
      return rTree
  result = inspect(ast)

plug it in your macros, and surprise it works:

import macros

proc replaceNodes*(ast: NimNode): NimNode =
  # Replace NimIdent and NimSym by a fresh ident node
  proc inspect(node: NimNode): NimNode =
    case node.kind:
    of {nnkIdent, nnkSym}:
      return ident($node)
    of nnkEmpty:
      return node
    of nnkLiterals:
      return node
    else:
      var rTree = node.kind.newTree()
      for child in node:
        rTree.add inspect(child)
      return rTree
  result = inspect(ast)

macro genMutating*(p: typed, name: untyped) =
  result = p.getImpl

  var call = nnkCall.newTree(result[0].strVal.ident)
  for i in 1..<result[3].len:
    for c in result[3][i]:
      if c.kind == nnkIdent: call.add c

  result[0] = nnkPostfix.newTree(ident "*", ident name.strVal)
  result[2] = result[5][1]
  result[3][0] = newEmptyNode()
  result[3][1][1] = nnkVarTy.newTree(result[3][1][1])
  result[5] = newEmptyNode()
  result[6] = nnkStmtList.newTree(nnkAsgn.newTree(result[3][1][0], call))

  result = replaceNodes(result)

type
  Color* = array[3, int] | array[4, int]
  Image*[T: Color] = seq[T]

func test*[T: Color](img: Image[T]): Image[T] = discard

test.genMutating res
# func res[T](img: var Image[T]) =
#   img = test(img)

var i = Image[array[3, int]](@[[1, 2, 3]])
i.res

---

What is happening?

In some cases, typed macro, or macro nested inside a generic or template (#11091), you will get symbols instead of idents for types. Symbols are already bound to a type T and you need to regenerate a fresh ident to avoid that, see detailed explanation in #8531 (comment).

[mratsim]

Tagging langage design.

Either getImpl is working as intended: i.e. when passed a generic typed proc it "instantiates" T to an abstract type and returns a symbol "T", in that case we should close this bug in favor of nim-lang/RFCs#44.

Or getImpl (and maybe getTypeImpl) should not instantiate generics and in that case this is an actual implementation bug.

[SolitudeSF]

it its working as intended, then maybe the replaceNodes helper, probably under a different name, should be added to macros.

[mratsim]

The proposed solution is to introduce a toUntypedAST that undo semchecks #11091 (comment).

I don't think it's working as intended but at this point the generic machine seems to be too time-consuming to change in-depth. A proper fix of the recurrent instantiation and semcheck issues reported would as far as I know require such in-depth changes.

[krux02]

@Araq, I think we should really document better that semchecked may not be modified. I think about the following solution: A pnode has the flags field where we can detect if a node is already semchecked or not. In opcNSetChild we can then check if nfSem is not part of the flags of that node. If it is, emit an error. This way result[0] = nnkPostfix.newTree(ident "*", ident name.strVal) would already emit an error message. A clear error message that explains that nodes that are already typechecked should be used read only.