distinctBase type trait for distinct types

[cooldome]

In quest for migration to ARC I was looking for mashal module replacement. I have tried a number of serialization libraries in last 2 days and all of them had one thing in common. They can't handle distinct types while marshal had no issues with such types. Major reason Nim doesn't provide much help to work with distinct types. Most libraries use fieldPairs iterator with operator overloading and when statements as a fallback.

when T is distinct already works for distinct types the last missing part of the puzzle is to get base type of distinct. I have added baseType() type trait to help with that. I have tried it on the samson library and it worked well.

Also, I found isNamedTuple trait useful for serialization, but because the way it was implemented it can't be used in when statements which is problem, so I decided to reimplement in evalTypeTrait .

[Araq]

Needs a changelog entry and a .since annotation or the corresponding when statement.

[ghost]

How is this different than sugar.distinctBase?
This one doesn't desymbolize the result, which I have had issues with before. Anything else aside from being magic?

Also the signature + name is rather generic for only working with distinct types.
Something like sugar's distinctBase(T: typedesc) or baseType(T: typedesc[distinct]) and let overload resolution provide the error.

[cooldome]

@dumjyl. Good comment thank you. I wasn't aware of sugar.distinctBase,
What I will do I will harmonize it, I will move distinctBase to typetraits because it is where it belongs and I will use name distinctBase then we don't need since annotation.

update: distinctBase is already mentioned in the manual, hence description remains actual.

[timotheecour]

+1 also for turning isNamedTuple into a magic; IIRC it was causing some issue and is definitely more efficient as a magic
(I wrote both the original isNamedTuple and distinctBase as macros but implementation is indeed simpler / more efficient as a magic)

This one doesn't desymbolize the result, which I have had issues with before.

/cc @dumjyl what do you mean? can you please explain by showing how they differ with an example ?

[cooldome]

I just discovered that ttypetraits.nim is a disabled test. I need to move my tests elsewhere

[timotheecour]

I just discovered that ttypetraits.nim is a disabled test. I need to move my tests elsewhere

there's already a ttypetraits2.nim, you can use that

let's merge #13041 so you can use use ttypetraits.nim after all

EDIT: was merged; just rebase against devel

[ghost]

@timotheecour

/cc @dumjyl what do you mean? can you please explain by showing how they differ with an example ?

sugar.distinctBase replaces nnkSym with nnkIdent so the wrong type may be bound.

Untested but I think this example triggers the assertion when it shouldn't:

# mod_a
type Bar* = string
type Foo* = distinct Bar

# mod_b
import mod_a, sugar
type Bar = int
type Detail = distincBase(Foo)
assert Detail is string

I believe some have had problems with constructors of nnkSym, but nnkSym -> nnkIdent is a one way transformation that can be done by the user.

[cooldome]

PR is ready. All comments address.
I have consolidated all distinctBase tests into ttypetraits. tsugar ended up being almost empty, so I moved all remaining tests and removed tsugar.

[timotheecour]

@dumjyl

sugar.distinctBase replaces nnkSym with nnkIdent so the wrong type may be bound.

thanks! indeed, I checked, the assert fails, it returns int instead of string, so sugar.distincBase has a pre-existing bug (gets fooled in that case by lookup with same names)

sugar.distinctBase is recursive, unlike typetraits.distinctBase

I had argued in #8531 (comment) that distinctBase should be non-recursive (with an option for recursive), however @Araq disagreed and wanted only recursive;
however, in this PR, typetraits.distinctBase is actually non-recursive:

    type T = distinct int
    doAssert distinctBase(T) is int
    doAssert: not compiles(distinctBase(int))
    type T2 = distinct T
    doAssert distinctBase(T2) is int # fails for `typetraits.distinctBase`

so we should arrive at a new consensus;

in any case, IMO we shouldn't keep old implementation around (especially because it has a bug, and especially if it has same name but different semantics)

proposal

• make sugar.distinctBase a deprecated alias of typetraits.distinctBase:
• and re-introduce the recursive flag in typetraits.distinctBase

# typetraits.nim
proc distinctBase*(T: typedesc, recursive: static[bool] = false): typedesc {.magic: "TypeTrait".}

# sugar.nim
import std/typetraits
# use `recursive = true` for compatibility (the compatibility argument doesn't apply to above mentioned bug, which is just a bug)
template distinctBase*(T): untyped {.deprecated: "use typetraits.distinctBase".} = typetraits.distinctBase(T, recursive = true)

[cooldome]

Tests are green, ready for review

[timotheecour]

one more thing to note (that doesn't require any change in this PR):

• before PR: this would fail: Error: attempt to access a nil address
  after PR: this works

  import std/typetraits
  proc fun2[T](a: openArray[T]): bool =
    isNamedTuple (int, T, T)
  static:
    doAssert not fun2(@[1,2])

• i had come up with another implementation that doesn't fail with above testcase and didn't use compiles but I still prefer this PR's version because it doesn't add a dependency on typetraits (and is likely easier on the compiler)

import std/macros

macro isNamedTuple*(T: typedesc): bool =
  # note: getTypeInst would work, but not getType
  var n = T
  n = getTypeImpl(n)
  while true:
    case n.kind
    of nnkBracketExpr: n=getTypeImpl(n[1])
    of nnkTupleTy: return newLit true
    of nnkTupleConstr: return newLit false
    of nnkSym:
      doAssert false # or maybe `return newLit false`
    else:
      return newLit false