[TODO] [easy] [typetraits] add `==` for type equality (different from `is` !) #8612

timotheecour · 2018-08-11T05:47:01Z

No description provided.

metagn · 2018-08-11T06:33:46Z

lib/pure/typetraits.nim

@@ -57,6 +57,16 @@ proc supportsCopyMem*(t: typedesc): bool {.magic: "TypeTrait".}
  ## This trait returns true iff the type ``t`` is safe to use for
  ## `copyMem`:idx:. Other languages name a type like these `blob`:idx:.

+proc sameType*(t1, t2: typedesc): bool =


I predict the same thing with name will happen where people will ask for this to be renamed to ==

i thought about it... not sure which is better; sameType is more searchable but == is more predictable; Note: there's also macros.sameType(Node,Node) although obviously that one can't be named to ==.
Overall == could be better ; should i go ahead and chage to ==?

done. replace with == ; which is nice because we get int != float for free.
makes the language nice and uniform

This will quickly turn into compareTypes(a, b, flags) anyway, == is for simple comparisons, type equality is anything but simple. In fact, I know few other examples where it gets as complex as it gets for types.

well nothing wrong w having both == for simple comparison and, when needed, add compareTypes(a, b, flags) to allow customizing what is meant by equality

timotheecour · 2018-08-11T08:26:17Z

PTAL

dom96 · 2018-08-15T11:01:02Z

How is this different to is?

This passes:

type T = int
doAssert T is int
doAssert int is T
doAssert int isnot float

GULPF · 2018-08-15T12:59:40Z

@dom96 is doesn't imply equality since it supports concepts & inheritance.

type
  C = concept x
    x is int

echo int is C # true
echo C is int # false

timotheecour · 2018-08-15T18:45:49Z

clarified documentation further to add what @GULPF said.
PTAL

dom96 · 2018-08-15T20:42:44Z

This should be demonstrated in the runnable examples.

dom96 · 2018-08-15T20:43:23Z

lib/pure/typetraits.nim

+  ## Returns wheter ``t1``, ``t2`` are the same type; this is different
+  ## from ``t1 is t2`` since the latter supports concepts & inheritance.
+  runnableExamples:
+    type T=int


Style nitpick: space before and after =.

dom96 · 2018-08-15T20:43:38Z

lib/pure/typetraits.nim

+    type T=int
+    doAssert T == int
+    doAssert int == T
+    doAssert: int != float


Please be consistent in your style, no need for this random :.

it's needed to avoid #8300
otherwise you get: Error: invalid indentation

I doubt it. != is not an unary keyword operator.

you can try for yourself with git fetch origin pull/8634/head:pull_8634 && git checkout pull_8634 and remove this semicolon then nim doc ./lib/pure/typetraits.nim to reproduce this error

This compiles:

import typetraits type T = int template `==`(x, y: typedesc): bool = (x is y) and (y is x) doAssert T == int doAssert int == T doAssert int != float

ok, this must've been fixed since that was reported

timotheecour · 2018-08-15T22:36:13Z

PTAL, all comments addressed

timotheecour · 2018-08-18T05:40:57Z

/cc @dom96 friendly ping

dom96

Small things. I'm happy but I want @Araq to take a look too.

dom96 · 2018-08-18T18:24:01Z

lib/pure/typetraits.nim

@@ -57,6 +57,16 @@ proc supportsCopyMem*(t: typedesc): bool {.magic: "TypeTrait".}
  ## This trait returns true iff the type ``t`` is safe to use for
  ## `copyMem`:idx:. Other languages name a type like these `blob`:idx:.

+proc `==`*(t1, t2: typedesc): bool =
+  ## Returns wheter ``t1``, ``t2`` are the same type; this is different


Typo: "whether".

I would also replace the comma with "and".

dom96 · 2018-08-18T18:24:18Z

lib/pure/typetraits.nim

@@ -57,6 +57,16 @@ proc supportsCopyMem*(t: typedesc): bool {.magic: "TypeTrait".}
  ## This trait returns true iff the type ``t`` is safe to use for
  ## `copyMem`:idx:. Other languages name a type like these `blob`:idx:.

+proc `==`*(t1, t2: typedesc): bool =
+  ## Returns wheter ``t1``, ``t2`` are the same type; this is different
+  ## from ``t1 is t2`` since the latter supports concepts & inheritance.


Actually, is supports concepts but not inheritance (that's of AFAIK).

it does:

type A=object of RootObj b1:int type B=object of A b2:int echo A is B # false echo B is A # true

Araq · 2018-08-18T18:49:16Z

I don't want it. People should use the is operator and learn Nim. There are two ways to design a programming language:

Keep it simple, well documented and expect people to read the manual.
Borrow everything that might be useful from other languages so that programmers can guess things correctly.

(2) is a never ending story and in the end you fail to produce a consistent system that people enjoy using after they got over the "I program in Nim for X days" phase. It's also after this phase when programmers become valuable contributors, generally speaking. We tried (2) for far too long, it's time to try (1).

bluenote10 · 2018-08-19T21:14:07Z

In my opinion having this function is a nice way to document this subtlety. Seeing the expression A is B and B is A in the wild would often raise the question why this is necessary. It wasn't even obvious to @dom96 and currently the manual section on the is operator also does not mention it. Implementing == allows to document this difference nicely in code + docstring. What's the drawback?

Araq · 2018-08-20T07:36:58Z

What's the drawback?

It doesn't come up often enough. Most of the time people would use it instead of is because they are not aware of is.

timotheecour · 2018-08-20T22:54:07Z

I changed implementation to use macros.sameType because some test cases were running into Error: internal error: genMagicExpr: mIs #8693
this introduces a dependency typetraits: import macros; if that's a problem I could move == to macros.nim instead although it seems more natural in typetraits.nim
I added a bunch of tests (see tests/metatype/ttypetraits2.nim)
I don't really see how [TODO] [easy] [typetraits] add == for type equality (different from is !) #8612 (comment) applies to this PR. is is different from ==, there are use cases for both.

It doesn't come up often enough. Most of the time people would use it instead of is because they are not aware of is.

I'm using both is and this PR's == quite a bit in my own code, in particular when writing tests. I don't follow this argument; is and == are 2 different tools with 2 different use cases; we shouldn't assume nim programmers are incapable of telling these apart ; so long documentation makes it clear, there is no drawback and it makes the language more regular / self-consistent. This has nothing to do with "Borrow everything that might be useful from other languages"

Clyybber · 2018-08-20T23:05:01Z

#8700 would fix #8693, so I think this PR should use the simpler old implementation.

timotheecour · 2018-08-20T23:56:13Z

well we can merge this first and update if/when #8700 gets merged later (looks like tests in #8700 are failing right now, and there are unresolved comments there)

Clyybber · 2018-08-21T12:50:16Z

The tests in #8700 are passing now.

Araq · 2018-08-21T13:20:42Z

Again, what are the use cases?

timotheecour · 2018-08-22T22:23:41Z

eg:

when you want to assert/condition on something is of a certain class and not a derived class, eg:type(expr) == A
when you want to assert/condition on 2 types being equal instead of related via is; especially for procs/templates operating on types or typedesc params
here's also a concrete example; there are lots of other (unrelated) examples

import unittest

proc equals[T1,T2](a1:T1, a2:T2): bool=
  # allows more efficient comparison; for example GC-free comparisons in tests,
  # eg for procs returning a seq, we can compare against an array, which doesn't
  # allocate
  
  # replacing `T1==T2` by `T1 is T2` would produce different results in some cases, eg inheritance
  when T1==T2 and isValueType(T1): # I implemented isValueType elsewhere; it's false if there are any nested references
    # from https://github.com/nim-lang/Nim/issues/8436
    return bitEquals(a1, a2)
  elif compiles(a1.len) and compiles(a2.len):
    if a1.len != a2.len: return false
    for i in 0..<a1.len:
      if not equals(a1[i], a2[i]): return false
    return true
  else:
    return a1 == a2

timotheecour · 2018-08-22T23:36:27Z

#8700 would fix #8693, so I think this PR should use the simpler old implementation.

done

Araq · 2018-08-23T05:49:47Z

Well I was asking for concrete examples and you provided one where I have to ask: What's the point of the check? when isValueType(T1) seems to suffice.

timotheecour · 2018-08-23T09:23:33Z

What's the point of the check? when isValueType(T1) seems to suffice.

with when isValueType(T1):
equals([1,2], @[1, 2]) => false (isValueType(T1) is true and bitEquals(a1, a2) is false: a seq has a different bit layout than an array (excluding the elements themselves)

with when T1==T2 and isValueType(T1):
equals([1,2], @[1, 2]) => true (T1==T2 is false and remainder of code returns true)

happy to provide full implementation for isValueType and test suite if you want but didn't wanna obscure this issue

I also have an example where replacing when T1==T2 and isValueType(T1) by when T1 is T2 and isValueType(T1) would return different results (with inheritance involved)

Araq · 2018-08-23T09:51:47Z

Your check for T1 == T2 doesn't fix your wrong isValueType predicate. I'm asking for real examples, I'm not asking for your workarounds or why == is different than is (I know!).

And the reason why I don't like this feature is because this whole idea of builtin specialized typetraits does not scale. What should be done instead is to make use of macros.getType() so that one can write real checks for the type properties one is interested in. Your isValueType is a good example here, you don't want isValueType, you want isBlob or similar.

timotheecour · 2018-08-23T10:15:13Z

you don't want isValueType, you want isBlob or similar.

I couldn't find isBlob in nim; so perhaps we actually mean the same thing? see below.

wrong isValueType predicate

without seeing implementation it's hard to say it's wrong, so here is my implementation for isValueType: (the name could be better; it basically means it's a POD (plain old data); ie doesn't any pointers / references; for array it's true; for seq or string it's false)

given this implementation, why is it wrong? (besides the TODO of using a whitelist instead of not(tuple|object|array|RefType) and fixing remaining cases of map etc)

i think it's a real example; it tries to handle as much cases as possible with bitEquals, reverting to manual behavior otherwise. it's not a workaround (of what?)

{.reorder:on.}
proc defaultTemp[T]():T=
  var a: T
  return a

# TODO: map? etc
type RefType = ptr|pointer|ref|seq|string
proc isValueType(T:type RefType): bool = false

proc isValueType(T:type[tuple|object]): bool =
  ## returns whether T is a POD type
  var a = defaultTemp[T]()
  for i in fields(a):
    if not isValueType(type(i)):
      return false
  return true

# proc isValueType(T:type array):bool = defaultTemp[T]()[0].type.isValueType
#or: 
proc isValueType[N, T2](T:type[array[N, T2]]):bool = isValueType(T2)

# TODO: use whitelist instead (more robust)
proc isValueType(T:type[not(tuple|object|array|RefType)]):bool = true

proc isValueTypeTest()=
  type A1=object
    a:int
    b:array[3, int]
    c:type((1,2))

  type A2=object
    a:int
    b:array[3, string]

  var a:A1
  var a2 = cast[pointer](a.addr)

  require:
    int.isValueType
    float.isValueType
    not string.isValueType
    not seq[int].isValueType
    (1,2).type.isValueType
    not (1,"").type.isValueType
    A1.isValueType
    not A2.isValueType
    not (ref A1).isValueType
    not (ptr int).isValueType
    a.type.isValueType
    not a.addr.type.isValueType
    not a2.type.isValueType
    not pointer.isValueType
  # make sure works at CT
  static:
    doAssert: not A2.isValueType

Araq · 2018-10-09T16:10:50Z

Still not convinced.

metagn reviewed Aug 11, 2018

View reviewed changes

timotheecour changed the title ~~add typetraits.sameType~~ add typetraits.== for type equality Aug 11, 2018

timotheecour changed the title ~~add typetraits.== for type equality~~ [typetraits add == for type equality Aug 11, 2018

timotheecour changed the title ~~[typetraits add == for type equality~~ [typetraits] add == for type equality Aug 11, 2018

timotheecour changed the title ~~[typetraits] add == for type equality~~ [easy] [typetraits] add == for type equality Aug 15, 2018

dom96 reviewed Aug 15, 2018

View reviewed changes

timotheecour mentioned this pull request Aug 15, 2018

assert not(1==2) and assert foo != bar gives compile error; a hand-crafted not doesn't that that error #8300

Closed

dom96 reviewed Aug 18, 2018

View reviewed changes

timotheecour mentioned this pull request Aug 20, 2018

Error: internal error: genMagicExpr: mIs #8693

Closed

timotheecour force-pushed the pr_sameType branch from 3f0e797 to 714d1e7 Compare August 20, 2018 19:20

timotheecour closed this Aug 20, 2018

timotheecour reopened this Aug 20, 2018

timotheecour changed the title ~~[easy] [typetraits] add == for type equality~~ [easy] [typetraits] add == for type equality (different from is !) Aug 21, 2018

timotheecour added 2 commits August 22, 2018 12:24

add typetraits.sameType

ca9cdf3

sameType => ==

d25b2db

timotheecour added 4 commits August 22, 2018 12:24

doc improvement

862391b

fixup

5a6c660

address comments

06de483

used a different implementation to bypass issue nim-lang#8693

a1e29f8

revert back to simpler implementation now that nim-lang#8693 was fixed

bfa0b66

timotheecour force-pushed the pr_sameType branch from 714d1e7 to bfa0b66 Compare August 22, 2018 23:35

timotheecour mentioned this pull request Aug 23, 2018

proposal: bitEquals in system.nim ? (has implementation + tests) ; to test bit pattern equality regardless of type #22 #8436

Closed

Araq closed this Oct 9, 2018

timotheecour changed the title ~~[easy] [typetraits] add == for type equality (different from is !)~~ [TODO] [easy] [typetraits] add == for type equality (different from is !) Oct 9, 2018

[TODO] [easy] [typetraits] add == for type equality (different from is !) #8612

[TODO] [easy] [typetraits] add == for type equality (different from is !) #8612

Conversation

timotheecour commented Aug 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timotheecour Aug 12, 2018 • edited Loading

Choose a reason for hiding this comment

timotheecour commented Aug 11, 2018

dom96 commented Aug 15, 2018

GULPF commented Aug 15, 2018

timotheecour commented Aug 15, 2018

dom96 commented Aug 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timotheecour Aug 15, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timotheecour commented Aug 15, 2018

timotheecour commented Aug 18, 2018 • edited Loading

dom96 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Araq commented Aug 18, 2018

bluenote10 commented Aug 19, 2018

Araq commented Aug 20, 2018

timotheecour commented Aug 20, 2018 • edited Loading

Clyybber commented Aug 20, 2018

timotheecour commented Aug 20, 2018 • edited Loading

Clyybber commented Aug 21, 2018

Araq commented Aug 21, 2018

timotheecour commented Aug 22, 2018 • edited Loading

timotheecour commented Aug 22, 2018

Araq commented Aug 23, 2018

timotheecour commented Aug 23, 2018 • edited Loading

Araq commented Aug 23, 2018

timotheecour commented Aug 23, 2018 • edited Loading

Araq commented Oct 9, 2018

[TODO] [easy] [typetraits] add `==` for type equality (different from `is` !) #8612

[TODO] [easy] [typetraits] add `==` for type equality (different from `is` !) #8612

timotheecour Aug 12, 2018 •

edited

Loading

timotheecour Aug 15, 2018 •

edited

Loading

timotheecour commented Aug 18, 2018 •

edited

Loading

timotheecour commented Aug 20, 2018 •

edited

Loading

timotheecour commented Aug 20, 2018 •

edited

Loading

timotheecour commented Aug 22, 2018 •

edited

Loading

timotheecour commented Aug 23, 2018 •

edited

Loading

timotheecour commented Aug 23, 2018 •

edited

Loading