Nim icon indicating copy to clipboard operation
Nim copied to clipboard

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

Open timotheecour opened this issue 5 years ago • 14 comments

/cc @Araq https://github.com/nim-lang/Nim/pull/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

  • 515b6d661e0def97cfe06303fd753b1443fef83e 1.1.1

timotheecour avatar Apr 01 '20 04:04 timotheecour

@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()

zah avatar Apr 04 '20 13:04 zah

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.}

timotheecour avatar Apr 04 '20 14:04 timotheecour

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.

zah avatar Apr 04 '20 14:04 zah

@zah @araq my experience with https://github.com/nim-lang/Nim/pull/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 avatar Apr 18 '20 11:04 timotheecour

@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:

  • pragma type macro is very recent so there's still time to fix this before 1.4 release (possibly with a legacy flag); after that (and the more we wait) it'll cause more issues for backward compatibility as more code would rely on it
  • it's not a big compiler change
  • it's causing recurring issues, eg see those recent issues now that people are starting to use it:
    • https://github.com/nim-lang/Nim/issues/15334
    • https://forum.nim-lang.org/t/6907
    • https://forum.nim-lang.org/t/6907#43253
    • makes enummap syntax worse as mentioned in #14008
  • the new proposed way is much more useful and consistent with pragma macros for procs

timotheecour avatar Oct 09 '20 22:10 timotheecour

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?

disruptek avatar Nov 15 '20 23:11 disruptek

Would TypeSectionStmt or whatever as in https://github.com/nim-lang/RFCs/issues/66 be preferred? Not mutually exclusive with the current behavior

metagn avatar Feb 15 '21 08:02 metagn

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

timotheecour avatar Feb 18 '21 19:02 timotheecour

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

akbcode avatar Jul 13 '21 05:07 akbcode

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.

Varriount avatar Jul 23 '21 21:07 Varriount

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

shirleyquirk avatar Oct 16 '21 15:10 shirleyquirk

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)).

metagn avatar Nov 24 '21 14:11 metagn

any idea on how to inject constant section from type pragma?

type
     ABC {.genConstants.} = enum
         ABC1, ABC2

----->

type
     ABC = enum
          ABC1, ABC2
const
     LegacyABC1 = ABC.ABC1
     LegacyABC2 = ABC.ABC2

YesDrX avatar Dec 07 '25 00:12 YesDrX

There has been a "workaround" for this for a while by generating AST in the form of:

type
  _ = (; # nkStmtListType node
    type ABC = enum ABC1, ABC2
    const
      LegacyABC1 = ABC.ABC1
      LegacyABC2 = ABC.ABC2
    void)

But since #19168 (first appears in version 2.0) this can be turned into:

type
  ABC = enum ABC1, ABC2
  _ = (; # nkStmtListTypeNode
    const
      LegacyABC1 = ABC.ABC1
      LegacyABC2 = ABC.ABC2
    void)

On older versions where _ didn't work with types you would generate a dummy genSym symbol.

The advantage of the second version is that any type definitions like ABC are clued into the recursive type behavior inside type sections, i.e.:

type
  AbcRef = ref Abc
  _ = (;
    type Abc = object
      next: AbcRef
    ...)

doesn't work, while it works when merged into a single type section.

There might also be other bugs with the StmtListType version, I don't know if it's especially protected behavior that symbols defined in type section bodies are defined in the same scope.

metagn avatar Dec 07 '25 03:12 metagn