Scala Programming Language
  1. Scala Programming Language
  2. SI-6534

value classes allow equals and hashCode overrides via inherited trait

    Details

      Description

      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
      

        Activity

        Hide
        Paul Phillips added a comment -

        Marked as blocker.

        Show
        Paul Phillips added a comment - Marked as blocker.
        Hide
        Adriaan Moors added a comment -

        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.

        Show
        Adriaan Moors added a comment - 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.
        Hide
        Martin Odersky added a comment -

        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

        Show
        Martin Odersky added a comment - 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
        Hide
        Adriaan Moors added a comment - - edited

        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.

        Show
        Adriaan Moors added a comment - - edited 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.
        Hide
        Paul Phillips added a comment -

        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.

        Show
        Paul Phillips added a comment - 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.
        Hide
        Paul Phillips added a comment -
        Show
        Paul Phillips added a comment - The latest is: https://github.com/scala/scala/pull/1526

          People

          • Assignee:
            Paul Phillips
            Reporter:
            Paul Phillips
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development