Skip to content

Commit d0255bb

Browse files
authored
Merge pull request #345 from scala/backport-lts-3.3-22865
Backport "Mix in the `productPrefix` hash statically in case class `hashCode`" to 3.3 LTS
2 parents e3c4028 + 639b51a commit d0255bb

File tree

4 files changed

+102
-32
lines changed

4 files changed

+102
-32
lines changed

compiler/src/dotty/tools/dotc/core/Definitions.scala

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -496,6 +496,9 @@ class Definitions {
496496
@tu lazy val ScalaRuntime_toArray: Symbol = ScalaRuntimeModule.requiredMethod(nme.toArray)
497497
@tu lazy val ScalaRuntime_toObjectArray: Symbol = ScalaRuntimeModule.requiredMethod(nme.toObjectArray)
498498

499+
@tu lazy val MurmurHash3Module: Symbol = requiredModule("scala.util.hashing.MurmurHash3")
500+
@tu lazy val MurmurHash3_productHash = MurmurHash3Module.info.member(termName("productHash")).suchThat(_.info.firstParamTypes.size == 3).symbol
501+
499502
@tu lazy val BoxesRunTimeModule: Symbol = requiredModule("scala.runtime.BoxesRunTime")
500503
@tu lazy val BoxesRunTimeModule_externalEquals: Symbol = BoxesRunTimeModule.info.decl(nme.equals_).suchThat(toDenot(_).info.firstParamTypes.size == 2).symbol
501504
@tu lazy val ScalaStaticsModule: Symbol = requiredModule("scala.runtime.Statics")

compiler/src/dotty/tools/dotc/transform/SyntheticMembers.scala

Lines changed: 29 additions & 31 deletions
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

@@ -95,6 +96,7 @@ class SyntheticMembers(thisPhase: DenotTransformer) {
9596
val isSimpleEnumValue = isEnumValue && !clazz.owner.isAllOf(EnumCase)
9697
val isJavaEnumValue = isEnumValue && clazz.derivesFrom(defn.JavaEnumClass)
9798
val isNonJavaEnumValue = isEnumValue && !isJavaEnumValue
99+
val ownName = clazz.name.stripModuleClassSuffix.toString
98100

99101
val symbolsToSynthesize: List[Symbol] =
100102
if clazz.is(Case) then
@@ -118,8 +120,7 @@ class SyntheticMembers(thisPhase: DenotTransformer) {
118120
def forwardToRuntime(vrefs: List[Tree]): Tree =
119121
ref(defn.runtimeMethodRef("_" + sym.name.toString)).appliedToTermArgs(This(clazz) :: vrefs)
120122

121-
def ownName: Tree =
122-
Literal(Constant(clazz.name.stripModuleClassSuffix.toString))
123+
def ownNameLit: Tree = Literal(Constant(ownName))
123124

124125
def nameRef: Tree =
125126
if isJavaEnumValue then
@@ -146,7 +147,7 @@ class SyntheticMembers(thisPhase: DenotTransformer) {
146147
Literal(Constant(candidate.get))
147148

148149
def toStringBody(vrefss: List[List[Tree]]): Tree =
149-
if (clazz.is(ModuleClass)) ownName
150+
if (clazz.is(ModuleClass)) ownNameLit
150151
else if (isNonJavaEnumValue) identifierRef
151152
else forwardToRuntime(vrefss.head)
152153

@@ -159,7 +160,7 @@ class SyntheticMembers(thisPhase: DenotTransformer) {
159160
case nme.ordinal => ordinalRef
160161
case nme.productArity => Literal(Constant(accessors.length))
161162
case nme.productPrefix if isEnumValue => nameRef
162-
case nme.productPrefix => ownName
163+
case nme.productPrefix => ownNameLit
163164
case nme.productElement => productElementBody(accessors.length, vrefss.head.head)
164165
case nme.productElementName => productElementNameBody(accessors.length, vrefss.head.head)
165166
}
@@ -300,39 +301,36 @@ class SyntheticMembers(thisPhase: DenotTransformer) {
300301
ref(accessors.head).select(nme.hashCode_).ensureApplied
301302
}
302303

303-
/** The class
304-
*
305-
* ```
306-
* case object C
307-
* ```
308-
*
309-
* gets the `hashCode` method:
310-
*
311-
* ```
312-
* def hashCode: Int = "C".hashCode // constant folded
313-
* ```
314-
*
315-
* The class
316-
*
317-
* ```
318-
* case class C(x: T, y: U)
319-
* ```
304+
/**
305+
* A `case object C` or a `case class C()` without parameters gets the `hashCode` method
306+
* ```
307+
* def hashCode: Int = "C".hashCode // constant folded
308+
* ```
320309
*
321-
* if none of `T` or `U` are primitive types, gets the `hashCode` method:
310+
* Otherwise, if none of the parameters are primitive types:
311+
* ```
312+
* def hashCode: Int = MurmurHash3.productHash(
313+
* this,
314+
* Statics.mix(0xcafebabe, "C".hashCode), // constant folded
315+
* ignorePrefix = true)
316+
* ```
322317
*
323-
* ```
324-
* def hashCode: Int = ScalaRunTime._hashCode(this)
325-
* ```
318+
* The implementation used to invoke `ScalaRunTime._hashCode`, but that implementation mixes in the result
319+
* of `productPrefix`, which causes scala/bug#13033. By setting `ignorePrefix = true` and mixing in the case
320+
* name into the seed, the bug can be fixed and the generated code works with the unchanged Scala library.
326321
*
327-
* else if either `T` or `U` are primitive, gets the `hashCode` method implemented by [[caseHashCodeBody]]
322+
* For case classes with primitive paramters, see [[caseHashCodeBody]].
328323
*/
329324
def chooseHashcode(using Context) =
330-
if (clazz.is(ModuleClass))
331-
Literal(Constant(clazz.name.stripModuleClassSuffix.toString.hashCode))
325+
if (accessors.isEmpty) Literal(Constant(ownName.hashCode))
332326
else if (accessors.exists(_.info.finalResultType.classSymbol.isPrimitiveValueClass))
333327
caseHashCodeBody
334328
else
335-
ref(defn.ScalaRuntime__hashCode).appliedTo(This(clazz))
329+
ref(defn.MurmurHash3Module).select(defn.MurmurHash3_productHash).appliedTo(
330+
This(clazz),
331+
Literal(Constant(Statics.mix(0xcafebabe, ownName.hashCode))),
332+
Literal(Constant(true))
333+
)
336334

337335
/** The class
338336
*
@@ -345,7 +343,7 @@ class SyntheticMembers(thisPhase: DenotTransformer) {
345343
* ```
346344
* def hashCode: Int = {
347345
* <synthetic> var acc: Int = 0xcafebabe
348-
* acc = Statics.mix(acc, this.productPrefix.hashCode());
346+
* acc = Statics.mix(acc, "C".hashCode);
349347
* acc = Statics.mix(acc, x);
350348
* acc = Statics.mix(acc, Statics.this.anyHash(y));
351349
* Statics.finalizeHash(acc, 2)
@@ -356,7 +354,7 @@ class SyntheticMembers(thisPhase: DenotTransformer) {
356354
val acc = newSymbol(ctx.owner, nme.acc, Mutable | Synthetic, defn.IntType, coord = ctx.owner.span)
357355
val accDef = ValDef(acc, Literal(Constant(0xcafebabe)))
358356
val mixPrefix = Assign(ref(acc),
359-
ref(defn.staticsMethod("mix")).appliedTo(ref(acc), This(clazz).select(defn.Product_productPrefix).select(defn.Any_hashCode).appliedToNone))
357+
ref(defn.staticsMethod("mix")).appliedTo(ref(acc), Literal(Constant(ownName.hashCode))))
360358
val mixes = for (accessor <- accessors) yield
361359
Assign(ref(acc), ref(defn.staticsMethod("mix")).appliedTo(ref(acc), hashImpl(accessor)))
362360
val finish = ref(defn.staticsMethod("finalizeHash")).appliedTo(ref(acc), Literal(Constant(accessors.size)))

tests/run-macros/tasty-extractors-2.check

Lines changed: 1 addition & 1 deletion
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(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(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())))

tests/run/t13033.scala

Lines changed: 69 additions & 0 deletions
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)