Found several minor thigns wrong with checkSens...

Found several minor thigns wrong with checkSensible, which tries to
issue warnings for comparisons which will always be true or false. No
review.

5 files changed, 80 insertions, 45 deletions

diff --git a/src/compiler/scala/tools/nsc/backend/JavaPlatform.scala b/src/compiler/scala/tools/nsc/backend/JavaPlatform.scala
index 8d77eef488..8373f9bfd6 100644
--- a/src/compiler/scala/tools/nsc/backend/JavaPlatform.scala
+++ b/src/compiler/scala/tools/nsc/backend/JavaPlatform.scala
@@ -33,12 +33,18 @@ trait JavaPlatform extends Platform[AbstractFile] {
   def externalEqualsNumChar = getMember(BoxesRunTimeClass, "equalsNumChar")
   def externalEqualsNumObject = getMember(BoxesRunTimeClass, "equalsNumObject")
 
+  /** We could get away with excluding BoxedBooleanClass for the
+   *  purpose of equality testing since it need not compare equal
+   *  to anything but other booleans, but it should be present in
+   *  case this is put to other uses.
+   */
   def isMaybeBoxed(sym: Symbol): Boolean = {
     import definitions._
     (sym == ObjectClass) ||
     (sym == JavaSerializableClass) ||
     (sym == ComparableClass) ||
     (sym isNonBottomSubClass BoxedNumberClass) ||
-    (sym isNonBottomSubClass BoxedCharacterClass)
+    (sym isNonBottomSubClass BoxedCharacterClass) ||
+    (sym isNonBottomSubClass BoxedBooleanClass)
   }
 }
diff --git a/src/compiler/scala/tools/nsc/symtab/Definitions.scala b/src/compiler/scala/tools/nsc/symtab/Definitions.scala
index 5a4fa51899..12a91a2a8c 100644
--- a/src/compiler/scala/tools/nsc/symtab/Definitions.scala
+++ b/src/compiler/scala/tools/nsc/symtab/Definitions.scala
@@ -801,11 +801,17 @@ trait Definitions extends reflect.generic.StandardDefinitions {
     def isValueClass(sym: Symbol): Boolean =
       (sym eq UnitClass) || (boxedClass contains sym)
 
+    /** If symbol is a value class or a boxed value class, return the value class: otherwise NoSymbol. */
+    def unboxedValueClass(sym: Symbol): Symbol =
+      if (isValueClass(sym)) sym
+      else if (sym == BoxedUnitClass) sym
+      else boxedClass.map(_.swap).getOrElse(sym, NoSymbol)
+
     /** Is symbol a numeric value class? */
     def isNumericValueClass(sym: Symbol): Boolean =
       numericWeight contains sym
 
-    /** Is symbol a numeric value class? */
+    /** Is type's symbol a numeric value class? */
     def isNumericValueType(tp: Type): Boolean = tp match {
       case TypeRef(_, sym, _) => isNumericValueClass(sym)
       case _ => false
diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
index f12d7939ea..3af22264ab 100644
--- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
@@ -870,24 +870,29 @@ abstract class RefChecks extends InfoTransform {
             sym = sym.info.bounds.hi.widen.typeSymbol
           sym
         }
-        val actual = underlyingClass(args.head.tpe)
+        val actual   = underlyingClass(args.head.tpe)
         val receiver = underlyingClass(qual.tpe)
 
+        /** Symbols which limit the warnings we can issue since they may be value types */
+        val isMaybeValue = Set(AnyClass, AnyRefClass, AnyValClass, ObjectClass, ComparableClass, SerializableClass)
+
         // Whether def equals(other: Any) is overridden
         def isUsingDefaultEquals      = {
           val m = receiver.info.member(nme.equals_)
           (m == Object_equals) || (m == Any_equals)
         }
-        // Whether this == or != is actually an overloaded version
+        // Whether this == or != is one of those defined in Any/AnyRef or an overload from elsewhere.
         def isUsingDefaultScalaOp = {
           val s = fn.symbol
-          (s == Object_==) || (s == Object_!=) || (s == Any_==) || (s == Any_!=) || (s == Object_eq) || (s == Object_ne)
+          (s == Object_==) || (s == Object_!=) || (s == Any_==) || (s == Any_!=)
         }
         // Whether the operands+operator represent a warnable combo (assuming anyrefs)
-        def isWarnable                = isReferenceOp || (isUsingDefaultEquals && isUsingDefaultScalaOp)
-        def isScalaNumber(s: Symbol)  = isNumericValueClass(s) || (s isSubClass BoxedNumberClass) || (s isSubClass ScalaNumberClass)
-        def isEitherNull              = (receiver == NullClass) || (actual == NullClass)
-        def isEitherNullable          = (NullClass.tpe <:< receiver.info) || (NullClass.tpe <:< actual.info)
+        def isWarnable               = isReferenceOp || (isUsingDefaultEquals && isUsingDefaultScalaOp)
+        def isEitherNull             = (receiver == NullClass) || (actual == NullClass)
+        def isEitherNullable         = (NullClass.tpe <:< receiver.info) || (NullClass.tpe <:< actual.info)
+        def isBoolean(s: Symbol)     = unboxedValueClass(s) == BooleanClass
+        def isUnit(s: Symbol)        = unboxedValueClass(s) == UnitClass
+        def isNumeric(s: Symbol)     = isNumericValueClass(unboxedValueClass(s)) || (s isSubClass ScalaNumberClass)
 
         def nonSensibleWarning(what: String, alwaysEqual: Boolean) = {
           val msg = alwaysEqual == (name == nme.EQ || name == nme.eq)
@@ -895,23 +900,37 @@ abstract class RefChecks extends InfoTransform {
         }
 
         // @MAT normalize for consistency in error message, otherwise only part is normalized due to use of `typeSymbol'
-        def nonSensible(pre: String, alwaysEqual: Boolean) =
-          nonSensibleWarning(pre+"values of types "+normalizeAll(qual.tpe.widen)+" and "+normalizeAll(args.head.tpe.widen),
-                             alwaysEqual)
-
-        if (receiver == BooleanClass && !(receiver isSubClass actual))  // true == 5
-          nonSensible("", false)
-        else if (receiver == UnitClass && actual == UnitClass)  // () == ()
-          nonSensible("", true)
-        else if (isNumericValueClass(receiver)) {
-          if (!isScalaNumber(actual) && !forMSIL) // 5 == "abc"
+        def nonSensible(pre: String, alwaysEqual: Boolean) = nonSensibleWarning(
+          pre+"values of types "+normalizeAll(qual.tpe.widen)+" and "+normalizeAll(args.head.tpe.widen),
+          alwaysEqual
+        )
+
+        if (isBoolean(receiver)) {
+          if (!isBoolean(actual) && !isMaybeValue(actual))    // true == 5
+            nonSensible("", false)
+        }
+        else if (isUnit(receiver)) {
+          if (isUnit(actual)) // () == ()
+            nonSensible("", true)
+          else if (!isUnit(actual) && !isMaybeValue(actual))  // () == "abc"
             nonSensible("", false)
         }
+        else if (isNumeric(receiver)) {
+          if (!isNumeric(actual) && !forMSIL)
+            if (isUnit(actual) || isBoolean(actual) || !isMaybeValue(actual))   // 5 == "abc"
+              nonSensible("", false)
+        }
         else if (isWarnable) {
-          if (receiver.isFinal && !isEitherNull && !(receiver isSubClass actual)) // object X, Y; X == Y
-            nonSensible((if (isEitherNullable) "non-null " else ""), false)
-          else if (isNew(qual) || (isNew(args.head) && (receiver.isFinal || isReferenceOp))) // new X == y or object X ; X == new Y
+          if (isNew(qual)) // new X == y
+            nonSensibleWarning("a fresh object", false)
+          else if (isNew(args.head) && (receiver.isFinal || isReferenceOp))   // object X ; X == new Y
             nonSensibleWarning("a fresh object", false)
+          else if (receiver.isFinal && !isEitherNull && !(receiver isSubClass actual)) {  // object X, Y; X == Y
+            if (isEitherNullable)
+              nonSensible("non-null ", false)
+            else
+              nonSensible("", false)
+          }
         }
       case _ =>
     }
diff --git a/test/files/neg/checksensible.check b/test/files/neg/checksensible.check
index b36b11bf98..67a76dd7be 100644
--- a/test/files/neg/checksensible.check
+++ b/test/files/neg/checksensible.check
@@ -28,64 +28,64 @@ checksensible.scala:27: error: comparing values of types Int and Unit using `=='
 checksensible.scala:29: error: comparing values of types Int and java.lang.String using `==' will always yield false
   1 == "abc"
     ^
-checksensible.scala:32: error: comparing a fresh object using `==' will always yield false
+checksensible.scala:34: error: comparing a fresh object using `==' will always yield false
   new AnyRef == 1
              ^
-checksensible.scala:33: error: comparing values of types Int and java.lang.Object using `==' will always yield false
-  1 == new AnyRef
+checksensible.scala:37: error: comparing values of types Int and java.lang.Boolean using `==' will always yield false
+  1 == (new java.lang.Boolean(true))
     ^
-checksensible.scala:35: error: comparing values of types Int and Boolean using `!=' will always yield true
+checksensible.scala:39: error: comparing values of types Int and Boolean using `!=' will always yield true
   1 != true
     ^
-checksensible.scala:36: error: comparing values of types Unit and Boolean using `==' will always yield false
+checksensible.scala:40: error: comparing values of types Unit and Boolean using `==' will always yield false
   () == true
      ^
-checksensible.scala:37: error: comparing values of types Unit and Unit using `==' will always yield true
+checksensible.scala:41: error: comparing values of types Unit and Unit using `==' will always yield true
   () == ()
      ^
-checksensible.scala:38: error: comparing values of types Unit and Unit using `==' will always yield true
+checksensible.scala:42: error: comparing values of types Unit and Unit using `==' will always yield true
   () == println
      ^
-checksensible.scala:40: error: comparing values of types Int and Unit using `!=' will always yield true
+checksensible.scala:44: error: comparing values of types Int and Unit using `!=' will always yield true
   (1 != println)
      ^
-checksensible.scala:41: error: comparing values of types Int and Symbol using `!=' will always yield true
+checksensible.scala:45: error: comparing values of types Int and Symbol using `!=' will always yield true
   (1 != 'sym)
      ^
-checksensible.scala:47: error: comparing a fresh object using `==' will always yield false
+checksensible.scala:51: error: comparing a fresh object using `==' will always yield false
   ((x: Int) => x + 1) == null
                       ^
-checksensible.scala:48: error: comparing values of types object Bep and (Int) => Int using `==' will always yield false
+checksensible.scala:52: error: comparing a fresh object using `==' will always yield false
   Bep == ((_: Int) + 1)
       ^
-checksensible.scala:50: error: comparing a fresh object using `==' will always yield false
+checksensible.scala:54: error: comparing a fresh object using `==' will always yield false
   new Object == new Object
              ^
-checksensible.scala:51: error: comparing a fresh object using `==' will always yield false
+checksensible.scala:55: error: comparing a fresh object using `==' will always yield false
   new Object == "abc"
              ^
-checksensible.scala:52: error: comparing a fresh object using `!=' will always yield true
+checksensible.scala:56: error: comparing a fresh object using `!=' will always yield true
   new Exception() != new Exception()
                   ^
-checksensible.scala:55: error: comparing values of types Int and Null using `==' will always yield false
+checksensible.scala:59: error: comparing values of types Int and Null using `==' will always yield false
   if (foo.length == null) "plante" else "plante pas"
                  ^
-checksensible.scala:60: error: comparing values of types Bip and Bop using `==' will always yield false
+checksensible.scala:64: error: comparing values of types Bip and Bop using `==' will always yield false
   (x1 == x2)
       ^
-checksensible.scala:71: error: comparing values of types EqEqRefTest.this.C3 and EqEqRefTest.this.Z1 using `==' will always yield false
+checksensible.scala:75: error: comparing values of types EqEqRefTest.this.C3 and EqEqRefTest.this.Z1 using `==' will always yield false
   c3 == z1
      ^
-checksensible.scala:72: error: comparing values of types EqEqRefTest.this.Z1 and EqEqRefTest.this.C3 using `==' will always yield false
+checksensible.scala:76: error: comparing values of types EqEqRefTest.this.Z1 and EqEqRefTest.this.C3 using `==' will always yield false
   z1 == c3
      ^
-checksensible.scala:73: error: comparing values of types EqEqRefTest.this.Z1 and EqEqRefTest.this.C3 using `!=' will always yield true
+checksensible.scala:77: error: comparing values of types EqEqRefTest.this.Z1 and EqEqRefTest.this.C3 using `!=' will always yield true
   z1 != c3
      ^
-checksensible.scala:74: error: comparing values of types EqEqRefTest.this.C3 and java.lang.String using `!=' will always yield true
+checksensible.scala:78: error: comparing values of types EqEqRefTest.this.C3 and java.lang.String using `!=' will always yield true
   c3 != "abc"
      ^
-checksensible.scala:83: error: comparing values of types Unit and Int using `!=' will always yield true
+checksensible.scala:87: error: comparing values of types Unit and Int using `!=' will always yield true
     while ((c = in.read) != -1)
                          ^
 30 errors found
diff --git a/test/files/neg/checksensible.scala b/test/files/neg/checksensible.scala
index f67e9ea57f..e68a6ce9c7 100644
--- a/test/files/neg/checksensible.scala
+++ b/test/files/neg/checksensible.scala
@@ -27,10 +27,14 @@ class EqEqValTest {
   0 == (c = 1)
 
   1 == "abc"
-  "abc" == 1  // doesn't warn since String defines an equals method
+  1 == ("abc": Any) // doesn't warn because an Any may be a boxed Int
+  1 == (1: Any)     // as above
+  "abc" == 1        // doesn't warn since String defines an equals method
 
   new AnyRef == 1
-  1 == new AnyRef
+  1 == new AnyRef                 // doesn't warn because it could be...
+  1 == (new java.lang.Integer(1)) // ...something like this
+  1 == (new java.lang.Boolean(true))
 
   1 != true
   () == true