From 8a4653637a2b693cdcc730a93e17badaac14d56e Mon Sep 17 00:00:00 2001 From: Sébastien Doeraene Date: Thu, 21 Apr 2016 15:06:25 +0200 Subject: Bring Statics.doubleHash in sync with BoxesRunTime.hashFromDouble. The two algorithms were different, and could result in different hash codes for some values, namely, valid long values that were not also valid int values. The other two functions `longHash` and `floatHash` are rewritten to keep a common style with `doubleHash`, but their algorithm does not change. --- src/library/scala/runtime/Statics.java | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) (limited to 'src') diff --git a/src/library/scala/runtime/Statics.java b/src/library/scala/runtime/Statics.java index 485511ecbb..93fe681856 100644 --- a/src/library/scala/runtime/Statics.java +++ b/src/library/scala/runtime/Statics.java @@ -36,10 +36,11 @@ public final class Statics { } public static int longHash(long lv) { - if ((int)lv == lv) - return (int)lv; - else - return (int)(lv ^ (lv >>> 32)); + int iv = (int)lv; + if (iv == lv) + return iv; + + return java.lang.Long.hashCode(lv); } public static int doubleHash(double dv) { @@ -47,16 +48,15 @@ public final class Statics { if (iv == dv) return iv; - float fv = (float)dv; - if (fv == dv) - return java.lang.Float.floatToIntBits(fv); - long lv = (long)dv; if (lv == dv) - return (int)lv; + return java.lang.Long.hashCode(lv); + + float fv = (float)dv; + if (fv == dv) + return java.lang.Float.hashCode(fv); - lv = Double.doubleToLongBits(dv); - return (int)(lv ^ (lv >>> 32)); + return java.lang.Double.hashCode(dv); } public static int floatHash(float fv) { @@ -66,9 +66,9 @@ public final class Statics { long lv = (long)fv; if (lv == fv) - return (int)(lv^(lv>>>32)); + return java.lang.Long.hashCode(lv); - return java.lang.Float.floatToIntBits(fv); + return java.lang.Float.hashCode(fv); } public static int anyHash(Object x) { -- cgit v1.2.3 From cce701650143d13c8b292bffd590bf7c8eb532d7 Mon Sep 17 00:00:00 2001 From: Sébastien Doeraene Date: Wed, 20 Apr 2016 15:12:14 +0200 Subject: Remove the duplicate implem of hash codes for numbers. Previously, there were two separate implementations of hash code for boxed number classes: * One in Statics, used by the codegen of case class methods. * One in ScalaRunTime + BoxesRunTime, used by everything else. This commit removes the variant implemented in ScalaRunTime + BoxesRunTime, and always uses Statics instead. We use Statics because the one from ScalaRunTime causes an unnecessary module load. The entry point ScalaRunTime.hash() is kept, as deprecated, for bootstrapping reasons. --- .../tools/nsc/backend/jvm/BCodeBodyBuilder.scala | 1 - .../scala/tools/nsc/backend/jvm/CoreBTypes.scala | 2 +- .../scala/tools/nsc/transform/Erasure.scala | 22 ++++---- .../tools/nsc/typechecker/SyntheticMethods.scala | 15 +++--- src/library/scala/reflect/ClassTag.scala | 2 +- src/library/scala/runtime/BoxesRunTime.java | 60 ---------------------- src/library/scala/runtime/ScalaRunTime.scala | 8 ++- src/library/scala/runtime/Statics.java | 28 ++++++++++ src/reflect/scala/reflect/internal/StdNames.scala | 5 +- test/files/run/equality.scala | 2 +- test/files/run/hashCodeBoxesRunTime.scala | 29 ----------- test/files/run/hashCodeStatics.scala | 28 ++++++++++ test/junit/scala/runtime/ScalaRunTimeTest.scala | 8 --- 13 files changed, 84 insertions(+), 126 deletions(-) delete mode 100644 test/files/run/hashCodeBoxesRunTime.scala create mode 100644 test/files/run/hashCodeStatics.scala (limited to 'src') diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala index 6d3d458324..bc09d78f42 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala @@ -1119,7 +1119,6 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder { /* Generate the scala ## method. */ def genScalaHash(tree: Tree, applyPos: Position): BType = { - genLoadModule(ScalaRunTimeModule) // TODO why load ScalaRunTimeModule if ## has InvokeStyle of Static(false) ? genLoad(tree, ObjectRef) genCallMethod(hashMethodSym, InvokeStyle.Static, applyPos) INT diff --git a/src/compiler/scala/tools/nsc/backend/jvm/CoreBTypes.scala b/src/compiler/scala/tools/nsc/backend/jvm/CoreBTypes.scala index 4d03f9851e..0b53ea2fb1 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/CoreBTypes.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/CoreBTypes.scala @@ -251,7 +251,7 @@ class CoreBTypes[BTFS <: BTypesFromSymbols[_ <: Global]](val bTypes: BTFS) { ) } - lazy val hashMethodSym: Symbol = getMember(ScalaRunTimeModule, nme.hash_) + lazy val hashMethodSym: Symbol = getMember(RuntimeStaticsModule, nme.anyHash) // TODO @lry avoiding going through through missingHook for every line in the REPL: https://github.com/scala/scala/commit/8d962ed4ddd310cc784121c426a2e3f56a112540 lazy val AndroidParcelableInterface : Symbol = getClassIfDefined("android.os.Parcelable") diff --git a/src/compiler/scala/tools/nsc/transform/Erasure.scala b/src/compiler/scala/tools/nsc/transform/Erasure.scala index ac794201a4..0301e06c87 100644 --- a/src/compiler/scala/tools/nsc/transform/Erasure.scala +++ b/src/compiler/scala/tools/nsc/transform/Erasure.scala @@ -1013,24 +1013,20 @@ abstract class Erasure extends AddInterfaces // erasure the ScalaRunTime.hash overload goes from Unit => Int to BoxedUnit => Int. // This must be because some earlier transformation is being skipped on ##, but so // far I don't know what. For null we now define null.## == 0. + def staticsCall(methodName: TermName): Tree = { + val newTree = gen.mkMethodCall(RuntimeStaticsModule, methodName, qual :: Nil) + global.typer.typed(newTree) + } + qual.tpe.typeSymbol match { case UnitClass | NullClass => LIT(0) case IntClass => qual case s @ (ShortClass | ByteClass | CharClass) => numericConversion(qual, s) case BooleanClass => If(qual, LIT(true.##), LIT(false.##)) - case _ => - // Since we are past typer, we need to avoid creating trees carrying - // overloaded types. This logic is custom (and technically incomplete, - // although serviceable) for def hash. What is really needed is for - // the overloading logic presently hidden away in a few different - // places to be properly exposed so we can just call "resolveOverload" - // after typer. Until then: - val alts = ScalaRunTimeModule.info.member(nme.hash_).alternatives - def alt1 = alts find (_.info.paramTypes.head =:= qual.tpe) - def alt2 = ScalaRunTimeModule.info.member(nme.hash_) suchThat (_.info.paramTypes.head.typeSymbol == AnyClass) - val newTree = gen.mkRuntimeCall(nme.hash_, qual :: Nil) setSymbol (alt1 getOrElse alt2) - - global.typer.typed(newTree) + case LongClass => staticsCall(nme.longHash) + case FloatClass => staticsCall(nme.floatHash) + case DoubleClass => staticsCall(nme.doubleHash) + case _ => staticsCall(nme.anyHash) } } else if (isPrimitiveValueClass(qual.tpe.typeSymbol)) { // Rewrite 5.getClass to ScalaRunTime.anyValClass(5) diff --git a/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala b/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala index 243a706745..6d883aee3d 100644 --- a/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala +++ b/src/compiler/scala/tools/nsc/typechecker/SyntheticMethods.scala @@ -93,11 +93,14 @@ trait SyntheticMethods extends ast.TreeDSL { def forwardToRuntime(method: Symbol): Tree = forwardMethod(method, getMember(ScalaRunTimeModule, (method.name prepend "_")))(mkThis :: _) - def callStaticsMethod(name: String)(args: Tree*): Tree = { - val method = termMember(RuntimeStaticsModule, name) + def callStaticsMethodName(name: TermName)(args: Tree*): Tree = { + val method = RuntimeStaticsModule.info.member(name) Apply(gen.mkAttributedRef(method), args.toList) } + def callStaticsMethod(name: String)(args: Tree*): Tree = + callStaticsMethodName(newTermName(name))(args: _*) + // Any concrete member, including private def hasConcreteImpl(name: Name) = clazz.info.member(name).alternatives exists (m => !m.isDeferred) @@ -245,10 +248,10 @@ trait SyntheticMethods extends ast.TreeDSL { case BooleanClass => If(Ident(sym), Literal(Constant(1231)), Literal(Constant(1237))) case IntClass => Ident(sym) case ShortClass | ByteClass | CharClass => Select(Ident(sym), nme.toInt) - case LongClass => callStaticsMethod("longHash")(Ident(sym)) - case DoubleClass => callStaticsMethod("doubleHash")(Ident(sym)) - case FloatClass => callStaticsMethod("floatHash")(Ident(sym)) - case _ => callStaticsMethod("anyHash")(Ident(sym)) + case LongClass => callStaticsMethodName(nme.longHash)(Ident(sym)) + case DoubleClass => callStaticsMethodName(nme.doubleHash)(Ident(sym)) + case FloatClass => callStaticsMethodName(nme.floatHash)(Ident(sym)) + case _ => callStaticsMethodName(nme.anyHash)(Ident(sym)) } } diff --git a/src/library/scala/reflect/ClassTag.scala b/src/library/scala/reflect/ClassTag.scala index 7f037ce17b..1811d3a00f 100644 --- a/src/library/scala/reflect/ClassTag.scala +++ b/src/library/scala/reflect/ClassTag.scala @@ -101,7 +101,7 @@ trait ClassTag[T] extends ClassManifestDeprecatedApis[T] with Equals with Serial // case class accessories override def canEqual(x: Any) = x.isInstanceOf[ClassTag[_]] override def equals(x: Any) = x.isInstanceOf[ClassTag[_]] && this.runtimeClass == x.asInstanceOf[ClassTag[_]].runtimeClass - override def hashCode = scala.runtime.ScalaRunTime.hash(runtimeClass) + override def hashCode = runtimeClass.## override def toString = { def prettyprint(clazz: jClass[_]): String = if (clazz.isArray) s"Array[${prettyprint(clazz.getComponentType)}]" else diff --git a/src/library/scala/runtime/BoxesRunTime.java b/src/library/scala/runtime/BoxesRunTime.java index b6b512d529..6b3874fc1f 100644 --- a/src/library/scala/runtime/BoxesRunTime.java +++ b/src/library/scala/runtime/BoxesRunTime.java @@ -198,66 +198,6 @@ public final class BoxesRunTime } } - /** Hashcode algorithm is driven by the requirements imposed - * by primitive equality semantics, namely that equal objects - * have equal hashCodes. The first priority are the integral/char - * types, which already have the same hashCodes for the same - * values except for Long. So Long's hashCode is altered to - * conform to Int's for all values in Int's range. - * - * Float is problematic because it's far too small to hold - * all the Ints, so for instance Int.MaxValue.toFloat claims - * to be == to each of the largest 64 Ints. There is no way - * to preserve equals/hashCode alignment without compromising - * the hashCode distribution, so Floats are only guaranteed - * to have the same hashCode for whole Floats in the range - * Short.MinValue to Short.MaxValue (2^16 total.) - * - * Double has its hashCode altered to match the entire Int range, - * but is not guaranteed beyond that. (But could/should it be? - * The hashCode is only 32 bits so this is a more tractable - * issue than Float's, but it might be better simply to exclude it.) - * - * Note: BigInt and BigDecimal, being arbitrary precision, could - * be made consistent with all other types for the Int range, but - * as yet have not. - * - * Note: Among primitives, Float.NaN != Float.NaN, but the boxed - * versions are equal. This still needs reconciliation. - */ - public static int hashFromLong(java.lang.Long n) { - int iv = n.intValue(); - if (iv == n.longValue()) return iv; - else return n.hashCode(); - } - public static int hashFromDouble(java.lang.Double n) { - int iv = n.intValue(); - double dv = n.doubleValue(); - if (iv == dv) return iv; - - long lv = n.longValue(); - if (lv == dv) return java.lang.Long.hashCode(lv); - - float fv = n.floatValue(); - if (fv == dv) return java.lang.Float.hashCode(fv); - else return n.hashCode(); - } - public static int hashFromFloat(java.lang.Float n) { - int iv = n.intValue(); - float fv = n.floatValue(); - if (iv == fv) return iv; - - long lv = n.longValue(); - if (lv == fv) return java.lang.Long.hashCode(lv); - else return n.hashCode(); - } - public static int hashFromNumber(java.lang.Number n) { - if (n instanceof java.lang.Long) return hashFromLong((java.lang.Long)n); - else if (n instanceof java.lang.Double) return hashFromDouble((java.lang.Double)n); - else if (n instanceof java.lang.Float) return hashFromFloat((java.lang.Float)n); - else return n.hashCode(); - } - private static int unboxCharOrInt(Object arg1, int code) { if (code == CHAR) return ((java.lang.Character) arg1).charValue(); diff --git a/src/library/scala/runtime/ScalaRunTime.scala b/src/library/scala/runtime/ScalaRunTime.scala index 02f39f6f5f..b31a94576a 100644 --- a/src/library/scala/runtime/ScalaRunTime.scala +++ b/src/library/scala/runtime/ScalaRunTime.scala @@ -158,11 +158,9 @@ object ScalaRunTime { } } - /** Implementation of `##`. */ - def hash(x: Any): Int = - if (x == null) 0 - else if (x.isInstanceOf[java.lang.Number]) BoxesRunTime.hashFromNumber(x.asInstanceOf[java.lang.Number]) - else x.hashCode + /** Old implementation of `##`. */ + @deprecated("Use scala.runtime.Statics.anyHash instead.", "2.12.0") + def hash(x: Any): Int = Statics.anyHash(x.asInstanceOf[Object]) /** Given any Scala value, convert it to a String. * diff --git a/src/library/scala/runtime/Statics.java b/src/library/scala/runtime/Statics.java index 93fe681856..62390cb9d0 100644 --- a/src/library/scala/runtime/Statics.java +++ b/src/library/scala/runtime/Statics.java @@ -71,6 +71,34 @@ public final class Statics { return java.lang.Float.hashCode(fv); } + /** + * Hashcode algorithm is driven by the requirements imposed + * by primitive equality semantics, namely that equal objects + * have equal hashCodes. The first priority are the integral/char + * types, which already have the same hashCodes for the same + * values except for Long. So Long's hashCode is altered to + * conform to Int's for all values in Int's range. + * + * Float is problematic because it's far too small to hold + * all the Ints, so for instance Int.MaxValue.toFloat claims + * to be == to each of the largest 64 Ints. There is no way + * to preserve equals/hashCode alignment without compromising + * the hashCode distribution, so Floats are only guaranteed + * to have the same hashCode for whole Floats in the range + * Short.MinValue to Short.MaxValue (2^16 total.) + * + * Double has its hashCode altered to match the entire Int range, + * but is not guaranteed beyond that. (But could/should it be? + * The hashCode is only 32 bits so this is a more tractable + * issue than Float's, but it might be better simply to exclude it.) + * + * Note: BigInt and BigDecimal, being arbitrary precision, could + * be made consistent with all other types for the Int range, but + * as yet have not. + * + * Note: Among primitives, Float.NaN != Float.NaN, but the boxed + * versions are equal. This still needs reconciliation. + */ public static int anyHash(Object x) { if (x == null) return 0; diff --git a/src/reflect/scala/reflect/internal/StdNames.scala b/src/reflect/scala/reflect/internal/StdNames.scala index 6abd7f2f19..0ac72e7d8b 100644 --- a/src/reflect/scala/reflect/internal/StdNames.scala +++ b/src/reflect/scala/reflect/internal/StdNames.scala @@ -641,6 +641,7 @@ trait StdNames { val accessor: NameType = "accessor" val add_ : NameType = "add" val annotation: NameType = "annotation" + val anyHash: NameType = "anyHash" val anyValClass: NameType = "anyValClass" val apply: NameType = "apply" val applyDynamic: NameType = "applyDynamic" @@ -670,6 +671,7 @@ trait StdNames { val delayedInit: NameType = "delayedInit" val delayedInitArg: NameType = "delayedInit$body" val dollarScope: NameType = "$scope" + val doubleHash: NameType = "doubleHash" val drop: NameType = "drop" val elem: NameType = "elem" val noSelfType: NameType = "noSelfType" @@ -688,13 +690,13 @@ trait StdNames { val finalize_ : NameType = "finalize" val find_ : NameType = "find" val flatMap: NameType = "flatMap" + val floatHash: NameType = "floatHash" val foreach: NameType = "foreach" val freshTermName: NameType = "freshTermName" val freshTypeName: NameType = "freshTypeName" val get: NameType = "get" val parameterTypes: NameType = "parameterTypes" val hashCode_ : NameType = "hashCode" - val hash_ : NameType = "hash" val head : NameType = "head" val immutable: NameType = "immutable" val implicitly: NameType = "implicitly" @@ -711,6 +713,7 @@ trait StdNames { val lang: NameType = "lang" val length: NameType = "length" val lengthCompare: NameType = "lengthCompare" + val longHash: NameType = "longHash" val macroContext : NameType = "c" val main: NameType = "main" val manifestToTypeTag: NameType = "manifestToTypeTag" diff --git a/test/files/run/equality.scala b/test/files/run/equality.scala index ff59898821..2af73691d8 100644 --- a/test/files/run/equality.scala +++ b/test/files/run/equality.scala @@ -1,7 +1,7 @@ // a quickly assembled test of equality. Needs work. object Test { - import scala.runtime.ScalaRunTime.hash + def hash(x: Any): Int = x.## // forces upcast to Any def makeFromInt(x: Int) = List( x.toByte, x.toShort, x.toInt, x.toLong, x.toFloat, x.toDouble, BigInt(x), BigDecimal(x) diff --git a/test/files/run/hashCodeBoxesRunTime.scala b/test/files/run/hashCodeBoxesRunTime.scala deleted file mode 100644 index 8ad94c252a..0000000000 --- a/test/files/run/hashCodeBoxesRunTime.scala +++ /dev/null @@ -1,29 +0,0 @@ -// This only tests direct access to the methods in BoxesRunTime, -// not the whole scheme. -object Test -{ - import java.{ lang => jl } - import scala.runtime.BoxesRunTime.hashFromNumber - import scala.runtime.ScalaRunTime.{ hash => hashFromAny } - - def allSame[T](xs: List[T]) = assert(xs.distinct.size == 1, "failed: " + xs) - - def mkNumbers(x: Int): List[Number] = - List(x.toByte, x.toShort, x, x.toLong, x.toFloat, x.toDouble) - - def testLDF(x: Long) = allSame(List[Number](x, x.toDouble, x.toFloat) map hashFromNumber) - - def main(args: Array[String]): Unit = { - List(Byte.MinValue, -1, 0, 1, Byte.MaxValue) foreach { n => - val hashes = mkNumbers(n) map hashFromNumber - allSame(hashes) - if (n >= 0) { - val charCode = hashFromAny(n.toChar: Character) - assert(charCode == hashes.head) - } - } - - testLDF(Short.MaxValue.toLong) - testLDF(Short.MinValue.toLong) - } -} diff --git a/test/files/run/hashCodeStatics.scala b/test/files/run/hashCodeStatics.scala new file mode 100644 index 0000000000..bff62cce18 --- /dev/null +++ b/test/files/run/hashCodeStatics.scala @@ -0,0 +1,28 @@ +// This only tests direct access to the methods in Statics, +// not the whole scheme. +object Test +{ + import java.{ lang => jl } + import scala.runtime.Statics.anyHash + + def allSame[T](xs: List[T]) = assert(xs.distinct.size == 1, "failed: " + xs) + + def mkNumbers(x: Int): List[Number] = + List(x.toByte, x.toShort, x, x.toLong, x.toFloat, x.toDouble) + + def testLDF(x: Long) = allSame(List[Number](x, x.toDouble, x.toFloat) map anyHash) + + def main(args: Array[String]): Unit = { + List(Byte.MinValue, -1, 0, 1, Byte.MaxValue) foreach { n => + val hashes = mkNumbers(n) map anyHash + allSame(hashes) + if (n >= 0) { + val charCode = anyHash(n.toChar: Character) + assert(charCode == hashes.head) + } + } + + testLDF(Short.MaxValue.toLong) + testLDF(Short.MinValue.toLong) + } +} diff --git a/test/junit/scala/runtime/ScalaRunTimeTest.scala b/test/junit/scala/runtime/ScalaRunTimeTest.scala index 9efbdd44de..5bfb12610e 100644 --- a/test/junit/scala/runtime/ScalaRunTimeTest.scala +++ b/test/junit/scala/runtime/ScalaRunTimeTest.scala @@ -8,14 +8,6 @@ import org.junit.runners.JUnit4 /** Tests for the runtime object ScalaRunTime */ @RunWith(classOf[JUnit4]) class ScalaRunTimeTest { - @Test - def hashConsistentWithStaticsHash(): Unit = { - val problematicValue: Double = -Math.pow(2, 53) + 1 - val scalaRunTimeHash = ScalaRunTime.hash(problematicValue) - val staticsHash = Statics.anyHash(problematicValue) - assertEquals(scalaRunTimeHash, staticsHash) - } - @Test def testStingOf() { import ScalaRunTime.stringOf -- cgit v1.2.3