Permalink
Browse files

SI-5779: Wrong warning message (comparing Number)

Massage the predicates for readability.

Thanks to Adriaan for the push.  As an outsider, one tries
to tread lightly by minimizing the number of chars changed
in the source, which is the wrong optimization.

This code remains as it was (not amenable to extension).
This commit also deletes the flag I'd introduced to say that an
exhaustive check had already been done.  For the record, I
renamed it to something less silly before I zapped it.
  • Loading branch information...
som-snytt committed May 22, 2012
1 parent e3b924e commit 17ed9675570487fc51b2b7be3efa7b4b99785a6c
Showing with 13 additions and 9 deletions.
  1. +13 −9 src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
@@ -1084,17 +1084,22 @@ abstract class RefChecks extends InfoTransform with reflect.internal.transform.R
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 isJavaNumeric(s: Symbol) = s isSubClass JavaNumberClass
def isSpecial(s: Symbol) = isPrimitiveValueClass(unboxedValueClass(s)) || (s isSubClass ScalaNumberClass) || isMaybeValue(s)
def isNumeric(s: Symbol) = isNumericValueClass(unboxedValueClass(s)) || isAnyNumber(s)
def isScalaNumber(s: Symbol) = s isSubClass ScalaNumberClass
// test is behind a platform guard
def isJavaNumber(s: Symbol) = !forMSIL && (s isSubClass JavaNumberClass)
// includes java.lang.Number if appropriate [SI-5779]
def isAnyNumber(s: Symbol) = isScalaNumber(s) || isJavaNumber(s)
def isMaybeAnyValue(s: Symbol) = isPrimitiveValueClass(unboxedValueClass(s)) || isMaybeValue(s)
// used to short-circuit unrelatedTypes check if both sides are special
def isSpecial(s: Symbol) = isMaybeAnyValue(s) || isAnyNumber(s)
// unused
def possibleNumericCount = onSyms(_ filter (x => isNumeric(x) || isMaybeValue(x)) size)
val nullCount = onSyms(_ filter (_ == NullClass) size)
var sensicality = false
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)
sensicality = true
}
def nonSensible(pre: String, alwaysEqual: Boolean) =
nonSensibleWarning(pre+"values of types "+typesString, alwaysEqual)
@@ -1126,11 +1131,10 @@ abstract class RefChecks extends InfoTransform with reflect.internal.transform.R
else if (!isUnit(actual) && !isMaybeValue(actual)) // () == "abc"
nonSensiblyNeq()
}
else if (isNumeric(receiver) || (!forMSIL && isJavaNumeric(receiver))) {
if (!isNumeric(actual) && !forMSIL && !isJavaNumeric(actual))
else if (isNumeric(receiver)) {
if (!isNumeric(actual) && !forMSIL)
if (isUnit(actual) || isBoolean(actual) || !isMaybeValue(actual)) // 5 == "abc"
nonSensiblyNeq()
sensicality = true
}
else if (isWarnable && !isCaseEquals) {
if (isNew(qual)) // new X == y
@@ -1146,7 +1150,7 @@ abstract class RefChecks extends InfoTransform with reflect.internal.transform.R
}
// possibleNumericCount is insufficient or this will warn on e.g. Boolean == j.l.Boolean
if (!sensicality && isWarnable && nullCount == 0 && !(isSpecial(receiver) && isSpecial(actual))) {
if (isWarnable && nullCount == 0 && !(isSpecial(receiver) && isSpecial(actual))) {
// better to have lubbed and lost
def warnIfLubless(): Unit = {
val common = global.lub(List(actual.tpe, receiver.tpe))

0 comments on commit 17ed967

Please sign in to comment.