Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] Working with types in macro is difficult. #44

Open
mratsim opened this issue Apr 28, 2018 · 8 comments
Open

[RFC] Working with types in macro is difficult. #44

mratsim opened this issue Apr 28, 2018 · 8 comments

Comments

@mratsim
Copy link
Collaborator

mratsim commented Apr 28, 2018

Context

I find working with types in macro quite difficult and I think Nim would gain a lot if it was easier.
The 2 main issues I've noted are:

  • getTypeInst only works in a typed macro, but it doesn't work in a compile-time proc. In an untyped context, this forces us to use a helper macro with getAST/quote do as we can't call a macro from a macro.
  • It is very easy to get Cannot instantiate or type mismatch because of forgetting to convert the NimSym given by getTypeInst to NimIdent if they are not used raw.

Example:

import macros

type Conv2DLayer[T] = object
  data: T
type Context[T] = ref object
  data: T

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 ctxSubtype(context: Context): untyped =
  ## Extract the subtype from a Context
  echo context.getTypeInst.treerepr
  result = context.getTypeInst[1] # This gives a cryptic error got <Foo> expected <bar>
  # result = replaceNodes(context.getTypeInst[1]) # If we replace the symbol by ident it works!

macro genModelFromCtx(modelType: untyped, context: Context): untyped =

  var records = nnkRecList.newTree

  let subtype = getAST(ctxSubtype(context))

  let field_type = nnkBracketExpr.newTree(
      ident("Conv2DLayer"), subtype
    )

  records.add nnkIdentDefs.newTree(
      newIdentNode("field_name"),
      field_type,
      newEmptyNode()
    )

  result = nnkStmtList.newTree(
    nnkTypeSection.newTree(
      nnkTypeDef.newTree(
        newIdentNode($modelType),
        newEmptyNode(),
        nnkObjectTy.newTree(
          newEmptyNode(),
          newEmptyNode(),
          records
        )
      )
    )
  )

var ctx: Context[seq[int]]

expandMacros:
  genModelFromCtx(FooType2, ctx)

## Output - before replaceNodes:
# got: <seq[int]>
# but expected: <T>

## Output fixed:
# type
#   FooType2 = object
#     field_name: Conv2DLayer[seq[int]]
@mratsim
Copy link
Collaborator Author

mratsim commented Apr 30, 2018

Not sure if a bug or expected but the following getAST gives node has no type while quote do works, reported in #7739:

import macros, typetraits

proc getSubType(T: NimNode): NimNode =
  echo getTypeInst(T).treerepr
  result = getTypeInst(T)[1]

macro typed_helper(x: typed): untyped =

  result = getSubType(x)

macro untyped_heavylifting(x: varargs[untyped]): untyped =

  let first = x[0]

  # result = quote do: # Works
  #   typed_helper(`first`)

  result = getAST(typed_helper(first)) # Node has no type

var a: seq[int]
echo untyped_heavylifting(a)

@CodeDoes
Copy link

CodeDoes commented Jul 6, 2018

result = getAST(typed_helper(first.type))

@mratsim
Copy link
Collaborator Author

mratsim commented Jul 6, 2018

That gives typedesc[NimNode]

@CodeDoes
Copy link

getAST(typed_helper(first.type))[0]?

@mratsim
Copy link
Collaborator Author

mratsim commented Jul 11, 2018

@CodeDoes first.type is of type NimNode because it's an AST node representing x, it is not x. You need to use one of the getType, getTypeInst or getTypeImpl to access the type of x.

@mratsim
Copy link
Collaborator Author

mratsim commented Aug 4, 2018

The linked #8531 is another example of the pitfalls of getTypeInst returning NimSym that has to be converted to NimIdent otherwise we get cryptic errors in certain cases. cc @timotheecour.

@iffy
Copy link

iffy commented Oct 11, 2018

I don't completely understand everything in this post, but I do know that trying to use quote to make ASTs in macros is causing problems because it generates nnkSymNode where I want nnkIdentNode.

To get my code working, I used a modified version of replaceNodes from above (adding the branch for nnkOpenSymChoice):

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
    of nnkOpenSymChoice:
      return inspect(node[0])
    else:
      var rTree = node.kind.newTree()
      for child in node:
        rTree.add inspect(child)
      return rTree
  result = inspect(ast)

For reference, here's my original problem: https://forum.nim-lang.org/t/4300#26770

@narimiran narimiran transferred this issue from nim-lang/Nim Jan 2, 2019
mratsim added a commit to mratsim/laser that referenced this issue Jul 7, 2019
mratsim added a commit to status-im/nimbus-eth2 that referenced this issue Jan 8, 2021
Tests that don't involve serialization are passing, tests that involve serialization are broken. Example command:

`nim c -r --hints:off --warnings:off --verbosity:0 --outdir:build tests/official/test_fixture_operations_attester_slashings.nim`
mratsim added a commit to mratsim/constantine that referenced this issue Jan 21, 2021
mratsim added a commit to mratsim/constantine that referenced this issue Jan 21, 2021
* Introduce Fr type: finite field over curve order. Need workaround for nim-lang/Nim#16774

* Split curve properties into core and derived

* Attach field properties to an instantiated field instead of the curve enum

* Workaround nim-lang/Nim#14021, yet another "working with types in macros" is difficult nim-lang/RFCs#44

* Implement finite field over prime order of a curve subgroup

* skip OpenSSL tests on windows
@mratsim
Copy link
Collaborator Author

mratsim commented Feb 14, 2022

Another: https://forum.nim-lang.org/t/8913

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants