Make case class hashCode take class into account #2159
Conversation
Previously, elements of the following classes had the same hash code:
case class A()
case class B()
Now they are distinguished.
|
What's the usecase for that? |
| * acc = Statics.mix(acc, x); | ||
| * acc = Statics.mix(acc, Statics.this.anyHash(y)); | ||
| * Statics.finalizeHash(acc, 2) | ||
| * } | ||
| */ | ||
| def caseHashCodeBody(implicit ctx: Context): Tree = { | ||
| val acc = ctx.newSymbol(ctx.owner, "acc".toTermName, Mutable | Synthetic, defn.IntType, coord = ctx.owner.pos) | ||
| val accDef = ValDef(acc, Literal(Constant(0xcafebabe))) | ||
| val accDef = ValDef(acc, This(clazz).select(defn.Product_productPrefix).select(defn.Any_hashCode).ensureApplied) |
There was a problem hiding this comment.
Given that this code generates also productPrefix itself, why not inline constant ownName here instead?
There was a problem hiding this comment.
Also, given that hashcodes on strings are fixed by the spec, we can actually compute the runtime value and inline it: Literal(ownName.toString.hashcode).
There was a problem hiding this comment.
@DarkDimius Neat! I'll change it.
Less hash collisions for unrelated things. |
Also, update check file.
|
That failure looks like a deadlock. /cc @felixmulder |
| * acc = Statics.mix(acc, x); | ||
| * acc = Statics.mix(acc, Statics.this.anyHash(y)); | ||
| * Statics.finalizeHash(acc, 2) | ||
| * } | ||
| */ | ||
| def caseHashCodeBody(implicit ctx: Context): Tree = { | ||
| val acc = ctx.newSymbol(ctx.owner, "acc".toTermName, Mutable | Synthetic, defn.IntType, coord = ctx.owner.pos) | ||
| val accDef = ValDef(acc, Literal(Constant(0xcafebabe))) | ||
| val accDef = ValDef(acc, Literal(Constant(clazz.fullName.toString.hashCode))) |
There was a problem hiding this comment.
scalac uses productPrefix (which is user definable), rather than the class name.
What is the definition of fullName for non-member classes?
There was a problem hiding this comment.
scalac uses productPrefix (which is user definable), rather than the class name.
What's the motivation behind this?
Product prefix isn't required to even return the same value, so one can break hash-code by returning different values there.
There was a problem hiding this comment.
Neither are the case accessors (they could be vars).
I guess a motivation would be to have more stable hash codes for a local case class so that it wasn't volatile wrt fresh name suffixes. Perhaps this implementation acheives the same by inlining fullName, rather than calling this.getClass.toString).
I'm just pointing out the deviation.
There was a problem hiding this comment.
AFAIK, The full name here won't include any fresh name suffices. They will be only added after Flatten. Two local classes with the same name will get the same constant.
There was a problem hiding this comment.
Neither are the case accessors (they could be vars).
I see now, though, that a mutable productPrefix would be worse that a var accessor, as equals does not check that this.productPrefix == that.productPrefix.
Previously, elements of the following classes had the same hash code:
Now they are distinguished.