Make Sym "XXX:ObjectType" works with macros.getImpl

[jangko]

Make Sym "XXX:ObjectType" works with macros.getImpl

Background story

We have 5 or 6 hasCustomPragma related issues:

misc issues:

1. hasCustomPragma fails on nnkVarTy/nnkBracketExpr nodes Nim#11923
2. cannot get pragma of attribute in macro Nim#11511
3. Custom pragma ignored on field of variant obj if in multiple branches Nim#11415

ref/ptr issues:
4. nim-lang/Nim#12523
5. nim-lang/Nim#8457
6. status-im/nim-json-serialization#11 (using alternate implementation of hasCustomPragma from status-im/stew)

This is about macros.hasCustomPragma. Current implementation of hasCustomPragma is barely readable.

That is why we implement an alternate version of hasCustomPragma in nim-stew/macros.

This new implementation of hasCustomPragma readability is far above macros.hasCustomPragma and also easier to use when designing a serialization library.
Issues no 1 to 3 practically can be eliminated easily.

However, issues no 4-6 remain the same for both implementation. stdlib-hasCustomPragma using various work-around to no avail.
nim-stew-hasCustomPragma actually very close to get it works.

nim-stew-hasCustomPragma approach is ignoring ref/ptr types altogether and only focused to regular object/tuple.
The algorithm only use simple getType + getImpl.
This approach make the code simpler. So how we expect ref/ptr to works? use a template:

template hasCustomPragma*(T: type, moreArgs): untyped =
  when T is ref|ptr:
    type TT = type(default(T)[])
    hasCustomPragma(TT, moreArgs)
  else:
    hasCustomPragma(T, moreArgs)

But, this approach unfortunately doesn't work. Sym "objectName:ObjectType" return nothing when you use getImpl.
Using combination of getType + getTypeImpl + getTypeInst + getImpl only make it worse and make it messier.
It also never reach the same usability compared to only use getType + getImpl.

Proposed solution

If we getType ref/ptr object, it will return synthetic symbol Sym "objectName:ObjectType" as a part of it's AST.
Currently, getImpl will return NilLit on this symbol. As a user, I don't care much how the compiler internal works.
Putting an AST to that synthetic symbol may have no use to the compiler itself, but as a user, who care?

If this synthetic symbol can works with getImpl, much of the problem mentioned above will be reduced.

Of course, we also can use dynamicBindSym as a work around to relate the objectName back to it's ref/ptr type.
But I prefer getImpl approach.

Implementation

It only involves NimNode tree copy + add + assign. Not too hard I think.
When we use getImpl on this symbol, it will return impl AST as if a regular object's symbol does.

Consequences

It will breaks some code. But I think very rare people depends on this NilLit return value.

Benefits

• It not only works with ref/ptr object, it will make inheritance/climb up parent tree much simpler without become messier.
• Both stdlib-hasCustomPragma and nim-stew-hasCustomPragma can be fused easier to get more power.
• Improve it's usage within generic context, hasCustomPragma can accept various type declarations such as:

type
  T   = object/ref/ptr/tuple
  TT1 = type(ref/ptr deref)
  TT2 = type(variable of T)
  TT3 = type(proc call return type)
  aliasofT = T
  aliasaliasofT = aliasofT
  etc

[krux02]

Here is a related PR nim-lang/Nim#11526

[Araq]

getImpl was never designed for this and only accidentically worked. I think it's not wise to use it for this.

[jangko]

and what is the wiser way to get pragmas from type/proc definition? currently, only getImpl able to return AST + pragmas. getType family always omit pragmas.

[Araq]

The getType family should return pragmas. Maybe we should add a flag to them to keep it.

[jangko]

something like proc getType*(n: NimNode, flags: GetTypeFlags = {KeepPragma}): NimNode?

[Araq]

Exactly. The default should be not to include them though for compatibility.

[jangko]

I agree backward compatibility should not be compromised here. We can add more features to getType family with this approach while retaining old behavior.

I think we reach consensus here.

[github-actions]

This RFC is stale because it has been open for 1095 days with no activity. Contribute a fix or comment on the issue, or it will be closed in 7 days.