Skip to content

int doesn't extend Comparable<int> #43763

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

Closed
znjameswu opened this issue Oct 12, 2020 · 11 comments
Closed

int doesn't extend Comparable<int> #43763

znjameswu opened this issue Oct 12, 2020 · 11 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-core type-enhancement A request for a change that isn't a bug

Comments

@znjameswu
Copy link

This is an old issue since Dart 1 and doesn't seem to have been fixed. #8741

This behavior is preventing the below API patterns from being efficiently implemented.

extension ListExtension<T> on List<T> {
  void sortBy<T2 extends Comparable<T2>>(T2 Function(T element) f) {
    this.sort((a, b) => f(a).compareTo(f(b)));
  }
}

Trying to invoke this function

[1].sortBy((x) => x);

will cause error: int doesn't extend Comparable<int>

We need to change generics parameter to num at every invocation site.

[1].sortBy<num>((x) => x);
@lrhn
Copy link
Member

lrhn commented Oct 12, 2020

The behavior is unfortunate, but not easily correctible.
Dart's int implements num which implements Comparable<num>. That means that int can't also implement Comparable<int>. A Dart 2 type can only implement one version of a generic interface.
Sometimes type inference trips over this, and it fails to recognize that it can choose T2 a num in the example given here and still work successfully. Maybe improving type inference could help, but at the risk of potentially getting worse results in other cases.

Since Comparable is actually a contravariant interface, maybe adding variance annotations can make a difference. Probably not. I think that with proper variance annotations, we can let a subclass implement a more specialized version of a covariant interface, and a more general version of a contravariant interface, but here we need a more specialized version of a contravariant interface.

@lrhn lrhn added area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-core type-enhancement A request for a change that isn't a bug labels Oct 12, 2020
@eernstg
Copy link
Member

eernstg commented Oct 12, 2020

If we do make the type argument of Comparable contravariant (that is, if it isn't too much of a breaking change),

class Comparable<in T> {...}

then int <: Comparable<num> <: Comparable<int>, so inference can succeed as [1].sortBy<int>((x) => x);, which would make the example work.

@lrhn lrhn closed this as completed Oct 12, 2020
@lrhn lrhn reopened this Oct 12, 2020
@agrapine
Copy link

Yep. This is quite annoying.

@mverver-google
Copy link

mverver-google commented Jul 9, 2021

I regularly run into this problem with Int64 from the fixnum package. This case is interesting because the workaround of explicitly specifying the type argument doesn't work.

The class hierarchy is as follows:

 abstract class IntX implements Comparable<Object> {}
 class Int32 implements IntX {}
 class Int64 implements IntX {}

IntX implements Comparable<object> because e.g. Int64.compareTo() can do a value comparison against Int64, Int32 or int, and Object is the common supertype. But this means that it's impossible to use methods like sortBy() with Int64 even when explicitly specifying the type argument, because IntX doesn't implement Comparable<IntX>, and Object doesn't implement Comparable<Object>.

If Comparable was considered contravariant, that would solve this problem.

P.S. I think this a design mistake in the fixnum package. IntX should implement Comparable<IntX>, which would still allow the method signature to be int compareTo(Object) to support comparison against integers. Maybe I'll file a separate bug about it, though it's probably impossible to fix now without breaking a bunch of existing code.

@mverver-google
Copy link

Another solution would be to allow multiple type constraints on a parameter, e.g. like this:

void sortBy<C, T2 extends C & Comparable<C>>(T2 Function(T element) f)

Currently this isn't supported by Dart, as discussed in Issue 1152.

For completeness sake, I'll point out that in Java the problem can be solved with a lower bounded wildcard:

void sortBy<T2 extends Comparable<? super T2>>(T2 Function(T element) f)

Java uses wildcards extensively instead of supporting covariant/contravariant typing. Java's system is more flexible and more sound than the Dart's (which assumes all generic types are covariant, which is nonsense of course), though it leads to more verbose type signatures. I suspect the Dart team would prefer to label types explicitly as covariant/contravariant like Erik suggested above, even if it's less flexible.

@eseidel
Copy link
Contributor

eseidel commented Oct 10, 2023

I think an easy solution may just be to mention this gotcha in the docs of sortedBy or wherever else this is easy to trip.

Here is a dartpad example:

import 'package:collection/collection.dart';

class Foo {
  Foo(this.value);
  
  int value;
  
  String toString() => value.toString();
}

void main() {
  final a = <Foo>[Foo(2), Foo(5), Foo(3)];

  // final b = a.sortedBy((f) => f.value);
  // Will fail with:
  //   lib/main.dart:13:15:
  // Error: Inferred type argument 'int' doesn't conform to the bound 'Comparable<K>' of the type variable 'K' on 'IterableExtension|sortedBy'.
  //  - 'Comparable' is from 'dart:core'.
  //   final b = a.sortedBy((f) => f.value);
  //               ^
  // /app/local_pub_cache/hosted/pub.dev/collection-1.17.2/lib/src/iterable_extensions.dart:65:20:
  // Info: This is the type variable whose bound isn't conformed to.
  //   List<T> sortedBy<K extends Comparable<K>>(K Function(T element) keyOf) {
  //                    ^
  // Error: Compilation failed.
  
  // final b = a.sortedBy<int>((f) => f.value);
  // Will fail with:
  //   lib/main.dart:25:18:
  // Error: Type argument 'int' doesn't conform to the bound 'Comparable<K>' of the type variable 'K' on 'IterableExtension|sortedBy'.
  //  - 'Comparable' is from 'dart:core'.
  //      final b = a.sortedBy<int>((f) => f.value);
  //                  ^
  // /app/local_pub_cache/hosted/pub.dev/collection-1.17.2/lib/src/iterable_extensions.dart:65:20:
  // Info: This is the type variable whose bound isn't conformed to.
  //   List<T> sortedBy<K extends Comparable<K>>(K Function(T element) keyOf) {
  //                    ^
  // Error: Compilation failed.

  // So you have to do:
  final b = a.sortedBy<num>((f) => f.value);
  // Or casting also works:
  // final b = a.sortedBy((f) => f.value as num);

  print(b);
}

@natebosch
Copy link
Member

@bwilkerson - would it be feasible to specialize the "Couldn't infer parameter" message in the case of Comparable<int>?

This could save us from having to add a warning about this in every method that uses <T extends Comparable<T>>.

@lrhn
Copy link
Member

lrhn commented Oct 11, 2023

We have the option of declaring num-specific extensions:

extension ComparableNum<T extends num> on Iterable<T> {
  List<T> sortedBy([int Function(T, T)? compare]) => [...this]..sort(compare ?? Comparable.compare<num>);
}

which should take precedence over extension ...<T extends Comparable<T>> on Iterable<T> when both match.

Then, at least, our own extensions won't give these errors.

It's annoying, and special-casey, but num is a place where we do that already.

But if we get variance annotations, and Comparable becomes Comparable<in T>, then that extra extension becomes dead-weight. That's why I'd prefer to have such workarounds in packages only. It's harder to change the platform libraries.

@natebosch
Copy link
Member

We have the option of declaring num-specific extensions:

I do not think that would help here. These are all generic methods, and we need to specialize on the generic argument to the method - not the generic argument for the collection. These methods are passed a keyOf callback instead of a compare callback, and the return type of the keyOf callback is what defines the method generic.

@eernstg
Copy link
Member

eernstg commented Oct 12, 2023

We should consider generalizing type inference such that it is able to find solutions in this kind of situation. Here is an example that works fine in Kotlin:

open class A<out X: A<X>>() {}
open class B() : A<B>() {}
open class C() : B() {}

fun <X: A<X>>f(x : X) {}

fun main() {
    f(C()); // Inferred type argument `B`.
}

The corresponding example in Dart fails, because it does not make an attempt to use B as the type argument:

class A<X extends A<X>> {}
class B extends A<B> {}
class C extends B {}

void f<X extends A<X>>(X x) {}

void main() {
  f<B>(C()); // OK.
  f(C()); // Compile-time error: "Couldn't infer ..".
}

This situation is not directly covered by the discussion in dart-lang/language#1194, but it does seem to be related: We need to take the bound of X into account during inference of the invocation of f.

@eernstg
Copy link
Member

eernstg commented Dec 15, 2024

Closing: The generalization of type inference that I mentioned here has been performed by means of the new feature known as 'inference-using-bounds', which is enabled by default in Dart 3.7.0. See dart-lang/language#3009 for further info.

In particular, the following examples are working in DartPad, main channel at this time:

void main() {
  print([1]..sortBy((x) => x));
}

extension ListExtension<T> on List<T> {
  void sortBy<T2 extends Comparable<T2>>(T2 Function(T element) f) {
    this.sort((a, b) => f(a).compareTo(f(b)));
  }
}
import 'package:collection/collection.dart';

class Foo {
  Foo(this.value);

  int value;

  String toString() => value.toString();
}

void main() {
  final a = <Foo>[Foo(2), Foo(5), Foo(3)];
  final b1 = a.sortedBy((f) => f.value);
  final b2 = a.sortedBy<num>((f) => f.value);
  final b3 = a.sortedBy((f) => f.value as num);
  print('$b1, $b2, $b3');

  // The following still fails, it doesn't satisfy the declare bounds:
  // final b = a.sortedBy<int>((f) => f.value);
}

@eernstg eernstg closed this as completed Dec 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-core type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

7 participants