Skip to content

Commit

Permalink
Fix #1770: Fix __scala_== with ScalaNumbers. (#1805)
Browse files Browse the repository at this point in the history 
The implementation of `java.lang.Number.__scala_==` was correct in
spirit, but failed in practice because of quirks of the linker and
`java.lang.Object.__scala_==`.

Normally, `Object.__scala_==` should be implemented as
`this.equals(that)`. For performance reasons, it is instead
implemented as `this eq that`, with the understanding that the
linker will patch the vtable of any class that overrides `equals`
so that `__scala_==` refers to that `equals`.

The issue is that this patching does not apply to super calls of the
form `super.__scala_==`, making any such call incorrect.

We fix the issue by directly calling `this.equals(that)` instead of
`super.__scala_==`, since the former would be the reasonable
implementation of `Object.__scala_==`.
  • Loading branch information
@LeeTibbert
LeeTibbert committed May 25, 2020
1 parent 9bfd3fa commit 83c86ef
Show file tree
Hide file tree
Showing 4 changed files with 312 additions and 2 deletions.
2 changes: 1 addition & 1 deletion javalib/src/main/scala/java/lang/Number.scala
View file
Expand Up @@ -14,7 +14,7 @@ abstract class Number extends java.lang._Object with java.io.Serializable {
if (other.isInstanceOf[ScalaNumber] && !this.isInstanceOf[ScalaNumber]) {
other.equals(this)
} else {
super.__scala_==(other)
this.equals(other)
}
}
}
253 changes: 253 additions & 0 deletions unit-tests/src/test/scala/java/lang/ScalaNumberSuite.scala
View file
@@ -0,0 +1,253 @@
package java.lang

// Exercise __scala_==

object ScalaNumberSuite extends tests.Suite {

test("BigInt") { // Section header, visually group tests
}

test(" BigInt == BigInt") {
val token = 2047L
val sbi1: scala.math.BigInt = scala.math.BigInt(token)
val sbi2: scala.math.BigInt = scala.math.BigInt(token)

assert(sbi1 == sbi2)
}

test(" BigInt.equals(BigInt)") {
val token = 2047L
val sbi1: scala.math.BigInt = scala.math.BigInt(token)
val sbi2: scala.math.BigInt = scala.math.BigInt(token)

assert(sbi1.equals(sbi2))
}

test(" BigInt does not == BigInt with different value") {
val token = 2047L
val sbi1: scala.math.BigInt = scala.math.BigInt(token)
// avoid powers of 2 because of possible caching.
val sbi2: scala.math.BigInt = scala.math.BigInt(token + 2)

assertFalse(sbi1 == sbi2)
}

test(" BigInt == j.l.Long") {
val token = Int.MaxValue + 2L
val sbi: scala.math.BigInt = scala.math.BigInt(token)
val jl: java.lang.Long = new java.lang.Long(token.toString)

assert(sbi == jl)
}

test(" BigInt does not == j.l.Long with different value") {
val token = Int.MaxValue + 2L
val sbi: scala.math.BigInt = scala.math.BigInt(token)
val jl: java.lang.Long = new java.lang.Long((token + 2).toString)

assertFalse(sbi == jl)
}

test(" j.l.Long == BigInt") {
val token = Int.MaxValue + 2L
val sbi: scala.math.BigInt = scala.math.BigInt(token)
val jl: java.lang.Long = new java.lang.Long(token.toString)

assert(jl == sbi)
}

test(" j.l.Long does not == BigInt with different value") {
val token = Int.MaxValue + 2L
val sbi: scala.math.BigInt = scala.math.BigInt(token)
val jl: java.lang.Long = new java.lang.Long((token + 2).toString)

assertFalse(jl == sbi)
}

test(" j.l.Long == j.l.Long") {
val token = 2047
val jl1: java.lang.Long = new java.lang.Long(token)
val jl2: java.lang.Long = new java.lang.Long(token)

assert(jl1 == jl2)
}

test(" j.l.Long does not == j.l.Long with different value") {
val token = 2047
val jl1: java.lang.Long = new java.lang.Long(token)
val jl2: java.lang.Long = new java.lang.Long(token + 2)

assertFalse(jl1 == jl2)
}

test(" BigInt == j.l.Integer") {
val token = 2047L
val sbi: scala.math.BigInt = scala.math.BigInt(token)
val ji: java.lang.Integer = new java.lang.Integer(token.toString)

assert(sbi == ji)
}

test(" BigInt does not == j.l.Integer with different value") {
val token = 2047L
val sbi: scala.math.BigInt = scala.math.BigInt(token)
val ji: java.lang.Integer = new java.lang.Integer((token + 2).toString)

assertFalse(sbi == ji)
}

test(" j.l.Integer == BigInt") {
val token = 2047L
val sbi: scala.math.BigInt = scala.math.BigInt(token)
val ji: java.lang.Integer = new java.lang.Integer(token.toString)

assert(ji == sbi)
}

test(" j.l.Integer does not == BigInt with different value") {
val token = 2047L
val sbi: scala.math.BigInt = scala.math.BigInt(token)
val ji: java.lang.Integer = new java.lang.Integer((token + 2).toString)

assertFalse(ji == sbi)
}

test(" j.l.Integer == j.l.Integer") {
val token = 2047
val ji1: java.lang.Integer = new java.lang.Integer(token)
val ji2: java.lang.Integer = new java.lang.Integer(token)

assert(ji1 == ji2)
}

test(" j.l.Integer does not == j.l.Integer with different value") {
val token = 2047
val ji1: java.lang.Integer = new java.lang.Integer(token)
val ji2: java.lang.Integer = new java.lang.Integer(token + 2)

assertFalse(ji1 == ji2)
}

test("BigDecimal") { // Section header, visually group tests
}

test(" BigDecimal == BigDecimal") {
val token = 2046.5
val sbd1: scala.math.BigDecimal = scala.math.BigDecimal(token)
val sbd2: scala.math.BigDecimal = scala.math.BigDecimal(token)

assert(sbd1 == sbd2)
}

test(" BigDecimal.equals(BigDecimal)") {
val token = 2046.5
val sbd1: scala.math.BigDecimal = scala.math.BigDecimal(token)
val sbd2: scala.math.BigDecimal = scala.math.BigDecimal(token)

assert(sbd1.equals(sbd2))
}

test(" BigDecimal does not == BigDecimal with different value") {
val token = 2046.5
val sbd1: scala.math.BigDecimal = scala.math.BigDecimal(token)
val sbd2: scala.math.BigDecimal = scala.math.BigDecimal(token - 2.0)

assertFalse(sbd1 == sbd2)
}

test(" BigDecimal == j.l.Double") {
val token = 2046.5
val sbd: scala.math.BigDecimal = scala.math.BigDecimal(token)
val jd: java.lang.Double = new java.lang.Double(token.toString)

assert(sbd == jd)
}

test(" BigDecimal does not == j.l.Double with different value") {
val token = 2046.5
val sbd: scala.math.BigDecimal = scala.math.BigDecimal(token)
val jd: java.lang.Double = new java.lang.Double((token - 2.0).toString)

assertFalse(sbd == jd)
}

test(" j.l.Double == BigDecimal") {
val token = 2046.5
val sbd: scala.math.BigDecimal = scala.math.BigDecimal(token)
val jd: java.lang.Double = new java.lang.Double(token.toString)

assert(jd == sbd)
}

test(" j.l.Double does not == BigDecimal with different value") {
val token = 2046.5
val sbd: scala.math.BigDecimal = scala.math.BigDecimal(token)
val jd: java.lang.Double = new java.lang.Double((token - 2.0).toString)

assertFalse(sbd == jd)
}

test(" j.l.Double == j.l.Double") {
val token = 2046.5
val jd1: java.lang.Double = new java.lang.Double(token)
val jd2: java.lang.Double = new java.lang.Double(token)

assert(jd1 == jd2)
}

test(" j.l.Double does not == j.l.Double with different value") {
val token = 2046.5
val jd1: java.lang.Double = new java.lang.Double(token)
val jd2: java.lang.Double = new java.lang.Double((token - 2.0).toString)

assertFalse(jd1 == jd2)
}

test(" BigDecimal == j.l.Float") {
val token = 2046.5F
val sbd: scala.math.BigDecimal = scala.math.BigDecimal(token)
val jf: java.lang.Float = new java.lang.Float(token.toString)

assert(sbd == jf)
}

test(" BigDecimal does not == j.l.Float with different value") {
val token = 2046.5F
val sbd: scala.math.BigDecimal = scala.math.BigDecimal(token)
val jf: java.lang.Float = new java.lang.Float((token - 2.0).toString)

assertFalse(sbd == jf)
}

test(" j.l.Float == BigDecimal") {
val token = 2046.5F
val sbd: scala.math.BigDecimal = scala.math.BigDecimal(token)
val jf: java.lang.Float = new java.lang.Float(token.toString)

assert(jf == sbd)
}

test(" j.l.Float does not == BigDecimal with different value") {
val token = 2046.5F
val sbd: scala.math.BigDecimal = scala.math.BigDecimal(token)
val jf: java.lang.Float = new java.lang.Float((token - 2.0).toString)

assertFalse(jf == sbd)
}

test(" j.l.Float == j.l.Float") {
val token = 2046.5F
val jf1: java.lang.Float = new java.lang.Float(token)
val jf2: java.lang.Float = new java.lang.Float(token)

assert(jf1 == jf2)
}

test(" j.l.Float does not == j.l.Float with different value") {
val token = 2046.5F
val jf1: java.lang.Float = new java.lang.Float(token)
val jf2: java.lang.Float = new java.lang.Float((token - 2.0).toString)

assertFalse(jf1 == jf2)
}
}
31 changes: 31 additions & 0 deletions unit-tests/src/test/scala/java/math/BigDecimalSuite.scala
View file
@@ -0,0 +1,31 @@
package java.math

object BigDecimalSuite extends tests.Suite {
// __scala_==

test("BigDecimal == BigDecimal") {
val token = 2046.5
val jbd1: java.math.BigDecimal = java.math.BigDecimal.valueOf(token)
val jbd2: java.math.BigDecimal = java.math.BigDecimal.valueOf(token)

// Depending upon possible caching, they may or may not be eq.
assert(jbd1 == jbd2)
}

test("BigDecimal.equals(BigDecimal)") {
val token = 2046.5
val jbd1: java.math.BigDecimal = java.math.BigDecimal.valueOf(token)
val jbd2: java.math.BigDecimal = java.math.BigDecimal.valueOf(token)

// Depending upon possible caching, they may or may not be reference eq.
assert(jbd1.equals(jbd2))
}

test("BigDecimal does not == BigDecimal with different value") {
val token = 2046.5
val jbd1: java.math.BigDecimal = java.math.BigDecimal.valueOf(token)
val jbd2: java.math.BigDecimal = java.math.BigDecimal.valueOf(token + 1.0)

assertFalse(jbd1 == jbd2)
}
}
28 changes: 27 additions & 1 deletion unit-tests/src/test/scala/java/math/BigIntegerSuite.scala
View file
@@ -1,7 +1,6 @@
package java.math

object BigIntegerSuite extends tests.Suite {

// byteValueExact

val byteMaxBi = new BigInteger(java.lang.Byte.MAX_VALUE.toString)
Expand Down Expand Up @@ -110,4 +109,31 @@ object BigIntegerSuite extends tests.Suite {
}
}

// __scala_==

test("BigInteger == BigInteger") {
val token = 2047L
val jbi1: java.math.BigInteger = java.math.BigInteger.valueOf(token)
val jbi2: java.math.BigInteger = java.math.BigInteger.valueOf(token)

// Depending upon possible caching, they may or may not be eq.
assert(jbi1 == jbi2)
}

test("BigInteger.equals(BigInteger)") {
val token = 2047L
val jbi1: java.math.BigInteger = java.math.BigInteger.valueOf(token)
val jbi2: java.math.BigInteger = java.math.BigInteger.valueOf(token)

// Depending upon possible caching, they may or may not be reference eq.
assert(jbi1.equals(jbi2))
}

test("BigInteger does not == BigInteger with different value") {
val token = 2047L
val jbi1: java.math.BigInteger = java.math.BigInteger.valueOf(token)
val jbi2: java.math.BigInteger = java.math.BigInteger.valueOf(token + 1)

assertFalse(jbi1 == jbi2)
}
}

0 comments on commit 83c86ef

Please sign in to comment.