Disallow equals and hashCode in universal traits

[smarter]

equals and hashCode should always be synthesised in value classes so that new Meter(3) == new Meter(4) can be rewritten as 3 == 4, so allowing equals and hashCode in universal traits doesn't make much sense.

[smarter]

@DarkDimius: feel free to explain the rewriting scheme you have in mind that could handle having custom equals and hashCode in value classes. After thinking about it I don't think that we can do something that will work properly because of virtual dispatch.

[DarkDimius]

Let me illustrate:

trait A extends Any {
  def equals(other: Object) = false
}

becomes

trait A extends Any {
  def equals(other: Object) = false
}

trait A$extension {
  def equals$extension[@specialized T](this: T)(other: Object) = false
}

Than if we have

class B(val s: Int) extends AnyVal with A

we rewrite as we do currently: virtual methods forward to static implementations:

class B(val s: Int) extends AnyVal with A {
 def equals(other: Object) =  B.equals$extension(s)(other)
}

object B extends A$extension

We remove previous rules that we had

new B(e) == new B(f) ⇒ e == f
new B(e) != new B(f) ⇒ e != f

Instead we handle them the same way how we rewrite calls to normal methods(Step 2 of SIP)

new B(e) == new B(f)

becomes simply

B.equals$extension(1)(2)

[smarter]

Yes, that part makes sense, but things start getting difficult when you have virtual dispatch:

trait A extends Any {
  def equals(other: Object) = foo
  def foo: Boolean = false
}

trait B extends A {
  override def foo: Boolean = true
}


class Foo(val x: Int) extends AnyVal with A
class Bar(val x: Int) extends AnyVal with B

Can you show how your rewriting would work in that case?

[DarkDimius]

Those universal traits create additional

trait A$extension {
  def equals$extension[@specialized T](this: T)(other: Object) = foo$extension(this)
  def foo$extension[@specialized T](this: T) = false
}

trait B$extension extends A$extension{
  def foo$extension[@specialized T](this: T) = true
}

Note that rhs of equals$extension was simply trasformed by step2 of SIP.

Foo and Bar companion objects will simply inherit respective traits, as they do in simpler example above. And you'll get the expected behavior.

Everything else remains the same

[smarter]

OK, what about:

trait A extends Any {
  def isMeter: Boolean = this.isInstanceOf[Meter]
}

[smarter]

I think my previous example can be dealt with by having a box$ method in the extension traits, assuming we can override specialized methods:

trait A$extension {
  def box$[@specialized T]($this: T): Any = sys.error("Should always be overriden")
  def isMeter$extension[@specialized T]($this: T): Boolean = box$($this).isInstanceOf[Meter]
}
object Meter extends A$extension {
  // Only the box specialized for Int is implemented, because the
  // underlying type of Meter is Int
  def box$$specializedForInt($this: Int): Meter = new Meter($this)
}

[DarkDimius]

@smarter, we do have box method already: #515

@odersky so, in the end, do you want this implemented? I assumed that we have already reached agreement that this(universal traits without for value classes without boxing, including equals and hashcode) is part of our roadmap on the Dotty meeting.

[smarter]

@DarkDimius : Not in every universal trait, no, and there are many details left to fill to see if this works.

[smarter]

But I agree that it's something we should experiment with.

[odersky]

No, I think for now we should restrict universal traits not to have equals
and hashCode. It's not at all urgent to lift the restriction, and it might
in fact be preferable to keep it.

On Wed, Jun 3, 2015 at 5:05 PM, Dmitry Petrashko [email protected]
wrote:

@smarter https://github.com/smarter, we do have box method already: #515
#515

@odersky https://github.com/odersky so, in the end, do you want this
implemented? I assumed that we have already reached agreement that
this(universal traits without for value classes without boxing, including
equals and hashcode) is part of our roadmap on the Dotty meeting.

—
Reply to this email directly or view it on GitHub
#629 (comment).

Martin Odersky
EPFL

[smarter]

So, I'm getting more and more convinced that we can avoid boxing when calling universal trait methods, but unfortunately it also seems that this won't help us avoid special casing equals and hashCode. The problem is that the signature of == is:

def == (that: Any): Boolean

So even if we can rewrite new Meter(3) == new Meter(4) to call an extension method, it will lead to Meter.==$extension(3, new Meter(4), so the right-hand side is still boxed.

[DarkDimius]

so the right-hand side is still boxed.

If user did define custom equals we need to box, unless we use inlining, or similar approaches. Simply because user could want to have Meter(1000) == Kilometer(1).

If user didn't define a custom equals we can create an overload, that doesn't, that will do the same as current scheme, as Meter argument will be erased to unboxed Int, without signature clashes before or after erasure.

[smarter]

Well, so far we have avoided overloading extension methods, I don't know if we want to break this now. Maybe it's better to wait until we decide what we want to do with == in general (replace it by a typeclass, ...)

[DarkDimius]

Well, so far we have avoided overloading extension methods

That's not true:

scala> class A(val a: Int) extends AnyVal{ def s(a: Int) = a; def s(b: Double) = b}
defined class A

the only problem with overloads in value classes is that changing their order is not a binary compatible change.

[DarkDimius]

and that's the main reason why extension methods have consecutive numbers in the name:

    public final int s$extension1(int $this, int a) {
        return a;
    }

    public final double s$extension0(int $this, double b) {
        return b;
    }

[smarter]

I mean, in your example you will get two extension methods s$extension0 and s$extension1, and the one that will be selected is the one whose signature matches the signature of the value class method. With == the signature is always ==(val other: Any), so you need some custom rule to choose the overload, or you need to do "real" overloading resolution which is what extension methods have avoided so far.

[DarkDimius]

@smarter with any method, including == you have overload resolution. Given it, you are free to define a custom method with such name that does not box. And if static types are good enough, it will be used:

scala> class A(val s: Int) { def ==(a: Int) = s == a }
defined class A

scala> new A(1) == 1
res0: Boolean = true

scala> new A(1) == "s"
res2: Boolean = false

No special rules are required in this proposal. The whole motivation behind this proposal is to make scheme more regular, without introducing new rules.

[smarter]

OK, but then that means that adding equals or hashCode to a value class or a universal trait is a binary incompatible change, are we OK with that?

[DarkDimius]

onсe again, it's easy to make this be binary compatible: always generate a forwarder. Even if user did define a custom equals, we make an equals, that simply forwards. This is not safe for arbitary classes, but is safe for final ones.

[smarter]

EDIT: Ah OK, ignore that, I now see that with a forwarder that boxes we should be able to preserve binary compatiblity.

The situation I'm thinking of is:

• V has no equals and hashCode members
• In another compilation unit we do new V(1) == new V(2) which gets translated to something like V.==$extension(1, 2)
• We add an equals member to V, so ==$extension(x: Int, y: Int) is no longer generated.

At least this result in a runtime crash and not a silent error, so it might not be that bad.

[DarkDimius]

Yes, and this situation is fixed by always generating the ==$extension method.

[dwijnand]

@smarter reopen if I'm wrong but I think we can close this as we can't change this now that 3.0 is out.