Strong mode and dart:core's "Function"

[matanlurey]

The Function interface is used by several packages, such as package:test to be able to declare the input/output is a Function, no matter how many arguments/signature. That's nice. But...

I'm getting a "Unsound implicit cast from Function to () -> dynamic".

I'm not sure how to solve this nicely. You can see a reproduction case on Dartpad, or below:

void main() {
  run(expectAsync((String t) {}));
}

Function expectAsync(Function callback) => callback;

/*=E*/ run/*<E>*/(/*=E*/ action(String t)) => action('Hello');

I realize this might not be "strong mode" safe. If so, what is the Future of Function?

/cc @vsmenon @leafpetersen @nex3

[nex3]

expectAsync() is a particularly tricky case—see #26420 for detailed reasons why. The tl;dr is that it's impossible to use in strong mode at this point, and we're still searching for the right way to fix that in the short term.

[matanlurey]

Thanks for the background.

[matanlurey]

I'm closing as a duplicate of dart-lang/test#436

[leafpetersen]

I forget where we left this exactly - is it possible to make this work without inference by requiring explicit instantiation? If so, is this high enough priority for us to go through two breaking changes in the API (one to make it usable via explicit instantiation, and one to make it usable via inference if/when we get support for Function<T>)?

[nex3]

We could theoretically define it as

Function expectAsync/*<T>*/(Function callback);

but that's a pretty heavy burden to impose on users, and is effectively a breaking change in the sense that all existing call sites will need to be modified. I'd really like to avoid any breaking changes in test's API, since it's so widely-used. Propagating out a major version bump would be pretty agonizing.

[leafpetersen]

Is there an API which would:

• be backward compatible with the existing API for non-strong users
• allow people who really want to use test now in strong mode to use it via explicit instantiation
• be forward compatible with the eventual API?

It seems like the API you define above is close to that. Non strong users can ignore it, strong mode users could use it via explicit instantiation if they really wanted to. Eventually we could make it Function<T> callback, and then inference would work, but the explicitly instantiated versions would also continue to work. Is that right, or am I missing something?

We'd discussed the following more complex signature in the other bug:

Function/*=F*/ expectAsync/*<T, F extends Function<T>>*/(Function/*=F*/ callback);

I think the benefit of this is that you don't have a cast on the output value, is that right?

@jmesserly I don't think our existing inference would do the right thing with this more complex signature (I don't think we consider bounds during inference). We should think about whether our constraint solving would generalize to this or whether it would open a big ol can of worms.

[nex3]

It seems like the API you define above is close to that. Non strong users can ignore it, strong mode users could use it via explicit instantiation if they really wanted to. Eventually we could make it Function<T> callback, and then inference would work, but the explicitly instantiated versions would also continue to work. Is that right, or am I missing something?

It's close, but it doesn't satisfy the third bullet point: it won't be forwards-compatible with the eventual solution. Which means there'll still be a breaking change coming down the pipe.

[matanlurey]

Dumb question: Could we offer a new API to help strong-mode users, i.e. expectAsyncTyped.

I'd be OK if it's marked with DANGER: This will change and here are the steps*

[floitschG]

Closing again as duplicate of dart-lang/test#436

[jmesserly]

I don't think we consider bounds during inference

FYI @leafpetersen ... we do consider bounds during inference:

sdk/pkg/analyzer/lib/src/generated/type_system.dart

Line 1561 in 685e4ff

_inferTypeParameterSubtypeOf(typeParam,

I believe it would work with current inference, but it'd be good to add a test for.

[jmesserly]

update: inference seems to work for that, but I found a different bug, which I'll file.