Skip to content
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

value classes allow equals and hashCode overrides via inherited trait #6534

Closed
scabug opened this issue Oct 17, 2012 · 7 comments
Closed

value classes allow equals and hashCode overrides via inherited trait #6534

scabug opened this issue Oct 17, 2012 · 7 comments
Assignees
Milestone

Comments

@scabug
Copy link

scabug commented Oct 17, 2012

This is true of 2.10.0-RC1, which is not listed as a valid version.

scala> class Foo(val x: Int) extends AnyVal { override def hashCode = 1 }
<console>:7: error: redefinition of hashCode method. See SIP-15, criterion 4. is not allowed in value class
       class Foo(val x: Int) extends AnyVal { override def hashCode = 1 }

But if an override is inherited, we get two different wrong behaviors: in the case of equals, no error is issued and it is ignored. In the case of hashCode, no error is issued and it is used.

scala> trait Foo extends Any { override def equals(x: Any) = false }
defined trait Foo

scala> class Bippy(val x: Int) extends AnyVal with Foo { }
defined class Bippy

scala> new Bippy(5) == new Bippy(5)
res0: Boolean = true

scala> trait Ding extends Any { override def hashCode = -1 }
defined trait Ding

scala> class Bip(val x: Int) extends AnyVal with Ding
defined class Bip

scala> new Bip(5) hashCode
warning: there were 1 feature warnings; re-run with -feature for details
res1: Int = -1
@scabug
Copy link
Author

scabug commented Oct 17, 2012

Imported From: https://issues.scala-lang.org/browse/SI-6534?orig=1
Reporter: @paulp
Other Milestones: 2.10.0

@scabug
Copy link
Author

scabug commented Oct 17, 2012

@paulp said:
Marked as blocker.

@scabug
Copy link
Author

scabug commented Oct 23, 2012

@adriaanm said:
Resolution: mark value classes as experimental. This was the last nail in the coffin. The alternative would be to disable universal traits outside of the scala package. Yet another irregularity.

@scabug
Copy link
Author

scabug commented Oct 24, 2012

@odersky said:
Sorry, but I think this is an over-reaction. Has anybody attempted to fix this? It seems not as the ticket is still unassigned.

This is a fairly easy diagnosis and fix: If you look at the generated code after post-erasure you see that an equals on a value class is replaced by the equals on the underlying object. The same optimization is not done for hashcode.

There are two possible fixes:

  1. Do the == optimization only if equals is not overridden anywhere

  2. Demand that universal traits may not implement equals and hashCode.

I would vote for 2), which should be really easy to implement.

Thanks

@scabug
Copy link
Author

scabug commented Oct 24, 2012

@adriaanm said (edited on Oct 24, 2012 5:06:25 PM UTC):
We always judge language extensions by their impact on the complexity of the spec.

Can we make a list of all the ways in which we had to restrict value classes because of current implementation restrictions?

I agree it's not a decision we should take lightly, but I feel the burden is on the extension to prove it's worth it.

Also, implicit classes that directly extend AnyVal should be supported, but maybe anything beyond directly extending AnyVal should be considered experimental for now.
I'll start a thread on scala-internals to discuss this in detail.

@scabug
Copy link
Author

scabug commented Oct 24, 2012

@paulp said:
I implemented option 2) yesterday, and emailed scala-internals about it.

https://github.com/paulp/scala/commits/issue/6534

The problem, which is what had me asking about moving equals earlier, is that it means a number of things no longer compile. So that's the rest of the changes.

@scabug
Copy link
Author

scabug commented Oct 31, 2012

@paulp said:
The latest is: scala/scala#1526

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants