diff options
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) |