Inconsistent runtime type errors for certain dynamic generic function invocations.

[Markzipan]

class Foo<T> {
  Foo();
}

typedef Foo FooOperation<T>([T value]);

class FooOperator<T> {
  FooOperation<T> op;
  FooOperator(this.op);
}

Foo<String> start() {
  FooOperator operation = FooOperator<String>(
    ([_]) => Foo<String>()
  );
  return operation.op();
}

main() {
  start();
}

The above snippet passes in DDC but fails in CFE with:

Unhandled exception:
type '([String]) => Foo<String>' is not a subtype of type '([dynamic]) => Foo<dynamic>'

It seems that DDC is missing the function type check (i.e., dynamicToFooOfString._check()).

What's the expected behavior here? This pattern's being used quite a bit internally, so making this stricter will require some cleanup.

[mraleph]

CFE behavior is correct to the best of my knowledge.

Given the code like this:

class G<T> {
  U f;
}

G<X> o;
use(o.f);  // (*)

If T occurs in non-covariant way in U then use(o.f); should have an implicit cast use(o.f as U') where U' is U with T substituted by X. Without this check function invocations would require parameter type checks on invocation.

[eernstg]

Agreeing with @mraleph about the issue, I think it's worth considering the example design as well.

The examples fails at the evaluation of operation.op, as part of return operation.op();). The dynamic error is caused by a caller-side check which is needed for soundness (but we are considering whether we can change the approach such that op could have a type which will not fail to hold at run time, cf. dart-lang/language#296).

The basic issue is that covariant generics is an unsound rule for a class that contains members whose type is not covariant; the type of op is an example of this.

But you could use a different approach which won't create the situation where you get a caller-side check at all (this is just a manual emulation of the approach proposed in dart-lang/language#296):

class Foo<T> {
  Foo();
}

typedef Foo FooOperation<T>([T value]);

// Specify a common supertype of `FooOperation<T>` for all `T`. PS: Change `Null`
// to `Never` when we switch to non-nullable types.
typedef Foo FooBound([Null value]);

class FooOperator<T> {
  FooBound op; // Use the common supertype.

  // Specify the parameter type, such that we get the same type check as before
  // when an instance of `FooOperator<T>` is created.
  FooOperator(FooOperator<T> this.op);
}

Foo<String> start() {
  var operation = FooOperator<String>(([_]) => Foo<String>());
  return operation.op();
}

main() {
  start();
}

With this approach you avoid using a non-covariant type for op, and then there is no need for a caller-side check.

The trade-off is that operation.op has a more general type, and you may then need to use a cast in order to obtain a typing where you can use the function.

In the example it is actually not necessary, but it would be needed if you had chosen to pass an argument, e.g., operation.op("Hello!"). You could use (operation.op as Function)("Hello!") or (operation.op as FooOperator<String>)("Hello!"), depending on how much information you have available about the actual type of op and the actual argument at the call site.

Another approach that might actually be the most convenient in practice would be to make op a regular method:

class Foo<T> {
  Foo();
}

typedef Foo FooOperation<T>([T value]);

class FooOperator<T> {
  FooOperation<T> _op;
  FooOperator(this._op);
  Foo op([T value]) => _op(value);
}

Foo<String> start() {
  var operation = FooOperator<String>(([_]) => Foo<String>());
  return operation.op();
}

main() {
  start();
}

The invocation _op(value) is safe because T is in scope and we already know that value has type T; this means that you only get the dynamic check on the actual argument being a T when op is called, and in particular there's no caller-side check anywhere. The trade-off is that we have an extra method invocation.

In order to avoid the dynamic checks entirely you would need to specify some kind of invariance (or contravariance, if you're only using contravariant-typed members like op). We are likely to get that, but it's a bit further into the future (cf. dart-lang/language#229).

[vsmenon]

@Markzipan - can you clarify - are you seeing different behavior with DDC and DDK?

[Markzipan]

Yes - DDK emits the proper error here. So there might be a few areas in google3 to fix up.

[vsmenon]

Thanks. Analyzer DDC should be injecting a check here as well, but marking p4.

[vsmenon]

@Markzipan - shall we close this?

[Markzipan]

Yep - issue resolved by fixing internal tests.