diff options
author | Paul Phillips <paulp@improving.org> | 2010-12-01 16:25:54 +0000 |
---|---|---|
committer | Paul Phillips <paulp@improving.org> | 2010-12-01 16:25:54 +0000 |
commit | a37284fdf7e2b2917c9e894a9b93c02d1defc983 (patch) | |
tree | 78e074b4ba89ed710ae15ea3bca11fbec606a695 /src | |
parent | 8a959d80f17eff0ebcc09defff16beac21909ed5 (diff) | |
download | scala-a37284fdf7e2b2917c9e894a9b93c02d1defc983.tar.gz scala-a37284fdf7e2b2917c9e894a9b93c02d1defc983.tar.bz2 scala-a37284fdf7e2b2917c9e894a9b93c02d1defc983.zip |
More fiddling with checkSensible.
warnings. Fixed some bugs revealed by said warnings, and made some minor
changes to avoid warnings. (Technically it's not a bug to have unrelated
classes compare as equal, but it so often is a bug that it behooves us
not to do it intentionally so the warnings stand out.) Disabled the most
useful warning for the moment since it'd be wrong with some frequency.
No review.
Diffstat (limited to 'src')
4 files changed, 42 insertions, 25 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 _ => } |