diff options
| -rw-r--r-- | src/compiler/scala/tools/nsc/typechecker/RefChecks.scala | 32 | ||||
| -rw-r--r-- | test/files/neg/t5663-badwarneq.check | 34 | ||||
| -rw-r--r-- | test/files/neg/t5663-badwarneq.scala | 18 |
3 files changed, 67 insertions, 17 deletions
diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala index 9a9ca6a654..7f8aeceeec 100644 --- a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala +++ b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala @@ -1102,12 +1102,15 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans val s = fn.symbol (s == Object_==) || (s == Object_!=) || (s == Any_==) || (s == Any_!=) } + def haveSubclassRelationship = (actual isSubClass receiver) || (receiver isSubClass actual) + // Whether the operands+operator represent a warnable combo (assuming anyrefs) // Looking for comparisons performed with ==/!= in combination with either an // equals method inherited from Object or a case class synthetic equals (for // which we know the logic.) def isWarnable = isReferenceOp || (isUsingDefaultScalaOp && isUsingWarnableEquals) def isEitherNullable = (NullClass.tpe <:< receiver.info) || (NullClass.tpe <:< actual.info) + def isEitherValueClass = actual.isDerivedValueClass || receiver.isDerivedValueClass def isBoolean(s: Symbol) = unboxedValueClass(s) == BooleanClass def isUnit(s: Symbol) = unboxedValueClass(s) == UnitClass def isNumeric(s: Symbol) = isNumericValueClass(unboxedValueClass(s)) || isAnyNumber(s) @@ -1122,6 +1125,12 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans // unused def possibleNumericCount = onSyms(_ filter (x => isNumeric(x) || isMaybeValue(x)) size) val nullCount = onSyms(_ filter (_ == NullClass) size) + def isNonsenseValueClassCompare = ( + !haveSubclassRelationship + && isUsingDefaultScalaOp + && isEitherValueClass + && !isCaseEquals + ) def nonSensibleWarning(what: String, alwaysEqual: Boolean) = { val msg = alwaysEqual == (name == nme.EQ || name == nme.eq) @@ -1133,10 +1142,13 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans def nonSensiblyNeq() = nonSensible("", false) def nonSensiblyNew() = nonSensibleWarning("a fresh object", false) + def unrelatedMsg = name match { + case nme.EQ | nme.eq => "never compare equal" + case _ => "always compare unequal" + } def unrelatedTypes() = { - val msg = if (name == nme.EQ || name == nme.eq) - "never compare equal" else "always compare unequal" - unit.warning(pos, typesString + " are unrelated: they will most likely " + msg) + val weaselWord = if (isEitherValueClass) "" else " most likely" + unit.warning(pos, s"$typesString are unrelated: they will$weaselWord $unrelatedMsg") } if (nullCount == 2) // null == null @@ -1175,15 +1187,19 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans } } + // warn if one but not the other is a derived value class + // this is especially important to enable transitioning from + // regular to value classes without silent failures. + if (isNonsenseValueClassCompare) + unrelatedTypes() // possibleNumericCount is insufficient or this will warn on e.g. Boolean == j.l.Boolean - if (isWarnable && nullCount == 0 && !(isSpecial(receiver) && isSpecial(actual))) { + else 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)) if (ObjectClass.tpe <:< common) unrelatedTypes() } - def eitherSubclasses = (actual isSubClass receiver) || (receiver isSubClass actual) // warn if actual has a case parent that is not same as receiver's; // if actual is not a case, then warn if no common supertype, as below if (isCaseEquals) { @@ -1196,14 +1212,12 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans //else // if a class, it must be super to thisCase (and receiver) since not <: thisCase if (!actual.isTrait && !(receiver isSubClass actual)) nonSensiblyNeq() - else if (!eitherSubclasses) warnIfLubless() + else if (!haveSubclassRelationship) warnIfLubless() case _ => } } - else if (actual isSubClass receiver) () - else if (receiver isSubClass actual) () // warn only if they have no common supertype below Object - else { + else if (!haveSubclassRelationship) { warnIfLubless() } } diff --git a/test/files/neg/t5663-badwarneq.check b/test/files/neg/t5663-badwarneq.check index 00c2234e9d..242be8de68 100644 --- a/test/files/neg/t5663-badwarneq.check +++ b/test/files/neg/t5663-badwarneq.check @@ -1,22 +1,40 @@ -t5663-badwarneq.scala:42: error: comparing case class values of types Some[Int] and None.type using `==' will always yield false +t5663-badwarneq.scala:47: error: comparing case class values of types Some[Int] and None.type using `==' will always yield false println(new Some(1) == None) // Should complain on type, was: spuriously complains on fresh object ^ -t5663-badwarneq.scala:43: error: comparing case class values of types Some[Int] and Thing using `==' will always yield false +t5663-badwarneq.scala:48: error: comparing case class values of types Some[Int] and Thing using `==' will always yield false println(Some(1) == new Thing(1)) // Should complain on type, was: spuriously complains on fresh object ^ -t5663-badwarneq.scala:51: error: ThingOne and Thingy are unrelated: they will most likely never compare equal +t5663-badwarneq.scala:56: error: ThingOne and Thingy are unrelated: they will most likely never compare equal println(t1 == t2) // true, but apparently unrelated, a compromise warning ^ -t5663-badwarneq.scala:52: error: ThingThree and Thingy are unrelated: they will most likely never compare equal +t5663-badwarneq.scala:57: error: ThingThree and Thingy are unrelated: they will most likely never compare equal println(t4 == t2) // true, complains because ThingThree is final and Thingy not a subclass, stronger claim than unrelated ^ -t5663-badwarneq.scala:55: error: comparing case class values of types ThingTwo and Some[Int] using `==' will always yield false +t5663-badwarneq.scala:60: error: comparing case class values of types ThingTwo and Some[Int] using `==' will always yield false println(t3 == Some(1)) // false, warn on different cases ^ -t5663-badwarneq.scala:56: error: comparing values of types ThingOne and Cousin using `==' will always yield false +t5663-badwarneq.scala:61: error: comparing values of types ThingOne and Cousin using `==' will always yield false println(t1 == c) // should warn ^ -t5663-badwarneq.scala:64: error: comparing case class values of types Simple and SimpleSibling.type using `==' will always yield false +t5663-badwarneq.scala:69: error: comparing case class values of types Simple and SimpleSibling.type using `==' will always yield false println(new Simple() == SimpleSibling) // like Some(1) == None, but needn't be final case ^ -7 errors found +t5663-badwarneq.scala:72: error: ValueClass1 and Int are unrelated: they will never compare equal + println(new ValueClass1(5) == 5) // bad + ^ +t5663-badwarneq.scala:74: error: comparing values of types Int and ValueClass1 using `==' will always yield false + println(5 == new ValueClass1(5)) // bad + ^ +t5663-badwarneq.scala:78: error: ValueClass2[String] and String are unrelated: they will never compare equal + println(new ValueClass2("abc") == "abc") // bad + ^ +t5663-badwarneq.scala:79: error: ValueClass2[Int] and ValueClass1 are unrelated: they will never compare equal + println(new ValueClass2(5) == new ValueClass1(5)) // bad - different value classes + ^ +t5663-badwarneq.scala:81: error: comparing values of types ValueClass3 and ValueClass2[Int] using `==' will always yield false + println(ValueClass3(5) == new ValueClass2(5)) // bad + ^ +t5663-badwarneq.scala:82: error: comparing values of types ValueClass3 and Int using `==' will always yield false + println(ValueClass3(5) == 5) // bad + ^ +13 errors found diff --git a/test/files/neg/t5663-badwarneq.scala b/test/files/neg/t5663-badwarneq.scala index 56ec389c03..3c0376914e 100644 --- a/test/files/neg/t5663-badwarneq.scala +++ b/test/files/neg/t5663-badwarneq.scala @@ -17,6 +17,11 @@ class SimpleParent case class Simple() extends SimpleParent case object SimpleSibling extends SimpleParent +// value classes +final class ValueClass1(val value: Int) extends AnyVal +final class ValueClass2[T](val value: T) extends AnyVal +final case class ValueClass3(val value: Int) extends AnyVal + /* It's not possible to run partest without -deprecation. * Since detecting the warnings requires a neg test with * -Xfatal-warnings, and deprecation terminates the compile, @@ -63,6 +68,19 @@ object Test { println(new Simple() == SimpleSibling) // like Some(1) == None, but needn't be final case + println(new ValueClass1(5) == new ValueClass1(5)) // ok + println(new ValueClass1(5) == 5) // bad + println(new ValueClass1(5) == (5: Any)) // ok, have to let it through + println(5 == new ValueClass1(5)) // bad + println((5: Any) == new ValueClass1(5) == (5: Any)) // ok + + println(new ValueClass2("abc") == new ValueClass2("abc")) // ok + println(new ValueClass2("abc") == "abc") // bad + println(new ValueClass2(5) == new ValueClass1(5)) // bad - different value classes + println(ValueClass3(5) == new ValueClass3(5)) // ok + println(ValueClass3(5) == new ValueClass2(5)) // bad + println(ValueClass3(5) == 5) // bad + /* val mine = new MyThing val some = new SomeThing |