Skip to content

[#11123] fix for equality of WrappedArray.ofRef#7156

Merged
szeiger merged 3 commits intoscala:2.12.xfrom
da-liii:fix_wrapped_array
Sep 7, 2018
Merged

[#11123] fix for equality of WrappedArray.ofRef#7156
szeiger merged 3 commits intoscala:2.12.xfrom
da-liii:fix_wrapped_array

Conversation

@da-liii
Copy link
Copy Markdown
Contributor

@da-liii da-liii commented Aug 31, 2018

override def hashCode = MurmurHash3.wrappedArrayHash(array)
override def equals(that: Any) = that match {
case that: ofRef[_] => Arrays.equals(array.asInstanceOf[Array[AnyRef]], that.array.asInstanceOf[Array[AnyRef]])
case that: ofRef[_] => that.array.canEqual(array) && array.sameElements(that.array)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that.array.canEqual(array) is always return true.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seq(1) == Seq(1.0) also invokes canEqual in IterableLike which always returns true.

This code snippet is taken from GenSeqLike's equals.

Well, since it always yields true, I should remove it.

@adriaanm adriaanm requested a review from szeiger August 31, 2018 07:58
@adriaanm
Copy link
Copy Markdown
Contributor

adriaanm commented Aug 31, 2018

Welcome, @sadhen! Thanks for the prompt fix. I'd like to give Stefan a chance to take a look before merging this (he's on vacation this week).

@adriaanm adriaanm added the welcome hello new contributor! label Aug 31, 2018
@szeiger szeiger merged commit 4ad99a1 into scala:2.12.x Sep 7, 2018
@adriaanm adriaanm added the release-notes worth highlighting in next release notes label Sep 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes worth highlighting in next release notes welcome hello new contributor!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants