Skip to content

Compare productPrefix in synthetic case class canEqual #10859

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

lrytz
Copy link
Member

@lrytz lrytz commented Sep 10, 2024

Since 2.13, case class hashCode mixes in the hash code of the
productPrefix string. The synthetic equals method has fallen
out of sync in that regard, so two instances can be equal but have
different hash codes.

This commit changes the synthetic canEqual method to also compare
the productPrefix of the two instances.

Fixes scala/bug#13033

@scala-jenkins scala-jenkins added this to the 2.13.16 milestone Sep 10, 2024
@lrytz lrytz requested a review from retronym September 10, 2024 13:45
@lrytz
Copy link
Member Author

lrytz commented Sep 10, 2024

(should forward-port/report to Scala 3 once approved)

@lrytz lrytz force-pushed the t13033 branch 2 times, most recently from cd7b384 to 7315f51 Compare September 11, 2024 11:54
@retronym
Copy link
Member

👍🏽 for the canEqual approach.

Since 2.13, case class `hashCode` mixes in the hash code of the
`productPrefix` string. The synthetic `equals` method has fallen
out of sync in that regard, so two instances can be equal but have
different hash codes.

This commit changes the synthetic `canEqual` method to also compare
the `productPrefix` of the two instances.
@lrytz lrytz changed the title Add deprecatedOverriding to synthetic case class productPrefix Compare productPrefix in synthetic case class canEqual Sep 12, 2024
@SethTisue
Copy link
Member

@lrytz do we need to release-note this, might it cause mysterious breakage?

@lrytz
Copy link
Member Author

lrytz commented Sep 16, 2024

Yes, it changes case class equality.

scala> case class C(x: Int)
class C

scala> class D extends C(1) { override def productPrefix = "D" }
class D

scala> C(1) == new D
val res0: Boolean = false

That is true in 2.13.15.

@SethTisue SethTisue added the release-notes worth highlighting in next release notes label Sep 17, 2024
@SethTisue
Copy link
Member

we'll need a comparable PR on the Scala 3 side, yes?

@lrytz
Copy link
Member Author

lrytz commented Sep 17, 2024

Yes. I can submit it there to probe what the 3 folks think about the semantic change.

@SethTisue
Copy link
Member

@lrytz where does this stand, after the discussion on scala/scala3#21606 ?

@lrytz lrytz modified the milestones: 2.13.16, 2.13.17 Nov 13, 2024
@lrytz
Copy link
Member Author

lrytz commented Mar 20, 2025

@sjrd points out that this fix goes the wrong way around; what we want is to define semantics of equals and then adjust the hashCode to be compatible. This PR does the opposite.

So here's my attempt to fix hashCode instead: #11023

@lrytz lrytz closed this Mar 20, 2025
@SethTisue SethTisue removed this from the 2.13.17 milestone Mar 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes worth highlighting in next release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Overriding productPrefix breaks case class hash code
4 participants