New quote ast 2

[krux02]

This pull request superseeds my old pull request from here, therefore it is also an implementation of my proposal nim-lang/RFCs#122.

My old pull request uses an uq(expr) syntax to inject an expression into the quoted AST. I replaced that syntax with explicit symbol injection. This syntax was proposed from #11722, this circomvents the problem that the uq(expr) syntax isn't allowed everywhere (explained in the "Limitations" section of the mentioned RFC).

As a side effect, this PR also introduces a proc to manually set the lineinfo for a NimNode.

The biggest fix compared to quote do is that identifiers are not automatically captured. This means clean error messages when you forget to capture your variables. Clean error messages when they have the incorrect type, and no automatically doing the wrong thing for you. This also means that things are a little bit more verbose, the usage of newLit and bindSym is enforced, it is not automatically done for you. I think in the future it is sane to add a shortcut syntax for the most common patterns. For example *myVal for myVal = newLit(myVal) and @myProc for myProc = bindSym"myProc". But that is just syntactic sugar and not part of this initial PR.

[zah]

Why is the use of getAst avoided here? If there are issues with it, perhaps they can be fixed instead?

I'm worried about the compilation speed impact of the approach taken here. If this makes it to macros.nim, it will:

• Increase the number of definition that the compiler has to process in the macros module.
• Perform all the AST heavy-lifting in the VM, instead of in the C code of the compiler.

It also introduces an alternative code path to do the same thing, which will be a source of inconsistencies and bugs when the rules of the language change (for the record, this is part of the reason why quote deteriorated so much).

[krux02]

I avoided getAst, because I think the usage of a local template and getAst is what causes most of the problems in quote do. This implementation is a proof of concept right now, by not using getAst and doing everything in a macro, I avoided to deal with the problems of getAst and I could focus on getting the behavior right. I have the control over everything and fast iteration times.

This is also a continuation of my PR with the uq(expr) syntax. Here I also thought about arbitrary neting quotoAst(uq(quoteAst(uq(...)))), and future directions, such as generator expressions quoteAst(var array = [uq(for i in 1 .. 10: yield newLit(i))...]). I did think that this would work nicely with the behavior of getAst.

Perform all the AST heavy-lifting in the VM, instead of in the C code of the compiler.

I am not doing heavy lifting in the VM, it is much more of a light lifting. I translate a block of code into a block of code that generates this code. This is what some people manually write because they don't want to deal with quote do, and they never complained about performance problems here.

But the vm runs out of registers quickly and I didn't figure out exactly why, yet. This is more concerning to me right now.

[krux02]

@zah I wanted to avoid getAst because there were several issues with getAst. One of them is that it erases comment statements. I found a workaround for that one. And in combination with dirty templates getAst seems to be becoma feasible. To fix the comment statement problem of getAst the documentation generator should be rewritten, and I don't know if that is worth the effort right now. I just hope that template instantiation won't get more hacks.

By now I removed most of the features that at one point I implemented and left only what I think is a bare bone quoteAst with only the necessary features and no magic going on. You might be interested to take another look on it.

[zah]

Why did you decide to avoid Timothee's approach where there is an extra argument controlling the dirtiness of the template? It seems to me that this can still be useful from time to time and it will make life easier for people switching to the new API.

Otherwise, can you please add a test for a nested use of quoteAst like the one here:
https://github.com/nim-lang/Nim/pull/11722/files#diff-f0de32b58ad2d14a98d80be161ed74f0R177-R182

genAst is perhaps a nicer name due to being shorter. It could be complemented by another macro called genCode that can be used to create a static block that generates a bunch of code:

template genCode*(body: untyped) =
  iterator generator: NimNode = body

  macro payload: untyped =
    result = newStmtList()
    for node in generator():
      result.add node

  payload()

proc main =
  genCode:
    for i in 1..10:
      yield newCall("echo", newLit(i))

Such blocks have a relatively wide-spread use in the Boo language.

[krux02]

@zah, at all costs the temporary template may not bind to local variables automatically if not explicitly requested to do so. This is one of the biggest problems that quote do has. Therefore non dirty templates are out of the question and no parameter controlling the dirtyness of the template. This was also one of the reasons why I didn't want to go with templates at all in the beginning.

For the generator expression, yes nice idea. I had a similar idea for when I still wanted to implement quoting with the unquote operator. But the whole idea of unquote as an expression did not work out at all. Nim does not allow arbitrary expressions everywhere. Yes you could use it to generate statement lists like in your example, but you would not be able to use it to fill up fiends in type definitions. And now since all genrating expressions have been forced to be written before the quoteAst command, it is also not that much of a difference to also generate the satement list in a local loop before the quoteAst command:

let body = newStmtList()
for i in 1..10:
  let i = newLit(i)
  body.addQuoteAst(i):
    echo i

result = quoteast(body):
  proc main() =
    body

I am thinking of a shortcut to inject newLit more easily though:

let body = newStmtList()
for i in 1..10:
  # prefix @ to inject newLit for i
  body.addQuoteAst(@i):
    echo i

I am also thinking about other prefixes, for example +myName for bindSym("myName", brOpen) and -myName for bindSym("myName", brClosed). But these are optional features. Not core essentials, therefore I left them out in this PR.

[zah]

Well, I'm looking forward to try the new quote replacements in our codebase. I'll try to provide some feedback regarding how painful it is to switch from non-dirty to dirty by default.

[timotheecour]

let's compare this PR with #11722 on a simple example:

#main.nim
import ./t0672c
echo foobar(1+2, true)

#t0672c.nim
import macros
import tables

type Kar = ref object
proc `$`(a: Kar): string = "custom"

when defined case_quoteAst: # using this PR #11823
  macro foobar*(arg: untyped, b: static bool): untyped =
    let b = newLit b
    let newTable = bindSym"newTable"
    let `[]=` = bindSym("[]=")
    let mySet = bindSym"[]="
    let myGet = bindSym"[]"
    let pairs = bindSym"pairs"
    let Kar = bindSym"Kar"
    let `$` = bindSym"$"
    result = quoteAst(arg, b, newTable, `[]=`, mySet, myGet, pairs, Kar, `$`):
      let t = newTable[string, Kar]()
      # t["foo1"] = Kar() # Error: type mismatch => doesn't work with `[]=`, ditto with `[]`
      mySet(t, "foo1", Kar())
      for k,v in pairs(t): echo (k,$v)
      (arg, b, myGet(t,"foo1"))

when defined case_genAst2: # using PR #11722
  macro foobar*(arg: untyped, b: static bool): untyped =
    result = genAst(arg, b):
      let t = newTable[string, Kar]()
      t["foo1"] = Kar()
      for k,v in pairs(t): echo (k,$v)
      (arg, b, t["foo1"])

• I much prefer the simplicity of genAst2
• all that's needed when migrating from quote do is to change a quoted ident to a param in genAst, ie the ones declared in the macro
• migrating from quoteAst requires passing every symbol referenced; and suffers from caller scope hijacking issue I mentioned in [superseded] new macros.genAst: sidesteps issues with quote do #11722 if some symbols are forgotten
• I couldn't get []= to work with quoteAst (without requiring caller to import tables), hence the mySet workaround mentioned above
• you'll also need renaming in practice: let b2 = newLit b instead of let b = newLit b in case b is used in macro body as well; that also hurts readability
• quoteAst works opposite of how templates work in nim (templates being hygienic by default instead of dirty by default)

As for #10430 (Comments are removed in quote do expressions), this PR doesn't have that issue, unlike genAst, which is nice; however this issue can be fixed, see #10430 (comment)

[Araq]

@timotheecour but that is not a fair comparison, quoteAst is designed to do what .dirty templates do. If you want bindSym etc everywhere, that's already what quote do does. And IMO it's rarely the behavior that is desired.

[krux02]

@timotheecour

• I much prefer the simplicity of genAst2

• I much prefer the explicity on quoteAst. More verbose to type, but simpler to learn and easier to understand when read.

• all that's needed when migrating from quote do is to change a quoted ident to a param in genAst, ie the ones declared in the macro

Here the focus is not on how little is to do when migrating, it is about how straight forward is it. Is everything that needs to be changed caught in an error message that the programmer can understand? Is it obvious what to do?

• migrating from quoteAst requires passing every symbol referenced; and suffers from caller scope hijacking issue I mentioned in [superseded] new macros.genAst: sidesteps issues with quote do #11722 if some symbols are forgotten

What do you mean? What is caller scope hijacking? Please explain what you mean.

• I couldn't get []= to work with quoteAst (without requiring caller to import tables), hence the mySet workaround mentioned above

Yea this is a bummer, good that you found it, I will fix it. The problem is []= doesn't exist as an identifier in the untyped AST. This is a problem of the Nim AST, but not too bad to not find a solution for it. How does your implementation work around this problem?

• you'll also need renaming in practice: let b2 = newLit b instead of let b = newLit b in case b is used in macro body as well; that also hurts readability

Can you provide an example for this? I don't see that problem.

• quoteAst works opposite of how templates work in nim (templates being hygienic by default instead of dirty by default)

Yea, that is an issue that I understand. The problem that I have is, In a macro I can only declare a tepmlate in the scope where a template is expanded. In this scope all locals variables are in scope. I don't know how to shield my template from these local symbols other than making the template dirty.

As for #10430 (Comments are removed in quote do expressions), this PR doesn't have that issue, unlike genAst, which is nice; however this issue can be fixed, see #10430 (comment)

Yea I found a workaround for it.

[timotheecour]

Here the focus is not on how little is to do when migrating, it is about how straight forward is it. Is everything that needs to be changed caught in an error message that the programmer can understand? Is it obvious what to do?

#11722 has examples showing the CT error msgs the user will encounter when he misuses genAst. IIRC the one that's not very friendly is var not init; this one can and should be improved (in separate PR, it can occur in pre-existing code)

Can you provide an example for this? I don't see that problem.

you have to rename let b2=newLit b instead of keeping same name let b=newLit b in several cases:

• when original b is still needed in macro body after quoteAst
• if it's a local proc, eg:

  import macros
  macro fun*(): untyped =
    proc `$`(a: ref int): string = "foo"
    # let `$` = bindSym"$"      # Error: redefinition of '$';
    let toString = bindSym"$" # have to rename
    result = quoteAst(toString):
      let b = new int
      discard toString(b)

so that makes it awkward for local operators, eg $, []= etc

What do you mean? What is caller scope hijacking? Please explain what you mean.

# main.nim
  import randomimport, mylib
  fun()
# randomimport.nim
proc `$`*(a: ref int): string = "foo1" # hijacks `$`
# mylib.nim
  import macros
  proc `$`(a: ref int): string = "foo2"
  macro fun*(): untyped =
    result = quoteAst():
      let b = new int
      echo $b

mylib library writer probably intended this to print "foo2", but this will print "foo1". Yes you can blame library writer for forgetting to use

let `$` = bindSym"$"
result = quoteAst():

but in complex examples, with lots of bindSym arguments passed to quoteAst, this type of errors is likely to happen (I had an example for that in #11722 tests/macros/tgenast.nim)

How does your implementation work around this problem?

it uses non-dirty templates so []= is bound in callee instead of caller; and as mentioned in #11722 (comment) (see mixin locafun1), it's easy to override that for a particular symbol to achieve what library writer wants.

[krux02]

#11722 has examples showing the CT error msgs the user will encounter when he misuses genAst. IIRC the one that's not very friendly is var not init; this one can and should be improved (in separate PR, it can occur in pre-existing code)

In that case my solution will show the error message "undeclared identifier a". To me that is a much better error message than var not init.

you have to rename let b2=newLit b instead of keeping same name let b=newLit b in several cases:

• when original b is still needed in macro body after quoteAst
• if it's a local proc, eg:

  import macros
  macro fun*(): untyped =
    proc `$`(a: ref int): string = "foo"
    # let `$` = bindSym"$"      # Error: redefinition of '$';
    let toString = bindSym"$" # have to rename
    result = quoteAst(toString):
      let b = new int
      discard toString(b)

so that makes it awkward for local operators, eg $, []= etc

Local procedures are for when the procedure accesses the environment (closure). This is exactly what bound symbols should never do. When you put $ proc to top level where it belongs, the let $ = bindSym"$" should work.

What do you mean? What is caller scope hijacking? Please explain what you mean.

# main.nim
  import randomimport, mylib
  fun()
# randomimport.nim
proc `$`*(a: ref int): string = "foo1" # hijacks `$`
# mylib.nim
  import macros
  proc `$`(a: ref int): string = "foo2"
  macro fun*(): untyped =
    result = quoteAst():
      let b = new int
      echo $b

mylib library writer probably intended this to print "foo2", but this will print "foo1". Yes you can blame library writer for forgetting to use

let `$` = bindSym"$"
result = quoteAst():

but in complex examples, with lots of bindSym arguments passed to quoteAst, this type of errors is likely to happen (I had an example for that in #11722 tests/macros/tgenast.nim)

Your example won't compile unless $ is hijacked. This means if the library programmer executes in macro only in a single test where the hijack isn't imported it will instantly fail with a compilation error. The local unexported proc cannot be called unless it is explicitly bound.

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. If you think it is still a valid PR, please rebase it on the latest devel; otherwise it will be closed. Thank you for your contributions.