More fiddling with checkSensible.

warnings. Fixed some bugs revealed by said warnings, and made some minor
changes to avoid warnings. (Technically it's not a bug to have unrelated
classes compare as equal, but it so often is a bug that it behooves us
not to do it intentionally so the warnings stand out.) Disabled the most
useful warning for the moment since it'd be wrong with some frequency.
No review.

6 files changed, 51 insertions, 33 deletions

diff --git a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala b/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
index ca9ab7007c..9bbedb6f53 100644
--- a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
+++ b/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
@@ -1800,15 +1800,11 @@ abstract class GenICode extends SubComponent  {
 
     /////////////////////// Context ////////////////////////////////
 
-    abstract class Cleanup;
-    case class MonitorRelease(m: Local) extends Cleanup {
-      override def hashCode = m.hashCode
-      override def equals(other: Any) = m == other;
-    }
-    case class Finalizer(f: Tree) extends Cleanup {
-      override def hashCode = f.hashCode
-      override def equals(other: Any) = f == other;
+    abstract class Cleanup(val value: AnyRef) {
+      def contains(x: AnyRef) = value == x
     }
+    case class MonitorRelease(m: Local) extends Cleanup(m) { }
+    case class Finalizer(f: Tree) extends Cleanup (f) { }
 
     def duplicateFinalizer(boundLabels: Set[Symbol], targetCtx: Context, finalizer: Tree) =  {
       (new DuplicateLabels(boundLabels))(targetCtx, finalizer)
@@ -1917,7 +1913,7 @@ abstract class GenICode extends SubComponent  {
       }
 
       def exitSynchronized(monitor: Local): this.type = {
-        assert(cleanups.head == monitor,
+        assert(cleanups.head contains monitor,
                "Bad nesting of cleanup operations: " + cleanups + " trying to exit from monitor: " + monitor)
         cleanups = cleanups.tail
         this
@@ -1929,7 +1925,7 @@ abstract class GenICode extends SubComponent  {
       }
 
       def removeFinalizer(f: Tree): this.type = {
-        assert(cleanups.head == f,
+        assert(cleanups.head contains f,
                "Illegal nesting of cleanup operations: " + cleanups + " while exiting finalizer " + f);
         cleanups = cleanups.tail
         this
diff --git a/src/compiler/scala/tools/nsc/symtab/Definitions.scala b/src/compiler/scala/tools/nsc/symtab/Definitions.scala
index 12a91a2a8c..4446f8c10d 100644
--- a/src/compiler/scala/tools/nsc/symtab/Definitions.scala
+++ b/src/compiler/scala/tools/nsc/symtab/Definitions.scala
@@ -413,7 +413,7 @@ trait Definitions extends reflect.generic.StandardDefinitions {
       	  (functionType.normalize match {
       	    case TypeRef(_, _, args) =>
       	      (delegateParams.map(pt => {
-                      if (pt == AnyClass.tpe) definitions.ObjectClass.tpe else pt})
+                      if (pt.tpe == AnyClass.tpe) definitions.ObjectClass.tpe else pt})
       	       ::: List(delegateReturn)) == args
       	    case _ => false
       	  })
diff --git a/src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala b/src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala
index 6de47e6c3d..ecd34d02b0 100644
--- a/src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala
+++ b/src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala
@@ -970,7 +970,7 @@ abstract class ICodeReader extends ClassfileParser {
 
       def checkValidIndex {
         locals.get(idx - 1) match {
-          case Some(others) if ((others find { x => x._1 == LONG || x._1 == DOUBLE}) != None) =>
+          case Some(others) if others exists (_._2.isWideType) =>
             error("Illegal index: " + idx + " points in the middle of another local")
           case _ => ()
         }
diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
index 3af22264ab..1e45ce72e7 100644
--- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
@@ -872,6 +872,11 @@ abstract class RefChecks extends InfoTransform {
         }
         val actual   = underlyingClass(args.head.tpe)
         val receiver = underlyingClass(qual.tpe)
+        def onTrees[T](f: List[Tree] => T) = f(List(qual, args.head))
+        def onSyms[T](f: List[Symbol] => T) = f(List(receiver, actual))
+
+        // @MAT normalize for consistency in error message, otherwise only part is normalized due to use of `typeSymbol'
+        def typesString = normalizeAll(qual.tpe.widen)+" and "+normalizeAll(args.head.tpe.widen)
 
         /** Symbols which limit the warnings we can issue since they may be value types */
         val isMaybeValue = Set(AnyClass, AnyRefClass, AnyValClass, ObjectClass, ComparableClass, SerializableClass)
@@ -887,25 +892,34 @@ abstract class RefChecks extends InfoTransform {
           (s == Object_==) || (s == Object_!=) || (s == Any_==) || (s == Any_!=)
         }
         // Whether the operands+operator represent a warnable combo (assuming anyrefs)
-        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 isWarnable           = isReferenceOp || (isUsingDefaultEquals && isUsingDefaultScalaOp)
+        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 possibleNumericCount = onSyms(_ filter (x => isNumeric(x) || isMaybeValue(x)) size)
+        val nullCount            = onSyms(_ filter (_ == NullClass) size)
 
         def nonSensibleWarning(what: String, alwaysEqual: Boolean) = {
           val msg = alwaysEqual == (name == nme.EQ || name == nme.eq)
           unit.warning(pos, "comparing "+what+" using `"+name.decode+"' will always yield " + msg)
         }
 
-        // @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
-        )
+        def nonSensible(pre: String, alwaysEqual: Boolean) =
+          nonSensibleWarning(pre+"values of types "+typesString, alwaysEqual)
+
+        def unrelatedTypes() =
+          unit.warning(pos, typesString + " are unrelated: should not compare equal")
 
-        if (isBoolean(receiver)) {
+        if (nullCount == 2)
+          nonSensible("", true)  // null == null
+        else if (nullCount == 1) {
+          if (onSyms(_ exists isValueClass)) // null == 5
+            nonSensible("", false)
+          else if (onTrees( _ exists isNew)) // null == new AnyRef
+            nonSensibleWarning("a fresh object", false)
+        }
+        else if (isBoolean(receiver)) {
           if (!isBoolean(actual) && !isMaybeValue(actual))    // true == 5
             nonSensible("", false)
         }
@@ -925,13 +939,20 @@ abstract class RefChecks extends InfoTransform {
             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
+          else if (receiver.isFinal && !(receiver isSubClass actual)) {  // object X, Y; X == Y
             if (isEitherNullable)
               nonSensible("non-null ", false)
             else
               nonSensible("", false)
           }
         }
+        // Warning on types without a parental relationship.  Uncovers a lot of
+        // bugs, but not always right to warn.
+        if (false) {
+          if (nullCount == 0 && possibleNumericCount < 2 && !(receiver isSubClass actual) && !(actual isSubClass receiver))
+            unrelatedTypes()
+        }
+
       case _ =>
     }
 
diff --git a/test/files/neg/checksensible.check b/test/files/neg/checksensible.check
index 67a76dd7be..9977739196 100644
--- a/test/files/neg/checksensible.check
+++ b/test/files/neg/checksensible.check
@@ -73,19 +73,19 @@ checksensible.scala:59: error: comparing values of types Int and Null using `=='
 checksensible.scala:64: error: comparing values of types Bip and Bop using `==' will always yield false
   (x1 == x2)
       ^
-checksensible.scala:75: error: comparing values of types EqEqRefTest.this.C3 and EqEqRefTest.this.Z1 using `==' will always yield false
+checksensible.scala:74: error: comparing values of types EqEqRefTest.this.C3 and EqEqRefTest.this.Z1 using `==' will always yield false
   c3 == z1
      ^
-checksensible.scala:76: error: comparing values of types EqEqRefTest.this.Z1 and EqEqRefTest.this.C3 using `==' will always yield false
+checksensible.scala:75: error: comparing values of types EqEqRefTest.this.Z1 and EqEqRefTest.this.C3 using `==' will always yield false
   z1 == c3
      ^
-checksensible.scala:77: error: comparing values of types EqEqRefTest.this.Z1 and EqEqRefTest.this.C3 using `!=' will always yield true
+checksensible.scala:76: error: comparing values of types EqEqRefTest.this.Z1 and EqEqRefTest.this.C3 using `!=' will always yield true
   z1 != c3
      ^
-checksensible.scala:78: error: comparing values of types EqEqRefTest.this.C3 and java.lang.String using `!=' will always yield true
+checksensible.scala:77: error: comparing values of types EqEqRefTest.this.C3 and java.lang.String using `!=' will always yield true
   c3 != "abc"
      ^
-checksensible.scala:87: error: comparing values of types Unit and Int using `!=' will always yield true
+checksensible.scala:88: 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 e68a6ce9c7..4872df4dfb 100644
--- a/test/files/neg/checksensible.scala
+++ b/test/files/neg/checksensible.scala
@@ -29,7 +29,7 @@ class EqEqValTest {
   1 == "abc"
   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
+  "abc" == 1        // doesn't generally warn since String defines an equals method, but can chatty warn
 
   new AnyRef == 1
   1 == new AnyRef                 // doesn't warn because it could be...
@@ -70,12 +70,13 @@ class EqEqRefTest {
   val z1 = new Z1
   val c3 = new C3
 
-  // all but c3 != z1 should warn
-  c3 != z1
+  // these should always warn
   c3 == z1
   z1 == c3
   z1 != c3
   c3 != "abc"
+  // this should warn when feeling chatty
+  c3 != z1
 
   // non-warners
   (null: AnyRef) == (null: AnyRef)