implementation for newLit on distinct types (fixes #13266)

[krux02]

newLit should work on all types, including distinct types. This fixes it.

Minor change, I removed some dead code of mine.

[krux02]

@Araq @narimiran @nitely @cooldome any feedback on this? Is it ok? Is it not ok?

Since I originally introduced most overloads of newLit, I would normally be resonsible to review this. The only reason that I didn't do this earlier is, I wasn't very familiar enough with distinct back then.

[Araq]

Needs a .since annotation and a changelog entry.

[Clyybber]

Can we use distinctBase instead?

[krux02]

Well distinctBase requires a new import/dependency. I don't like that. But more importantly distinctBase works on typedesc. typedesc is a source of problems, bugs and complications. If I want my code to work reliably I avoid it. That is possible here without adding much boilerplate code.

[timotheecour]

distinctBase doesn't require an import dependency now that it's a magic.

proc distinctBase(T: typedesc): typedesc {.magic: "TypeTrait".}

Having 2 different ways to do implement the safe feature is not good (if it's broken, please show an example where it breaks)

[krux02]

@timotheecour Just an example: #12582
That is a very typical bug that you hit just because you are using typedesc. If you want to avoid accidentally hitting compiler bugs like that, the only advice that works is, don't use typedesc.

But here are more typedesc related problems. Of cours the reported problems are only showing the tip of the iceberg. I encountered several more problems of typedesc that I did not report.

[timotheecour]

I'm still not aware of a bug involving distinctBase specifically, if you find one, please file an issue.

[krux02]

@Araq this PR is ready and can be merged.

[timotheecour]

good PR, but:

• needs rebase

• macro undistinct[T: distinct](arg: T): untyped = (even if private) should definitely reference typetraits.distinctBase as a doc comment (doc comments still useful for those) since it does the same thing as template distinctBase[T](a: T): untyped (recently added overload)

• can you please add:

block:  # test 2 level distinct
  type Rune2 = distinct Rune
  macro testNewLitDistinct2(): untyped = newLit(Rune2(123))
  doAssert testNewLitDistinct2() is Rune2

(i checked it works, but useful for regression testing)

• your tests would fail with --styleCheck:error

[timotheecour]

since: (1,3)

[timotheecour]

@Araq @krux02 this PR is useful, can we move towards merging it after the needed conflict rebase? either addressing the comments i raised or doing that in followup PR's (which i can do if necessary)

[Araq]

But why should this work? It feels like you don't understand distinct types. Distinct types hide their implementation, yes newLit(D(1)) doesn't work. So what? That's not a literal... Maybe construct D(1) as newCall(bindSym"D", newLit(1)) then. A macro system is somewhat like a type system ... it works better if not everything compiles just because "omg it's useful in generic programming" (what isn't?)

[nitely]

@Araq what about containers of distinct types, and objects with distinct type fields? I don't know why distinct types cannot be literals, or why adding a distinct type field to an object makes it not a literal anymore. Aside from that, it'd be nice if macros had some way to do this conversion. On the other hand I'm the first one to ever encounter this (or at least report it), so maybe is not common/worth while.

[timotheecour]

what about containers of distinct types, and objects with distinct type fields? I don't know why distinct types cannot be literals, or why adding a distinct type field to an object makes it not a literal anymore.

+1 to that, no need to add arbitrary restrictions on litterals

[Araq]

D(1) is not a literal, it's like you don't know what a literal is ... "restrictions on literals" -- give me a break. "Abitrary restrictions on what a tree can be. Restrictions on what a house can look like".

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. If you think it is still a valid PR, please rebase it on the latest devel; otherwise it will be closed. Thank you for your contributions.