Comparable.compare should be generic

[nex3]

The Comparable.compare static function currently has no generic annotations, which makes it unusable in strong mode. Comparable.compare/*<T>*/(Comparable/*<T>*/ a, Comparable/*<T>*/ b) is probably the way to go.

[jmesserly]

IIRC @munificent and I looked into this one, the way it's used (as the default comparator for ordered Maps/Sets) it probably needs to be less typed. Something along the lines of: Comparable.compare(Object a, Object b) and use a dynamic test in the body, throwing if neither a nor b implements Comparable. FWIW, that is the C# implementation of a similar construct.

Also CC @leafpetersen

[nex3]

That would actually work better in my use-case.

[lrhn]

I'm not even sure the suggested generification would work correctly. A Comparable<T> has the method int compareTo(T other). To be correct, the compare function would be Comparable.compare<T>(Comparable<T> a, T b) => a.compareTo(b);.

The the generification of the existing signature (taking Comparable as second parameter) will work when T is Object, dynamic or Comparable.

How about Comparable.compare<T extends Comparable<T>>(T a, T b) => a.compareTo(b); ? (Using Object is probably simpler).

[munificent]

How about Comparable.compare<T extends Comparable<T>>(T a, T b) => a.compareTo(b); ?

Yup, I think that's the right answer.

[kevmoo]

Everyone subscribed: what's the story on this? Still an issue? (Just curious...)

[jmesserly]

it appears to still be an issue: https://api.dartlang.org/stable/1.24.2/dart-core/Comparable/compare.html

[lrhn]

The problem with going for

int compare<T extends Comparable<T>>(T a, T b) => a.compareTo(b);

as the declaration is that it's a recursive type constraint which can make inference hard. An untyped tear-off won't work at all: var compare = Comparable.compare; will now assign the generic function to the variable, and it is-not a Comparator. You'd have to write Comparator<int> compare = Comparable.compare; to enforce the specialization (we don't even allow var compare = Comparable.compare<num>; syntactically yet).

On the other hand, the recursively typed declaration will make compare<num> actually be a Comparator<num>, which is something we want. That's also why

int compare<T>(Comparable<T> a, T b) => a.compareTo(b);

isn't a good alternative - then compare<num> is not a Comparator<num> (not exactly at least – int Function(Comparable<num>, num) is assignable to int Function(num, num), but if if you do var c = (a, b)=>Comparable<int>(a, b); then the c variable won't get a type that accepts a Comparator<num>).

There are existing uses of the plain Comparable.compare that will be wrong. In splay_tree.dart we write:

Object compare = Comparable.compare;

That would not instantiate the function, so compare here will be the generic function and not a Comparator.

I'm not actually sure whether an instantiation of a generic static function is a compile-time constant expression, so if there are any uses of Comparable.compare as a parameter default value, that will potentially also break.

And then there is the problem that inference might pick a type different from dynamic (if it succeeds at all and doesn't reject the program as untypable).

// Here we have to be certain that it infers T as `num`, because `int` is! Comparable<int>.
// I don't know it it does, and I don't expect users to be able to predict it.
print(Comparable.compare(2, 3));

Making the change is simple. It will not affect Dart 1 at all (the syntax, it does nothing!), so maybe the analyzer or DDC team can test how the change would affect strong mode?

I've made a CL: https://codereview.chromium.org/2959443002 You can try it out and see what breaks when compiling with DDC - I only fund the one test in corelib_strong and language_strong that needed change.

[srawlins]

This affects the removal of dynamic as bottom, #29630, as well. Right now, to assign Comparable.compare to a Comparator, you'd have to type a comparator as Comparator<Null> or have a type variable T extends Comparable in the context. DartPad.