Fix comparisons involving NaN

Floating point comparisons involving NaN should always return false,
except for !=. Fixes a regression introduced by #4963.

4 files changed, 52 insertions, 17 deletions

diff --git a/src/compiler/scala/tools/nsc/backend/ScalaPrimitives.scala b/src/compiler/scala/tools/nsc/backend/ScalaPrimitives.scala
index 00771b6b8c..dfd5b07a3b 100644
--- a/src/compiler/scala/tools/nsc/backend/ScalaPrimitives.scala
+++ b/src/compiler/scala/tools/nsc/backend/ScalaPrimitives.scala
@@ -61,8 +61,8 @@ abstract class ScalaPrimitives {
   final val NE = 43                            // x != y
   final val LT = 44                            // x < y
   final val LE = 45                            // x <= y
-  final val GE = 46                            // x > y
-  final val GT = 47                            // x >= y
+  final val GT = 46                            // x > y
+  final val GE = 47                            // x >= y
 
   // Boolean unary operations
   final val ZNOT = 50                          // !x
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala
index 5d152ef0e8..d7106ae908 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala
@@ -1110,22 +1110,19 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
     }
 
     /* Emit code to compare the two top-most stack values using the 'op' operator. */
-    private def genCJUMP(success: asm.Label, failure: asm.Label, op: TestOp, tk: BType, targetIfNoJump: asm.Label) {
-      if (targetIfNoJump == success) genCJUMP(failure, success, op.negate, tk, targetIfNoJump)
+    private def genCJUMP(success: asm.Label, failure: asm.Label, op: TestOp, tk: BType, targetIfNoJump: asm.Label, negated: Boolean = false) {
+      if (targetIfNoJump == success) genCJUMP(failure, success, op.negate, tk, targetIfNoJump, negated = !negated)
       else {
         if (tk.isIntSizedType) { // BOOL, BYTE, CHAR, SHORT, or INT
           bc.emitIF_ICMP(op, success)
         } else if (tk.isRef) { // REFERENCE(_) | ARRAY(_)
           bc.emitIF_ACMP(op, success)
         } else {
+          def useCmpG = if (negated) op == TestOp.GT || op == TestOp.GE else op == TestOp.LT || op == TestOp.LE
           (tk: @unchecked) match {
             case LONG   => emit(asm.Opcodes.LCMP)
-            case FLOAT  =>
-              if (op == TestOp.LT || op == TestOp.LE) emit(asm.Opcodes.FCMPG)
-              else emit(asm.Opcodes.FCMPL)
-            case DOUBLE =>
-              if (op == TestOp.LT || op == TestOp.LE) emit(asm.Opcodes.DCMPG)
-              else emit(asm.Opcodes.DCMPL)
+            case FLOAT  => emit(if (useCmpG) asm.Opcodes.FCMPG else asm.Opcodes.FCMPL)
+            case DOUBLE => emit(if (useCmpG) asm.Opcodes.DCMPG else asm.Opcodes.DCMPL)
           }
           bc.emitIF(op, success)
         }
@@ -1134,8 +1131,8 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
     }
 
     /* Emits code to compare (and consume) stack-top and zero using the 'op' operator */
-    private def genCZJUMP(success: asm.Label, failure: asm.Label, op: TestOp, tk: BType, targetIfNoJump: asm.Label) {
-      if (targetIfNoJump == success) genCZJUMP(failure, success, op.negate, tk, targetIfNoJump)
+    private def genCZJUMP(success: asm.Label, failure: asm.Label, op: TestOp, tk: BType, targetIfNoJump: asm.Label, negated: Boolean = false) {
+      if (targetIfNoJump == success) genCZJUMP(failure, success, op.negate, tk, targetIfNoJump, negated = !negated)
       else {
         if (tk.isIntSizedType) { // BOOL, BYTE, CHAR, SHORT, or INT
           bc.emitIF(op, success)
@@ -1145,18 +1142,17 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
             case TestOp.NE => bc emitIFNONNULL success
           }
         } else {
+          def useCmpG = if (negated) op == TestOp.GT || op == TestOp.GE else op == TestOp.LT || op == TestOp.LE
           (tk: @unchecked) match {
             case LONG   =>
               emit(asm.Opcodes.LCONST_0)
               emit(asm.Opcodes.LCMP)
             case FLOAT  =>
               emit(asm.Opcodes.FCONST_0)
-              if (op == TestOp.LT || op == TestOp.LE) emit(asm.Opcodes.FCMPG)
-              else emit(asm.Opcodes.FCMPL)
+              emit(if (useCmpG) asm.Opcodes.FCMPG else asm.Opcodes.FCMPL)
             case DOUBLE =>
               emit(asm.Opcodes.DCONST_0)
-              if (op == TestOp.LT || op == TestOp.LE) emit(asm.Opcodes.DCMPG)
-              else emit(asm.Opcodes.DCMPL)
+              emit(if (useCmpG) asm.Opcodes.DCMPG else asm.Opcodes.DCMPL)
           }
           bc.emitIF(op, success)
         }
@@ -1171,8 +1167,8 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder {
       case scalaPrimitives.NE => TestOp.NE
       case scalaPrimitives.LT => TestOp.LT
       case scalaPrimitives.LE => TestOp.LE
-      case scalaPrimitives.GE => TestOp.GE
       case scalaPrimitives.GT => TestOp.GT
+      case scalaPrimitives.GE => TestOp.GE
     }
 
     /** Some useful equality helpers. */
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala
index a5744983b2..5a5747c81f 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala
@@ -1336,6 +1336,7 @@ object BCodeHelpers {
   }
 
   object TestOp {
+    // the order here / op numbers are important to get the correct result when calling opcodeIF
     val EQ = new TestOp(0)
     val NE = new TestOp(1)
     val LT = new TestOp(2)
diff --git a/test/junit/scala/lang/primitives/NaNTest.scala b/test/junit/scala/lang/primitives/NaNTest.scala
new file mode 100644
index 0000000000..f4c4258395
--- /dev/null
+++ b/test/junit/scala/lang/primitives/NaNTest.scala
@@ -0,0 +1,38 @@
+package scala.lang.primitives
+
+import org.junit.Assert._
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+
+import scala.tools.testing.RunTesting
+
+@RunWith(classOf[JUnit4])
+class NaNTest extends RunTesting {
+
+  @Test
+  def compNaNFalse(): Unit = {
+    def code(tp: String) =
+      s"""val n = $tp.NaN
+          |def ne(x: $tp, y: $tp) = x != y
+          |val fs: List[($tp, $tp) => Boolean] = List(_ < _, _ <= _, _ > _, _ >= _,  _ == _, (x, y) => !ne(x, y))
+          |val vs = List[$tp](n, 1, -1, 0)
+          |for (f <- fs; v <- vs; (x, y) <- List((n, v), (v, n))) yield f(x, y)
+      """.stripMargin
+
+    runner.run[List[Boolean]](code("Double")).foreach(assertFalse)
+    runner.run[List[Boolean]](code("Float")).foreach(assertFalse)
+  }
+
+  @Test
+  def genericEqNe(): Unit = {
+    def code(tp: String) =
+      s"""def a[T](x: T, y: T) = x == y
+         |def b[T](x: T, y: T) = x != y
+         |val n = $tp.NaN
+         |a(n, n) :: a(n, 0) :: a (0, n) :: !b(n, n) :: !b(n, 0) :: !b(0, n) :: Nil
+      """.stripMargin
+    runner.run[List[Boolean]](code("Double")).foreach(assertFalse)
+    runner.run[List[Boolean]](code("Float")).foreach(assertFalse)
+  }
+}