diff options
| -rw-r--r-- | src/compiler/scala/tools/nsc/backend/JavaPlatform.scala | 8 | ||||
| -rw-r--r-- | src/compiler/scala/tools/nsc/symtab/Definitions.scala | 8 | ||||
| -rw-r--r-- | src/compiler/scala/tools/nsc/typechecker/RefChecks.scala | 59 | ||||
| -rw-r--r-- | test/files/neg/checksensible.check | 42 | ||||
| -rw-r--r-- | test/files/neg/checksensible.scala | 8 |
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 |