Skip to content

Commit 6bd8a0f

Browse files
authored
Mix in the productPrefix hash statically in case class hashCode (#22865)
Since 2.13, case class `hashCode` mixes in the hash code of the `productPrefix` string. This is inconsistent with the `equals` method, subclasses of case classes that override `productPrefix` compare equal but have a different `hashCode` (scala/bug#13033). This commit changes `hashCode` to mix in the `productPrefix.hashCode` statically instead of invoking `productPrefix` at runtime. For case classes without primitive fields, the synthetic `hashCode` invokes `ScalaRunTime._hashCode`, which mixes in the result of `productPrefix`. To fix that, the synthetic hashCode is changed to invoke `MurmurHash3.productHash` directly and mix in the name to the seed statically. Scala 3 forward port of scala/scala#11023
2 parents a8cf11d + d2fc358 commit 6bd8a0f

File tree

4 files changed

+105
-35
lines changed

4 files changed

+105
-35
lines changed

Diff for: compiler/src/dotty/tools/dotc/core/Definitions.scala

+3
Original file line numberDiff line numberDiff line change
@@ -505,6 +505,9 @@ class Definitions {
505505
@tu lazy val ScalaRuntime_toArray: Symbol = ScalaRuntimeModule.requiredMethod(nme.toArray)
506506
@tu lazy val ScalaRuntime_toObjectArray: Symbol = ScalaRuntimeModule.requiredMethod(nme.toObjectArray)
507507

508+
@tu lazy val MurmurHash3Module: Symbol = requiredModule("scala.util.hashing.MurmurHash3")
509+
@tu lazy val MurmurHash3_productHash = MurmurHash3Module.info.member(termName("productHash")).suchThat(_.info.firstParamTypes.size == 3).symbol
510+
508511
@tu lazy val BoxesRunTimeModule: Symbol = requiredModule("scala.runtime.BoxesRunTime")
509512
@tu lazy val BoxesRunTimeModule_externalEquals: Symbol = BoxesRunTimeModule.info.decl(nme.equals_).suchThat(toDenot(_).info.firstParamTypes.size == 2).symbol
510513
@tu lazy val ScalaStaticsModule: Symbol = requiredModule("scala.runtime.Statics")

Diff for: compiler/src/dotty/tools/dotc/transform/SyntheticMembers.scala

+32-34
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import util.Property
1515
import util.Spans.Span
1616
import config.Printers.derive
1717
import NullOpsDecorator.*
18+
import scala.runtime.Statics
1819

1920
object SyntheticMembers {
2021

@@ -101,6 +102,7 @@ class SyntheticMembers(thisPhase: DenotTransformer) {
101102
val isSimpleEnumValue = isEnumValue && !clazz.owner.isAllOf(EnumCase)
102103
val isJavaEnumValue = isEnumValue && clazz.derivesFrom(defn.JavaEnumClass)
103104
val isNonJavaEnumValue = isEnumValue && !isJavaEnumValue
105+
val ownName = clazz.name.stripModuleClassSuffix.toString
104106

105107
val symbolsToSynthesize: List[Symbol] =
106108
if clazz.is(Case) then
@@ -124,8 +126,7 @@ class SyntheticMembers(thisPhase: DenotTransformer) {
124126
def forwardToRuntime(vrefs: List[Tree]): Tree =
125127
ref(defn.runtimeMethodRef("_" + sym.name.toString)).appliedToTermArgs(This(clazz) :: vrefs)
126128

127-
def ownName: Tree =
128-
Literal(Constant(clazz.name.stripModuleClassSuffix.toString))
129+
def ownNameLit: Tree = Literal(Constant(ownName))
129130

130131
def nameRef: Tree =
131132
if isJavaEnumValue then
@@ -152,7 +153,7 @@ class SyntheticMembers(thisPhase: DenotTransformer) {
152153
Literal(Constant(candidate.get))
153154

154155
def toStringBody(vrefss: List[List[Tree]]): Tree =
155-
if (clazz.is(ModuleClass)) ownName
156+
if (clazz.is(ModuleClass)) ownNameLit
156157
else if (isNonJavaEnumValue) identifierRef
157158
else forwardToRuntime(vrefss.head)
158159

@@ -165,7 +166,7 @@ class SyntheticMembers(thisPhase: DenotTransformer) {
165166
case nme.ordinal => ordinalRef
166167
case nme.productArity => Literal(Constant(accessors.length))
167168
case nme.productPrefix if isEnumValue => nameRef
168-
case nme.productPrefix => ownName
169+
case nme.productPrefix => ownNameLit
169170
case nme.productElement =>
170171
if ctx.settings.YcompileScala2Library.value then productElementBodyForScala2Compat(accessors.length, vrefss.head.head)
171172
else productElementBody(accessors.length, vrefss.head.head)
@@ -335,39 +336,36 @@ class SyntheticMembers(thisPhase: DenotTransformer) {
335336
ref(accessors.head).select(nme.hashCode_).ensureApplied
336337
}
337338

338-
/** The class
339-
*
340-
* ```
341-
* case object C
342-
* ```
343-
*
344-
* gets the `hashCode` method:
345-
*
346-
* ```
347-
* def hashCode: Int = "C".hashCode // constant folded
348-
* ```
349-
*
350-
* The class
351-
*
352-
* ```
353-
* case class C(x: T, y: U)
354-
* ```
355-
*
356-
* if none of `T` or `U` are primitive types, gets the `hashCode` method:
357-
*
358-
* ```
359-
* def hashCode: Int = ScalaRunTime._hashCode(this)
360-
* ```
361-
*
362-
* else if either `T` or `U` are primitive, gets the `hashCode` method implemented by [[caseHashCodeBody]]
339+
/**
340+
* A `case object C` or a `case class C()` without parameters gets the `hashCode` method
341+
* ```
342+
* def hashCode: Int = "C".hashCode // constant folded
343+
* ```
344+
*
345+
* Otherwise, if none of the parameters are primitive types:
346+
* ```
347+
* def hashCode: Int = MurmurHash3.productHash(
348+
* this,
349+
* Statics.mix(0xcafebabe, "C".hashCode), // constant folded
350+
* ignorePrefix = true)
351+
* ```
352+
*
353+
* The implementation used to invoke `ScalaRunTime._hashCode`, but that implementation mixes in the result
354+
* of `productPrefix`, which causes scala/bug#13033. By setting `ignorePrefix = true` and mixing in the case
355+
* name into the seed, the bug can be fixed and the generated code works with the unchanged Scala library.
356+
*
357+
* For case classes with primitive paramters, see [[caseHashCodeBody]].
363358
*/
364359
def chooseHashcode(using Context) =
365-
if (clazz.is(ModuleClass))
366-
Literal(Constant(clazz.name.stripModuleClassSuffix.toString.hashCode))
360+
if (accessors.isEmpty) Literal(Constant(ownName.hashCode))
367361
else if (accessors.exists(_.info.finalResultType.classSymbol.isPrimitiveValueClass))
368362
caseHashCodeBody
369363
else
370-
ref(defn.ScalaRuntime__hashCode).appliedTo(This(clazz))
364+
ref(defn.MurmurHash3Module).select(defn.MurmurHash3_productHash).appliedTo(
365+
This(clazz),
366+
Literal(Constant(Statics.mix(0xcafebabe, ownName.hashCode))),
367+
Literal(Constant(true))
368+
)
371369

372370
/** The class
373371
*
@@ -380,7 +378,7 @@ class SyntheticMembers(thisPhase: DenotTransformer) {
380378
* ```
381379
* def hashCode: Int = {
382380
* <synthetic> var acc: Int = 0xcafebabe
383-
* acc = Statics.mix(acc, this.productPrefix.hashCode());
381+
* acc = Statics.mix(acc, "C".hashCode);
384382
* acc = Statics.mix(acc, x);
385383
* acc = Statics.mix(acc, Statics.this.anyHash(y));
386384
* Statics.finalizeHash(acc, 2)
@@ -391,7 +389,7 @@ class SyntheticMembers(thisPhase: DenotTransformer) {
391389
val acc = newSymbol(ctx.owner, nme.acc, Mutable | Synthetic, defn.IntType, coord = ctx.owner.span)
392390
val accDef = ValDef(acc, Literal(Constant(0xcafebabe)))
393391
val mixPrefix = Assign(ref(acc),
394-
ref(defn.staticsMethod("mix")).appliedTo(ref(acc), This(clazz).select(defn.Product_productPrefix).select(defn.Any_hashCode).appliedToNone))
392+
ref(defn.staticsMethod("mix")).appliedTo(ref(acc), Literal(Constant(ownName.hashCode))))
395393
val mixes = for (accessor <- accessors) yield
396394
Assign(ref(acc), ref(defn.staticsMethod("mix")).appliedTo(ref(acc), hashImpl(accessor)))
397395
val finish = ref(defn.staticsMethod("finalizeHash")).appliedTo(ref(acc), Literal(Constant(accessors.size)))

Diff for: tests/run-macros/tasty-extractors-2.check

+1-1
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ TypeRef(ThisType(TypeRef(NoPrefix(), "scala")), "Unit")
4949
Inlined(None, Nil, Block(List(ClassDef("Foo", DefDef("<init>", List(TermParamClause(Nil)), Inferred(), None), List(Apply(Select(New(Inferred()), "<init>"), Nil)), None, List(DefDef("a", Nil, Inferred(), Some(Literal(IntConstant(0))))))), Literal(UnitConstant())))
5050
TypeRef(ThisType(TypeRef(NoPrefix(), "scala")), "Unit")
5151

52-
Inlined(None, Nil, Block(List(ClassDef("Foo", DefDef("<init>", List(TermParamClause(Nil)), Inferred(), None), List(Apply(Select(New(Inferred()), "<init>"), Nil), TypeSelect(Select(Ident("_root_"), "scala"), "Product"), TypeSelect(Select(Ident("_root_"), "scala"), "Serializable")), None, List(DefDef("hashCode", List(TermParamClause(Nil)), Inferred(), Some(Apply(Ident("_hashCode"), List(This(Some("Foo")))))), DefDef("equals", List(TermParamClause(List(ValDef("x$0", Inferred(), None)))), Inferred(), Some(Apply(Select(Apply(Select(This(Some("Foo")), "eq"), List(TypeApply(Select(Ident("x$0"), "$asInstanceOf$"), List(Inferred())))), "||"), List(Match(Ident("x$0"), List(CaseDef(Bind("x$0", Typed(Wildcard(), Inferred())), None, Apply(Select(Literal(BooleanConstant(true)), "&&"), List(Apply(Select(Ident("x$0"), "canEqual"), List(This(Some("Foo"))))))), CaseDef(Wildcard(), None, Literal(BooleanConstant(false))))))))), DefDef("toString", List(TermParamClause(Nil)), Inferred(), Some(Apply(Ident("_toString"), List(This(Some("Foo")))))), DefDef("canEqual", List(TermParamClause(List(ValDef("that", Inferred(), None)))), Inferred(), Some(TypeApply(Select(Ident("that"), "isInstanceOf"), List(Inferred())))), DefDef("productArity", Nil, Inferred(), Some(Literal(IntConstant(0)))), DefDef("productPrefix", Nil, Inferred(), Some(Literal(StringConstant("Foo")))), DefDef("productElement", List(TermParamClause(List(ValDef("n", Inferred(), None)))), Inferred(), Some(Match(Ident("n"), List(CaseDef(Wildcard(), None, Apply(Ident("throw"), List(Apply(Select(New(Inferred()), "<init>"), List(Apply(Select(Ident("n"), "toString"), Nil)))))))))), DefDef("productElementName", List(TermParamClause(List(ValDef("n", Inferred(), None)))), Inferred(), Some(Match(Ident("n"), List(CaseDef(Wildcard(), None, Apply(Ident("throw"), List(Apply(Select(New(Inferred()), "<init>"), List(Apply(Select(Ident("n"), "toString"), Nil)))))))))), DefDef("copy", List(TermParamClause(Nil)), Inferred(), Some(Apply(Select(New(Inferred()), "<init>"), Nil))))), ValDef("Foo", TypeIdent("Foo$"), Some(Apply(Select(New(TypeIdent("Foo$")), "<init>"), Nil))), ClassDef("Foo$", DefDef("<init>", List(TermParamClause(Nil)), Inferred(), None), List(Apply(Select(New(Inferred()), "<init>"), Nil), Inferred()), Some(ValDef("_", Singleton(Ident("Foo")), None)), List(DefDef("apply", List(TermParamClause(Nil)), Inferred(), Some(Apply(Select(New(Inferred()), "<init>"), Nil))), DefDef("unapply", List(TermParamClause(List(ValDef("x$1", Inferred(), None)))), Singleton(Literal(BooleanConstant(true))), Some(Literal(BooleanConstant(true)))), DefDef("toString", Nil, Inferred(), Some(Literal(StringConstant("Foo")))), TypeDef("MirroredMonoType", TypeBoundsTree(Inferred(), Inferred())), DefDef("fromProduct", List(TermParamClause(List(ValDef("x$0", Inferred(), None)))), Inferred(), Some(Block(Nil, Apply(Select(New(Inferred()), "<init>"), Nil))))))), Literal(UnitConstant())))
52+
Inlined(None, Nil, Block(List(ClassDef("Foo", DefDef("<init>", List(TermParamClause(Nil)), Inferred(), None), List(Apply(Select(New(Inferred()), "<init>"), Nil), TypeSelect(Select(Ident("_root_"), "scala"), "Product"), TypeSelect(Select(Ident("_root_"), "scala"), "Serializable")), None, List(DefDef("hashCode", List(TermParamClause(Nil)), Inferred(), Some(Literal(IntConstant(70822)))), DefDef("equals", List(TermParamClause(List(ValDef("x$0", Inferred(), None)))), Inferred(), Some(Apply(Select(Apply(Select(This(Some("Foo")), "eq"), List(TypeApply(Select(Ident("x$0"), "$asInstanceOf$"), List(Inferred())))), "||"), List(Match(Ident("x$0"), List(CaseDef(Bind("x$0", Typed(Wildcard(), Inferred())), None, Apply(Select(Literal(BooleanConstant(true)), "&&"), List(Apply(Select(Ident("x$0"), "canEqual"), List(This(Some("Foo"))))))), CaseDef(Wildcard(), None, Literal(BooleanConstant(false))))))))), DefDef("toString", List(TermParamClause(Nil)), Inferred(), Some(Apply(Ident("_toString"), List(This(Some("Foo")))))), DefDef("canEqual", List(TermParamClause(List(ValDef("that", Inferred(), None)))), Inferred(), Some(TypeApply(Select(Ident("that"), "isInstanceOf"), List(Inferred())))), DefDef("productArity", Nil, Inferred(), Some(Literal(IntConstant(0)))), DefDef("productPrefix", Nil, Inferred(), Some(Literal(StringConstant("Foo")))), DefDef("productElement", List(TermParamClause(List(ValDef("n", Inferred(), None)))), Inferred(), Some(Match(Ident("n"), List(CaseDef(Wildcard(), None, Apply(Ident("throw"), List(Apply(Select(New(Inferred()), "<init>"), List(Apply(Select(Ident("n"), "toString"), Nil)))))))))), DefDef("productElementName", List(TermParamClause(List(ValDef("n", Inferred(), None)))), Inferred(), Some(Match(Ident("n"), List(CaseDef(Wildcard(), None, Apply(Ident("throw"), List(Apply(Select(New(Inferred()), "<init>"), List(Apply(Select(Ident("n"), "toString"), Nil)))))))))), DefDef("copy", List(TermParamClause(Nil)), Inferred(), Some(Apply(Select(New(Inferred()), "<init>"), Nil))))), ValDef("Foo", TypeIdent("Foo$"), Some(Apply(Select(New(TypeIdent("Foo$")), "<init>"), Nil))), ClassDef("Foo$", DefDef("<init>", List(TermParamClause(Nil)), Inferred(), None), List(Apply(Select(New(Inferred()), "<init>"), Nil), Inferred()), Some(ValDef("_", Singleton(Ident("Foo")), None)), List(DefDef("apply", List(TermParamClause(Nil)), Inferred(), Some(Apply(Select(New(Inferred()), "<init>"), Nil))), DefDef("unapply", List(TermParamClause(List(ValDef("x$1", Inferred(), None)))), Singleton(Literal(BooleanConstant(true))), Some(Literal(BooleanConstant(true)))), DefDef("toString", Nil, Inferred(), Some(Literal(StringConstant("Foo")))), TypeDef("MirroredMonoType", TypeBoundsTree(Inferred(), Inferred())), DefDef("fromProduct", List(TermParamClause(List(ValDef("x$0", Inferred(), None)))), Inferred(), Some(Block(Nil, Apply(Select(New(Inferred()), "<init>"), Nil))))))), Literal(UnitConstant())))
5353
TypeRef(ThisType(TypeRef(NoPrefix(), "scala")), "Unit")
5454

5555
Inlined(None, Nil, Block(List(ClassDef("Foo1", DefDef("<init>", List(TermParamClause(List(ValDef("a", TypeIdent("Int"), None)))), Inferred(), None), List(Apply(Select(New(Inferred()), "<init>"), Nil)), None, List(ValDef("a", Inferred(), None)))), Literal(UnitConstant())))

Diff for: tests/run/t13033.scala

+69
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
// This method will be in the 2.13.17 standard library. Until this test declares a copy of it.
2+
// import scala.util.hashing.MurmurHash3.caseClassHash
3+
def caseClassHash(x: Product, caseClassName: String = null): Int =
4+
import scala.runtime.Statics._
5+
val arr = x.productArity
6+
val aye = (if (caseClassName != null) caseClassName else x.productPrefix).hashCode
7+
if (arr == 0) aye
8+
else {
9+
var h = 0xcafebabe
10+
h = mix(h, aye)
11+
var i = 0
12+
while (i < arr) {
13+
h = mix(h, x.productElement(i).##)
14+
i += 1
15+
}
16+
finalizeHash(h, arr)
17+
}
18+
19+
20+
case class C1(a: Int)
21+
class C2(a: Int) extends C1(a) { override def productPrefix = "C2" }
22+
class C3(a: Int) extends C1(a) { override def productPrefix = "C3" }
23+
case class C4(a: Int) { override def productPrefix = "Sea4" }
24+
case class C5()
25+
case object C6
26+
case object C6b { override def productPrefix = "Sea6b" }
27+
case class C7(s: String) // hashCode forwards to ScalaRunTime._hashCode if there are no primitives
28+
class C8(s: String) extends C7(s) { override def productPrefix = "C8" }
29+
30+
case class VCC(x: Int) extends AnyVal
31+
32+
object Test extends App {
33+
val c1 = C1(1)
34+
val c2 = new C2(1)
35+
val c3 = new C3(1)
36+
assert(c1 == c2)
37+
assert(c2 == c1)
38+
assert(c2 == c3)
39+
assert(c1.hashCode == c2.hashCode)
40+
assert(c2.hashCode == c3.hashCode)
41+
42+
assert(c1.hashCode == caseClassHash(c1))
43+
// `caseClassHash` mixes in the `productPrefix.hashCode`, while `hashCode` mixes in the case class name statically
44+
assert(c2.hashCode != caseClassHash(c2))
45+
assert(c2.hashCode == caseClassHash(c2, c1.productPrefix))
46+
47+
val c4 = C4(1)
48+
assert(c4.hashCode != caseClassHash(c4))
49+
assert(c4.hashCode == caseClassHash(c4, "C4"))
50+
51+
assert((1, 2).hashCode == caseClassHash(1 -> 2))
52+
assert(("", "").hashCode == caseClassHash("" -> ""))
53+
54+
assert(C5().hashCode == caseClassHash(C5()))
55+
assert(C6.hashCode == caseClassHash(C6))
56+
assert(C6b.hashCode == caseClassHash(C6b, "C6b"))
57+
58+
val c7 = C7("hi")
59+
val c8 = new C8("hi")
60+
assert(c7.hashCode == caseClassHash(c7))
61+
assert(c7 == c8)
62+
assert(c7.hashCode == c8.hashCode)
63+
assert(c8.hashCode != caseClassHash(c8))
64+
assert(c8.hashCode == caseClassHash(c8, "C7"))
65+
66+
67+
assert(VCC(1).canEqual(VCC(1)))
68+
assert(!VCC(1).canEqual(1))
69+
}

0 commit comments

Comments
 (0)