Skip to content

Commit

Permalink
SI-5779: Wrong warning message (comparing Number)
Browse files Browse the repository at this point in the history
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 17ed967
Showing 1 changed file with 13 additions and 9 deletions.
22 changes: 13 additions & 9 deletions src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
Expand Up @@ -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 isEitherNullable = (NullClass.tpe <:< receiver.info) || (NullClass.tpe <:< actual.info)
def isBoolean(s: Symbol) = unboxedValueClass(s) == BooleanClass def isBoolean(s: Symbol) = unboxedValueClass(s) == BooleanClass
def isUnit(s: Symbol) = unboxedValueClass(s) == UnitClass def isUnit(s: Symbol) = unboxedValueClass(s) == UnitClass
def isNumeric(s: Symbol) = isNumericValueClass(unboxedValueClass(s)) || (s isSubClass ScalaNumberClass) def isNumeric(s: Symbol) = isNumericValueClass(unboxedValueClass(s)) || isAnyNumber(s)
def isJavaNumeric(s: Symbol) = s isSubClass JavaNumberClass def isScalaNumber(s: Symbol) = s isSubClass ScalaNumberClass
def isSpecial(s: Symbol) = isPrimitiveValueClass(unboxedValueClass(s)) || (s isSubClass ScalaNumberClass) || isMaybeValue(s) // 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) def possibleNumericCount = onSyms(_ filter (x => isNumeric(x) || isMaybeValue(x)) size)
val nullCount = onSyms(_ filter (_ == NullClass) size) val nullCount = onSyms(_ filter (_ == NullClass) size)


var sensicality = false
def nonSensibleWarning(what: String, alwaysEqual: Boolean) = { def nonSensibleWarning(what: String, alwaysEqual: Boolean) = {
val msg = alwaysEqual == (name == nme.EQ || name == nme.eq) val msg = alwaysEqual == (name == nme.EQ || name == nme.eq)
unit.warning(pos, "comparing "+what+" using `"+name.decode+"' will always yield " + msg) unit.warning(pos, "comparing "+what+" using `"+name.decode+"' will always yield " + msg)
sensicality = true
} }
def nonSensible(pre: String, alwaysEqual: Boolean) = def nonSensible(pre: String, alwaysEqual: Boolean) =
nonSensibleWarning(pre+"values of types "+typesString, alwaysEqual) nonSensibleWarning(pre+"values of types "+typesString, alwaysEqual)
Expand Down Expand Up @@ -1126,11 +1131,10 @@ abstract class RefChecks extends InfoTransform with reflect.internal.transform.R
else if (!isUnit(actual) && !isMaybeValue(actual)) // () == "abc" else if (!isUnit(actual) && !isMaybeValue(actual)) // () == "abc"
nonSensiblyNeq() nonSensiblyNeq()
} }
else if (isNumeric(receiver) || (!forMSIL && isJavaNumeric(receiver))) { else if (isNumeric(receiver)) {
if (!isNumeric(actual) && !forMSIL && !isJavaNumeric(actual)) if (!isNumeric(actual) && !forMSIL)
if (isUnit(actual) || isBoolean(actual) || !isMaybeValue(actual)) // 5 == "abc" if (isUnit(actual) || isBoolean(actual) || !isMaybeValue(actual)) // 5 == "abc"
nonSensiblyNeq() nonSensiblyNeq()
sensicality = true
} }
else if (isWarnable && !isCaseEquals) { else if (isWarnable && !isCaseEquals) {
if (isNew(qual)) // new X == y if (isNew(qual)) // new X == y
Expand All @@ -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 // 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 // better to have lubbed and lost
def warnIfLubless(): Unit = { def warnIfLubless(): Unit = {
val common = global.lub(List(actual.tpe, receiver.tpe)) val common = global.lub(List(actual.tpe, receiver.tpe))
Expand Down

0 comments on commit 17ed967

Please sign in to comment.