eclipse-archived / ceylon

The Ceylon compiler, language module, and command line tools
http://ceylon-lang.org
Apache License 2.0
395 stars 62 forks source link

what to do about Comparable #7411

Closed gavinking closed 5 years ago

gavinking commented 5 years ago

After #7395, I changed Comparable to be invariant.

@jvasileff points out that a bunch of our functions including sort() and max() don't work as well when you have things that indirectly implement comparable. It is possible to rescue all these functions by adding a new type parameter C and having parameters of type, for example, Element & Comparable<C> or {Element & Comparable<C>*}.

However, that's pretty messy and I don't see it as acceptable. There's two reasonable solutions:

  1. Make Comparable contravariant and remove its self type.
  2. Taking advantage of #7408, define Comparable as follows:

    shared interface Comparable of Other given Other satisfies Comparable { ... }

    Then functions like sort() and max() just use the same type constraint given Other satisfies Comparable<in Other>.

I like solution 2, assuming that #7408 doesn't run into some terrible snag.

jvasileff commented 5 years ago

Is option 2 still sound re coverage?

On Sep 14, 2018, at 3:10 PM, Gavin King notifications@github.com wrote:

After #7395, I changed Comparable to be invariant.

@jvasileff points out that a bunch of our functions including sort() and max() don't work as well when you have things that indirectly implement comparable. It is possible to rescue all these functions by adding a new type parameter C and having parameters of type, for example, Element & Comparable or {Element & Comparable*}.

However, that's pretty messy and I don't see it as acceptable. There's two reasonable solutions:

Make Comparable contravariant and remove its self type.

Taking advantage of #7408, define Comparable as follows:

shared interface Comparable of Other given Other satisfies Comparable { ... } Then functions like sort() and max() just use the same type constraint given Other satisfies Comparable.

I like solution 2, assuming that #7408 doesn't run into some terrible snag.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

gavinking commented 5 years ago

Is option 2 still sound re coverage?

I believe so, yes, since T is not considered to cover Comparable<in T>.

gavinking commented 5 years ago

I have pushed my proposed solution #2, so that people can try it out.

gavinking commented 5 years ago

After sleeping on this, I think the cleanest solution is just to:

  1. Make Comparable contravariant and remove its self type.

So I've done that. Thanks to @jvasileff for pointing out the problems with making Comparable invariant.

jvasileff commented 5 years ago

Option 1 seems reasonable to me given that it makes things easy for developers (they don't have to remember the in annotation).

Although with 1, I don't see a justification for given Other satisfies Comparable<Other>. I mean, if we can have class C() satisfies Comparable<Integer>, how is it relevant to C's satisfaction that Integer is comparable to itself?

gavinking commented 5 years ago

@jvasileff pfff, TBH I don't even remember. But I guess the idea is that a total order is a binary relation on a given set. It's not a binary relation between members of one set and members of a second distinct set.

What would it mean to say that a string is smaller than an integer?

jvasileff commented 5 years ago

What would it mean to say that a string is smaller than an integer?

Yeah, I think that sort of thing just creates problems. But the constraint doesn't prevent that.

dlkw commented 5 years ago

I think it's rather strange if you are allowed to write a <= b, but b>a gives a compile error. This is quite unexpected for a total order.

dlkw commented 5 years ago

It's been a long day. Should have written a < b vs. b > a.

jvasileff commented 5 years ago

@dlkw Yeah, types like that shouldn't be written. But I think the original question (option 1 vs. 2) is about whether it's important for the type system to prevent someone from creating such types, verse the convenience of having Comparable be contravariant.

gavinking commented 5 years ago

Right.