Override equals and hashCode for WrappedArray#5551
Override equals and hashCode for WrappedArray#5551alexeyr wants to merge 1 commit intoscala:2.12.xfrom alexeyr:wrappedarray-hashcode-equals
Conversation
|
The error is |
|
@alexeyr See the log of |
| def update(index: Int, elem: Unit) { array(index) = elem } | ||
| override def hashCode = MurmurHash3.wrappedArrayHash(array) | ||
| override def equals(that: Any) = that match { | ||
| case that: ofUnit => Arrays.equals(array.asInstanceOf[Array[AnyRef]], that.array.asInstanceOf[Array[AnyRef]]) |
There was a problem hiding this comment.
just compare by length?
case that: ofUnit => array.length == that.array.lengthThere was a problem hiding this comment.
That was my initial thought, but we could somehow end up with an Array[Unit] containing null at runtime and I decided to err in favor of safety.
There was a problem hiding this comment.
If you're hoping for any kind of correctness, null would have to be treated as being identical to () in this case. This is true for all primitive types. You can get null instead of the type's proper default value due to the way in which Scala abstracts over primitive types (and practically atavistic types such as void).
Just some of the weirdness:
scala> class A[T] { var x: T = _ } // a convenient way to get nulls of primitive types
defined class A
scala> new A[Unit]
res0: A[Unit] = A@359b650b
scala> res0.x // looks like we have ()
scala> println(res0.x) // null leaking when "converting" back to AnyRef
null
scala> res0.x == ((): Unit) // even scalac believes that this has to be true
<console>:13: warning: comparing values of types Unit and Unit using `==' will always yield true
res0.x == ((): Unit)
^
res4: Boolean = false
scala> Array[Unit](res0.x)
res6: Array[Unit] = Array(())
scala> res6(0)
scala> println(res6(0)) // whoa, this one's actually correct
()
scala> new Array[Unit](1)
res9: Array[Unit] = Array(())
scala> println(res9(0)) // default is null again
null
scala> res9(0) = res0.x
scala> println(res9(0)) // still null
null
There was a problem hiding this comment.
|
There is also the question of whether overrides for |
|
I wouldn't bother with |
|
Ah, I should have merged this before merge conflicts start creeping in. The Conflict will probably resolve itself after #5590 though. |
|
Closing in favor of the rebased one. |
Avoid unnecessary boxing when calling
equalsorhashCodeonWrappedArrays.