Skip to content
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

Unwanted Object-inference induced type errors. #3156

Closed
modulovalue opened this issue Jun 20, 2023 · 16 comments
Closed

Unwanted Object-inference induced type errors. #3156

modulovalue opened this issue Jun 20, 2023 · 16 comments
Labels
request Requests to resolve a particular developer problem

Comments

@modulovalue
Copy link

modulovalue commented Jun 20, 2023

Recently, I was surprised by unexpected type errors at runtime.

They seem to have been introduced when the return type of a function was migrated to a different type.

Consider the following:

void main() {
  // int dataSource() => 123;
  String dataSource() => "data";
  Foo<int> fooSource() => Foo<int>();
  bar(
    foo: fooSource(),
    data: dataSource(),
  );
}

void bar<T>({
  required final Foo<T> foo,
  required final T data,
}) {
  print(T);
  foo.baz(data);
}

class Foo<T> {
  void baz(T t) {}
}

Notice how a dataSource that returns a String causes the T in bar to be an Object (foo.baz throws), and a dataSource that returns an int causes the T in bar to an int (foo.baz does not throw).

I can't think of many practical situations where I'd want T of bar to be inferred to an Object. And in the rare cases where Object might be wanted there, I'd much rather prefer to be explicit about the type argument and specify T to be Object myself.

One way to prevent this would be to be more explicit and always specify all type arguments. However, I'd prefer not to do that because that would be really verbose.

Could something like implicit-dynamic (e.g. implicit-object) help here? (or could maybe strict-inference be extended to help prevent errors like these?)

@jakemac53
Copy link
Contributor

jakemac53 commented Jun 21, 2023

It sounds like what you really need here is a way to disable the covariance, so that a Foo<int> is not considered a subtype of Foo<Object>.

In particular, the variance proposal is I believe what you want, so you would define Foo as:

class Foo<in T> {...}

This would make Foo be contravariant instead of covariant for its generics. This article has a decent introduction https://medium.com/dartlang/dart-declaration-site-variance-5c0e9c5f18a5.

@modulovalue
Copy link
Author

Thank you, Jake.

Should sound variance land, and if it will be able to help with completely avoiding the pitfalls related to this issue, then I think that that would be great and definitely a solution.

However, I still think that it feels a little unexpected, even without type errors, that, for example, in the following example:

void main() {
  bar(
    data1: 123,
    data2: "data",
  );
}

void bar<T>({
  required final T data1,
  required final T data2,
}) {
  print(T);
}

T is an Object. I can't recall any cases where that is something I'd prefer over a warning that "no type that is more specific than Object could be inferred for T".

@jakemac53
Copy link
Contributor

This problem really is not constraint to Object - the inevitable conclusion is an argument that we should never compute the LUB at all, and require explicit type arguments where we can't infer directly matching types.

For instance if you consider your original example, and change dataSource to double dataSource() => 123.0;, that would also crash at runtime, even though it gets an inferred type of num and not Object.

It is true that Object is very likely the most common failure mode here, but the problem isn't really about Object specifically at all. It is about LUB interacting poorly with covariant generics.

I could see a special lint or something (see the linter repo) that warned you when LUB resulted in Object being inferred, especially if there is a generic type involved, but I don't think it would fit well as a part of the language itself.

@eernstg
Copy link
Member

eernstg commented Jun 21, 2023

You might want to emulate invariance if you need anything other than covariance (invariance is the most strict requirement, which means that it subsumes contravariance):

void main() {
  // int dataSource() => 123;
  String dataSource() => "data";
  Foo<int> fooSource() => Foo<int>();
  bar(
    foo: fooSource(), // Compile-time error. OK when using the other `dataSource`.
    data: dataSource(),
  );
}

void bar<T>({
  required final Foo<T> foo,
  required final T data,
}) {
  print(T);
  foo.baz(data);
}

// Foo stuff, perhaps in a different library (such that privacy matters).

typedef _Inv<X> = X Function(X);

typedef Foo<X> = _Foo<X, _Inv<X>>;

class _Foo<T, Invariance> {
  void baz(T t) {}
}

(and it's also a compile-time error to use double dataSource() => 12.3.)

@modulovalue
Copy link
Author

You might want to emulate invariance

Thank you, Erik. This seems like it indeed solves the issue here. Unfortunately, I don't think that it would be practical for me to go and update all my code to use this technique.

So it looks like I'll have to wait for #524.

@modulovalue
Copy link
Author

modulovalue commented Jun 21, 2023

I could see a special lint or something

My understanding is that lints have somewhat looser correctness guarantees. There are some analyses that the language team appears to maintain. I'm not sure if there are plans to update them (especially because this would potentially be a breaking change), but:

[...] that warned you when LUB resulted in Object being inferred,

I think it would be great if any of those analyses (or a new one) could include this rule (or something similar that would help with this issue).

@eernstg
Copy link
Member

eernstg commented Jun 21, 2023

I don't think that it would be practical

You did notice that the declaration of Foo is modified, but there are no modifications of client code? ;-)

@modulovalue
Copy link
Author

You did notice that the declaration of Foo is modified, but there are no modifications of client code? ;-)

Yes, and I think that this trick is pretty cool. However, it still requires at least an extra declaration per declaration. It might not seem like a lot, but if I were to update thousands of declarations, just to use this trick (which I would need to do), that would be extremely tedious.

@eernstg
Copy link
Member

eernstg commented Jun 22, 2023

Sorry about pushing on this one again, but I'm curious: Given that we have had dynamically checked covariance for more than a decade, and the number of people who are really enthusiastic about adding statically checked declaration-site variance is rather small, it looks like the dynamic errors occur in specialized situations. The use of a "contravariant member" is an obvious example.

Are you sure you'd need to do this with thousands of classes, and not just a handful?

@modulovalue
Copy link
Author

Sorry about pushing on this one again

No, I appreciate that, thank you. It's always a possibility that I have made a mistake somewhere.


I try to use the soundness properties of the Dart type system to give me strong guarantees that my programs are statically verified not to have certain classes of errors.

The most economical way (that I can think of) to "prove" (with a high degree of certainty) that I have eliminated the class of type errors that this issue deals with, is to attempt to use the technique that you've mentioned on all of my generic declarations, and to see which ones need it or not.

Are you sure you'd need to do this with thousands of classes, and not just a handful?

That could be the case, but I don't want to rely on my intuition here because It can make mistakes. I would like to eventually be able to assume that errors from this class of errors will never appear again.


Given that we have had dynamically checked covariance for more than a decade, and the number of people who are really enthusiastic about adding statically checked declaration-site variance is rather small [...]

I've translated the Dart program (that throws at runtime) to Java, Kotlin, Swift, Scala, TypeScript and C++ and they all reject that program at compile-time. Maybe people are just not aware that this is an issue or what the issue actually is? People also don't seem to be enthusiastic about seat belts, street lamps or hospitals, they just take them for granted, so I don't think that upvotes or the amount of excitement would necessarily correlate with how important a language feature is.

Java rejects this program:

public class Main {
    public static void main(String[] args) {
        bar(new Foo<Integer>(), "data");
    }

    public static <T> void bar(Foo<T> foo, T data) {}

    public static class Foo<T> {
        public void baz(T t) {
        }
    }
}
Main.java:3: error: method bar in class Main cannot be applied to given types;
        bar(new Foo<Integer>(), "data");
        ^
required: Foo<T>,T
  found:    Foo<Integer>,String
  reason: inference variable T has incompatible bounds
    equality constraints: Integer
    lower bounds: String
  where T is a type-variable:
    T extends Object declared in method <T>bar(Foo<T>,T)
1 error

Swift rejects this program:

bar(foo: Foo<Int>(), data: "data")

func bar<T>(foo: Foo<T>, data: T) {
    print(T.self)
    foo.baz(data: data)
}

class Foo<T> {
    func baz(data: T) {}
}
error: cannot convert value of type 'String' to expected argument type 'Int'
bar(foo: Foo<Int>(), data: "data")

Kotlin rejects this program:

fun main() {
    bar(Foo<Int>(), "data")
}

fun <T> bar(foo: Foo<T>, data: T) {}

class Foo<T> {
    fun baz(t: T) {}
}
Type mismatch: inferred type is String but Int was expected

Scala rejects this program:

object Main {
  def main(args: Array[String]): Unit = {
    bar(new Foo[Int](), "data")
  }

  def bar[T](foo: Foo[T], data: T): Unit = {}

  class Foo[T] {
    def baz(t: T): Unit = {}
  }
}
error: type mismatch;
 found   : Main.Foo[Int]
 required: Main.Foo[Any]
Note: Int <: Any, but class Foo is invariant in type T.
You may wish to define T as +T instead. (SLS 4.5)
    bar(new Foo[Int](), "data")
        ^

TypeScript rejects this program:

function main(): void {
  bar(new Foo<number>(), "data");
}

function bar<T>(foo: Foo<T>, data: T): void {}

class Foo<T> {
  baz(t: T): void {
  }
}
Argument of type 'string' is not assignable to parameter of type 'number'.

C++ rejects this program:

#include <iostream>

template<typename T>
class Foo {
public:
    void baz(T t) {
    }
};

template<typename T>
void bar(Foo<T> foo, T data) {}

int main() {
    bar(Foo<int>(), "data");
    return 0;
}
main.cpp:15:5: error: no matching function for call to 'bar'
    bar(Foo<int>(), "data");
    ^~~
main.cpp:12:6: note: candidate template ignored: deduced conflicting types for parameter 'T' ('int' vs. 'const char *')
void bar(Foo<T> foo, T data) {}
     ^
1 error generated.

@jakemac53
Copy link
Contributor

The most economical way (that I can think of) to "prove" (with a high degree of certainty) that I have eliminated the class of type errors that this issue deals with, is to attempt to use the technique that you've mentioned on all of my generic declarations, and to see which ones need it or not.

Afaik know, what you need to look for is any class which has a method with a parameter whose type contains a generic type parameter from the class. This means it takes something of the generic type "in" as an argument, which means it can't be safely treated as covariant.

@modulovalue
Copy link
Author

Afaik know, what you need to look for is any class which has a method with a parameter whose type contains a generic type parameter from the class.

I agree that that might work very well for cases like the one in the issue description. However, consider, for example, when Foo is part of a nested structure (such as an AST):

FooFriendFriendFriend
void main() {
  bar(
    foo: FooFriendFriendFriend<int>(),
    data: "data",
  );
}

void bar<T>({
  required final FooFriendFriendFriend<T> foo,
  required final T data,
}) {
  print(T);
  foo.foo.foo.foo.baz(data);
}


class FooFriendFriendFriend<T> {
  FooFriendFriend<T> foo = FooFriendFriend();
}

class FooFriendFriend<T> {
  FooFriend<T> foo = FooFriend();
}

class FooFriend<T> {
  Foo<T> foo = Foo();
}

typedef _Inv<X> = X Function(X);

typedef Foo<X> = _Foo<X, _Inv<X>>;

class _Foo<T, _$I> {
  void baz(
    final T t,
  ) {}
}

Running the program above throws:

type '_Foo<int, (int) => int>' is not a subtype of type '_Foo<Object, (Object) => Object>'

That is, I think, we would have to make FooFriend, FooFriendFriend, FooFriendFriendFriend and everybody else that "stores" any of them also use that technique to guarantee safety.

Of course, this is doable, but I think this would be a lot of work.

@eernstg
Copy link
Member

eernstg commented Jun 23, 2023

@modulovalue wrote:

use the technique that you've mentioned on all of my generic declarations, and to see which ones need it or not.

Right, one way to do that would be to --enable-experiment=variance and add out to every type variable. A class which gives rise to type checks at run time will then have compile-time errors (at each occurrence of a type variable in a non-covariant position in a member signature).

However, I suspect that the lowest hanging fruit is to make a type variable invariant if it is used in a non-covariant position in a member signature which is not the type of a formal parameter. That is, typically, when a type variable is used in the type of an instance variable or in a return type which is a function type:

class C<X> {
  final void Function(X) f; // Danger! "Contravariant member".
  ...
}

Maybe people are just not aware that this is an issue or what the issue actually is?

That's possible. It is also possible that the danger may be considered acceptable (and the issue about dynamically checked variance in general isn't being debated so widely) because certain elements of programming style tend to make these errors infrequent. For instance, if lists (or Foo etc.) are often created and accessed as part of an implementation then there may not be any covariant subtyping, in which case those dynamic type errors will not occur (invariance is safe when it is statically enforced, but equally safe when it is true even though it isn't enforced).

class C<X> {
  final _list1 = <X>[], _list2 = <X>[];
  final X _x;

  C(this._x);

  void baz() {
    _list1.add(x);
    if (_list2.isNotEmpty) _list1.add(_list2.first);
  }
}

Invocations of add on _list1 and _list2 are perfectly type safe as long as the actual argument has type X. This is relevant for a _list1 and similar members as long as they are managed by the class C itself. As soon as they are accessed by any other object than this, we may have covariance and there may be dynamic type errors.

I'm obviously in favor of introducing the statically checked kind of variance, but I must also admit that the dynamically checked covariance has been working much more smoothly in practice than I had expected.

Considering the "friend" example that causes a run-time type error, here's a variant that emulates invariance:

void main() {
  bar(
    foo: FooFriendFriendFriend<int>(),
    data: "data", // Compile-time error. An `int` is OK.
  );
}

void bar<T>({
  required final FooFriendFriendFriend<T> foo,
  required final T data,
}) {
  print(T);
  foo.foo.foo.foo.baz(data);
}

typedef FooFriendFriendFriend<X> = _FooFriendFriendFriend<X, _Inv<X>>;
class _FooFriendFriendFriend<T, _$I> {
  var foo = FooFriendFriend<T>();
}

typedef FooFriendFriend<X> = _FooFriendFriend<X, _Inv<X>>;
class _FooFriendFriend<T, _$I> {
  var foo = FooFriend();
}

typedef FooFriend<X> = _FooFriend<X, _Inv<X>>;
class _FooFriend<T, _$I> {
  Foo<T> foo = Foo();
}

typedef Foo<X> = _Foo<X, _Inv<X>>;
class _Foo<T, _$I> {
  void baz(T t) {}
}

typedef _Inv<X> = X Function(X);

Granted, it's ugly! ;-)

However, note that it is necessary to mark the type variable as inout in each of those Friend declarations as well, in order to get the same typing structure using a proper language mechanism.

Perhaps it's good that it is so ugly: If people start doing this a bit more often, they might vote for #524. ;-)

@modulovalue
Copy link
Author

@eernstg I agree with everything you said, and thank you and @jakemac53 for your help.

So to conclude:

@eernstg
Copy link
Member

eernstg commented May 16, 2024

@modulovalue, perhaps this issue should be closed at this time?

I think the behavior of the tools in the current language is as expected, and language enhancements that would address the issues described here would amount to #524 (and possibly some other proposals that are similar).

@lrhn lrhn added the request Requests to resolve a particular developer problem label May 16, 2024
@modulovalue
Copy link
Author

@eernstg I think so, too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
request Requests to resolve a particular developer problem
Projects
None yet
Development

No branches or pull requests

4 participants