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

[Macro] NimNode of Forward (Prototype) Procedure Defined with quote Cause Error #13484

Closed
loloicci opened this issue Feb 24, 2020 · 9 comments · Fixed by #15091
Closed

[Macro] NimNode of Forward (Prototype) Procedure Defined with quote Cause Error #13484

loloicci opened this issue Feb 24, 2020 · 9 comments · Fixed by #15091

Comments

@loloicci
Copy link
Contributor

loloicci commented Feb 24, 2020

When we use the forward procedure defined with quote, compiler issues following error.

Example

proc defForwardQuo(id: NimNode): NimNode =
  result = quote do:
    proc `id`(): int

proc defForwardNew(id: NimNode): NimNode =
  result = newProc(id, @[newIdentNode("int")])

proc def(id: NimNode): NimNode =
  result = quote do:
    proc `id`(): int =
      result = 42

proc call(id: NimNode): NimNode =
  result = quote do:
    `id`()

macro defCallQuo(): untyped =
  # cause an error
  let
    procId = genSym(nskProc)
    protoNode = defForwardQuo(procId)
    defNode = def(procId)
    callNode = call(procId)
  result = quote do:
    `protoNode`
    `defNode`
    echo `callNode`
  echo result.toStrLit

macro defCallNew(): untyped =
  # cause no error
  let
    procId = genSym(nskProc)
    protoNode = defForwardNew(procId)
    defNode = def(procId)
    callNode = call(procId)
  result = quote do:
    `protoNode`
    `defNode`
    echo `callNode`
  echo result.toStrLit

# cause an error
defCallQuo()

# cause no error
# defCallNew()

Current Output

proc `:tmp`_141170(): int
proc `:tmp`_141170(): int =
  result = 42

echo `:tmp_141170`()
/.../test.nim(31, 7) Hint: 'defCallNew' is declare
d but not used [XDeclaredButNotUsed]                                             
/.../test.nim(21, 20) Error: internal error: still
 forwarded: :tmp                                                                 
No stack traceback available
To create a stacktrace, rerun compilation with ./koch temp c <file>

and the result of ./koch temp c test.nim is as following

proc `:tmp`_185170(): int
proc `:tmp`_185170(): int =
  result = 42

echo `:tmp_185170`()
/.../test.nim(31, 7) Hint: 'defCallNew' is declare
d but not used [XDeclaredButNotUsed]                                             
/.../test.nim(21, 20) Error: internal error: still
 forwarded: :tmp                                                                 
Traceback (most recent call last)
/.../Nim/compiler/nim.nim(118) nim
/.../Nim/compiler/nim.nim(95) handleCmdLine
/.../Nim/compiler/cmdlinehelper.nim(98) loadConfigsAndRunMainC
ommand                                                                           
/.../Nim/compiler/main.nim(188) mainCommand
/.../Nim/compiler/main.nim(95) commandCompileToC
/.../Nim/compiler/cgen.nim(2024) cgenWriteModules
/.../Nim/compiler/cgen.nim(2013) genForwardedProcs
/.../Nim/compiler/msgs.nim(561) internalError
/.../Nim/compiler/msgs.nim(532) liMessage
/.../Nim/compiler/msgs.nim(357) handleError
/.../Nim/compiler/msgs.nim(347) quit
FAILURE

Expected Output

proc `:tmp`_185170(): int
proc `:tmp`_185170(): int =
  result = 42

echo `:tmp_185170`()
42

Possible Solution

I have no idea.

Additional Information

$ nim -v
Nim Compiler Version 1.0.6 [MacOSX: amd64]
Compiled at 2020-01-23
Copyright (c) 2006-2019 by Andreas Rumpf

git hash: 89b39ee8fe271d0e1b75c15a4c6cf82eb9c13aea
active boot switches: -d:release

and

$ ./koch temp -v
/.../.nimble/bin/nim c -d:debug --debugger:native -d:nimBetterRun  -v -
d:leanCompiler /.../Nim/compiler/nim                         
Nim Compiler Version 1.0.6 [MacOSX: amd64]
Compiled at 2020-01-23
Copyright (c) 2006-2019 by Andreas Rumpf

git hash: 89b39ee8fe271d0e1b75c15a4c6cf82eb9c13aea
active boot switches: -d:release
@krux02
Copy link
Contributor

krux02 commented Feb 26, 2020

newProc does not create a forward declaration, it creates a definition with an empty body. So defForwardNew does not create a forward declaration. But there is still a bug here.

@loloicci
Copy link
Contributor Author

loloicci commented Feb 26, 2020

defForwardNew does not create a forward declaration. But there is still a bug here.

import macros

proc defForwardQuo(id: NimNode): NimNode =
  result = quote do:
    proc `id`(n: int): bool

proc defForwardNew(id: NimNode): NimNode =
  result = newProc(id, @[newIdentNode("bool"), newIdentDefs(ident("n"), ident("int"))])

proc defEven(evenid, oddid: NimNode): NimNode =
  result = quote do:
    proc `evenid`(n: int): bool =
      if n == 0:
        return true
      else:
        return `oddid`(n - 1)

proc defOdd(evenid, oddid: NimNode): NimNode =
  result = quote do:
    proc `oddid`(n: int): bool =
      if n == 0:
        return false
      else:
        return `evenid`(n - 1)

proc call(id, param: NimNode): NimNode =
  result = quote do:
    `id`(`param`)

macro testEvenOdd(): untyped =
  let
    evenid = genSym(nskProc)
    oddid = genSym(nskProc)
    oddForwardNew = defForwardNew(oddid)
    oddForwardQuo = defForwardQuo(oddid)
    even = defEven(evenid, oddid)
    odd = defOdd(evenid, oddid)
    callEven42 = call(evenid, newLit(42))
    callOdd42 = call(oddid, newLit(42))
  result = quote do:
    `oddForwardNew`
    `even`
    `odd`
    echo `callEven42`
    echo `callOdd42`

testEvenOdd()

Although this succeeds, when I use oddForwardQuo instead of oddForwardNew it fails.
I think this means defForwardNew makes a forward declaration node and defForwardQuo does not.

@krux02 What is the difference between "definition with an empty body" and "forward declaration"?

@krux02
Copy link
Contributor

krux02 commented Feb 26, 2020

Look at this. The forward declaration as an empty node at the end. If it does not have a empty node the declaration is a definition. Having two identical definitions should raise a compilation error.

import macros

dumpTree:
  proc foo()

  proc foo() =
    discard

@loloicci
Copy link
Contributor Author

loloicci commented Feb 26, 2020

@krux02
Did you mean forward declaration node should be defined with newProc(..., body=newEmptyNode(),...), right?
(But, in my previous example, a definition with an empty body works as a forward declaration. Is it an unexpected behavior?)

import macros

macro showNewProc(): untyped =
  let
    id = genSym(nskProc)
    prams = @[newIdentNode("bool")]
    # should be newProc(id, prams, body=newEmptyNode())
    procNode = newProc(id, prams)

  echo procNode.treeRepr

showNewProc()

returns:

ProcDef
  Sym ":tmp"
  Empty
  Empty
  FormalParams
    Ident "bool"
  Empty
  Empty
  StmtList    # <- NonEmpty. should empty if it is a foward declaration

@krux02
Copy link
Contributor

krux02 commented Feb 26, 2020

Did you mean forward declaration node should be defined with newProc(..., body=newEmptyNode(),...), right?

That is exactly what I mean.

@loloicci
Copy link
Contributor Author

loloicci commented Feb 26, 2020

I think the body of a forward declaration should be an empty StmtList instead of an EmptyNode.

It is the same as my previous example except defForwardNew.
This defForwardNew makes a forward declaration with newProc(..., body=newEmptyNode(), ...)

import macros

proc defForwardQuo(id: NimNode): NimNode =
  result = quote do:
    proc `id`(n: int): bool

proc defForwardNew(id: NimNode): NimNode =
  result = newProc(id, @[newIdentNode("bool"), newIdentDefs(ident("n"), ident("int"))], body=newEmptyNode())

proc defEven(evenid, oddid: NimNode): NimNode =
  result = quote do:
    proc `evenid`(n: int): bool =
      if n == 0:
        return true
      else:
        return `oddid`(n - 1)

proc defOdd(evenid, oddid: NimNode): NimNode =
  result = quote do:
    proc `oddid`(n: int): bool =
      if n == 0:
        return false
      else:
        return `evenid`(n - 1)

proc call(id, param: NimNode): NimNode =
  result = quote do:
    `id`(`param`)

macro testEvenOdd(): untyped =
  let
    evenid = genSym(nskProc)
    oddid = genSym(nskProc)
    oddForwardNew = defForwardNew(oddid)
    oddForwardQuo = defForwardQuo(oddid)
    even = defEven(evenid, oddid)
    odd = defOdd(evenid, oddid)
    callEven42 = call(evenid, newLit(42))
    callOdd42 = call(oddid, newLit(42))
  result = quote do:
    `oddForwardNew`
    `even`
    `odd`
    echo `callEven42`
    echo `callOdd42`

testEvenOdd()

the compiler issues an error the same as the first (when it issue is opened) example.

$ koch temp c evenodd.nim 
/....nimble/bin/nim c -d:debug --debugger:native -d:nimBetterRun  -d:l
eanCompiler /...Nim/compiler/nim                            
Hint: used config file '/.../.choosenim/toolchains/nim-1.0.6/config/nim
.cfg' [Conf]                                                                     
Hint: used config file '/.../Nim/compiler/nim.cfg' [Conf]
Hint: operation successful (329 lines compiled; 0.190 sec total; 5.004MiB peakmem;
 Debug Build) [SuccessX]                                                         
/.../Nim/bin/nim_temp  c evenodd.nim
Hint: used config file '/.../Nim/config/nim.cfg' [Conf]
Hint: used config file '/.../Nim/config/config.nims' [Conf]
Hint: system [Processing]
Hint: widestrs [Processing]
Hint: io [Processing]
Hint: evenodd [Processing]
Hint: macros [Processing]
/.../evenodd.nim(35, 5) Hint: 'oddForwardQuo' is decla
red but not used [XDeclaredButNotUsed]                                           
/.../evenodd.nim(33, 19) Error: internal error: still 
forwarded: :tmp                                                                  
Traceback (most recent call last)
/.../Nim/compiler/nim.nim(118) nim
/.../Nim/compiler/nim.nim(95) handleCmdLine
/.../Nim/compiler/cmdlinehelper.nim(98) loadConfigsAndRunMainC
ommand                                                                           
/.../Nim/compiler/main.nim(188) mainCommand
/.../Nim/compiler/main.nim(95) commandCompileToC
/.../Nim/compiler/cgen.nim(2024) cgenWriteModules
/.../Nim/compiler/cgen.nim(2013) genForwardedProcs
/.../Nim/compiler/msgs.nim(561) internalError
/.../Nim/compiler/msgs.nim(532) liMessage
/.../Nim/compiler/msgs.nim(357) handleError
/.../Nim/compiler/msgs.nim(347) quit
FAILURE

and the following causes no error.

macro testEvenOdd2(): untyped =
  let
    evenid = genSym(nskProc)
    oddid = genSym(nskProc)
    oddForwardNew = defForwardNew(oddid)
    oddForwardQuo = defForwardQuo(oddid)
    even = defEven(evenid, oddid)
    odd = defOdd(evenid, oddid)
    callEven42 = call(evenid, newLit(42))
    callOdd42 = call(oddid, newLit(42))
  # oddForwardQuo[6] is the body of the procedure.
  oddForwardQuo[6] = newStmtList()
  result = quote do:
    `oddForwardQuo`
    `even`
    `odd`
    echo `callEven42`
    echo `callOdd42`

@loloicci
Copy link
Contributor Author

loloicci commented Feb 26, 2020

The same problem is seen in quoteAST in 585df74 .
(forward decleration made by quoteAST has an EmptyNode as their body. It should be an empty StmtList)

@krux02
Copy link
Contributor

krux02 commented Feb 27, 2020

forward decleration made by quoteAST has an EmptyNode as their body. It should be an empty StmtList

Not really. How do you think the compiler distinguishes a forward declaration from an implementation that does nothing. It is not the discard statement, that one is only required for the parser.

import macros

dumpTree:
  proc foo()

  proc foo() =
    discard

@loloicci
Copy link
Contributor Author

loloicci commented Feb 27, 2020

Sorry @krux02, you are right.
But, ProcNode's behavior differs depending on whose id.
When the id is generated with genSym, to treat the ProcNode as a forward declaration, its body needs to be an empty StmtList.
Maybe it is wrong behavior.

To detail, see the following example.

import macros

proc defForward(id, nid: NimNode): NimNode =
  result = newProc(id, @[newIdentNode("bool"), newIdentDefs(nid, newIdentNode("int"))], body=newEmptyNode())

proc defEven(evenid, oddid, nid: NimNode): NimNode =
  result = quote do:
    proc `evenid`(`nid`: int): bool =
      if `nid` == 0:
        return true
      else:
        return `oddid`(`nid` - 1)

proc defOdd(evenid, oddid, nid: NimNode): NimNode =
  result = quote do:
    proc `oddid`(`nid`: int): bool =
      if `nid` == 0:
        return false
      else:
        return `evenid`(`nid` - 1)

proc callNode(funid, param: NimNode): NimNode =
  result = quote do:
    `funid`(`param`)

macro testEvenOdd3(): untyped =
  let
    evenid = newIdentNode("even3")
    oddid = newIdentNode("odd3")
    nid = newIdentNode("n")
    oddForward = defForward(oddid, nid)
    even = defEven(evenid, oddid, nid)
    odd = defOdd(evenid, oddid, nid)
    callEven = callNode(evenid, newLit(42))
    callOdd = callNode(oddid, newLit(42))
  result = quote do:
    `oddForward`
    `even`
    `odd`
    echo `callEven`
    echo `callOdd`

macro testEvenOdd4(): untyped =
  let
    evenid = newIdentNode("even4")
    oddid = newIdentNode("odd4")
    nid = newIdentNode("n")
    oddForward = defForward(oddid, nid)
    even = defEven(evenid, oddid, nid)
    odd = defOdd(evenid, oddid, nid)
    callEven = callNode(evenid, newLit(42))
    callOdd = callNode(oddid, newLit(42))
  # rewrite the body of proc node.
  oddForward[6] = newStmtList()
  result = quote do:
    `oddForward`
    `even`
    `odd`
    echo `callEven`
    echo `callOdd`

macro testEvenOdd5(): untyped =
  let
    evenid = genSym(nskProc, "even5")
    oddid = genSym(nskProc, "odd5")
    nid = newIdentNode("n")
    oddForward = defForward(oddid, nid)
    even = defEven(evenid, oddid, nid)
    odd = defOdd(evenid, oddid, nid)
    callEven = callNode(evenid, newLit(42))
    callOdd = callNode(oddid, newLit(42))
  result = quote do:
    `oddForward`
    `even`
    `odd`
    echo `callEven`
    echo `callOdd`

macro testEvenOdd6(): untyped =
  let
    evenid = genSym(nskProc, "even6")
    oddid = genSym(nskProc, "odd6")
    nid = newIdentNode("n")
    oddForward = defForward(oddid, nid)
    even = defEven(evenid, oddid, nid)
    odd = defOdd(evenid, oddid, nid)
    callEven = callNode(evenid, newLit(42))
    callOdd = callNode(oddid, newLit(42))
  # rewrite the body of proc node.
  oddForward[6] = newStmtList()
  result = quote do:
    `oddForward`
    `even`
    `odd`
    echo `callEven`
    echo `callOdd`

# it works
testEvenOdd3()

# it causes an error (redefinition of odd4)
# testEvenOdd4()

# it causes an error (still forwarded: odd5)
# testEvenOdd5()

# it works
testEvenOdd6()

The results are the same if defForward is as follows.

proc defForward(id, nid: NimNode): NimNode =
  result = quote do:
    proc `id`(`nid`: int): bool

Clyybber added a commit to Clyybber/Nim that referenced this issue Jul 29, 2020
Araq pushed a commit that referenced this issue Jul 29, 2020
* Fix forward declaration issues in template/macro context

* Correct forward declaration resolving for overloads

* Remove old dead code

* WIP consistent gensym ids

* Minimize diff

* Remove obsoleted hack

* Add templInstCounter to give unique IDs to template instantiations

* Remove obsoleted code

* Eh, init in myOpen, not myProcess...

* Remove optNimV019

* Add testcase for #13484
mildred pushed a commit to mildred/Nim that referenced this issue Jan 11, 2021
)

* Fix forward declaration issues in template/macro context

* Correct forward declaration resolving for overloads

* Remove old dead code

* WIP consistent gensym ids

* Minimize diff

* Remove obsoleted hack

* Add templInstCounter to give unique IDs to template instantiations

* Remove obsoleted code

* Eh, init in myOpen, not myProcess...

* Remove optNimV019

* Add testcase for nim-lang#13484
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

Successfully merging a pull request may close this issue.

2 participants