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 pragmas in type section should apply to TypeSection, not TypeDef #13830

Open
timotheecour opened this issue Apr 1, 2020 · 12 comments · May be fixed by #24245
Open

macro pragmas in type section should apply to TypeSection, not TypeDef #13830

timotheecour opened this issue Apr 1, 2020 · 12 comments · May be fixed by #24245

Comments

@timotheecour
Copy link
Member

timotheecour commented Apr 1, 2020

/cc @Araq
#13778 introduced macro pragmas in type sections. It's very useful as it allows things like "export all fields" in library code, or other custom logic.

However the semantics are too restrictive because we're passing a TypeDef AST which is not representable in regular nim code, preventing many useful cases (eg using a template instead of a macro, discarding the typedef, adding a statement before the type etc, all of which are possible with pragmas for other declarations (including routines and let statements)).

It should be changed to something very similar but much more useful, see "Possible Solution" below.

In particular, pure forwarding via template m2(def: untyped): untyped = def works for other declarations:

template ma1(def: untyped): untyped = def
proc fun(){.ma1.}=discard

but not for type sections.

Example

when true:
  # only `m0,m1` works, all others give `Error: illformed AST`

  import macros
  ## these are ok
  macro m0(def: untyped) = def
  macro m1(def: untyped) = result =
    quote do: `def`

  ## these all give CT error when replacing the `m2`

  template m2(def: untyped): untyped = def # BUG
  template m3(def: untyped): untyped =
    type Foo1b = int
  template m4(def: untyped): untyped =
    Foo1b = int
  macro m5(def: untyped): untyped =
    quote do:
      Foo1b = int

  macro m6(def: untyped): untyped =
    quote do:
      type Foo1b = int
  macro m7(def: untyped): untyped =
    result = newStmtList()
  macro m8(def: untyped): untyped = discard # discard typedef
  macro m9(def: untyped): untyped =
    result = quote do:
      static: echo "introducing a new type" # insert an instruction before typedef
      `def`

  type
    Foo1 {.m2.} = object ## replace `m2` with one of the above

Current Output

`Error: illformed AST`

Expected Output

should work for m2, m3, m6, m7, m8, m9
(m4, m5 was just to verify this doesn't work either)

Possible Solution: pass the TypeSection, not the TypeDef

  • when the type section contains a single element, the whole TypeSection AST should be passed to the macro (or template), instead of passing the TypeDef. This will make all examples pass m2, m3, m6, m7, m8, m9 + m0,m1
# for reference, here is the AST:
# repr:
type
  Foo1 = object

# AST:
  TypeSection
    TypeDef
      Ident "Foo1"
      Empty
      ObjectTy
        Empty
        Empty
        Empty
  • when the type section contains more than 1 element, it's a bit more tricky but works too: if the AST produced by the macro is a TypeSection, it's elements are inserted in the parent type section:
macro m(def): untyped =
  # silly hardcoded example just for illustration
  quote do:
    type 
      TFoo2* = object
      Foo2* = ref object

type
  Foo1 = object
    seq[Foo3] # don't break type section!
  Foo2 {.m.} = object
  Foo3 = ref Foo1

# transformed to:
  Foo1 = object
    seq[Foo3] # don't break type section!
  TFoo2* = object # this gets inserted in parent type section
  Foo2* = ref object
  Foo3 = ref Foo1
  • if the AST produced by the macro is not a TypeSection, we could either issue a Error: illformed AST (simplest) or split the type section accordingly, eg:
type
  Foo1 = object
    seq[Foo3] # don't break type section!
# insert the `StmtList` obtained from `Foo2 {.m.} = object`
type
  Foo3 = ref Foo1

Additional Information

@zah
Copy link
Member

zah commented Apr 4, 2020

@timotheecour, you'll be surprised by the flexibility you are given with nnkStmtListType.
Play a bit with this program:

template makeType: type =
  type
    Foo = object
      a: int
      b: int

    Bar = distinct Foo

  template a*(obj: Bar): untyped = Foo(obj).a
  template b*(obj: Bar): untyped = Foo(obj).b

  Bar

type
  MyType = makeType()

@timotheecour
Copy link
Member Author

timotheecour commented Apr 4, 2020

yes but that's missing the point though, both routine pragmas and var pragmas allow complete rewrite of the AST (including omitting the declaration or arbitrary other rewrites), but that's not yet the case with type pragmas, but would be under this RFC.

As for your approach, you can't express simple things like:

  • add export marker (say, depending on some condition)
  • omit the type declaration for MyType altogether
  • rename MyType
type
  MyType = makeType()

(at least not unless you also combine with type pragma, but then again it still won't help eg for omit type declaration depending on some condition)

All of this is possible with this RFC, and results in simpler user code, eg:

type Foo {.maybeExport.} = Bar

{.push maybeExportTypeAndFields.}
type
  Foo1 = Bar1
  Foo2 = Bar1
{.pop.}

@zah
Copy link
Member

zah commented Apr 4, 2020

My example was just meant to demonstrate how nnkStmtListType works. The subtext was that you can produce such a statement list in the rewrite happening in the type section macro.

You can do a rename or add export marker by modifying the returned nnkTypeDef row. To add extra types or definitions, you just make the right-hand side a nnkStmtListType that includes what you need. There are probably some limitations for sure, but if you need more you can always use a different syntax such as:

specialTypeSection:
  type
    Foo = ...
    Bar = ...
     

EDIT: Maybe it's not clear from my explanation that the body of the template represents the nnkStmtListType. Notice how it includes exported symbols for operations over the newly produced type.

@timotheecour
Copy link
Member Author

timotheecour commented Apr 18, 2020

@zah @Araq my experience with #14008 confirms exactly what I wrote in this RFC.

with the proposed change change, we can write these interchangeably, the 1st form being more user friendly.
Furthermore, this requires 0 change in enumMap!

type EnumWithHole {.enumMap.} = enum
  k1=1
  k2=3

enumMap:
  type EnumWithHole = enum
    k1=1
    k2=3

with the current state of affair, this just isn't possible (please prove me wrong); (and even if it were possible, it'd require a complex rewrite of enumMap)

With proposed change, the rewrite rule is the exact analog of macro pragma for routines and var, instead of some weird thing that takes a broken piece of AST (a TypeDef can't be written, say in a template, unlike a TypeSection, VarSection or ProcDef etc), thus prevents defining it via templates for example;

The very fact that you'd need to jump through hoops to make type EnumWithHole {.enumMap.} = enum work should be proof enough (and I'm not even sure it's possible)

@timotheecour
Copy link
Member Author

timotheecour commented Oct 9, 2020

@Araq I had opened this issue right after type macro pragma was implemented in #13778 but I'm now adding this issue as a blocker for milestone 1.4 (I hope you don't mind) for the following reason:

@disruptek
Copy link
Contributor

I don't understand. The macro should apply to the ast where you're manipulating the type; ie. the TypeDef. I use it successfully in CPS and never had any concerns. This is like suggesting that a pragma on a LHS in a variable in a VarSection should get lifted to the section depending upon the output of other rewrites. Sounds like a very poor idea to me, but again, I really don't understand -- perhaps you could provide a more compelling example?

@metagn
Copy link
Collaborator

metagn commented Feb 15, 2021

Would TypeSectionStmt or whatever as in nim-lang/RFCs#66 be preferred? Not mutually exclusive with the current behavior

@timotheecour
Copy link
Member Author

timotheecour commented Feb 18, 2021

you mean this?

type 
  A = B
  E = variant: # maybe we can skip the colon
    ...

that syntax looks very foreign, and doesn't make much sense of variant conditionally skips or renames E; I also don't see how you'd use it for E = enum etc.

EDIT:
@hlaaftana Note, that there may be a way (temporary or not) to allow both current behavior and proposed behavior, via (as usual) pragmas, eg: {.typesection.}:

template m1(body): untyped {.typesection.} =
  when not defined(js): body

type
  Foo1 = object
  Foo2 {.m1.} = object
  Foo3 = object

alternatively, via type system would be possible too:

template m1(body: TypeSectionStmt): untyped =
  when not defined(js): body

type
  Foo1 = object
  Foo2 {.m1.} = object
  Foo3 = object

and then user would choose:

template m1(body: TypeSectionStmt): untyped =. # applies to TypeSection
template m2(body: TypeDefStmt): untyped = ... # applies to TypeDef

=> no breaking change

@Araq Araq removed this from the 1.4.0 milestone Mar 9, 2021
@akbcode
Copy link

akbcode commented Jul 13, 2021

This change would be useful for generating custom getters and setters for fields.

@Varriount
Copy link
Contributor

Varriount commented Jul 23, 2021

I feel that that a "push" macro should just be passed the AST of the entire section it applies to - this offers the most flexibility, and would be a nice alternative to wrapping giant sections of code (for instance, an entire module) under a macro invocation.

shirleyquirk added a commit to shirleyquirk/nim_helpers that referenced this issue Oct 16, 2021
took a surprisingly long time to figure out how to attach procs to a type declaration.
thanks to zah's comment on this github issue nim-lang/Nim#13830 (comment)
@shirleyquirk
Copy link
Contributor

custom procs (getters,setters,constructors) can be generated with @zah's nnkStmtListType idea.

here's a POC constructor generator:

import macros,sugar
template helper(body:untyped):untyped =
  body
macro dataclass*(x:untyped):untyped =
  ## dataclass macro replaces a typedef with itself, plus some
  ## helper procs. for now, just an initFoo() proc
  ## input is, e.g.
  ## type
  ##   Foo {. pragmas.. .} = object
  ##     x*:int
  ##     y,z*,w: float
  ## TODO: variants
  ##
  ## output is:
  ## template anonymous:type =
  ##   type Foo {. pragmas..,inject .} = object
  ##     x*:int
  ##     y,z*,w: float
  ##   proc initFoo(x:int,y,z,w:float):auto = Foo(x:x,y:y,z:z,w:w)
  ##   Foo
  ## type
  ##   FooDataClass {. pragmas.. .} = anonymous()
  
  result = x.copyNimTree()
  
  x.expectKind(nnkTypeDef)

  let basename = x[0]
  
  let outname = basename.copyNimTree()
  outname[0] = ident(outname[0].strval & "Dataclass")
  #type BaseNameDataclass{. pragmalist.. .} = helper(templatebody)

  basename[1].add(ident"inject")
  # type BaseName{.inject.} = object

  x[2].expectKind(nnkObjectTy)
  let typedef = x[2]
  let identdefs = typedef[2].copyNimTree()
  var ids: seq[NimNode]
  
  #get identdef, ident seqs, stripped of `*` postfix
  for i in 0..<identdefs.len:
    for j in 0..<identdefs[i].len - 2: #last two are type and i think pragma?
      if identdefs[i][j].kind == nnkPostfix:
        identdefs[i][j] = identdefs[i][j][1]
      ids.add identdefs[i][j]
   
  let params = @[ident"auto"] & collect(newSeq,for c in identdefs.children: c)
  let assignments = @[basename[0]] & collect(newSeq, for i in ids: nnkExprColonExpr.newTree(i,i))

  let procdef = newProc(ident("init" & basename[0].strval),
                        params,
                        nnkObjConstr.newTree( assignments )
                       )

  let templatebody = nnkStmtList.newTree(
    nnkTypeSection.newTree(
      nnkTypeDef.newTree(
        basename,
        newEmptyNode(),
        typedef
      )
    ),
    procdef,
    basename[0]
  )
  result[0] = outname
  result[2] = newCall(ident"helper",[templatebody])

type
  Foo = int
  Bar{.dataclass.} = object
    x*: int
    y,z*,w: float
  Baz = float


let y = initBar(3,1.0,2.0,3.0)
echo y

@metagn
Copy link
Collaborator

metagn commented Nov 24, 2021

m3 and m6 should work now. Other examples should have a workaround with gensym and statement lists that return types (i.e. type _ = (static: echo "foo"; void)).

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.

8 participants