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

macros.getImpl still return incorrect AST for Sym "XXX:ObjectType" #16110

Closed
jangko opened this issue Nov 24, 2020 · 1 comment · Fixed by #16117
Closed

macros.getImpl still return incorrect AST for Sym "XXX:ObjectType" #16110

jangko opened this issue Nov 24, 2020 · 1 comment · Fixed by #16117
Labels

Comments

@jangko
Copy link
Contributor

jangko commented Nov 24, 2020

First of all, I would like to thank you for all improvements since nim-lang/RFCs#176.
Instead of returning nilLit, macros.getImpl now gives us some AST. But unfortunately, the AST is still incorrect.

import macros

macro see(x: type): untyped =
  let z = getType(x)[1]
  let y = getImpl(z)
  debugEcho getType(x).treeRepr
  debugEcho "---"
  debugEcho y.treeRepr

We will use the same macro code above for all examples below.

I'll start with ordinary object:

type
  Tire = object
    code: int

see(Tire)

The result is:

BracketExpr
  Sym "typeDesc"
  Sym "Tire"
---
TypeDef
  Sym "Tire"
  Empty
  ObjectTy
    Empty
    Empty
    RecList
      IdentDefs
        Ident "code"
        Sym "int"
        Empty

Now, we will use ref object:

type
  TireRef = ref Tire

var x: TireRef
type XX = type x[]  # should be equal to `Tire`
see(XX)

output still ok:

BracketExpr
  Sym "typeDesc"
  Sym "Tire"
---
TypeDef
  Sym "Tire"
  Empty
  ObjectTy
    Empty
    Empty
    RecList
      IdentDefs
        Ident "code"
        Sym "int"
        Empty

Last, we will try it with ref/ptr shortcut:

type
  TirePtr = ptr object
    code: int

var z: TirePtr
type ZZ = type z[] # the deref works but the internal representation is somewhat weird
see(ZZ)

the result is still PtrTy, not ObjectTy, and the Sym is incorrect:

BracketExpr
  Sym "typeDesc"
  Sym "TirePtr:ObjectType"  # this already correct
---
TypeDef
  Sym "TirePtr"   # how could Sym "TirePtr:ObjectType" become Sym "TirePtr"?
  Empty
  PtrTy                # `PtrTy` should be removed
    ObjectTy        # we only need this `ObjectTy` part
      Empty
      Empty
      RecList
        IdentDefs
          Ident "code"
          Sym "int"
          Empty

Expected output

BracketExpr
  Sym "typeDesc"
  Sym "TirePtr:ObjectType"
---
TypeDef
  Sym "TirePtr:ObjectType"
  Empty
  ObjectTy
    Empty
    Empty
    RecList
      IdentDefs
        Ident "code"
        Sym "int"
        Empty

getImpl should return the impl of Sym "TirePtr:ObjectType" and not of Sym "TirePtr".
Please fix this inconsistency, it will help us a lot writing sane macro based code.

cooldome added a commit that referenced this issue Nov 24, 2020
cooldome added a commit that referenced this issue Nov 24, 2020
This reverts commit 2bab2a2.
cooldome added a commit that referenced this issue Nov 24, 2020
This reverts commit f8b9d8c.
@cooldome cooldome reopened this Nov 24, 2020
cooldome added a commit that referenced this issue Nov 24, 2020
@cooldome cooldome mentioned this issue Nov 24, 2020
Araq pushed a commit that referenced this issue Nov 24, 2020
narimiran pushed a commit that referenced this issue Nov 24, 2020
(cherry picked from commit 1d14b2c)
@jangko
Copy link
Contributor Author

jangko commented Nov 24, 2020

Thank you for the quick response and fix.
But the PtrTy /RefTy still there, and the test also incomplete.
too quick?

@Araq Araq reopened this Nov 24, 2020
cooldome added a commit that referenced this issue Nov 25, 2020
ringabout pushed a commit to ringabout/Nim that referenced this issue Nov 25, 2020
ringabout pushed a commit to ringabout/Nim that referenced this issue Nov 25, 2020
ringabout pushed a commit to ringabout/Nim that referenced this issue Nov 25, 2020
This reverts commit 2bab2a2.
ringabout pushed a commit to ringabout/Nim that referenced this issue Nov 25, 2020
This reverts commit f8b9d8c.
ringabout pushed a commit to ringabout/Nim that referenced this issue Nov 25, 2020
mildred pushed a commit to mildred/Nim that referenced this issue Jan 11, 2021
mildred pushed a commit to mildred/Nim that referenced this issue Jan 11, 2021
mildred pushed a commit to mildred/Nim that referenced this issue Jan 11, 2021
This reverts commit 2bab2a2.
mildred pushed a commit to mildred/Nim that referenced this issue Jan 11, 2021
This reverts commit f8b9d8c.
mildred pushed a commit to mildred/Nim that referenced this issue Jan 11, 2021
mildred pushed a commit to mildred/Nim that referenced this issue Jan 11, 2021
* fix nim-lang#16110

* refs nim-lang#16110

* fix comment

* Trigger build

* use shallowCopy for efficiency
ardek66 pushed a commit to ardek66/Nim that referenced this issue Mar 26, 2021
ardek66 pushed a commit to ardek66/Nim that referenced this issue Mar 26, 2021
ardek66 pushed a commit to ardek66/Nim that referenced this issue Mar 26, 2021
This reverts commit 2bab2a2.
ardek66 pushed a commit to ardek66/Nim that referenced this issue Mar 26, 2021
This reverts commit f8b9d8c.
ardek66 pushed a commit to ardek66/Nim that referenced this issue Mar 26, 2021
ardek66 pushed a commit to ardek66/Nim that referenced this issue Mar 26, 2021
* fix nim-lang#16110

* refs nim-lang#16110

* fix comment

* Trigger build

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

Successfully merging a pull request may close this issue.

4 participants