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

untyped macro nested in template: getting sym instead of idents #11091

Open
mratsim opened this issue Apr 23, 2019 · 5 comments
Open

untyped macro nested in template: getting sym instead of idents #11091

mratsim opened this issue Apr 23, 2019 · 5 comments

Comments

@mratsim
Copy link
Collaborator

mratsim commented Apr 23, 2019

When an untyped macro is nested in a template, the identifiers are transformed into nnkSym instead of being left as raw nnkIdent. We also get the OpenSymChoice instead of nnkBracketExpr.

Test case:

import macros

macro foo(body: untyped): untyped =
  echo body.treerepr
  body.expectKind nnkStmtList
  body[0].expectKind nnkProcDef
  body[0][3].expectKind nnkFormalParams
  body[0][3][1].expectKind nnkIdentDefs
  assert body[0][3][1][0].eqIdent "a"
  body[0][3][1][0].expectKind nnkIdent

  echo "Success!\n##############\n"
  result = newStmtList()


foo:
  proc bar[T](a, b: seq[T]): seq[T]

####################################

template baz(): untyped =
  foo:
    proc bar[T](a, b: seq[T]): seq[T]

baz()

First parsing output:

StmtList
  ProcDef
    Ident "bar"
    Empty
    GenericParams
      IdentDefs
        Ident "T"
        Empty
        Empty
    FormalParams
      BracketExpr
        Ident "seq"
        Ident "T"
      IdentDefs
        Ident "a"
        Ident "b"
        BracketExpr
          Ident "seq"
          Ident "T"
        Empty
    Empty
    Empty
    Empty

Second:

StmtList
  ProcDef
    Ident "bar"
    Empty
    GenericParams
      IdentDefs
        Ident "T"
        Empty
        Empty
    FormalParams
      Call
        OpenSymChoice
          Sym "[]"
          Sym "[]"
          Sym "[]"
          Sym "[]"
          Sym "[]"
          Sym "[]"
          Sym "[]"
          Sym "[]"
          Sym "[]"
          Sym "[]"
          Sym "[]"
          Sym "[]"
          Sym "[]"
        Sym "seq"
        Ident "T"
      IdentDefs
        Sym "a"
        Sym "b"
        Call
          OpenSymChoice
            Sym "[]"
            Sym "[]"
            Sym "[]"
            Sym "[]"
            Sym "[]"
            Sym "[]"
            Sym "[]"
            Sym "[]"
            Sym "[]"
            Sym "[]"
            Sym "[]"
            Sym "[]"
            Sym "[]"
          Sym "seq"
          Ident "T"
        Empty
    Empty
    Empty
    Empty
@Araq
Copy link
Member

Araq commented Apr 23, 2019

Macros need to be able to handle syms and sym choices. We must be able to do "partial" symbol lookups in templates.

@krux02
Copy link
Contributor

krux02 commented Apr 23, 2019

Workaround

template baz(): untyped {.dirty.} =
  foo:
    proc bar[T](a, b: seq[T]): seq[T]

Templates bind all their identifiers, so that templates can call functions that are not exported in the same module. This is expected behavior, but in your case it is totally unwanted, and I can see that it causes other problems as well. To avoid this early symbol binding, you should use the {.dirty.} tag, but be aware that it also eliminates the hygienic-ness of the template.

@mratsim
Copy link
Collaborator Author

mratsim commented Apr 23, 2019

@krux02 regarding dirty I'm aware of it, the main issue is that I want to have a compile macro for my ML compiler that would do something like this:

template scope1(): untyped =
  var a {.compileTime.} = input("a") # This is a custom AST type for my compiler
  let b {.compileTime.} = input("b")

  static:
    a += b

  compile([a, b]):
    proc foobar[T](a: var T, b: T)

template scope2(): untyped =
  let
    a {.compileTime.} = input("a") # This is a custom AST type for my compiler
    b {.compileTime.} = input("b")
    c {.compileTime.} = input("c")

    foo {.compileTime.} = a + b + c
    bar {.compileTime.} = foo * 2

  compile([a, b, c, foo, bar]):
    proc foobar[T](a, b, c: T): tuple[foo, bar: T]

I would have a conflict of variable if those templates were dirty and in the same file.

@Araq, the main issue is that with untyped macro I expect to have the raw parsed AST and this is what is described in the manual for untyped as well. Furthermore when you have scope nesting (if/case/block/for loop block) the inner scope shadows the outer scope usually but for template with inner macro, the template applies first.

If it's not possible to have inner scope macro expansion done before outer scope template symbol resolution, I see 2 workarounds forward.

  1. Allow fine-grained {.dirty.} pragma, for example if we use a syntax similar to fine-grained noRewrite
template scope1(): untyped =
  var a {.compileTime.} = input("a") # This is a custom AST type for my compiler
  let b {.compileTime.} = input("b")

  static:
    a += b

  {.dirty.}:
    compile([a, b]):
      proc foobar[T](a: var T, b: T)

It's ugly in my opinion but well.

  1. Have a unresolveSym helper that undo early symbol resolution. I thought early symbol resolution was restricted to generics/static ([Meta] Generics/Static early symbol resolution #8677) and there was way around it for templates following the closing of strformat doesn't work properly inside generics #7632 but apparently it's still ongoing for template as well ("undeclared identifier" error when using fmt from strformat on devel inside a template  #10977 and add strformat limitations section #10982).

I basically have to unresolve symbols in many of my projects and expanding the standard library starts to get quite costly due to similar symbol resolution issues, see strformat issues linked previously.

Note that working with types in macro, for example with getTypeInst, is also hard (nim-lang/RFCs#44) because it also requires to unresolve symbols, see distinctBase #8531 (comment)

@Araq
Copy link
Member

Araq commented Apr 24, 2019

We could introduce a macros.toUntypedAst that understands what it means to un-sem'check an AST. Library additions are better than language additions at this point.

@krux02
Copy link
Contributor

krux02 commented Apr 24, 2019

Well, I have no idea what you are doing, but you can leave out body[0][3][1][0].expectKind nnkIdent in your ast sanity check or replace it with body[0][3][1][0].expectKind {nnkIdent,nnkSym, nnkOpenSymChoice, nnkClosedSymChoice}. eqIdent is written in a way that it also accepts symbols and (open/closed) sym choices with the given name as well. I think this exception where symbols might leak into untyped macro parameters really needs to be documented better.

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

No branches or pull requests

3 participants